Reuse McpToolOutput in McpHandler (#14229)

We already have a type to represent the MCP tool output, reuse it
instead of the custom McpHandlerOutput

codex-rs/core/src/tools/context.rs

@@ -7,10 +7,10 @@ use crate::truncate::TruncationPolicy;
 use crate::truncate::formatted_truncate_text;
 use crate::turn_diff_tracker::TurnDiffTracker;
 use crate::unified_exec::resolve_max_tokens;
-use codex_protocol::mcp::CallToolResult;
 use codex_protocol::models::FunctionCallOutputBody;
 use codex_protocol::models::FunctionCallOutputContentItem;
 use codex_protocol::models::FunctionCallOutputPayload;
+use codex_protocol::models::McpToolOutput;
 use codex_protocol::models::ResponseInputItem;
 use codex_protocol::models::ShellToolCallParams;
 use codex_protocol::models::function_call_output_content_items_to_text;
@@ -82,23 +82,21 @@ pub trait ToolOutput: Send {
     }
 }
 
-pub struct McpToolOutput {
-    pub result: Result<CallToolResult, String>,
-}
-
 impl ToolOutput for McpToolOutput {
     fn log_preview(&self) -> String {
-        format!("{:?}", self.result)
+        let output = self.as_function_call_output_payload();
+        let preview = output.body.to_text().unwrap_or_else(|| output.to_string());
+        telemetry_preview(&preview)
     }
 
     fn success_for_logging(&self) -> bool {
-        self.result.is_ok()
+        self.success
     }
 
     fn to_response_item(&self, call_id: &str, _payload: &ToolPayload) -> ResponseInputItem {
         ResponseInputItem::McpToolCallOutput {
             call_id: call_id.to_string(),
-            result: self.result.clone(),
+            output: self.clone(),
         }
     }
 }
@@ -273,15 +271,14 @@ fn response_input_to_code_mode_result(response: ResponseInputItem) -> JsonValue
                 content_items_to_code_mode_result(&items)
             }
         },
-        ResponseInputItem::McpToolCallOutput { result, .. } => match result {
-            Ok(result) => match FunctionCallOutputPayload::from(&result).body {
+        ResponseInputItem::McpToolCallOutput { output, .. } => {
+            match output.as_function_call_output_payload().body {
                 FunctionCallOutputBody::Text(text) => JsonValue::String(text),
                 FunctionCallOutputBody::ContentItems(items) => {
                     content_items_to_code_mode_result(&items)
                 }
-            },
-            Err(error) => JsonValue::String(error),
-        },
+            }
+        }
     }
 }
 

codex-rs/core/src/tools/handlers/mcp.rs

@@ -3,47 +3,16 @@ use std::sync::Arc;
 
 use crate::function_tool::FunctionCallError;
 use crate::mcp_tool_call::handle_mcp_tool_call;
-use crate::tools::context::FunctionToolOutput;
-use crate::tools::context::McpToolOutput;
 use crate::tools::context::ToolInvocation;
 use crate::tools::context::ToolPayload;
 use crate::tools::registry::ToolHandler;
 use crate::tools::registry::ToolKind;
-use codex_protocol::models::ResponseInputItem;
+use codex_protocol::models::McpToolOutput;
 
 pub struct McpHandler;
-
-pub enum McpHandlerOutput {
-    Mcp(McpToolOutput),
-    Function(FunctionToolOutput),
-}
-
-impl crate::tools::context::ToolOutput for McpHandlerOutput {
-    fn log_preview(&self) -> String {
-        match self {
-            Self::Mcp(output) => output.log_preview(),
-            Self::Function(output) => output.log_preview(),
-        }
-    }
-
-    fn success_for_logging(&self) -> bool {
-        match self {
-            Self::Mcp(output) => output.success_for_logging(),
-            Self::Function(output) => output.success_for_logging(),
-        }
-    }
-
-    fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem {
-        match self {
-            Self::Mcp(output) => output.to_response_item(call_id, payload),
-            Self::Function(output) => output.to_response_item(call_id, payload),
-        }
-    }
-}
-
 #[async_trait]
 impl ToolHandler for McpHandler {
-    type Output = McpHandlerOutput;
+    type Output = McpToolOutput;
 
     fn kind(&self) -> ToolKind {
         ToolKind::Mcp
@@ -74,7 +43,7 @@ impl ToolHandler for McpHandler {
         let (server, tool, raw_arguments) = payload;
         let arguments_str = raw_arguments;
 
-        let response = handle_mcp_tool_call(
+        let output = handle_mcp_tool_call(
             Arc::clone(&session),
             &turn,
             call_id.clone(),
@@ -84,26 +53,6 @@ impl ToolHandler for McpHandler {
         )
         .await;
 
-        match response {
-            ResponseInputItem::McpToolCallOutput { result, .. } => {
-                Ok(McpHandlerOutput::Mcp(McpToolOutput { result }))
-            }
-            ResponseInputItem::FunctionCallOutput { output, .. } => {
-                let success = output.success;
-                match output.body {
-                    codex_protocol::models::FunctionCallOutputBody::Text(text) => Ok(
-                        McpHandlerOutput::Function(FunctionToolOutput::from_text(text, success)),
-                    ),
-                    codex_protocol::models::FunctionCallOutputBody::ContentItems(content) => {
-                        Ok(McpHandlerOutput::Function(
-                            FunctionToolOutput::from_content(content, success),
-                        ))
-                    }
-                }
-            }
-            _ => Err(FunctionCallError::RespondToModel(
-                "mcp handler received unexpected response variant".to_string(),
-            )),
-        }
+        Ok(output)
     }
 }

codex-rs/core/src/tools/js_repl/mod.rs

@@ -620,29 +620,23 @@ impl JsReplManager {
                     output,
                 )
             }
-            ResponseInputItem::McpToolCallOutput { result, .. } => match result {
-                Ok(result) => {
-                    let output = FunctionCallOutputPayload::from(result);
-                    let mut summary = Self::summarize_function_output_payload(
-                        "mcp_tool_call_output",
-                        JsReplToolCallPayloadKind::McpResult,
-                        &output,
-                    );
-                    summary.payload_item_count = Some(result.content.len());
-                    summary.structured_content_present = Some(result.structured_content.is_some());
-                    summary.result_is_error = Some(result.is_error.unwrap_or(false));
-                    summary
-                }
-                Err(error) => {
-                    let mut summary = Self::summarize_text_payload(
-                        Some("mcp_tool_call_output"),
-                        JsReplToolCallPayloadKind::McpErrorResult,
-                        error,
-                    );
-                    summary.result_is_error = Some(true);
-                    summary
-                }
-            },
+            ResponseInputItem::McpToolCallOutput { output, .. } => {
+                let function_output = output.as_function_call_output_payload();
+                let payload_kind = if output.success {
+                    JsReplToolCallPayloadKind::McpResult
+                } else {
+                    JsReplToolCallPayloadKind::McpErrorResult
+                };
+                let mut summary = Self::summarize_function_output_payload(
+                    "mcp_tool_call_output",
+                    payload_kind,
+                    &function_output,
+                );
+                summary.payload_item_count = Some(output.content.len());
+                summary.structured_content_present = Some(output.structured_content.is_some());
+                summary.result_is_error = Some(!output.success);
+                summary
+            }
         }
     }
 

codex-rs/core/src/tools/parallel.rs

@@ -124,7 +124,9 @@ impl ToolCallRuntime {
             },
             ToolPayload::Mcp { .. } => ResponseInputItem::McpToolCallOutput {
                 call_id: call.call_id.clone(),
-                result: Err(Self::abort_message(call, secs)),
+                output: codex_protocol::models::McpToolOutput::from_error_text(
+                    Self::abort_message(call, secs),
+                ),
             },
             _ => ResponseInputItem::FunctionCallOutput {
                 call_id: call.call_id.clone(),