From 96d8e34712e31ad4051577f777b3235bd1ad6424 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 24 Jun 2026 20:52:36 +0100 Subject: [PATCH] Follow directory symlinks in filesystem walks (#29844) Stack 3 of 3. Stacked on #29842. ## What changes Adds an opt-in `followDirectorySymlinks` setting to `fs/walk`. When enabled, the walk follows directory symlinks but continues to ignore symlinked files. Canonical directory identities prevent symlink cycles, while normal paths keep their existing spelling. Environment skill discovery enables the setting so symlinked skill directories continue to work with the new single-RPC scan. --- .../core-skills/src/loader/environment.rs | 1 + .../exec-server/tests/file_system/shared.rs | 5 +++ .../exec-server/tests/file_system_unix.rs | 43 ++++++++++++++++--- codex-rs/file-system/src/lib.rs | 41 ++++++++++++++++-- 4 files changed, 81 insertions(+), 9 deletions(-) diff --git a/codex-rs/core-skills/src/loader/environment.rs b/codex-rs/core-skills/src/loader/environment.rs index 414593430..5dc29ebfb 100644 --- a/codex-rs/core-skills/src/loader/environment.rs +++ b/codex-rs/core-skills/src/loader/environment.rs @@ -116,6 +116,7 @@ pub async fn load_environment_skills_from_root( max_depth: MAX_SCAN_DEPTH, max_directories: MAX_SKILLS_DIRS_PER_ROOT, max_entries: MAX_SKILLS_ENTRIES_PER_ROOT, + follow_directory_symlinks: true, }, /*sandbox*/ None, ) diff --git a/codex-rs/exec-server/tests/file_system/shared.rs b/codex-rs/exec-server/tests/file_system/shared.rs index 0f12cd592..261c6b07d 100644 --- a/codex-rs/exec-server/tests/file_system/shared.rs +++ b/codex-rs/exec-server/tests/file_system/shared.rs @@ -401,6 +401,7 @@ async fn file_system_walk_returns_a_bounded_tree( max_depth: 4, max_directories: 10, max_entries: 10, + follow_directory_symlinks: false, }, /*sandbox*/ None, ) @@ -445,6 +446,7 @@ async fn file_system_walk_returns_a_bounded_tree( max_depth: 0, max_directories: 10, max_entries: 10, + follow_directory_symlinks: false, }, /*sandbox*/ None, ) @@ -466,6 +468,7 @@ async fn file_system_walk_returns_a_bounded_tree( max_depth: 4, max_directories: 1, max_entries: 10, + follow_directory_symlinks: false, }, /*sandbox*/ None, ) @@ -487,6 +490,7 @@ async fn file_system_walk_returns_a_bounded_tree( max_depth: 4, max_directories: 10, max_entries: 1, + follow_directory_symlinks: false, }, /*sandbox*/ None, ) @@ -530,6 +534,7 @@ async fn file_system_walk_honors_read_sandbox( max_depth: 1, max_directories: 2, max_entries: 2, + follow_directory_symlinks: false, }, Some(&sandbox), ) diff --git a/codex-rs/exec-server/tests/file_system_unix.rs b/codex-rs/exec-server/tests/file_system_unix.rs index 320f52108..f65804544 100644 --- a/codex-rs/exec-server/tests/file_system_unix.rs +++ b/codex-rs/exec-server/tests/file_system_unix.rs @@ -273,17 +273,23 @@ async fn file_system_get_metadata_reports_symlink_targets( #[test_case(FileSystemImplementation::Local ; "local")] #[test_case(FileSystemImplementation::Remote ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_walk_ignores_symlinks(implementation: FileSystemImplementation) -> Result<()> { +async fn file_system_walk_handles_directory_symlinks( + implementation: FileSystemImplementation, +) -> Result<()> { let context = create_file_system_context(implementation).await?; let file_system = context.file_system; let tmp = TempDir::new()?; let root = tmp.path().join("root"); - let target = root.join("target"); + let target = tmp.path().join("target"); let target_file = target.join("note.txt"); + let target_link = root.join("target-link"); + let root_link = target.join("root-link"); + std::fs::create_dir_all(&root)?; std::fs::create_dir_all(&target)?; std::fs::write(&target_file, "target")?; - symlink(&target, root.join("target-link"))?; + symlink(&target, &target_link)?; + symlink(&root, &root_link)?; let outcome = file_system .walk( @@ -292,6 +298,29 @@ async fn file_system_walk_ignores_symlinks(implementation: FileSystemImplementat max_depth: 2, max_directories: 4, max_entries: 8, + follow_directory_symlinks: false, + }, + /*sandbox*/ None, + ) + .await + .with_context(|| format!("mode={implementation}"))?; + assert_eq!( + outcome, + WalkOutcome { + entries: Vec::new(), + errors: Vec::new(), + truncated: false, + } + ); + + let outcome = file_system + .walk( + &PathUri::from_host_native_path(&root)?, + WalkOptions { + max_depth: 2, + max_directories: 4, + max_entries: 8, + follow_directory_symlinks: true, }, /*sandbox*/ None, ) @@ -302,13 +331,17 @@ async fn file_system_walk_ignores_symlinks(implementation: FileSystemImplementat WalkOutcome { entries: vec![ WalkEntry { - path: PathUri::from_host_native_path(&target)?, + path: PathUri::from_host_native_path(&target_link)?, kind: WalkEntryKind::Directory, }, WalkEntry { - path: PathUri::from_host_native_path(target_file)?, + path: PathUri::from_host_native_path(target_link.join("note.txt"))?, kind: WalkEntryKind::File, }, + WalkEntry { + path: PathUri::from_host_native_path(target_link.join("root-link"))?, + kind: WalkEntryKind::Directory, + }, ], errors: Vec::new(), truncated: false, diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 3ae4b5675..07e096adc 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -12,6 +12,7 @@ use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use futures::Stream; +use std::collections::HashSet; use std::collections::VecDeque; use std::future::Future; use std::io; @@ -72,6 +73,8 @@ pub struct WalkOptions { pub max_directories: usize, /// Maximum number of directory entries that may be examined. pub max_entries: usize, + /// Whether directory symlinks should be followed. + pub follow_directory_symlinks: bool, } /// Type of a filesystem entry returned by a walk. @@ -82,7 +85,7 @@ pub enum WalkEntryKind { File, } -/// One non-symlink entry returned by a walk. +/// One entry returned by a walk. #[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)] #[serde(rename_all = "camelCase")] pub struct WalkEntry { @@ -300,7 +303,7 @@ pub trait ExecutorFileSystem: Send + Sync { sandbox: Option<&'a FileSystemSandboxContext>, ) -> ExecutorFileSystemFuture<'a, Vec>; - /// Recursively lists descendants, skipping symlinks. + /// Recursively lists descendants, optionally following directory symlinks. fn walk<'a>( &'a self, path: &'a PathUri, @@ -351,12 +354,20 @@ async fn walk( } let root_metadata = file_system.get_metadata(root, sandbox).await?; - if root_metadata.is_symlink || !root_metadata.is_directory { + if !root_metadata.is_directory + || (root_metadata.is_symlink && !options.follow_directory_symlinks) + { return Ok(WalkOutcome::default()); } + let root_identity = if options.follow_directory_symlinks { + file_system.canonicalize(root, sandbox).await? + } else { + root.clone() + }; let mut outcome = WalkOutcome::default(); let mut queue = VecDeque::from([(root.clone(), 0usize)]); + let mut visited_directories = HashSet::from([root_identity]); let mut directory_count = 1usize; let mut entry_count = 0usize; let mut response_bytes = 0usize; @@ -409,7 +420,8 @@ async fn walk( continue; } }; - if metadata.is_symlink { + if metadata.is_symlink && (!options.follow_directory_symlinks || !metadata.is_directory) + { continue; } @@ -433,6 +445,27 @@ async fn walk( }); if kind == WalkEntryKind::Directory && depth < options.max_depth { + let directory_identity = if options.follow_directory_symlinks { + match file_system.canonicalize(&path, sandbox).await { + Ok(path) => path, + Err(error) => { + if !push_walk_error( + &mut outcome, + &mut response_bytes, + path, + error.to_string(), + ) { + return Ok(outcome); + } + continue; + } + } + } else { + path.clone() + }; + if !visited_directories.insert(directory_identity) { + continue; + } if directory_count == options.max_directories { outcome.truncated = true; } else {