Add Codex Apps sediment file remapping#15197
Conversation
479d6f8 to
ce24846
Compare
|
there's one real bug here: in also, can we split out the unrelated cleanup from this PR? the nextest / approval-matrix / code-mode timing changes, |
will investigate
wilco, was trying to remove as much of that as possible but kept breaking CI. if i can make it work without i'll try to remove |
|
Removing local fallback entirely, we should always assume it's |
e4acb53 to
3f800e2
Compare
f370d7f to
2c77356
Compare
| let interrupt_writer = writer_tx.clone(); | ||
| let interrupt_task = tokio::spawn(async move { | ||
| sleep(Duration::from_secs(2)).await; | ||
| let interrupt_delay = if cfg!(target_os = "macos") { |
There was a problem hiding this comment.
Can we clean up the unrelated diffs here and in other places.
| sess: Arc<Session>, | ||
| turn_context: &Arc<TurnContext>, | ||
| call_id: String, | ||
| qualified_tool: QualifiedMcpTool<'_>, |
There was a problem hiding this comment.
Why do we need change the core flow here? Can we move the file transformation logic out of the core flow as much as possible? Essentially you only need to rewrite a few special params and I don't think you have to change the whole core flow to achieve that.
| session, | ||
| turn, | ||
| call_id, | ||
| tool_name, |
There was a problem hiding this comment.
Same here, do we have to change the core flow?
| impl ToolSearchHandler { | ||
| pub fn new(tools: HashMap<String, ToolInfo>) -> Self { | ||
| Self { tools } | ||
| pub fn new(tools: HashMap<String, ToolInfo>, openai_file_bridge_enabled: bool) -> Self { |
There was a problem hiding this comment.
I don't think you need to add it to tool search.
There was a problem hiding this comment.
There's two main things that we have to make sure to expose to the model, and the tool results seems to be the best thing. First is that we need to modify the parameter so that it is showing the model file shape instead of the path. The second is that we need to indicate to the model that that is what is happening on both the upload and the download flows.
I've gone ahead and tried to streamline it as much as possible to isolate it to just the case that we're caring about. If you feel like we can go further I'm happy to take a shot.
|
|
||
| async fn wait_for_file_contents(path: &Path) -> Result<String> { | ||
| let deadline = Instant::now() + Duration::from_secs(5); | ||
| let file_wait_timeout = if cfg!(target_os = "macos") { |
There was a problem hiding this comment.
Also please clean up here.
codex-rs/core/src/codex.rs
Outdated
| } | ||
|
|
||
| impl Session { | ||
| fn add_session_openai_file_root_to_sandbox( |
There was a problem hiding this comment.
Can you confirm with @bolinfest if this is ok with the sandbox policy?
68912ab to
4557a14
Compare
Bridge Codex Apps file params and outputs through sediment handles, add a dedicated download tool, and harden test/runtime paths used during verification. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Tighten Codex Apps MCP file argument rewriting so remote handles must be explicit sediment:// URIs, and trim unrelated CI/test/runtime changes from this branch. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
a661bc0 to
c217e5c
Compare
Summary
apps_file_bridgefeature flagopenai/fileParamsfields and rewrite them into canonical OpenAI file payload objectsopenai/fileOutputssediment://{file_id}results into managed temp paths and expose the explicitdownload_openai_filetool when the feature is enabledNotes
sediment://...; barefile_*/file-*strings are treated as local paths.model_availability_nuxtest relaxation reproduced the local Bazel failureunexpected exit code from codex resume: 1; output: ^C, so that narrow allowance remains.suite::shell_snapshot::shell_command_snapshot_still_intercepts_apply_patchbecause removing it reproduced the macOS x86 local Bazel failuretimed out waiting for file .../snapshot-apply.txt.Verification
cargo test -p codex-core rewrite_single_argument_string_ -- --nocapturecargo test -p codex-core openai_file_ -- --nocapturecargo test -p codex-core mcp_tool_to_openai_tool_ -- --nocapturecargo test -p codex-core prompt_tools_are_consistent_across_requests -- --nocapturecargo test -p codex-core serialize_tool_search_output_tools_ -- --nocapturecargo test -p codex-features~/.cargo/bin/just fmt