diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index e2877349db..bd425090c2 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -558,46 +558,21 @@ async fn discover_skills_under_root( } let path = dir.join(&file_name); - let path_uri = PathUri::from_abs_path(&path); - let metadata = match fs.get_metadata(&path_uri, /*sandbox*/ None).await { - Ok(metadata) => metadata, - Err(e) => { - error!("failed to stat skills path {}: {e:#}", path.display()); - continue; - } - }; - - if metadata.is_symlink { - if !follow_symlinks { - continue; - } - match fs.read_directory(&path_uri, /*sandbox*/ None).await { - Ok(_) => { - let resolved_dir = canonicalize_for_skill_identity(fs, &path).await; - enqueue_dir( - &mut queue, - &mut visited_dirs, - &mut truncated_by_dir_limit, - resolved_dir, - depth + 1, - ); - } - Err(err) - if matches!( - err.kind(), - io::ErrorKind::NotADirectory | io::ErrorKind::NotFound - ) => {} - Err(err) => { - error!( - "failed to read skills symlink dir {}: {err:#}", - path.display() - ); - } + if entry.is_symlink { + if follow_symlinks && entry.is_directory { + let resolved_dir = canonicalize_for_skill_identity(fs, &path).await; + enqueue_dir( + &mut queue, + &mut visited_dirs, + &mut truncated_by_dir_limit, + resolved_dir, + depth + 1, + ); } continue; } - if metadata.is_directory { + if entry.is_directory { let resolved_dir = canonicalize_for_skill_identity(fs, &path).await; enqueue_dir( &mut queue, @@ -609,7 +584,7 @@ async fn discover_skills_under_root( continue; } - if metadata.is_file && file_name == SKILLS_FILENAME { + if entry.is_file && file_name == SKILLS_FILENAME { match parse_skill_file(fs, &path, scope, plugin_id, plugin_root.as_ref()).await { Ok(skill) => { outcome.skills.push(skill); diff --git a/codex-rs/exec-server/src/fs_helper.rs b/codex-rs/exec-server/src/fs_helper.rs index f4db844b5b..bb95083a3c 100644 --- a/codex-rs/exec-server/src/fs_helper.rs +++ b/codex-rs/exec-server/src/fs_helper.rs @@ -257,6 +257,7 @@ pub(crate) async fn run_direct_request( file_name: entry.file_name, is_directory: entry.is_directory, is_file: entry.is_file, + is_symlink: entry.is_symlink, }) .collect(); Ok(FsHelperPayload::ReadDirectory(FsReadDirectoryResponse { diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 3129606d6b..36fae4c0f1 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -597,13 +597,23 @@ impl DirectFileSystem { let mut entries = Vec::new(); let mut read_dir = tokio::fs::read_dir(path.as_path()).await?; while let Some(entry) = read_dir.next_entry().await? { - let Ok(metadata) = tokio::fs::metadata(entry.path()).await else { + let Ok(file_type) = entry.file_type().await else { continue; }; + let is_symlink = file_type.is_symlink(); + let (is_directory, is_file) = if is_symlink { + let Ok(metadata) = tokio::fs::metadata(entry.path()).await else { + continue; + }; + (metadata.is_dir(), metadata.is_file()) + } else { + (file_type.is_dir(), file_type.is_file()) + }; entries.push(ReadDirectoryEntry { file_name: entry.file_name().to_string_lossy().into_owned(), - is_directory: metadata.is_dir(), - is_file: metadata.is_file(), + is_directory, + is_file, + is_symlink, }); } Ok(entries) diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index 32271ed207..08564b6dc6 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -320,6 +320,8 @@ pub struct FsReadDirectoryEntry { pub file_name: String, pub is_directory: bool, pub is_file: bool, + #[serde(default)] + pub is_symlink: bool, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index f9136e1461..16ea67fdf4 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -179,6 +179,7 @@ impl RemoteFileSystem { file_name: entry.file_name, is_directory: entry.is_directory, is_file: entry.is_file, + is_symlink: entry.is_symlink, }) .collect()) } diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index d0beb57a43..7abafb7f07 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -196,6 +196,7 @@ impl SandboxedFileSystem { file_name: entry.file_name, is_directory: entry.is_directory, is_file: entry.is_file, + is_symlink: entry.is_symlink, }) .collect()) } diff --git a/codex-rs/exec-server/src/server/file_system_handler.rs b/codex-rs/exec-server/src/server/file_system_handler.rs index ddf17f21cd..68e3c0cd5f 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -193,6 +193,7 @@ impl FileSystemHandler { file_name: entry.file_name, is_directory: entry.is_directory, is_file: entry.is_file, + is_symlink: entry.is_symlink, }) .collect(); Ok(FsReadDirectoryResponse { entries }) diff --git a/codex-rs/exec-server/tests/file_system/shared.rs b/codex-rs/exec-server/tests/file_system/shared.rs index 08ba2efdd8..ea4c2fa50f 100644 --- a/codex-rs/exec-server/tests/file_system/shared.rs +++ b/codex-rs/exec-server/tests/file_system/shared.rs @@ -343,11 +343,13 @@ async fn file_system_read_directory_lists_entries( file_name: "nested".to_string(), is_directory: true, is_file: false, + is_symlink: false, }, ReadDirectoryEntry { file_name: "root.txt".to_string(), is_directory: false, is_file: true, + is_symlink: false, }, ] ); diff --git a/codex-rs/exec-server/tests/file_system_unix.rs b/codex-rs/exec-server/tests/file_system_unix.rs index 57c3a82e97..1da24da0f3 100644 --- a/codex-rs/exec-server/tests/file_system_unix.rs +++ b/codex-rs/exec-server/tests/file_system_unix.rs @@ -23,6 +23,7 @@ use codex_exec_server::CreateDirectoryOptions; #[cfg(target_os = "linux")] use codex_exec_server::Environment; use codex_exec_server::FileMetadata; +use codex_exec_server::ReadDirectoryEntry; use codex_exec_server::RemoveOptions; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; @@ -79,6 +80,51 @@ fn assert_normalized_path_rejected(error: &std::io::Error) { } } +#[test_case(FileSystemImplementation::Local ; "local")] +#[test_case(FileSystemImplementation::Remote ; "remote")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn file_system_read_directory_reports_symlink_target_kind( + implementation: FileSystemImplementation, +) -> Result<()> { + let context = create_file_system_context(implementation).await?; + let file_system = context.file_system; + + let tmp = TempDir::new()?; + let source_dir = tmp.path().join("source"); + let target_dir = tmp.path().join("target-dir"); + let target_file = tmp.path().join("target.txt"); + std::fs::create_dir_all(&source_dir)?; + std::fs::create_dir_all(&target_dir)?; + std::fs::write(&target_file, "hello")?; + symlink(&target_dir, source_dir.join("directory-link"))?; + symlink(&target_file, source_dir.join("file-link"))?; + + let mut entries = file_system + .read_directory(&PathUri::from_path(&source_dir)?, /*sandbox*/ None) + .await + .with_context(|| format!("mode={implementation}"))?; + entries.sort_by(|left, right| left.file_name.cmp(&right.file_name)); + assert_eq!( + entries, + vec![ + ReadDirectoryEntry { + file_name: "directory-link".to_string(), + is_directory: true, + is_file: false, + is_symlink: true, + }, + ReadDirectoryEntry { + file_name: "file-link".to_string(), + is_directory: false, + is_file: true, + is_symlink: true, + }, + ] + ); + + Ok(()) +} + fn alias_root_candidate() -> Result> { for root in [Path::new("/tmp").to_path_buf(), std::env::temp_dir()] { if root.is_dir() && root.canonicalize().is_ok_and(|canonical| canonical != root) { diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index 1eff53c3b9..604f0fbbbc 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -60,12 +60,14 @@ impl SyntheticFileSystem { file_name: "skill".to_string(), is_directory: true, is_file: false, + is_symlink: false, }]) } else if path == self.canonical_root.join("skill") { Ok(vec![ReadDirectoryEntry { file_name: "SKILL.md".to_string(), is_directory: false, is_file: true, + is_symlink: false, }]) } else { Err(io::Error::new(io::ErrorKind::NotFound, "not found")) @@ -146,7 +148,17 @@ impl ExecutorFileSystem for SyntheticFileSystem { path: &'a PathUri, _sandbox: Option<&'a FileSystemSandboxContext>, ) -> ExecutorFileSystemFuture<'a, FileMetadata> { - Box::pin(async move { self.metadata(&path.to_abs_path()?) }) + Box::pin(async move { + let path = path.to_abs_path()?; + if path == self.canonical_root { + self.metadata(&path) + } else { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "synthetic filesystem exposes child entry types through read_directory", + )) + } + }) } fn read_directory<'a>( diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 3ba7874404..4b37ef5176 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -54,6 +54,7 @@ pub struct ReadDirectoryEntry { pub file_name: String, pub is_directory: bool, pub is_file: bool, + pub is_symlink: bool, } #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]