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 8a323efce..03f3db95f 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -5,16 +5,25 @@ use std::time::Duration; use anyhow::Result; use app_test_support::McpProcess; +use app_test_support::create_final_assistant_message_sse_response; +use app_test_support::create_mock_responses_server_sequence; use app_test_support::to_response; use app_test_support::write_mock_responses_config_toml; use axum::Router; +use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::McpServerToolCallParams; use codex_app_server_protocol::McpServerToolCallResponse; +use codex_app_server_protocol::McpToolCallStatus; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; +use codex_app_server_protocol::UserInput as V2UserInput; +use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use core_test_support::responses; use pretty_assertions::assert_eq; use rmcp::handler::server::ServerHandler; @@ -42,6 +51,7 @@ use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); const TEST_SERVER_NAME: &str = "tool_server"; const TEST_TOOL_NAME: &str = "echo_tool"; +const LARGE_RESPONSE_MESSAGE: &str = "large"; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_returns_tool_result() -> Result<()> { @@ -161,6 +171,137 @@ async fn mcp_server_tool_call_returns_error_for_unknown_thread() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn mcp_tool_call_completion_notification_contains_truncated_large_result() -> Result<()> { + let call_id = "call-large-mcp"; + let namespace = format!("mcp__{TEST_SERVER_NAME}__"); + let responses = vec![ + responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_function_call_with_namespace( + call_id, + &namespace, + TEST_TOOL_NAME, + &serde_json::to_string(&json!({ + "message": LARGE_RESPONSE_MESSAGE, + }))?, + ), + responses::ev_completed("resp-1"), + ]), + create_final_assistant_message_sse_response("done")?, + ]; + let responses_server = create_mock_responses_server_sequence(responses).await; + let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let codex_home = TempDir::new()?; + write_mock_responses_config_toml( + codex_home.path(), + &responses_server.uri(), + &BTreeMap::new(), + /*auto_compact_limit*/ 1_000_000, + /*requires_openai_auth*/ None, + "mock_provider", + "compact", + )?; + + let config_path = codex_home.path().join("config.toml"); + let mut config_toml = std::fs::read_to_string(&config_path)?; + config_toml.push_str(&format!( + r#" +[mcp_servers.{TEST_SERVER_NAME}] +url = "{mcp_server_url}/mcp" +"# + )); + std::fs::write(config_path, config_toml)?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let thread_start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response(thread_start_resp)?; + + let turn_start_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id, + input: vec![V2UserInput::Text { + text: "Call the large MCP tool".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + let turn_start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_start_id)), + ) + .await??; + let TurnStartResponse { turn, .. } = to_response(turn_start_resp)?; + + let completed = wait_for_mcp_tool_call_completed(&mut mcp, call_id).await?; + assert_eq!(completed.turn_id, turn.id); + + let ThreadItem::McpToolCall { + id, + server, + tool, + status, + result: Some(result), + error, + .. + } = completed.item + else { + panic!("expected completed MCP tool call item"); + }; + assert_eq!(id, call_id); + assert_eq!(server, TEST_SERVER_NAME); + assert_eq!(tool, TEST_TOOL_NAME); + assert_eq!(status, McpToolCallStatus::Completed); + assert_eq!(error, None); + assert_eq!(result.structured_content, None); + assert_eq!(result.meta, None); + assert_eq!(result.content.len(), 1); + + let text = result.content[0] + .get("text") + .and_then(serde_json::Value::as_str) + .expect("truncated MCP event result should be represented as text content"); + assert!(text.contains("truncated")); + assert!(text.len() < DEFAULT_OUTPUT_BYTES_CAP + 1024); + + let serialized_item = serde_json::to_string(&ThreadItem::McpToolCall { + id, + server, + tool, + status, + arguments: json!({ "message": LARGE_RESPONSE_MESSAGE }), + mcp_app_resource_uri: None, + result: Some(result), + error: None, + duration_ms: None, + })?; + assert!(serialized_item.len() < DEFAULT_OUTPUT_BYTES_CAP * 2 + 2048); + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + mcp_server_handle.abort(); + let _ = mcp_server_handle.await; + + Ok(()) +} + #[derive(Clone, Default)] struct ToolAppsMcpServer; @@ -224,6 +365,16 @@ impl ServerHandler for ToolAppsMcpServer { let mut meta = Meta::new(); meta.0.insert("calledBy".to_string(), json!("mcp-app")); + if message == LARGE_RESPONSE_MESSAGE { + let large_text = "large-mcp-content-".repeat(DEFAULT_OUTPUT_BYTES_CAP / 8); + let mut result = CallToolResult::structured(json!({ + "large": "structured-value-".repeat(DEFAULT_OUTPUT_BYTES_CAP / 8), + })); + result.content = vec![Content::text(large_text)]; + result.meta = Some(meta); + return Ok(result); + } + let mut result = CallToolResult::structured(json!({ "echoed": message, "threadId": thread_id, @@ -250,3 +401,23 @@ async fn start_mcp_server() -> Result<(String, JoinHandle<()>)> { Ok((format!("http://{addr}"), handle)) } + +async fn wait_for_mcp_tool_call_completed( + mcp: &mut McpProcess, + call_id: &str, +) -> Result { + loop { + let notification = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("item/completed"), + ) + .await??; + let Some(params) = notification.params else { + continue; + }; + let completed: ItemCompletedNotification = serde_json::from_value(params)?; + if matches!(&completed.item, ThreadItem::McpToolCall { id, .. } if id == call_id) { + return Ok(completed); + } + } +} diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 415a60198..5a442bff6 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -452,7 +452,7 @@ impl ContextManager { } } -fn truncate_function_output_payload( +pub(crate) fn truncate_function_output_payload( output: &FunctionCallOutputPayload, policy: TruncationPolicy, ) -> FunctionCallOutputPayload { diff --git a/codex-rs/core/src/context_manager/mod.rs b/codex-rs/core/src/context_manager/mod.rs index 853f8af5a..9dd85d1ed 100644 --- a/codex-rs/core/src/context_manager/mod.rs +++ b/codex-rs/core/src/context_manager/mod.rs @@ -7,3 +7,4 @@ pub(crate) use history::TotalTokenUsageBreakdown; pub(crate) use history::estimate_response_item_model_visible_bytes; pub(crate) use history::is_codex_generated_item; pub(crate) use history::is_user_turn_boundary; +pub(crate) use history::truncate_function_output_payload; diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 161994654..d2e990f44 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -60,6 +60,9 @@ use codex_rmcp_client::ElicitationAction; use codex_rmcp_client::ElicitationResponse; use codex_rollout::state_db; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::truncate_text; +use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use rmcp::model::ToolAnnotations; use serde::Deserialize; use serde::Serialize; @@ -81,6 +84,7 @@ const MCP_RESULT_TELEMETRY_TARGET_ID_SPAN_ATTR: &str = "codex.mcp.target.id"; const MCP_RESULT_TELEMETRY_SERVER_USER_FLOW_SPAN_ATTR: &str = "codex.mcp.server_user_flow.triggered"; const MCP_RESULT_TELEMETRY_TARGET_ID_MAX_CHARS: usize = 256; +const MCP_TOOL_CALL_EVENT_RESULT_MAX_BYTES: usize = DEFAULT_OUTPUT_BYTES_CAP; /// Handles the specified tool call dispatches the appropriate /// `McpToolCallBegin` and `McpToolCallEnd` events to the `Session`. @@ -362,7 +366,7 @@ async fn handle_approved_mcp_tool_call( invocation, mcp_app_resource_uri, duration, - result: result.clone(), + result: truncate_mcp_tool_result_for_event(&result), }); notify_mcp_tool_call_event(sess, turn_context, tool_call_end_event.clone()).await; maybe_track_codex_app_used(sess, turn_context, &server, &tool_name).await; @@ -648,6 +652,50 @@ fn sanitize_mcp_tool_result_for_model( }) } +fn truncate_mcp_tool_result_for_event( + result: &Result, +) -> Result { + match result { + Ok(call_tool_result) => { + // The app-server rebuilds `ThreadItem::McpToolCall` from this event, + // so avoid persisting multi-megabyte results in rollout storage. + let Ok(serialized) = serde_json::to_string(call_tool_result) else { + return Ok(call_tool_result.clone()); + }; + if serialized.len() <= MCP_TOOL_CALL_EVENT_RESULT_MAX_BYTES { + return Ok(call_tool_result.clone()); + } + + // A huge MCP result can put bytes in `content`, `structuredContent`, + // or `_meta`. Collapse the event copy to a text preview of the whole + // serialized result so the UI still has useful context without + // preserving a multi-megabyte structured payload. + // + // This budget applies to the preview text, not the final event JSON. + // The preview is itself serialized into a JSON string, so quotes and + // backslashes can be escaped again and the stored event may end up + // somewhat larger than this byte budget. + let truncated = truncate_text( + &serialized, + TruncationPolicy::Bytes(MCP_TOOL_CALL_EVENT_RESULT_MAX_BYTES), + ); + Ok(CallToolResult { + content: vec![serde_json::json!({ + "type": "text", + "text": truncated, + })], + structured_content: None, + is_error: call_tool_result.is_error, + meta: None, + }) + } + Err(message) => Err(truncate_text( + message, + TruncationPolicy::Bytes(MCP_TOOL_CALL_EVENT_RESULT_MAX_BYTES), + )), + } +} + async fn notify_mcp_tool_call_event(sess: &Session, turn_context: &TurnContext, event: EventMsg) { sess.send_event(turn_context, event).await; } @@ -1939,7 +1987,7 @@ async fn notify_mcp_tool_call_skip( invocation, mcp_app_resource_uri, duration: Duration::ZERO, - result: Err(message.clone()), + result: truncate_mcp_tool_result_for_event(&Err(message.clone())), }); notify_mcp_tool_call_event(sess, turn_context, tool_call_end_event).await; Err(message) diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 37df6dc6e..f9244a7fa 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -838,6 +838,72 @@ fn sanitize_mcp_tool_result_for_model_preserves_image_when_supported() { assert_eq!(got, original); } +#[test] +fn truncate_mcp_tool_result_for_event_preserves_small_result() { + let original = CallToolResult { + content: vec![serde_json::json!({ + "type": "text", + "text": "hello", + })], + structured_content: Some(serde_json::json!({"x": 1})), + is_error: Some(false), + meta: Some(serde_json::json!({"k": "v"})), + }; + + let got = truncate_mcp_tool_result_for_event(&Ok(original.clone())) + .expect("small result should remain successful"); + + assert_eq!(got, original); +} + +#[test] +fn truncate_mcp_tool_result_for_event_bounds_large_result() { + let original = CallToolResult { + content: vec![serde_json::json!({ + "type": "text", + "text": "long-message-with-newlines-\n".repeat(200_000), + })], + structured_content: Some(serde_json::json!({ + "structured": "structured-value-".repeat(200_000), + })), + is_error: Some(false), + meta: Some(serde_json::json!({ + "meta": "meta-value-".repeat(200_000), + })), + }; + + let got = truncate_mcp_tool_result_for_event(&Ok(original)) + .expect("large result should remain successful"); + let serialized = serde_json::to_string(&got).expect("truncated result should serialize"); + + // The truncated preview is embedded as a JSON string, so quotes and + // backslashes can be escaped again. That can roughly double the preview + // bytes in the worst case. The extra buffer covers the small result wrapper + // and marker. + assert!(serialized.len() < MCP_TOOL_CALL_EVENT_RESULT_MAX_BYTES * 2 + 1024); + assert_eq!(got.structured_content, None); + assert_eq!(got.meta, None); + assert_eq!(got.is_error, Some(false)); + assert!( + got.content[0] + .get("text") + .and_then(serde_json::Value::as_str) + .is_some_and(|text| text.contains("truncated")), + "large event result should contain a truncation marker: {got:?}" + ); +} + +#[test] +fn truncate_mcp_tool_result_for_event_bounds_large_error() { + let got = truncate_mcp_tool_result_for_event(&Err("error-message-".repeat(200_000))) + .expect_err("large error should remain an error"); + + // `truncate_text` includes its own marker, so allow a small amount of + // overhead beyond the requested byte budget. + assert!(got.len() < MCP_TOOL_CALL_EVENT_RESULT_MAX_BYTES + 1024); + assert!(got.contains("truncated")); +} + #[tokio::test] async fn mcp_tool_call_request_meta_includes_turn_metadata_for_custom_server() { let (_, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index f65baeb6d..cf6b3fac9 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -1,3 +1,4 @@ +use crate::context_manager::truncate_function_output_payload; use crate::original_image_detail::sanitize_original_image_detail; use crate::session::session::Session; use crate::session::turn_context::TurnContext; @@ -142,6 +143,7 @@ pub struct McpToolOutput { pub tool_input: JsonValue, pub wall_time: Duration, pub original_image_detail_supported: bool, + pub truncation_policy: TruncationPolicy, } impl ToolOutput for McpToolOutput { @@ -199,7 +201,13 @@ impl McpToolOutput { } } - payload + // This is the context-injection form, so keep it aligned with the + // function-call output truncation that conversation history already + // applies. Code-mode consumers still get the raw `CallToolResult`. + // + // The text is serialized again inside the Responses payload, so allow + // a small buffer for JSON escaping and wrapper overhead. + truncate_function_output_payload(&payload, self.truncation_policy * 1.2) } } diff --git a/codex-rs/core/src/tools/context_tests.rs b/codex-rs/core/src/tools/context_tests.rs index 4f40342e4..29adec5bd 100644 --- a/codex-rs/core/src/tools/context_tests.rs +++ b/codex-rs/core/src/tools/context_tests.rs @@ -101,6 +101,7 @@ fn mcp_tool_output_response_item_includes_wall_time() { tool_input: json!({}), wall_time: std::time::Duration::from_millis(1250), original_image_detail_supported: false, + truncation_policy: TruncationPolicy::Bytes(1024), }; let response = output.to_response_item( @@ -137,6 +138,51 @@ fn mcp_tool_output_response_item_includes_wall_time() { } } +#[test] +fn mcp_tool_output_response_item_truncates_large_structured_content() { + let output = McpToolOutput { + result: CallToolResult { + content: vec![serde_json::json!({ + "type": "text", + "text": "ignored when structured content is present", + })], + structured_content: Some(serde_json::json!({ + "items": "large structured value ".repeat(1_000), + })), + is_error: Some(false), + meta: None, + }, + tool_input: json!({}), + wall_time: std::time::Duration::from_millis(1250), + original_image_detail_supported: false, + truncation_policy: TruncationPolicy::Bytes(128), + }; + + let response = output.to_response_item( + "mcp-call-large", + &ToolPayload::Mcp { + server: "server".to_string(), + tool: "tool".to_string(), + raw_arguments: "{}".to_string(), + }, + ); + + match response { + ResponseInputItem::FunctionCallOutput { call_id, output } => { + assert_eq!(call_id, "mcp-call-large"); + assert_eq!(output.success, Some(true)); + let text = output + .body + .to_text() + .expect("MCP output should serialize as text"); + assert!(text.starts_with("Wall time: 1.2500 seconds\nOutput:\n")); + assert!(text.contains("chars truncated")); + assert!(!text.contains("ignored when structured content is present")); + } + other => panic!("expected FunctionCallOutput, got {other:?}"), + } +} + #[test] fn mcp_tool_output_response_item_preserves_content_items() { let image_url = "data:image/png;base64,AAA"; @@ -154,6 +200,7 @@ fn mcp_tool_output_response_item_preserves_content_items() { tool_input: json!({}), wall_time: std::time::Duration::from_millis(500), original_image_detail_supported: false, + truncation_policy: TruncationPolicy::Bytes(1024), }; let response = output.to_response_item( @@ -193,6 +240,7 @@ fn mcp_tool_output_response_item_preserves_content_items() { #[test] fn mcp_tool_output_code_mode_result_stays_raw_call_tool_result() { + let large_content = "large structured value ".repeat(1_000); let output = McpToolOutput { result: CallToolResult { content: vec![serde_json::json!({ @@ -200,7 +248,7 @@ fn mcp_tool_output_code_mode_result_stays_raw_call_tool_result() { "text": "ignored", })], structured_content: Some(serde_json::json!({ - "content": "done", + "content": large_content, })), is_error: Some(false), meta: None, @@ -208,6 +256,7 @@ fn mcp_tool_output_code_mode_result_stays_raw_call_tool_result() { tool_input: json!({}), wall_time: std::time::Duration::from_millis(1250), original_image_detail_supported: false, + truncation_policy: TruncationPolicy::Bytes(64), }; let result = output.code_mode_result(&ToolPayload::Mcp { @@ -224,7 +273,7 @@ fn mcp_tool_output_code_mode_result_stays_raw_call_tool_result() { "text": "ignored", }], "structuredContent": { - "content": "done", + "content": "large structured value ".repeat(1_000), }, "isError": false, }) diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index d828119d9..568e45615 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -96,6 +96,7 @@ impl ToolHandler for McpHandler { tool_input: result.tool_input, wall_time: started.elapsed(), original_image_detail_supported: can_request_original_image_detail(&turn.model_info), + truncation_policy: turn.truncation_policy, }) } } @@ -181,6 +182,7 @@ mod tests { }), wall_time: Duration::from_millis(42), original_image_detail_supported: true, + truncation_policy: codex_utils_output_truncation::TruncationPolicy::Bytes(1024), }; let (session, turn) = make_session_and_context().await; let invocation = ToolInvocation {