core/core-agent-ide
8
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Reuse McpToolOutput in McpHandler (#14229)

Browse source 
We already have a type to represent the MCP tool output, reuse it
instead of the custom McpHandlerOutput
This commit is contained in:
pakrym-oai 2026-03-10 10:41:41 -07:00 • committed by Michael Bolin
parent 52a7f4b68b
commit c4d35084f5
8 changed files with 119 additions and 141 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
codex-rs/core/src/codex_tests.rs
View file

@ -58,6 +58,7 @@ use codex_app_server_protocol::AppInfo;
use codex_otel::TelemetryAuthMode;
use codex_protocol::models::BaseInstructions;
use codex_protocol::models::ContentItem;
use codex_protocol::models::McpToolOutput;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ModelsResponse;
@ -1607,7 +1608,7 @@ fn prefers_structured_content_when_present() {
meta: None,
};
let got = FunctionCallOutputPayload::from(&ctr);
let got = McpToolOutput::from(&ctr).into_function_call_output_payload();
let expected = FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(
serde_json::to_string(&json!({
@ -1689,7 +1690,7 @@ fn falls_back_to_content_when_structured_is_null() {
meta: None,
};
let got = FunctionCallOutputPayload::from(&ctr);
let got = McpToolOutput::from(&ctr).into_function_call_output_payload();
let expected = FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(
serde_json::to_string(&vec![text_block("hello"), text_block("world")]).unwrap(),
@ -1709,7 +1710,7 @@ fn success_flag_reflects_is_error_true() {
meta: None,
};
let got = FunctionCallOutputPayload::from(&ctr);
let got = McpToolOutput::from(&ctr).into_function_call_output_payload();
let expected = FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(
serde_json::to_string(&json!({ "message": "bad" })).unwrap(),
@ -1729,7 +1730,7 @@ fn success_flag_true_with_no_error_and_content_used() {
meta: None,
};
let got = FunctionCallOutputPayload::from(&ctr);
let got = McpToolOutput::from(&ctr).into_function_call_output_payload();
let expected = FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(
serde_json::to_string(&vec![text_block("alpha")]).unwrap(),

20
codex-rs/core/src/mcp_tool_call.rs
View file

@ -29,9 +29,7 @@ use crate::protocol::McpToolCallBeginEvent;
use crate::protocol::McpToolCallEndEvent;
use crate::state_db;
use codex_protocol::mcp::CallToolResult;
use codex_protocol::models::FunctionCallOutputBody;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::McpToolOutput;
use codex_protocol::openai_models::InputModality;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
@ -58,7 +56,7 @@ pub(crate) async fn handle_mcp_tool_call(
server: String,
tool_name: String,
arguments: String,
) -> ResponseInputItem {
) -> McpToolOutput {
// Parse the `arguments` as JSON. An empty string is OK, but invalid JSON
// is not.
let arguments_value = if arguments.trim().is_empty() {
@ -68,13 +66,7 @@ pub(crate) async fn handle_mcp_tool_call(
Ok(value) => Some(value),
Err(e) => {
error!("failed to parse tool call arguments: {e}");
return ResponseInputItem::FunctionCallOutput {
call_id: call_id.clone(),
output: FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(format!("err: {e}")),
success: Some(false),
},
};
return McpToolOutput::from_error_text(format!("err: {e}"));
}
}
};
@ -118,7 +110,7 @@ pub(crate) async fn handle_mcp_tool_call(
turn_context
.session_telemetry
.counter("codex.mcp.call", 1, &[("status", status)]);
return ResponseInputItem::McpToolCallOutput { call_id, result };
return McpToolOutput::from_result(result);
}
if let Some(decision) = maybe_request_mcp_tool_approval(
@ -212,7 +204,7 @@ pub(crate) async fn handle_mcp_tool_call(
.session_telemetry
.counter("codex.mcp.call", 1, &[("status", status)]);
return ResponseInputItem::McpToolCallOutput { call_id, result };
return McpToolOutput::from_result(result);
}
let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent {
@ -258,7 +250,7 @@ pub(crate) async fn handle_mcp_tool_call(
.session_telemetry
.counter("codex.mcp.call", 1, &[("status", status)]);
ResponseInputItem::McpToolCallOutput { call_id, result }
McpToolOutput::from_result(result)
}
async fn maybe_mark_thread_memory_mode_polluted(sess: &Session, turn_context: &TurnContext) {

10
codex-rs/core/src/stream_events_utils.rs
View file

@ -359,14 +359,8 @@ pub(crate) fn response_input_to_response_item(input: &ResponseInputItem) -> Opti
output: output.clone(),
})
}
ResponseInputItem::McpToolCallOutput { call_id, result } => {
let output = match result {
Ok(call_tool_result) => FunctionCallOutputPayload::from(call_tool_result),
Err(err) => FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(err.clone()),
success: Some(false),
},
};
ResponseInputItem::McpToolCallOutput { call_id, output } => {
let output = output.as_function_call_output_payload();
Some(ResponseItem::FunctionCallOutput {
call_id: call_id.clone(),
output,

23
codex-rs/core/src/tools/context.rs
View file

@ -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),
},
}
}
}
}

59
codex-rs/core/src/tools/handlers/mcp.rs
View file

@ -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)
}
}

40
codex-rs/core/src/tools/js_repl/mod.rs
View file

@ -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
}
}
}

4
codex-rs/core/src/tools/parallel.rs
View file

@ -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(),

95
codex-rs/protocol/src/models.rs
View file

@ -206,7 +206,7 @@ pub enum ResponseInputItem {
},
McpToolCallOutput {
call_id: String,
result: Result<CallToolResult, String>,
output: McpToolOutput,
},
CustomToolCallOutput {
call_id: String,
@ -843,14 +843,8 @@ impl From<ResponseInputItem> for ResponseItem {
ResponseInputItem::FunctionCallOutput { call_id, output } => {
Self::FunctionCallOutput { call_id, output }
}
ResponseInputItem::McpToolCallOutput { call_id, result } => {
let output = match result {
Ok(result) => FunctionCallOutputPayload::from(&result),
Err(tool_call_err) => FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(format!("err: {tool_call_err:?}")),
success: Some(false),
},
};
ResponseInputItem::McpToolCallOutput { call_id, output } => {
let output = output.into_function_call_output_payload();
Self::FunctionCallOutput { call_id, output }
}
ResponseInputItem::CustomToolCallOutput { call_id, output } => {
@ -1190,25 +1184,59 @@ impl<'de> Deserialize<'de> for FunctionCallOutputPayload {
}
}
impl From<&CallToolResult> for FunctionCallOutputPayload {
fn from(call_tool_result: &CallToolResult) -> Self {
let CallToolResult {
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
pub struct McpToolOutput {
pub content: Vec<serde_json::Value>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub structured_content: Option<serde_json::Value>,
pub success: bool,
}
impl McpToolOutput {
pub fn from_result(result: Result<CallToolResult, String>) -> Self {
match result {
Ok(result) => Self::from(&result),
Err(error) => Self::from_error_text(error),
}
}
pub fn from_error_text(text: String) -> Self {
Self {
content: vec![serde_json::json!({
"type": "text",
"text": text,
})],
structured_content: None,
success: false,
}
}
pub fn into_call_tool_result(self) -> CallToolResult {
let Self {
content,
structured_content,
is_error,
meta: _,
} = call_tool_result;
success,
} = self;
let is_success = is_error != &Some(true);
CallToolResult {
content,
structured_content,
is_error: Some(!success),
meta: None,
}
}
if let Some(structured_content) = structured_content
pub fn as_function_call_output_payload(&self) -> FunctionCallOutputPayload {
if let Some(structured_content) = &self.structured_content
&& !structured_content.is_null()
{
match serde_json::to_string(structured_content) {
Ok(serialized_structured_content) => {
return FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(serialized_structured_content),
success: Some(is_success),
success: Some(self.success),
};
}
Err(err) => {
@ -1220,7 +1248,7 @@ impl From<&CallToolResult> for FunctionCallOutputPayload {
}
}
let serialized_content = match serde_json::to_string(content) {
let serialized_content = match serde_json::to_string(&self.content) {
Ok(serialized_content) => serialized_content,
Err(err) => {
return FunctionCallOutputPayload {
@ -1230,7 +1258,7 @@ impl From<&CallToolResult> for FunctionCallOutputPayload {
}
};
let content_items = convert_mcp_content_to_items(content);
let content_items = convert_mcp_content_to_items(&self.content);
let body = match content_items {
Some(content_items) => FunctionCallOutputBody::ContentItems(content_items),
@ -1239,7 +1267,28 @@ impl From<&CallToolResult> for FunctionCallOutputPayload {
FunctionCallOutputPayload {
body,
success: Some(is_success),
success: Some(self.success),
}
}
pub fn into_function_call_output_payload(self) -> FunctionCallOutputPayload {
self.as_function_call_output_payload()
}
}
impl From<&CallToolResult> for McpToolOutput {
fn from(call_tool_result: &CallToolResult) -> Self {
let CallToolResult {
content,
structured_content,
is_error,
meta: _,
} = call_tool_result;
Self {
content: content.clone(),
structured_content: structured_content.clone(),
success: is_error != &Some(true),
}
}
}
@ -1833,7 +1882,7 @@ mod tests {
meta: None,
};
let payload = FunctionCallOutputPayload::from(&call_tool_result);
let payload = McpToolOutput::from(&call_tool_result).into_function_call_output_payload();
assert_eq!(payload.success, Some(true));
let Some(items) = payload.content_items() else {
panic!("expected content items");
@ -1900,7 +1949,7 @@ mod tests {
meta: None,
};
let payload = FunctionCallOutputPayload::from(&call_tool_result);
let payload = McpToolOutput::from(&call_tool_result).into_function_call_output_payload();
let Some(items) = payload.content_items() else {
panic!("expected content items");
};