From 14d515700be9222ac9bb186cc40ccea9453ac60b Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 22 Aug 2025 13:34:28 -0700 Subject: [PATCH] fix: prefer sending MCP structuredContent as the function call response, if available --- codex-rs/core/src/codex.rs | 55 +++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 7d616f96e0..bd6b1df208 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1403,24 +1403,21 @@ async fn run_task( Some(ResponseInputItem::McpToolCallOutput { call_id, result }), ) => { items_to_record_in_conversation_history.push(item); - let (content, success): (String, Option) = match result { - Ok(CallToolResult { - content, - is_error, - structured_content: _, - }) => match serde_json::to_string(content) { - Ok(content) => (content, *is_error), - Err(e) => { - warn!("Failed to serialize MCP tool call output: {e}"); - (e.to_string(), Some(true)) - } + let output = match result { + Ok(call_tool_result) => { + convert_call_tool_result_to_function_call_output_payload( + call_tool_result, + ) + } + Err(err) => FunctionCallOutputPayload { + content: err.clone(), + success: Some(false), }, - Err(e) => (e.clone(), Some(true)), }; items_to_record_in_conversation_history.push( ResponseItem::FunctionCallOutput { call_id: call_id.clone(), - output: FunctionCallOutputPayload { content, success }, + output, }, ); } @@ -2546,3 +2543,35 @@ async fn drain_to_completed( } } } + +fn convert_call_tool_result_to_function_call_output_payload( + call_tool_result: &CallToolResult, +) -> FunctionCallOutputPayload { + let CallToolResult { + content, + is_error, + structured_content, + } = call_tool_result; + + // In terms of what to send back to the model, we prefer structured_content, + // if available, and fallback to content, otherwise. + let mut is_success = is_error != &Some(true); + let content = if let Some(structured_content) = structured_content + && structured_content != &serde_json::Value::Null + && let Ok(serialized_structured_content) = serde_json::to_string(&structured_content) + { + serialized_structured_content + } else if let Ok(serialized_content) = serde_json::to_string(&content) { + serialized_content + } else { + // If we could not serialize either content or structured_content to + // JSON, flag this as an error. + is_success = false; + "".to_owned() + }; + + FunctionCallOutputPayload { + content, + success: Some(is_success), + } +}