Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions codex-rs/exec-server/src/local_file_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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)
Expand Down
40 changes: 39 additions & 1 deletion codex-rs/exec-server/tests/file_system_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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?;
Expand Down Expand Up @@ -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(())
}

Expand Down
16 changes: 5 additions & 11 deletions codex-rs/ext/memories/src/local/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand Down
21 changes: 15 additions & 6 deletions codex-rs/ext/memories/src/local/path.rs
Original file line number Diff line number Diff line change
@@ -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(
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/ext/memories/src/local/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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;
}
Expand Down
74 changes: 74 additions & 0 deletions codex-rs/ext/memories/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
30 changes: 20 additions & 10 deletions codex-rs/rollout/src/compression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
}
Expand Down
Loading