[codex] Add size to internal filesystem metadata#27927
Conversation
pakrym-oai
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 138e4eeeea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pub is_directory: bool, | ||
| pub is_file: bool, | ||
| pub is_symlink: bool, | ||
| pub size: u64, |
There was a problem hiding this comment.
With mixed-version remotes, older exec-server fs/getMetadata replies omit size; making it required makes serde fail before RemoteFileSystem::get_metadata returns metadata. Please default/optionalize this wire field or negotiate protocol while filling it when available. guidance
Useful? React with 👍 / 👎.
2 participants
Why
ExecutorFileSystem::get_metadatareports 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'sFileStat.sizefilesystem convention.What changed
size: u64to internalFileMetadataThe new field is intentionally not exposed through the app-server API.
Testing
just test -p codex-exec-server get_metadatajust test -p codex-exec-server file_system_sandboxed_metadata_and_read_allow_readable_rootjust test -p codex-core-pluginsjust test -p codex-skills-extension