openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-08-23 13:09:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Avoid redundant filesystem metadata probes (#36898)

Browse Source 
## What changed

- Reuse directory-entry file types in local memory listing while continuing to
  exclude symlinks, and reuse rollout metadata when reading modification times.
- Avoid following non-symlinks twice in direct filesystem metadata and directory
  listing operations while preserving target classification for valid symlinks.

## Testing

- Cover symlink handling in local memory listing and search.
- Extend Unix filesystem tests for followed file and directory symlinks and
  dangling metadata links.

GitOrigin-RevId: e4e24576e2e9db704f9da54727928e121f81dc86
This commit is contained in:
Charlie Marsh
2026-08-04 13:51:19 +00:00
committed by copyberry
parent 17df7545a3
commit 40e5de94e9
7 changed files with 171 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
codex-rs/exec-server/src/local_file_system.rs
View File

@@ -596,12 +596,17 @@ impl DirectFileSystem {
) -> FileSystemResult<FileMetadata> {
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)

40
codex-rs/exec-server/tests/file_system_unix.rs
View File

@@ -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(())
}

16
codex-rs/ext/memories/src/local/list.rs
View File

@@ -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;

21
codex-rs/ext/memories/src/local/path.rs
View File

@@ -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<Vec<PathBuf>, MemoriesBackendError> {
) -> Result<Vec<(PathBuf, FileType)>, 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(

4
codex-rs/ext/memories/src/local/search.rs
View File

@@ -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;
}

74
codex-rs/ext/memories/src/tests.rs
View File

@@ -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::<Vec<_>>();
assert_eq!(paths, vec!["a.md", "nested/z.md"]);
}
#[tokio::test]
async fn search_tool_accepts_multiple_queries() {
let tempdir = tempfile::tempdir().expect("tempdir");

30
codex-rs/rollout/src/compression.rs
View File

@@ -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<Option<time::OffsetDateTime>> {
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<PathBuf> {
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<PathBuf> {
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
}