diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 281fd1eeb356..b0689e8384e1 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -596,12 +596,17 @@ impl DirectFileSystem { ) -> FileSystemResult { reject_sandbox_context(sandbox)?; let path = path.to_abs_path()?; - let metadata = tokio::fs::metadata(path.as_path()).await?; let symlink_metadata = tokio::fs::symlink_metadata(path.as_path()).await?; + let is_symlink = symlink_metadata.is_symlink(); + let metadata = if is_symlink { + tokio::fs::metadata(path.as_path()).await? + } else { + symlink_metadata + }; Ok(FileMetadata { is_directory: metadata.is_dir(), is_file: metadata.is_file(), - is_symlink: symlink_metadata.file_type().is_symlink(), + is_symlink, size: metadata.len(), created_at_ms: metadata.created().ok().map_or(0, system_time_to_unix_ms), modified_at_ms: metadata.modified().ok().map_or(0, system_time_to_unix_ms), @@ -618,13 +623,19 @@ 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(mut file_type) = entry.file_type().await else { continue; }; + if file_type.is_symlink() { + let Ok(metadata) = tokio::fs::metadata(entry.path()).await else { + continue; + }; + file_type = metadata.file_type(); + } 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: file_type.is_dir(), + is_file: file_type.is_file(), }); } Ok(entries) diff --git a/codex-rs/exec-server/tests/file_system_unix.rs b/codex-rs/exec-server/tests/file_system_unix.rs index e89eca7aad7c..4e5980e345ac 100644 --- a/codex-rs/exec-server/tests/file_system_unix.rs +++ b/codex-rs/exec-server/tests/file_system_unix.rs @@ -24,6 +24,7 @@ use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::Environment; use codex_exec_server::FileMetadata; use codex_exec_server::FileSystemSandboxContext; +use codex_exec_server::ReadDirectoryEntry; use codex_exec_server::RemoveOptions; use codex_exec_server::WalkEntry; use codex_exec_server::WalkEntryKind; @@ -297,7 +298,7 @@ async fn remote_read_file_preserves_empty_workspace_roots() -> Result<()> { #[test_case(FileSystemImplementation::Local ; "local")] #[test_case(FileSystemImplementation::Remote ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_get_metadata_reports_symlink_targets( +async fn file_system_metadata_and_directory_listing_follow_symlinks( implementation: FileSystemImplementation, ) -> Result<()> { let context = create_file_system_context(implementation).await?; @@ -351,6 +352,43 @@ async fn file_system_get_metadata_reports_symlink_targets( } ); + let dangling_symlink_path = tmp.path().join("dangling-link"); + symlink(tmp.path().join("missing"), &dangling_symlink_path)?; + let error = file_system + .get_metadata( + &PathUri::from_host_native_path(&dangling_symlink_path)?, + /*sandbox*/ None, + ) + .await + .expect_err("dangling symlink should not resolve"); + assert_eq!(error.kind(), std::io::ErrorKind::NotFound); + + let mut entries = file_system + .read_directory( + &PathUri::from_host_native_path(tmp.path())?, + /*sandbox*/ None, + ) + .await + .with_context(|| format!("mode={implementation}"))?; + entries.retain(|entry| entry.file_name.contains("link")); + entries.sort_by(|left, right| left.file_name.cmp(&right.file_name)); + + assert_eq!( + entries, + vec![ + ReadDirectoryEntry { + file_name: "note-link.txt".to_string(), + is_directory: false, + is_file: true, + }, + ReadDirectoryEntry { + file_name: "notes-link".to_string(), + is_directory: true, + is_file: false, + }, + ] + ); + Ok(()) } diff --git a/codex-rs/ext/memories/src/local/list.rs b/codex-rs/ext/memories/src/local/list.rs index 60915aa1f9d8..f5b1acabcbc1 100644 --- a/codex-rs/ext/memories/src/local/list.rs +++ b/codex-rs/ext/memories/src/local/list.rs @@ -8,7 +8,7 @@ use crate::backend::MemoryEntryType; use super::LocalMemoriesBackend; use super::path::display_relative_path; use super::path::is_hidden_path; -use super::path::read_sorted_dir_paths; +use super::path::read_sorted_dir_entries; use super::path::reject_symlink; pub(super) async fn list( @@ -37,20 +37,14 @@ pub(super) async fn list( }] } else if metadata.is_dir() { let mut entries = Vec::new(); - for path in read_sorted_dir_paths(&start).await? { - if is_hidden_path(&path) { - continue; - } - let Some(metadata) = LocalMemoriesBackend::metadata_or_none(&path).await? else { - continue; - }; - if metadata.file_type().is_symlink() { + for (path, file_type) in read_sorted_dir_entries(&start).await? { + if is_hidden_path(&path) || file_type.is_symlink() { continue; } - let entry_type = if metadata.is_dir() { + let entry_type = if file_type.is_dir() { MemoryEntryType::Directory - } else if metadata.is_file() { + } else if file_type.is_file() { MemoryEntryType::File } else { continue; diff --git a/codex-rs/ext/memories/src/local/path.rs b/codex-rs/ext/memories/src/local/path.rs index 2052967f68b2..9766d2b64c35 100644 --- a/codex-rs/ext/memories/src/local/path.rs +++ b/codex-rs/ext/memories/src/local/path.rs @@ -1,23 +1,32 @@ +use std::fs::FileType; use std::path::Component; use std::path::Path; use std::path::PathBuf; use crate::backend::MemoriesBackendError; -pub(super) async fn read_sorted_dir_paths( +/// Returns lexically sorted paths and their cached, non-symlink-following file types. +/// +/// Missing directories and entries that disappear during iteration are ignored. +pub(super) async fn read_sorted_dir_entries( dir_path: &Path, -) -> Result, MemoriesBackendError> { +) -> Result, MemoriesBackendError> { let mut dir = match tokio::fs::read_dir(dir_path).await { Ok(dir) => dir, Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), Err(err) => return Err(err.into()), }; - let mut paths = Vec::new(); + let mut entries = Vec::new(); while let Some(entry) = dir.next_entry().await? { - paths.push(entry.path()); + let file_type = match entry.file_type().await { + Ok(file_type) => file_type, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue, + Err(err) => return Err(err.into()), + }; + entries.push((entry.path(), file_type)); } - paths.sort(); - Ok(paths) + entries.sort_by(|(left, _), (right, _)| left.cmp(right)); + Ok(entries) } pub(super) fn reject_symlink( diff --git a/codex-rs/ext/memories/src/local/search.rs b/codex-rs/ext/memories/src/local/search.rs index 85551fd88a87..2a539aa7dc96 100644 --- a/codex-rs/ext/memories/src/local/search.rs +++ b/codex-rs/ext/memories/src/local/search.rs @@ -11,7 +11,7 @@ use crate::backend::SearchMemoriesResponse; use super::LocalMemoriesBackend; use super::path::display_relative_path; use super::path::is_hidden_path; -use super::path::read_sorted_dir_paths; +use super::path::read_sorted_dir_entries; use super::path::reject_symlink; pub(super) async fn search( @@ -106,7 +106,7 @@ async fn search_entries( let mut pending = vec![current.to_path_buf()]; while let Some(dir_path) = pending.pop() { - for path in read_sorted_dir_paths(&dir_path).await? { + for (path, _) in read_sorted_dir_entries(&dir_path).await? { if is_hidden_path(&path) { continue; } diff --git a/codex-rs/ext/memories/src/tests.rs b/codex-rs/ext/memories/src/tests.rs index cd13c062e75b..e0ad227891b7 100644 --- a/codex-rs/ext/memories/src/tests.rs +++ b/codex-rs/ext/memories/src/tests.rs @@ -19,6 +19,13 @@ use codex_utils_output_truncation::TruncationPolicy; use pretty_assertions::assert_eq; use serde_json::json; +use crate::backend::ListMemoriesRequest; +use crate::backend::ListMemoriesResponse; +use crate::backend::MemoriesBackend; +use crate::backend::MemoryEntry; +use crate::backend::MemoryEntryType; +use crate::backend::SearchMatchMode; +use crate::backend::SearchMemoriesRequest; use crate::extension::MemoriesExtension; use crate::extension::MemoriesExtensionConfig; use crate::local::LocalMemoriesBackend; @@ -324,6 +331,73 @@ async fn read_tool_reads_memory_file() { ); } +#[tokio::test] +async fn local_listing_and_search_ignore_symlinks() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let memory_root = tempdir.path().join("memories"); + let outside_root = tempdir.path().join("outside"); + std::fs::create_dir_all(memory_root.join("nested")).expect("create memories directory"); + std::fs::create_dir_all(&outside_root).expect("create outside directory"); + for (path, content) in [ + (memory_root.join("a.md"), "visible needle"), + (memory_root.join("nested/z.md"), "nested needle"), + (outside_root.join("secret.md"), "outside needle"), + ] { + std::fs::write(path, content).expect("write memory fixture"); + } + #[cfg(unix)] + std::os::unix::fs::symlink(&outside_root, memory_root.join("linked-directory")) + .expect("create memory fixture symlink"); + + let backend = LocalMemoriesBackend::from_memory_root(&memory_root); + let listing = backend + .list(ListMemoriesRequest { + path: None, + cursor: None, + max_results: 10, + }) + .await + .expect("list visible memories"); + assert_eq!( + listing, + ListMemoriesResponse { + path: None, + entries: vec![ + MemoryEntry { + path: "a.md".to_string(), + entry_type: MemoryEntryType::File, + }, + MemoryEntry { + path: "nested".to_string(), + entry_type: MemoryEntryType::Directory, + }, + ], + next_cursor: None, + truncated: false, + } + ); + + let response = backend + .search(SearchMemoriesRequest { + queries: vec!["needle".to_string()], + match_mode: SearchMatchMode::Any, + path: None, + cursor: None, + context_lines: 0, + case_sensitive: false, + normalized: false, + max_results: 10, + }) + .await + .expect("search visible memories"); + let paths = response + .matches + .iter() + .map(|matched| matched.path.as_str()) + .collect::>(); + assert_eq!(paths, vec!["a.md", "nested/z.md"]); +} + #[tokio::test] async fn search_tool_accepts_multiple_queries() { let tempdir = tempfile::tempdir().expect("tempdir"); diff --git a/codex-rs/rollout/src/compression.rs b/codex-rs/rollout/src/compression.rs index a3e14e9fe02b..ef3589da2270 100644 --- a/codex-rs/rollout/src/compression.rs +++ b/codex-rs/rollout/src/compression.rs @@ -32,12 +32,10 @@ pub fn spawn_rollout_compression_worker(codex_home: PathBuf) { /// Returns the modified time for the existing plain or compressed rollout file. pub(crate) async fn file_modified_time(path: &Path) -> io::Result> { - let Some(path) = path::existing_rollout_path(path).await else { - return Ok(None); - }; - let meta = tokio::fs::metadata(path).await?; - let modified = meta.modified().ok(); - Ok(modified.map(time::OffsetDateTime::from)) + Ok(path::existing_rollout_with_metadata(path) + .await + .and_then(|(_, metadata)| metadata.modified().ok()) + .map(time::OffsetDateTime::from)) } /// Opens a rollout line reader that transparently handles plain `.jsonl` and `.jsonl.zst` files. @@ -936,6 +934,7 @@ pub async fn existing_rollout_path(path: &Path) -> Option { mod path { use std::ffi::OsStr; + use std::fs::Metadata; use std::path::Path; use std::path::PathBuf; @@ -974,15 +973,26 @@ mod path { } pub(super) async fn existing_rollout_path(path: &Path) -> Option { + existing_rollout_with_metadata(path) + .await + .map(|(path, _)| path) + } + + /// Resolves the plain rollout before its compressed sibling and retains the lookup metadata. + /// + /// Returning the metadata lets callers inspect the selected file without a second stat. + pub(super) async fn existing_rollout_with_metadata(path: &Path) -> Option<(PathBuf, Metadata)> { let plain_path = plain_rollout_path(path); - if matches!(tokio::fs::metadata(plain_path.as_path()).await, Ok(metadata) if metadata.is_file()) + if let Ok(metadata) = tokio::fs::metadata(plain_path.as_path()).await + && metadata.is_file() { - return Some(plain_path); + return Some((plain_path, metadata)); } let compressed_path = compressed_rollout_path(plain_path.as_path()); - if matches!(tokio::fs::metadata(compressed_path.as_path()).await, Ok(metadata) if metadata.is_file()) + if let Ok(metadata) = tokio::fs::metadata(compressed_path.as_path()).await + && metadata.is_file() { - return Some(compressed_path); + return Some((compressed_path, metadata)); } None }