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 {