From 4c65f72117f160f0badfbfdfef3e1fa87942b81e Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 15 Jan 2026 17:31:15 -0800 Subject: [PATCH] fix(mcp): include threadId in both content and structuredContent in CallToolResult --- codex-rs/docs/codex_mcp_interface.md | 19 ++++++++++++ codex-rs/mcp-server/src/codex_tool_runner.rs | 30 +++++++++++++------ codex-rs/mcp-server/src/message_processor.rs | 16 ++++------ codex-rs/mcp-server/tests/suite/codex_tool.rs | 21 +++++++++++++ 4 files changed, 66 insertions(+), 20 deletions(-) diff --git a/codex-rs/docs/codex_mcp_interface.md b/codex-rs/docs/codex_mcp_interface.md index edd5ac1b2d..14a8727001 100644 --- a/codex-rs/docs/codex_mcp_interface.md +++ b/codex-rs/docs/codex_mcp_interface.md @@ -105,6 +105,25 @@ While a conversation runs, the server sends notifications: Clients should render events and, when present, surface approval requests (see next section). +## Tool responses + +The `codex` and `codex-reply` tools return standard MCP `CallToolResult` payloads. For +compatibility with MCP clients that prefer `structuredContent`, Codex mirrors the +assistant text inside `structuredContent` alongside the `threadId`. + +Example: + +```json +{ + "content": [{ "type": "text", "text": "Hello from Codex" }], + "structuredContent": { + "threadId": "019bbed6-1e9e-7f31-984c-a05b65045719", + "text": "Hello from Codex", + "content": [{ "type": "text", "text": "Hello from Codex" }] + } +} +``` + ## Approvals (server → client) When Codex needs approval to apply changes or run commands, the server issues JSON‑RPC requests to the client: diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 6aafdf6de9..d5c837041e 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -34,21 +34,27 @@ pub(crate) const INVALID_PARAMS_ERROR_CODE: i64 = -32602; /// To adhere to MCP `tools/call` response format, include the Codex /// `threadId` in the `structured_content` field of the response. -fn create_call_tool_result_with_thread_id( +/// Some MCP clients ignore `content` when `structuredContent` is present, so +/// mirror the text content there as well. +pub(crate) fn create_call_tool_result_with_thread_id( thread_id: ThreadId, text: String, is_error: Option, ) -> CallToolResult { + let content = vec![ContentBlock::TextContent(TextContent { + r#type: "text".to_string(), + text: text.clone(), + annotations: None, + })]; + let structured_content = json!({ + "threadId": thread_id, + "text": text, + "content": content.clone(), + }); CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_string(), - text, - annotations: None, - })], + content, is_error, - structured_content: Some(json!({ - "threadId": thread_id, - })), + structured_content: Some(structured_content), } } @@ -398,6 +404,12 @@ mod tests { result.structured_content, Some(json!({ "threadId": thread_id, + "text": "done", + "content": vec![ContentBlock::TextContent(TextContent { + r#type: "text".to_string(), + text: "done".to_string(), + annotations: None, + })], })) ); } diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index 33bad85d75..9d947cda3f 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -498,17 +498,11 @@ impl MessageProcessor { Ok(c) => c, Err(_) => { tracing::warn!("Session not found for thread_id: {thread_id}"); - let result = CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_owned(), - text: format!("Session not found for thread_id: {thread_id}"), - annotations: None, - })], - is_error: Some(true), - structured_content: Some(json!({ - "threadId": thread_id, - })), - }; + let result = crate::codex_tool_runner::create_call_tool_result_with_thread_id( + thread_id, + format!("Session not found for thread_id: {thread_id}"), + Some(true), + ); outgoing.send_response(request_id, result).await; return; } diff --git a/codex-rs/mcp-server/tests/suite/codex_tool.rs b/codex-rs/mcp-server/tests/suite/codex_tool.rs index dfe3651205..30980f3958 100644 --- a/codex-rs/mcp-server/tests/suite/codex_tool.rs +++ b/codex-rs/mcp-server/tests/suite/codex_tool.rs @@ -162,6 +162,13 @@ async fn shell_command_approval_triggers_elicitation() -> anyhow::Result<()> { ], "structuredContent": { "threadId": params.thread_id, + "text": "File created!", + "content": [ + { + "text": "File created!", + "type": "text" + } + ] } }), }, @@ -323,6 +330,13 @@ async fn patch_approval_triggers_elicitation() -> anyhow::Result<()> { ], "structuredContent": { "threadId": params.thread_id, + "text": "Patch has been applied successfully!", + "content": [ + { + "text": "Patch has been applied successfully!", + "type": "text" + } + ] } }), }, @@ -394,6 +408,13 @@ async fn codex_tool_passes_base_instructions() -> anyhow::Result<()> { .and_then(|v| v.get("threadId")) .and_then(serde_json::Value::as_str) .expect("codex tool response should include structuredContent.threadId"), + "text": "Enjoy!", + "content": [ + { + "text": "Enjoy!", + "type": "text" + } + ] } }) );