refactor(core): isolate openai file broker

Route openai/fileParams rewriting through an OpenAI file broker wrapper that keeps the current behavior while making file input resolution explicit for follow-up transports.

Co-authored-by: Codex <noreply@openai.com>

codex-rs/core/src/mcp_openai_file.rs

@@ -14,6 +14,7 @@ use crate::session::session::Session;
 use crate::session::turn_context::TurnContext;
 use codex_api::upload_local_file;
 use codex_login::CodexAuth;
+use codex_utils_absolute_path::AbsolutePathBuf;
 use serde_json::Value as JsonValue;
 
 pub(crate) async fn rewrite_mcp_tool_arguments_for_openai_files(
@@ -26,75 +27,162 @@ pub(crate) async fn rewrite_mcp_tool_arguments_for_openai_files(
         return Ok(arguments_value);
     };
 
-    let Some(arguments_value) = arguments_value else {
-        return Ok(None);
-    };
-    let Some(arguments) = arguments_value.as_object() else {
-        return Ok(Some(arguments_value));
-    };
     let auth = sess.services.auth_manager.auth().await;
-    let mut rewritten_arguments = arguments.clone();
-
-    for field_name in openai_file_input_params {
-        let Some(value) = arguments.get(field_name) else {
-            continue;
-        };
-        let Some(uploaded_value) =
-            rewrite_argument_value_for_openai_files(turn_context, auth.as_ref(), field_name, value)
-                .await?
-        else {
-            continue;
-        };
-        rewritten_arguments.insert(field_name.clone(), uploaded_value);
-    }
-
-    if rewritten_arguments == *arguments {
-        return Ok(Some(arguments_value));
-    }
-
-    Ok(Some(JsonValue::Object(rewritten_arguments)))
+    OpenAiFileBroker::new(turn_context, auth.as_ref())
+        .rewrite_tool_arguments(arguments_value, openai_file_input_params)
+        .await
 }
 
+struct OpenAiFileBroker<'a> {
+    turn_context: &'a TurnContext,
+    auth: Option<&'a CodexAuth>,
+}
+
+impl<'a> OpenAiFileBroker<'a> {
+    fn new(turn_context: &'a TurnContext, auth: Option<&'a CodexAuth>) -> Self {
+        Self { turn_context, auth }
+    }
+
+    async fn rewrite_tool_arguments(
+        &self,
+        arguments_value: Option<JsonValue>,
+        openai_file_input_params: &[String],
+    ) -> Result<Option<JsonValue>, String> {
+        let Some(arguments_value) = arguments_value else {
+            return Ok(None);
+        };
+        let Some(arguments) = arguments_value.as_object() else {
+            return Ok(Some(arguments_value));
+        };
+        let mut rewritten_arguments = arguments.clone();
+
+        for field_name in openai_file_input_params {
+            let Some(value) = arguments.get(field_name) else {
+                continue;
+            };
+            let Some(uploaded_value) = self.rewrite_argument_value(field_name, value).await? else {
+                continue;
+            };
+            rewritten_arguments.insert(field_name.clone(), uploaded_value);
+        }
+
+        if rewritten_arguments == *arguments {
+            return Ok(Some(arguments_value));
+        }
+
+        Ok(Some(JsonValue::Object(rewritten_arguments)))
+    }
+
+    async fn rewrite_argument_value(
+        &self,
+        field_name: &str,
+        value: &JsonValue,
+    ) -> Result<Option<JsonValue>, String> {
+        match value {
+            JsonValue::String(path_or_file_ref) => {
+                let rewritten = self
+                    .build_uploaded_argument_value(
+                        field_name,
+                        /*index*/ None,
+                        path_or_file_ref,
+                    )
+                    .await?;
+                Ok(Some(rewritten))
+            }
+            JsonValue::Array(values) => {
+                let mut rewritten_values = Vec::with_capacity(values.len());
+                for (index, item) in values.iter().enumerate() {
+                    let Some(path_or_file_ref) = item.as_str() else {
+                        return Ok(None);
+                    };
+                    let rewritten = self
+                        .build_uploaded_argument_value(field_name, Some(index), path_or_file_ref)
+                        .await?;
+                    rewritten_values.push(rewritten);
+                }
+                Ok(Some(JsonValue::Array(rewritten_values)))
+            }
+            _ => Ok(None),
+        }
+    }
+
+    async fn build_uploaded_argument_value(
+        &self,
+        field_name: &str,
+        index: Option<usize>,
+        file_path: &str,
+    ) -> Result<JsonValue, String> {
+        let file_input = FileBrokerInput::local_path(self.turn_context, file_path);
+        let Some(auth) = self.auth else {
+            return Err(
+                "ChatGPT auth is required to upload local files for Codex Apps tools".to_string(),
+            );
+        };
+        if !auth.uses_codex_backend() {
+            return Err(
+                "ChatGPT auth is required to upload local files for Codex Apps tools".to_string(),
+            );
+        }
+        let upload_auth = codex_model_provider::auth_provider_from_auth(auth);
+        let uploaded = upload_local_file(
+            self.turn_context
+                .config
+                .chatgpt_base_url
+                .trim_end_matches('/'),
+            upload_auth.as_ref(),
+            &file_input.resolved_path,
+        )
+        .await
+        .map_err(|error| match index {
+            Some(index) => {
+                format!(
+                    "failed to upload `{}` for `{field_name}[{index}]`: {error}",
+                    file_input.original
+                )
+            }
+            None => format!(
+                "failed to upload `{}` for `{field_name}`: {error}",
+                file_input.original
+            ),
+        })?;
+        Ok(serde_json::json!({
+            "download_url": uploaded.download_url,
+            "file_id": uploaded.file_id,
+            "mime_type": uploaded.mime_type,
+            "file_name": uploaded.file_name,
+            "uri": uploaded.uri,
+            "file_size_bytes": uploaded.file_size_bytes,
+        }))
+    }
+}
+
+struct FileBrokerInput<'a> {
+    original: &'a str,
+    resolved_path: AbsolutePathBuf,
+}
+
+impl<'a> FileBrokerInput<'a> {
+    fn local_path(turn_context: &TurnContext, path: &'a str) -> Self {
+        Self {
+            original: path,
+            resolved_path: turn_context.resolve_path(Some(path.to_string())),
+        }
+    }
+}
+
+#[cfg(test)]
 async fn rewrite_argument_value_for_openai_files(
     turn_context: &TurnContext,
     auth: Option<&CodexAuth>,
     field_name: &str,
     value: &JsonValue,
 ) -> Result<Option<JsonValue>, String> {
-    match value {
-        JsonValue::String(path_or_file_ref) => {
-            let rewritten = build_uploaded_local_argument_value(
-                turn_context,
-                auth,
-                field_name,
-                /*index*/ None,
-                path_or_file_ref,
-            )
-            .await?;
-            Ok(Some(rewritten))
-        }
-        JsonValue::Array(values) => {
-            let mut rewritten_values = Vec::with_capacity(values.len());
-            for (index, item) in values.iter().enumerate() {
-                let Some(path_or_file_ref) = item.as_str() else {
-                    return Ok(None);
-                };
-                let rewritten = build_uploaded_local_argument_value(
-                    turn_context,
-                    auth,
-                    field_name,
-                    Some(index),
-                    path_or_file_ref,
-                )
-                .await?;
-                rewritten_values.push(rewritten);
-            }
-            Ok(Some(JsonValue::Array(rewritten_values)))
-        }
-        _ => Ok(None),
-    }
+    OpenAiFileBroker::new(turn_context, auth)
+        .rewrite_argument_value(field_name, value)
+        .await
 }
 
+#[cfg(test)]
 async fn build_uploaded_local_argument_value(
     turn_context: &TurnContext,
     auth: Option<&CodexAuth>,
@@ -102,38 +190,9 @@ async fn build_uploaded_local_argument_value(
     index: Option<usize>,
     file_path: &str,
 ) -> Result<JsonValue, String> {
-    let resolved_path = turn_context.resolve_path(Some(file_path.to_string()));
-    let Some(auth) = auth else {
-        return Err(
-            "ChatGPT auth is required to upload local files for Codex Apps tools".to_string(),
-        );
-    };
-    if !auth.uses_codex_backend() {
-        return Err(
-            "ChatGPT auth is required to upload local files for Codex Apps tools".to_string(),
-        );
-    }
-    let upload_auth = codex_model_provider::auth_provider_from_auth(auth);
-    let uploaded = upload_local_file(
-        turn_context.config.chatgpt_base_url.trim_end_matches('/'),
-        upload_auth.as_ref(),
-        &resolved_path,
-    )
-    .await
-    .map_err(|error| match index {
-        Some(index) => {
-            format!("failed to upload `{file_path}` for `{field_name}[{index}]`: {error}")
-        }
-        None => format!("failed to upload `{file_path}` for `{field_name}`: {error}"),
-    })?;
-    Ok(serde_json::json!({
-        "download_url": uploaded.download_url,
-        "file_id": uploaded.file_id,
-        "mime_type": uploaded.mime_type,
-        "file_name": uploaded.file_name,
-        "uri": uploaded.uri,
-        "file_size_bytes": uploaded.file_size_bytes,
-    }))
+    OpenAiFileBroker::new(turn_context, auth)
+        .build_uploaded_argument_value(field_name, index, file_path)
+        .await
 }
 
 #[cfg(test)]

codex-rs/core/tests/suite/openai_file_mcp.rs

@@ -15,11 +15,13 @@ use core_test_support::apps_test_server::DOCUMENT_EXTRACT_TEXT_RESOURCE_URI;
 use core_test_support::hooks::trust_discovered_hooks;
 use core_test_support::responses::ev_assistant_message;
 use core_test_support::responses::ev_completed;
+use core_test_support::responses::ev_custom_tool_call;
 use core_test_support::responses::ev_function_call_with_namespace;
 use core_test_support::responses::ev_response_created;
 use core_test_support::responses::mount_sse_sequence;
 use core_test_support::responses::sse;
 use core_test_support::responses::start_mock_server;
+use core_test_support::skip_if_no_network;
 use core_test_support::test_codex::test_codex;
 use pretty_assertions::assert_eq;
 use serde_json::Value;
@@ -245,3 +247,139 @@ async fn codex_apps_file_params_upload_local_paths_before_mcp_tool_call() -> Res
     server.verify().await;
     Ok(())
 }
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn code_mode_nested_mcp_file_params_upload_local_paths_before_tool_call() -> Result<()> {
+    skip_if_no_network!(Ok(()));
+
+    let server = start_mock_server().await;
+    let apps_server = AppsTestServer::mount(&server).await?;
+
+    Mock::given(method("POST"))
+        .and(path("/files"))
+        .and(header("chatgpt-account-id", "account_id"))
+        .and(body_json(json!({
+            "file_name": "report.txt",
+            "file_size": 11,
+            "use_case": "codex",
+        })))
+        .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+            "file_id": "file_123",
+            "upload_url": format!("{}/upload/file_123", server.uri()),
+        })))
+        .expect(1)
+        .mount(&server)
+        .await;
+    Mock::given(method("PUT"))
+        .and(path("/upload/file_123"))
+        .and(header("content-length", "11"))
+        .respond_with(ResponseTemplate::new(200))
+        .expect(1)
+        .mount(&server)
+        .await;
+    Mock::given(method("POST"))
+        .and(path("/files/file_123/uploaded"))
+        .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+            "status": "success",
+            "download_url": format!("{}/download/file_123", server.uri()),
+            "file_name": "report.txt",
+            "mime_type": "text/plain",
+            "file_size_bytes": 11,
+        })))
+        .expect(1)
+        .mount(&server)
+        .await;
+
+    let code = r#"
+const result = await tools.mcp__codex_apps__calendar_extract_text({
+  file: "report.txt",
+});
+text(result.content?.[0]?.text ?? "missing result");
+"#;
+    mount_sse_sequence(
+        &server,
+        vec![
+            sse(vec![
+                ev_response_created("resp-1"),
+                ev_custom_tool_call("call-1", "exec", code),
+                ev_completed("resp-1"),
+            ]),
+            sse(vec![
+                ev_response_created("resp-2"),
+                ev_assistant_message("msg-1", "done"),
+                ev_completed("resp-2"),
+            ]),
+        ],
+    )
+    .await;
+
+    let mut builder = test_codex()
+        .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
+        .with_config(move |config| {
+            configure_apps(config, apps_server.chatgpt_base_url.as_str());
+            config
+                .features
+                .enable(Feature::CodeMode)
+                .expect("test config should allow feature update");
+        });
+    let test = builder.build(&server).await?;
+    tokio::fs::write(test.cwd.path().join("report.txt"), b"hello world").await?;
+
+    test.submit_turn_with_approval_and_permission_profile(
+        "Use code mode to extract the report text with the app tool.",
+        AskForApproval::Never,
+        PermissionProfile::Disabled,
+    )
+    .await?;
+
+    let apps_tool_call = server
+        .received_requests()
+        .await
+        .unwrap_or_default()
+        .into_iter()
+        .find_map(|request| {
+            let body: Value = serde_json::from_slice(&request.body).ok()?;
+            (request.url.path() == "/api/codex/apps"
+                && body.get("method").and_then(Value::as_str) == Some("tools/call")
+                && body.pointer("/params/name").and_then(Value::as_str)
+                    == Some("calendar_extract_text"))
+            .then_some(body)
+        })
+        .expect("apps calendar_extract_text tools/call request should be recorded");
+
+    assert_eq!(
+        apps_tool_call.pointer("/params/arguments/file"),
+        Some(&json!({
+            "download_url": format!("{}/download/file_123", server.uri()),
+            "file_id": "file_123",
+            "mime_type": "text/plain",
+            "file_name": "report.txt",
+            "uri": "sediment://file_123",
+            "file_size_bytes": 11,
+        }))
+    );
+    let codex_apps_meta = apps_tool_call
+        .pointer("/params/_meta/_codex_apps")
+        .expect("codex apps metadata should be forwarded");
+    assert!(
+        codex_apps_meta
+            .pointer("/call_id")
+            .and_then(Value::as_str)
+            .is_some_and(|call_id| call_id.starts_with("exec-"))
+    );
+    assert_eq!(
+        codex_apps_meta.pointer("/resource_uri"),
+        Some(&json!(DOCUMENT_EXTRACT_TEXT_RESOURCE_URI))
+    );
+    assert_eq!(
+        codex_apps_meta.pointer("/contains_mcp_source"),
+        Some(&json!(true))
+    );
+    assert_eq!(
+        codex_apps_meta.pointer("/connector_id"),
+        Some(&json!("calendar"))
+    );
+
+    server.verify().await;
+    Ok(())
+}