From 3a9df58d0611e51dc059d2e9175f4b338d8b99a1 Mon Sep 17 00:00:00 2001 From: Rennie Date: Tue, 21 Apr 2026 10:09:46 -0700 Subject: [PATCH] Propagate thread id in MCP tool metadata (#18093) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - attach the authoritative Codex thread id to MCP tool request `_meta.threadId` for model-initiated tool calls - attach the same thread id for manual `mcpServer/tool/call` requests before invoking the MCP server - cover both metadata helper behavior and the manual app-server MCP path in tests needed because the Rust app-server is the last place that still has authoritative knowledge of “this model-generated MCP tool call belongs to conversation/thread X” before the request leaves Codex and reaches Hoopa. It adds threadId to MCP request metadata in the model-generated tool-call path, using sess.conversation_id, and also does the same for the manual mcpServer/tool/call path. ## Test plan - `cargo test -p codex-core mcp_tool_call_thread_id_meta_is_added_to_request_meta --lib` - `cargo test -p codex-app-server mcp_server_tool_call_returns_tool_result` Paired Hoopa consumer PR: https://github.com/openai/openai/pull/833263 --- .../app-server/src/codex_message_processor.rs | 32 +++++++++++++++++-- .../app-server/tests/suite/v2/mcp_tool.rs | 13 ++++++-- codex-rs/core/src/mcp_tool_call.rs | 27 ++++++++++++++++ codex-rs/core/src/mcp_tool_call_tests.rs | 29 +++++++++++++++++ 4 files changed, 97 insertions(+), 4 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index f522908be..a60a07989 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -6046,17 +6046,19 @@ impl CodexMessageProcessor { params: McpServerToolCallParams, ) { let outgoing = Arc::clone(&self.outgoing); - let (_, thread) = match self.load_thread(¶ms.thread_id).await { + let thread_id = params.thread_id.clone(); + let (_, thread) = match self.load_thread(&thread_id).await { Ok(thread) => thread, Err(error) => { self.outgoing.send_error(request_id, error).await; return; } }; + let meta = with_mcp_tool_call_thread_id_meta(params.meta, &thread_id); tokio::spawn(async move { let result = thread - .call_mcp_tool(¶ms.server, ¶ms.tool, params.arguments, params.meta) + .call_mcp_tool(¶ms.server, ¶ms.tool, params.arguments, meta) .await; match result { Ok(result) => { @@ -9896,6 +9898,32 @@ fn thread_store_archive_error(operation: &str, err: ThreadStoreError) -> JSONRPC } } +const MCP_TOOL_THREAD_ID_META_KEY: &str = "threadId"; + +fn with_mcp_tool_call_thread_id_meta( + meta: Option, + thread_id: &str, +) -> Option { + match meta { + Some(serde_json::Value::Object(mut map)) => { + map.insert( + MCP_TOOL_THREAD_ID_META_KEY.to_string(), + serde_json::Value::String(thread_id.to_string()), + ); + Some(serde_json::Value::Object(map)) + } + None => { + let mut map = serde_json::Map::new(); + map.insert( + MCP_TOOL_THREAD_ID_META_KEY.to_string(), + serde_json::Value::String(thread_id.to_string()), + ); + Some(serde_json::Value::Object(map)) + } + other => other, + } +} + fn summary_from_stored_thread( thread: StoredThread, fallback_provider: &str, diff --git a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs index 87556c836..8a323efce 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -83,10 +83,11 @@ url = "{mcp_server_url}/mcp" ) .await??; let ThreadStartResponse { thread, .. } = to_response(thread_start_resp)?; + let thread_id = thread.id.clone(); let tool_call_request_id = mcp .send_mcp_server_tool_call_request(McpServerToolCallParams { - thread_id: thread.id, + thread_id: thread_id.clone(), server: TEST_SERVER_NAME.to_string(), tool: TEST_TOOL_NAME.to_string(), arguments: Some(json!({ @@ -114,6 +115,7 @@ url = "{mcp_server_url}/mcp" response.structured_content, Some(json!({ "echoed": "hello from app", + "threadId": thread_id, })) ); assert_eq!(response.is_error, Some(false)); @@ -203,7 +205,7 @@ impl ServerHandler for ToolAppsMcpServer { async fn call_tool( &self, request: CallToolRequestParams, - _context: RequestContext, + context: RequestContext, ) -> Result { assert_eq!(request.name.as_ref(), TEST_TOOL_NAME); let message = request @@ -212,12 +214,19 @@ impl ServerHandler for ToolAppsMcpServer { .and_then(|arguments| arguments.get("message")) .and_then(|value| value.as_str()) .unwrap_or_default(); + let thread_id = context + .meta + .0 + .get("threadId") + .and_then(|value| value.as_str()) + .unwrap_or_default(); let mut meta = Meta::new(); meta.0.insert("calledBy".to_string(), json!("mcp-app")); let mut result = CallToolResult::structured(json!({ "echoed": message, + "threadId": thread_id, })); result.content = vec![Content::text(format!("echo: {message}"))]; result.meta = Some(meta); diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index c5d4068d2..82bbc7ca4 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -477,6 +477,8 @@ async fn execute_mcp_tool_call( metadata.and_then(|metadata| metadata.openai_file_input_params.as_deref()), ) .await?; + let request_meta = + with_mcp_tool_call_thread_id_meta(request_meta, &sess.conversation_id.to_string()); let request_meta = augment_mcp_tool_request_meta_with_sandbox_state(sess, turn_context, server, request_meta) .await @@ -660,6 +662,7 @@ pub(crate) struct McpToolApprovalMetadata { const MCP_TOOL_CODEX_APPS_META_KEY: &str = "_codex_apps"; const MCP_TOOL_OPENAI_OUTPUT_TEMPLATE_META_KEY: &str = "openai/outputTemplate"; const MCP_TOOL_UI_RESOURCE_URI_META_KEY: &str = "ui/resourceUri"; +const MCP_TOOL_THREAD_ID_META_KEY: &str = "threadId"; fn custom_mcp_tool_approval_mode( turn_context: &TurnContext, @@ -714,6 +717,30 @@ fn build_mcp_tool_call_request_meta( (!request_meta.is_empty()).then_some(serde_json::Value::Object(request_meta)) } +fn with_mcp_tool_call_thread_id_meta( + meta: Option, + thread_id: &str, +) -> Option { + match meta { + Some(serde_json::Value::Object(mut map)) => { + map.insert( + MCP_TOOL_THREAD_ID_META_KEY.to_string(), + serde_json::Value::String(thread_id.to_string()), + ); + Some(serde_json::Value::Object(map)) + } + None => { + let mut map = serde_json::Map::new(); + map.insert( + MCP_TOOL_THREAD_ID_META_KEY.to_string(), + serde_json::Value::String(thread_id.to_string()), + ); + Some(serde_json::Value::Object(map)) + } + other => other, + } +} + #[derive(Clone, Copy)] struct McpToolApprovalPromptOptions { allow_session_remember: bool, diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 1893ef054..d20f23395 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -651,6 +651,35 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps ); } +#[test] +fn mcp_tool_call_thread_id_meta_is_added_to_request_meta() { + assert_eq!( + with_mcp_tool_call_thread_id_meta( + Some(serde_json::json!({ + "source": "test-client", + "threadId": "stale-thread", + })), + "thread-live", + ), + Some(serde_json::json!({ + "source": "test-client", + "threadId": "thread-live", + })) + ); + + assert_eq!( + with_mcp_tool_call_thread_id_meta(/*meta*/ None, "thread-live"), + Some(serde_json::json!({ + "threadId": "thread-live", + })) + ); + + assert_eq!( + with_mcp_tool_call_thread_id_meta(Some(serde_json::json!("invalid-meta")), "thread-live"), + Some(serde_json::json!("invalid-meta")) + ); +} + #[test] fn accepted_elicitation_content_converts_to_request_user_input_response() { let response = request_user_input_response_from_elicitation_content(Some(serde_json::json!(