From 5c01595a2d85543d8a95ae59d8c8d5884bf6ed0e Mon Sep 17 00:00:00 2001 From: jif Date: Tue, 4 Aug 2026 11:07:46 +0000 Subject: [PATCH] Preserve complete MCP namespace descriptions (#36882) ## What changed - Keep complete MCP namespace descriptions in tool-search source metadata. - Raise the namespace tool-spec description limit from 1,000 bytes to 512 KiB, truncating at a UTF-8 character boundary only when the new limit is exceeded. ## Testing - Cover descriptions beyond the former limit and multibyte truncation at 512 KiB. - Update SSE and stdio MCP tests to verify that complete server instructions are preserved without hiding tools. GitOrigin-RevId: 000bfcafb3df348065ae451685bfbf978a0e3248 --- codex-rs/core/src/tools/handlers/mcp.rs | 10 ++--- .../src/tools/handlers/mcp_search_tests.rs | 37 +++++++++++++++---- codex-rs/core/tests/suite/rmcp_client.rs | 14 +++---- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index b8ac8925b4..a8f35690b4 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -31,7 +31,7 @@ use serde_json::Value; const LEGACY_MCP_TOOL_NAME_PREFIX: &str = "mcp__"; const MCP_TOOL_NAME_DELIMITER: &str = "__"; -const MAX_MCP_NAMESPACE_DESCRIPTION_BYTES: usize = 1_000; +const MAX_MCP_NAMESPACE_DESCRIPTION_BYTES: usize = 512 * 1024; pub struct McpHandler { tool_info: ToolInfo, @@ -106,7 +106,6 @@ impl ToolExecutor for McpHandler { .as_deref() .map(str::trim) .filter(|description| !description.is_empty()) - .map(bounded_mcp_namespace_description) .map(str::to_string), }); @@ -258,15 +257,12 @@ fn create_tool_spec(tool_info: &ToolInfo) -> Result Ok(ToolSpec::Namespace(ResponsesApiNamespace { name: tool_info.callable_namespace.clone(), - description: bounded_mcp_namespace_description(&description).to_string(), + description: take_bytes_at_char_boundary(&description, MAX_MCP_NAMESPACE_DESCRIPTION_BYTES) + .to_string(), tools: vec![ResponsesApiNamespaceTool::Function(tool)], })) } -fn bounded_mcp_namespace_description(description: &str) -> &str { - take_bytes_at_char_boundary(description, MAX_MCP_NAMESPACE_DESCRIPTION_BYTES) -} - fn mcp_hook_tool_input(raw_arguments: &str) -> Value { if raw_arguments.trim().is_empty() { return Value::Object(Map::new()); diff --git a/codex-rs/core/src/tools/handlers/mcp_search_tests.rs b/codex-rs/core/src/tools/handlers/mcp_search_tests.rs index 335750a7ea..af8797f02c 100644 --- a/codex-rs/core/src/tools/handlers/mcp_search_tests.rs +++ b/codex-rs/core/src/tools/handlers/mcp_search_tests.rs @@ -43,9 +43,8 @@ fn search_info_uses_connector_name_for_output_namespace_description() { } #[test] -fn mcp_namespace_descriptions_are_bounded_without_mutating_metadata() { - let expected_description = "é".repeat(499); - let full_description = format!("{expected_description}🦀keep the complete app metadata"); +fn mcp_namespace_descriptions_preserve_complete_metadata() { + let full_description = format!("{}🦀keep the complete app metadata", "é".repeat(499)); let mut info = tool_info(); info.namespace_description = Some(full_description.clone()); let handler = McpHandler::new(info).expect("MCP tool spec should build"); @@ -55,17 +54,39 @@ fn mcp_namespace_descriptions_are_bounded_without_mutating_metadata() { search_info.source_info, Some(ToolSearchSourceInfo { name: "Calendar".to_string(), - description: Some(expected_description.clone()), + description: Some(full_description.clone()), + }) + ); + let LoadableToolSpec::Namespace(namespace) = search_info.entry.output else { + panic!("expected namespace search output"); + }; + assert_eq!(namespace.description, full_description); + assert_eq!( + handler.tool_info.namespace_description, + Some(full_description) + ); +} + +#[test] +fn mcp_namespace_descriptions_are_bounded_at_512_kib() { + let expected_description = "é".repeat(MAX_MCP_NAMESPACE_DESCRIPTION_BYTES / 2 - 1); + let full_description = format!("{expected_description}🦀overflow"); + let mut info = tool_info(); + info.namespace_description = Some(full_description.clone()); + let handler = McpHandler::new(info).expect("MCP tool spec should build"); + let search_info = handler.search_info().expect("MCP search info"); + + assert_eq!( + search_info.source_info, + Some(ToolSearchSourceInfo { + name: "Calendar".to_string(), + description: Some(full_description), }) ); let LoadableToolSpec::Namespace(namespace) = search_info.entry.output else { panic!("expected namespace search output"); }; assert_eq!(namespace.description, expected_description); - assert_eq!( - handler.tool_info.namespace_description, - Some(full_description) - ); } fn tool_info() -> ToolInfo { diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index d6f005aab4..d965a60987 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -545,7 +545,7 @@ fn assert_cwd_tool_output(structured: &Value, expected_cwd: &Path) { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn mcp_namespace_instructions_are_bounded_without_hiding_tools() -> anyhow::Result<()> { +async fn mcp_namespace_instructions_are_preserved_without_hiding_tools() -> anyhow::Result<()> { skip_if_wine_exec!( Ok(()), "requires a Windows test_stdio_server in the Wine-exec environment" @@ -553,8 +553,8 @@ async fn mcp_namespace_instructions_are_bounded_without_hiding_tools() -> anyhow skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; - let expected_description = "é".repeat(499); - let instructions = format!("{expected_description}🦀keep the valid MCP server"); + let expected_description = format!("{}🦀keep the valid MCP server", "é".repeat(499)); + let instructions = expected_description.clone(); let response = mount_sse_once( &server, responses::sse(vec![ @@ -614,7 +614,7 @@ async fn mcp_namespace_instructions_are_bounded_without_hiding_tools() -> anyhow ); assert!( responses::namespace_child_tool(&body, "mcp__bounded", "echo").is_some(), - "bounding the namespace must not hide a valid MCP tool" + "preserving the namespace must not hide a valid MCP tool" ); Ok(()) } @@ -672,8 +672,8 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { .await; let expected_env_value = "propagated-env"; - let expected_description = "é".repeat(499); - let instructions = format!("{expected_description}🦀keep the complete MCP metadata"); + let expected_description = format!("{}🦀keep the complete MCP metadata", "é".repeat(499)); + let instructions = expected_description.clone(); let rmcp_test_server_bin = remote_aware_stdio_server_bin()?; let fixture = test_codex() @@ -769,7 +769,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { "the complete tool search description must remain bounded" ); assert!(search_description.contains(&format!("- rmcp: {expected_description}"))); - assert!(!search_description.contains("🦀keep the complete MCP metadata")); + assert!(search_description.contains("🦀keep the complete MCP metadata")); let search_output = call_mock .single_request()