mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Add size to internal filesystem metadata (#27927)
## Why `ExecutorFileSystem::get_metadata` reports file kind and timestamps but not size. Internal callers that need to enforce a size limit therefore have to read the complete file first, which is especially wasteful for remote filesystems. This adds the missing internal metadata so consumers can reject oversized files before transferring or buffering them. The field is named `size`, matching VS Code's `FileStat.size` filesystem convention. ## What changed - add `size: u64` to internal `FileMetadata` - populate it from the underlying filesystem metadata - carry it through sandbox-helper and remote exec-server responses - cover files, directories, symlink targets, and sandboxed reads across local and remote filesystem implementations The new field is intentionally not exposed through the app-server API. ## Testing - `just test -p codex-exec-server get_metadata` - `just test -p codex-exec-server file_system_sandboxed_metadata_and_read_allow_readable_root` - `just test -p codex-core-plugins` - `just test -p codex-skills-extension`
This commit is contained in:
committed by
GitHub
Unverified
parent
b6baa77eec
commit
76d8f20241
@@ -129,6 +129,7 @@ impl ExecutorFileSystem for SyntheticPluginFileSystem {
|
||||
is_directory,
|
||||
is_file,
|
||||
is_symlink: false,
|
||||
size: 0,
|
||||
created_at_ms: 0,
|
||||
modified_at_ms: 0,
|
||||
})
|
||||
|
||||
@@ -233,6 +233,7 @@ pub(crate) async fn run_direct_request(
|
||||
is_directory: metadata.is_directory,
|
||||
is_file: metadata.is_file,
|
||||
is_symlink: metadata.is_symlink,
|
||||
size: metadata.size,
|
||||
created_at_ms: metadata.created_at_ms,
|
||||
modified_at_ms: metadata.modified_at_ms,
|
||||
}))
|
||||
|
||||
@@ -483,6 +483,7 @@ impl DirectFileSystem {
|
||||
is_directory: metadata.is_dir(),
|
||||
is_file: metadata.is_file(),
|
||||
is_symlink: symlink_metadata.file_type().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),
|
||||
})
|
||||
|
||||
@@ -244,6 +244,7 @@ pub struct FsGetMetadataResponse {
|
||||
pub is_directory: bool,
|
||||
pub is_file: bool,
|
||||
pub is_symlink: bool,
|
||||
pub size: u64,
|
||||
pub created_at_ms: i64,
|
||||
pub modified_at_ms: i64,
|
||||
}
|
||||
|
||||
@@ -132,6 +132,7 @@ impl RemoteFileSystem {
|
||||
is_directory: response.is_directory,
|
||||
is_file: response.is_file,
|
||||
is_symlink: response.is_symlink,
|
||||
size: response.size,
|
||||
created_at_ms: response.created_at_ms,
|
||||
modified_at_ms: response.modified_at_ms,
|
||||
})
|
||||
|
||||
@@ -164,6 +164,7 @@ impl SandboxedFileSystem {
|
||||
is_directory: response.is_directory,
|
||||
is_file: response.is_file,
|
||||
is_symlink: response.is_symlink,
|
||||
size: response.size,
|
||||
created_at_ms: response.created_at_ms,
|
||||
modified_at_ms: response.modified_at_ms,
|
||||
})
|
||||
|
||||
@@ -103,6 +103,7 @@ impl FileSystemHandler {
|
||||
is_directory: metadata.is_directory,
|
||||
is_file: metadata.is_file,
|
||||
is_symlink: metadata.is_symlink,
|
||||
size: metadata.size,
|
||||
created_at_ms: metadata.created_at_ms,
|
||||
modified_at_ms: metadata.modified_at_ms,
|
||||
})
|
||||
|
||||
@@ -2,6 +2,7 @@ use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_exec_server::CopyOptions;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::FileMetadata;
|
||||
use codex_exec_server::ReadDirectoryEntry;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
@@ -65,18 +66,34 @@ async fn file_system_get_metadata_reports_files_and_directories(
|
||||
.get_metadata(&PathUri::from_path(&file_path)?, /*sandbox*/ None)
|
||||
.await
|
||||
.with_context(|| format!("mode={implementation}"))?;
|
||||
assert_eq!(file_metadata.is_directory, false);
|
||||
assert_eq!(file_metadata.is_file, true);
|
||||
assert_eq!(file_metadata.is_symlink, false);
|
||||
assert_eq!(
|
||||
file_metadata,
|
||||
FileMetadata {
|
||||
is_directory: false,
|
||||
is_file: true,
|
||||
is_symlink: false,
|
||||
size: 5,
|
||||
created_at_ms: file_metadata.created_at_ms,
|
||||
modified_at_ms: file_metadata.modified_at_ms,
|
||||
}
|
||||
);
|
||||
assert!(file_metadata.modified_at_ms > 0);
|
||||
|
||||
let directory_metadata = file_system
|
||||
.get_metadata(&PathUri::from_path(&directory_path)?, /*sandbox*/ None)
|
||||
.await
|
||||
.with_context(|| format!("mode={implementation}"))?;
|
||||
assert_eq!(directory_metadata.is_directory, true);
|
||||
assert_eq!(directory_metadata.is_file, false);
|
||||
assert_eq!(directory_metadata.is_symlink, false);
|
||||
assert_eq!(
|
||||
directory_metadata,
|
||||
FileMetadata {
|
||||
is_directory: true,
|
||||
is_file: false,
|
||||
is_symlink: false,
|
||||
size: std::fs::metadata(&directory_path)?.len(),
|
||||
created_at_ms: directory_metadata.created_at_ms,
|
||||
modified_at_ms: directory_metadata.modified_at_ms,
|
||||
}
|
||||
);
|
||||
assert!(directory_metadata.modified_at_ms > 0);
|
||||
|
||||
Ok(())
|
||||
@@ -395,7 +412,7 @@ async fn file_system_copy_rejects_directory_without_recursive(
|
||||
#[test_case(FileSystemImplementation::Local ; "local")]
|
||||
#[test_case(FileSystemImplementation::Remote ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn file_system_sandboxed_read_allows_readable_root(
|
||||
async fn file_system_sandboxed_metadata_and_read_allow_readable_root(
|
||||
implementation: FileSystemImplementation,
|
||||
) -> Result<()> {
|
||||
let context = create_file_system_context(implementation).await?;
|
||||
@@ -408,6 +425,22 @@ async fn file_system_sandboxed_read_allows_readable_root(
|
||||
std::fs::write(&file_path, "sandboxed hello")?;
|
||||
let sandbox = read_only_sandbox(allowed_dir);
|
||||
|
||||
let metadata = file_system
|
||||
.get_metadata(&PathUri::from_path(&file_path)?, Some(&sandbox))
|
||||
.await
|
||||
.with_context(|| format!("mode={implementation}"))?;
|
||||
assert_eq!(
|
||||
metadata,
|
||||
FileMetadata {
|
||||
is_directory: false,
|
||||
is_file: true,
|
||||
is_symlink: false,
|
||||
size: 15,
|
||||
created_at_ms: metadata.created_at_ms,
|
||||
modified_at_ms: metadata.modified_at_ms,
|
||||
}
|
||||
);
|
||||
|
||||
let contents = file_system
|
||||
.read_file(&PathUri::from_path(&file_path)?, Some(&sandbox))
|
||||
.await
|
||||
|
||||
@@ -21,6 +21,7 @@ use codex_exec_server::CopyOptions;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
#[cfg(target_os = "linux")]
|
||||
use codex_exec_server::Environment;
|
||||
use codex_exec_server::FileMetadata;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -222,9 +223,17 @@ async fn file_system_get_metadata_reports_symlink_targets(
|
||||
.get_metadata(&PathUri::from_path(&symlink_path)?, /*sandbox*/ None)
|
||||
.await
|
||||
.with_context(|| format!("mode={implementation}"))?;
|
||||
assert_eq!(symlink_metadata.is_directory, false);
|
||||
assert_eq!(symlink_metadata.is_file, true);
|
||||
assert_eq!(symlink_metadata.is_symlink, true);
|
||||
assert_eq!(
|
||||
symlink_metadata,
|
||||
FileMetadata {
|
||||
is_directory: false,
|
||||
is_file: true,
|
||||
is_symlink: true,
|
||||
size: 5,
|
||||
created_at_ms: symlink_metadata.created_at_ms,
|
||||
modified_at_ms: symlink_metadata.modified_at_ms,
|
||||
}
|
||||
);
|
||||
assert!(symlink_metadata.modified_at_ms > 0);
|
||||
|
||||
let dir_path = tmp.path().join("notes");
|
||||
@@ -238,9 +247,17 @@ async fn file_system_get_metadata_reports_symlink_targets(
|
||||
)
|
||||
.await
|
||||
.with_context(|| format!("mode={implementation}"))?;
|
||||
assert_eq!(dir_symlink_metadata.is_directory, true);
|
||||
assert_eq!(dir_symlink_metadata.is_file, false);
|
||||
assert_eq!(dir_symlink_metadata.is_symlink, true);
|
||||
assert_eq!(
|
||||
dir_symlink_metadata,
|
||||
FileMetadata {
|
||||
is_directory: true,
|
||||
is_file: false,
|
||||
is_symlink: true,
|
||||
size: std::fs::metadata(&dir_path)?.len(),
|
||||
created_at_ms: dir_symlink_metadata.created_at_ms,
|
||||
modified_at_ms: dir_symlink_metadata.modified_at_ms,
|
||||
}
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -85,6 +85,7 @@ impl SyntheticFileSystem {
|
||||
is_directory,
|
||||
is_file,
|
||||
is_symlink: false,
|
||||
size: 0,
|
||||
created_at_ms: 0,
|
||||
modified_at_ms: 0,
|
||||
})
|
||||
|
||||
@@ -34,6 +34,8 @@ pub struct FileMetadata {
|
||||
pub is_directory: bool,
|
||||
pub is_file: bool,
|
||||
pub is_symlink: bool,
|
||||
/// Size in bytes.
|
||||
pub size: u64,
|
||||
pub created_at_ms: i64,
|
||||
pub modified_at_ms: i64,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user