From 7579a2b41353470efaef93c08b4a21068a366b7f Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Wed, 29 Jul 2026 18:00:54 +0000 Subject: [PATCH] Resolve MCP file uploads with environment-native paths (#36002) ## Why File arguments for MCP app tools can use a path convention that differs from the host running Codex. Resolving those arguments with host-native path handling can therefore point uploads at the wrong file. ## What changed - Resolve file arguments against the selected turn environment's working directory with `PathUri::join`. - Use the resolved URI's inferred native spelling for errors and derive the uploaded filename without converting through a host-native path. ## Testing Run the existing MCP file upload integration test under Wine and reduce its streaming fixture to 2 MiB. GitOrigin-RevId: 0a659c19e73f5a26ddfc864e39dad6a4a404057b --- codex-rs/core/src/mcp_openai_file.rs | 29 ++++++++++---------- codex-rs/core/tests/suite/openai_file_mcp.rs | 6 +--- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/codex-rs/core/src/mcp_openai_file.rs b/codex-rs/core/src/mcp_openai_file.rs index 837a9af655..9075923115 100644 --- a/codex-rs/core/src/mcp_openai_file.rs +++ b/codex-rs/core/src/mcp_openai_file.rs @@ -17,7 +17,6 @@ use codex_api::OPENAI_FILE_UPLOAD_LIMIT_BYTES; use codex_api::upload_openai_file; use codex_http_client::RouteAwareClientPool; use codex_login::CodexAuth; -use codex_utils_path_uri::PathUri; use serde_json::Value as JsonValue; use std::collections::HashMap; @@ -139,14 +138,10 @@ async fn build_uploaded_argument_value( "no primary turn environment is available".to_string(), )); }; - // TODO(anp): Resolve app tool file arguments using the selected environment's native path - // convention so uploads can read relative paths from foreign environments. - let native_environment_cwd = turn_environment + let path_uri = turn_environment .cwd() - .to_abs_path() + .join(file_path) .map_err(|error| contextualize_error(error.to_string()))?; - let resolved_path = native_environment_cwd.join(file_path); - let path_uri = PathUri::from_abs_path(&resolved_path); let fs = turn_environment.environment.get_filesystem(); let metadata = fs .get_metadata(&path_uri, /*sandbox*/ None) @@ -155,13 +150,13 @@ async fn build_uploaded_argument_value( if !metadata.is_file { return Err(contextualize_error(format!( "path `{}` is not a file", - resolved_path.display() + path_uri.inferred_native_path_string() ))); } if metadata.size > OPENAI_FILE_UPLOAD_LIMIT_BYTES { return Err(contextualize_error(format!( "file `{}` is too large: {} bytes exceeds the limit of {} bytes", - resolved_path.display(), + path_uri.inferred_native_path_string(), metadata.size, OPENAI_FILE_UPLOAD_LIMIT_BYTES, ))); @@ -170,11 +165,17 @@ async fn build_uploaded_argument_value( .read_file_stream(&path_uri, /*sandbox*/ None) .await .map_err(|error| contextualize_error(error.to_string()))?; - let file_name = resolved_path - .file_name() - .and_then(|value| value.to_str()) - .unwrap_or("file") - .to_string(); + let file_name = path_uri + .basename() + .or_else(|| { + path_uri.infer_path_convention().and_then(|convention| { + convention + .path_segments(file_path) + .rfind(|segment| !segment.is_empty()) + .map(str::to_string) + }) + }) + .unwrap_or_else(|| "file".to_string()); let upload_auth = codex_model_provider::auth_provider_from_auth(auth); let uploaded = upload_openai_file( turn_context.config.chatgpt_base_url.trim_end_matches('/'), diff --git a/codex-rs/core/tests/suite/openai_file_mcp.rs b/codex-rs/core/tests/suite/openai_file_mcp.rs index 2390071ac9..dad1dbbd42 100644 --- a/codex-rs/core/tests/suite/openai_file_mcp.rs +++ b/codex-rs/core/tests/suite/openai_file_mcp.rs @@ -27,7 +27,6 @@ use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::namespace_child_tool; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; -use core_test_support::skip_if_wine_exec; use core_test_support::test_codex::TestCodex; use pretty_assertions::assert_eq; use serde_json::Value; @@ -40,7 +39,7 @@ use wiremock::matchers::header; use wiremock::matchers::method; use wiremock::matchers::path; -const STREAMED_FILE_SIZE: usize = 13 * 1024 * 1024; +const STREAMED_FILE_SIZE: usize = 2 * 1024 * 1024; fn write_post_tool_use_hook(home: &Path) -> Result<()> { let script_path = home.join("post_tool_use_hook.py"); @@ -181,9 +180,6 @@ async fn run_extract_turn(test: &TestCodex, server: &MockServer) -> Result Result<()> { - // TODO(anp): Remove after file-upload fixtures support target-native Windows paths. - skip_if_wine_exec!(Ok(()), "uses a host-native file-upload path"); - let server = start_mock_server().await; let apps_server = AppsTestServer::mount(&server).await?; mount_file_upload_mocks(&server, STREAMED_FILE_SIZE as u64).await;