From e6896840f6906a5a2808ff7f1a26ec8ad7a42900 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 7 Jul 2026 22:01:57 -0400 Subject: [PATCH] Reduce redundant filesystem syscalls --- codex-rs/exec-server/src/environment_toml.rs | 46 ++++++++------- codex-rs/exec-server/src/local_file_system.rs | 9 ++- .../exec-server/tests/file_system_unix.rs | 25 ++++++++ codex-rs/file-search/src/lib.rs | 57 +++++++++++++++---- codex-rs/tui/src/lib.rs | 4 -- codex-rs/utils/path-utils/src/lib.rs | 5 +- 6 files changed, 104 insertions(+), 42 deletions(-) diff --git a/codex-rs/exec-server/src/environment_toml.rs b/codex-rs/exec-server/src/environment_toml.rs index bfc25873baff..5058f35bcac8 100644 --- a/codex-rs/exec-server/src/environment_toml.rs +++ b/codex-rs/exec-server/src/environment_toml.rs @@ -209,16 +209,10 @@ pub(crate) fn environment_provider_from_codex_home( codex_home: &Path, ) -> Result, ExecServerError> { let path = codex_home.join(ENVIRONMENTS_TOML_FILE); - if !path.try_exists().map_err(|err| { - ExecServerError::Protocol(format!( - "failed to inspect environment config `{}`: {err}", - path.display() - )) - })? { + let Some(environments) = load_environments_toml(&path)? else { return Ok(Box::new(DefaultEnvironmentProvider::from_env())); - } + }; - let environments = load_environments_toml(&path)?; Ok(Box::new(TomlEnvironmentProvider::new_with_config_dir( environments, Some(codex_home), @@ -307,20 +301,26 @@ fn validate_websocket_url(url: String) -> Result { Ok(url.to_string()) } -fn load_environments_toml(path: &Path) -> Result { - let contents = std::fs::read_to_string(path).map_err(|err| { - ExecServerError::Protocol(format!( - "failed to read environment config `{}`: {err}", - path.display() - )) - })?; +fn load_environments_toml(path: &Path) -> Result, ExecServerError> { + let contents = match std::fs::read_to_string(path) { + Ok(contents) => contents, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => { + return Err(ExecServerError::Protocol(format!( + "failed to read environment config `{}`: {err}", + path.display() + ))); + } + }; - toml::from_str(&contents).map_err(|err| { - ExecServerError::Protocol(format!( - "failed to parse environment config `{}`: {err}", - path.display() - )) - }) + toml::from_str(&contents) + .map_err(|err| { + ExecServerError::Protocol(format!( + "failed to parse environment config `{}`: {err}", + path.display() + )) + }) + .map(Some) } mod option_duration_secs { @@ -766,7 +766,9 @@ CODEX_LOG = "debug" ) .expect("write environments.toml"); - let environments = load_environments_toml(&path).expect("environments.toml"); + let environments = load_environments_toml(&path) + .expect("environments.toml") + .expect("environments.toml should exist"); assert_eq!(environments.default.as_deref(), Some("ssh-dev")); assert_eq!(environments.include_local, Some(false)); diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 281fd1eeb356..4969da822038 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.file_type().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), diff --git a/codex-rs/exec-server/tests/file_system_unix.rs b/codex-rs/exec-server/tests/file_system_unix.rs index f658045441f8..20d844d27c12 100644 --- a/codex-rs/exec-server/tests/file_system_unix.rs +++ b/codex-rs/exec-server/tests/file_system_unix.rs @@ -270,6 +270,31 @@ async fn file_system_get_metadata_reports_symlink_targets( Ok(()) } +#[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_broken_symlinks_as_missing( + implementation: FileSystemImplementation, +) -> Result<()> { + let context = create_file_system_context(implementation).await?; + let file_system = context.file_system; + + let tmp = TempDir::new()?; + let symlink_path = tmp.path().join("missing-link.txt"); + symlink(tmp.path().join("missing.txt"), &symlink_path)?; + + let error = file_system + .get_metadata( + &PathUri::from_host_native_path(&symlink_path)?, + /*sandbox*/ None, + ) + .await + .expect_err("broken symlink metadata should follow the missing target"); + assert_eq!(error.kind(), std::io::ErrorKind::NotFound); + + Ok(()) +} + #[test_case(FileSystemImplementation::Local ; "local")] #[test_case(FileSystemImplementation::Remote ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/codex-rs/file-search/src/lib.rs b/codex-rs/file-search/src/lib.rs index c49e57aeb976..f032653315e3 100644 --- a/codex-rs/file-search/src/lib.rs +++ b/codex-rs/file-search/src/lib.rs @@ -67,6 +67,11 @@ pub enum MatchType { Directory, } +struct IndexedEntry { + full_path: Arc, + match_type: MatchType, +} + impl FileMatch { pub fn full_path(&self) -> PathBuf { self.root.join(&self.path) @@ -411,7 +416,7 @@ fn get_file_path<'a>(path: &'a Path, search_directories: &[PathBuf]) -> Option<( fn walker_worker( inner: Arc, override_matcher: Option, - injector: Injector>, + injector: Injector, ) { let Some(first_root) = inner.search_directories.first() else { let _ = inner.work_tx.send(WorkSignal::WalkComplete); @@ -463,9 +468,19 @@ fn walker_worker( return ignore::WalkState::Continue; }; if let Some((_, relative_path)) = get_file_path(path, &search_directories) { - injector.push(Arc::from(full_path), |_, cols| { - cols[0] = Utf32String::from(relative_path); - }); + let match_type = match entry.file_type() { + Some(file_type) if file_type.is_dir() => MatchType::Directory, + _ => MatchType::File, + }; + injector.push( + IndexedEntry { + full_path: Arc::from(full_path), + match_type, + }, + |_, cols| { + cols[0] = Utf32String::from(relative_path); + }, + ); } n += 1; if n >= CHECK_INTERVAL { @@ -483,7 +498,7 @@ fn walker_worker( fn matcher_worker( inner: Arc, work_rx: Receiver, - mut nucleo: Nucleo>, + mut nucleo: Nucleo, ) -> anyhow::Result<()> { const TICK_TIMEOUT_MS: u64 = 10; let config = Config::DEFAULT.match_paths(); @@ -547,7 +562,7 @@ fn matcher_worker( .take(limit) .filter_map(|match_| { let item = snapshot.get_item(match_.idx)?; - let full_path = item.data.as_ref(); + let full_path = item.data.full_path.as_ref(); let (root_idx, relative_path) = get_file_path(Path::new(full_path), &inner.search_directories)?; let indices = if let Some(indices_matcher) = indices_matcher.as_mut() { let mut idx_vec = Vec::::new(); @@ -559,15 +574,10 @@ fn matcher_worker( } else { None }; - let match_type = if Path::new(full_path).is_dir() { - MatchType::Directory - } else { - MatchType::File - }; Some(FileMatch { score: match_.score, path: PathBuf::from(relative_path), - match_type, + match_type: item.data.match_type, root: inner.search_directories[root_idx].clone(), indices, }) @@ -1012,6 +1022,29 @@ mod tests { })); } + #[cfg(unix)] + #[test] + fn run_classifies_followed_directory_symlink_as_directory() { + use std::os::unix::fs::symlink; + + let dir = tempfile::tempdir().unwrap(); + fs::create_dir(dir.path().join("guides")).unwrap(); + symlink(dir.path().join("guides"), dir.path().join("guides-link")).unwrap(); + + let results = run( + "guides-link", + vec![dir.path().to_path_buf()], + FileSearchOptions::default(), + /*cancel_flag*/ None, + ) + .expect("run ok"); + + assert!(results.matches.iter().any(|file_match| { + file_match.path == Path::new("guides-link") + && file_match.match_type == MatchType::Directory + })); + } + #[test] fn cancel_exits_run() { let dir = create_temp_tree(/*file_count*/ 200); diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index dc8262f7db30..21bd08f43769 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -411,10 +411,6 @@ async fn connect_remote_app_server( #[cfg(unix)] async fn maybe_probe_default_daemon_socket(codex_home: &Path) -> Option { let socket_path = codex_app_server_client::app_server_control_socket_path(codex_home).ok()?; - if !socket_path.as_path().try_exists().unwrap_or(false) { - return None; - } - match tokio::time::timeout( AUTO_CONNECT_DAEMON_CONNECT_TIMEOUT, tokio::net::UnixStream::connect(socket_path.as_path()), diff --git a/codex-rs/utils/path-utils/src/lib.rs b/codex-rs/utils/path-utils/src/lib.rs index 71b0a6473ded..49b390771cfd 100644 --- a/codex-rs/utils/path-utils/src/lib.rs +++ b/codex-rs/utils/path-utils/src/lib.rs @@ -6,6 +6,7 @@ pub use env::is_wsl; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashSet; use std::io; +use std::io::Write; use std::path::Path; use std::path::PathBuf; use tempfile::NamedTempFile; @@ -126,8 +127,8 @@ pub fn write_atomically(write_path: &Path, contents: &str) -> io::Result<()> { ) })?; std::fs::create_dir_all(parent)?; - let tmp = NamedTempFile::new_in(parent)?; - std::fs::write(tmp.path(), contents)?; + let mut tmp = NamedTempFile::new_in(parent)?; + tmp.as_file_mut().write_all(contents.as_bytes())?; tmp.persist(write_path)?; Ok(()) }