diff --git a/codex-rs/core/src/mcp_tool_exposure.rs b/codex-rs/core/src/mcp_tool_exposure.rs index fa9b6eb61b..e7aa97993a 100644 --- a/codex-rs/core/src/mcp_tool_exposure.rs +++ b/codex-rs/core/src/mcp_tool_exposure.rs @@ -28,7 +28,6 @@ pub(crate) struct McpHandlerCache { } struct CachedMcpHandler { - tool_info: McpToolInfo, agent_plugin: bool, schema_max_bytes: Option, handler: Arc, @@ -98,7 +97,7 @@ fn append_mcp_tools( // Handlers contain immutable tool metadata, not a connection or authorization snapshot. // Preserve their identity across equivalent bindings so the search index can also be reused. let handler = if let Some(cached) = handlers.get(&tool_name).filter(|cached| { - cached.tool_info == *tool + cached.handler.tool_info() == tool && cached.agent_plugin == agent_plugin && cached.schema_max_bytes == tool_input_schema_max_bytes }) { @@ -123,7 +122,6 @@ fn append_mcp_tools( handlers.insert( tool_name.clone(), CachedMcpHandler { - tool_info: tool.clone(), agent_plugin, schema_max_bytes: tool_input_schema_max_bytes, handler: Arc::clone(&handler), diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index e165a53ab2..98d61aff08 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -50,6 +50,7 @@ const MAX_MCP_NAMESPACE_DESCRIPTION_BYTES: usize = 512 * 1024; pub struct McpHandler { tool_info: ToolInfo, + agent_plugin: bool, spec: Arc, code_mode_tool_definitions: OnceLock<(Option, bool, Vec)>, @@ -80,23 +81,10 @@ impl McpHandler { } fn with_agent_plugin( - mut tool_info: ToolInfo, + tool_info: ToolInfo, agent_plugin: bool, schema_max_bytes: Option, ) -> Result { - if agent_plugin { - tool_info.namespace_description = - tool_info - .namespace_description - .as_deref() - .map(|description| { - take_bytes_at_char_boundary( - description, - MAX_AGENT_PLUGIN_MCP_NAMESPACE_DESCRIPTION_BYTES, - ) - .to_string() - }); - } let spec = Arc::new(create_tool_spec( &tool_info, agent_plugin, @@ -104,11 +92,16 @@ impl McpHandler { )?); Ok(Self { tool_info, + agent_plugin, spec, code_mode_tool_definitions: OnceLock::new(), }) } + pub(crate) fn tool_info(&self) -> &ToolInfo { + &self.tool_info + } + pub(crate) fn model_spec_bytes(&self) -> Result { serde_json::to_vec(&self.spec).map(|spec| spec.len()) } @@ -169,17 +162,14 @@ impl ToolExecutor for McpHandler { .unwrap_or_else(|| self.tool_info.server_name.trim()); let source_info = (!source_name.is_empty()).then(|| ToolSearchSourceInfo { name: source_name.to_string(), - description: self - .tool_info - .namespace_description - .as_deref() + description: effective_namespace_description(&self.tool_info, self.agent_plugin) .map(str::trim) .filter(|description| !description.is_empty()) .map(str::to_string), }); ToolSearchInfo::from_shared_spec( - build_mcp_search_text(&self.tool_info), + build_mcp_search_text(&self.tool_info, self.agent_plugin), Arc::clone(&self.spec), source_info, ) @@ -520,9 +510,7 @@ fn create_tool_spec( } else { mcp_tool_to_responses_api_tool(&tool_name, &tool_info.tool, schema_max_bytes)? }; - let description = tool_info - .namespace_description - .as_deref() + let description = effective_namespace_description(tool_info, agent_plugin) .map(str::trim) .filter(|description| !description.is_empty()) .map(str::to_string) @@ -544,6 +532,18 @@ fn create_tool_spec( })) } +fn effective_namespace_description(tool_info: &ToolInfo, agent_plugin: bool) -> Option<&str> { + let description = tool_info.namespace_description.as_deref()?; + Some(if agent_plugin { + take_bytes_at_char_boundary( + description, + MAX_AGENT_PLUGIN_MCP_NAMESPACE_DESCRIPTION_BYTES, + ) + } else { + description + }) +} + fn mcp_hook_tool_input(raw_arguments: &str) -> Value { if raw_arguments.trim().is_empty() { return Value::Object(Map::new()); @@ -552,7 +552,7 @@ fn mcp_hook_tool_input(raw_arguments: &str) -> Value { serde_json::from_str(raw_arguments).unwrap_or_else(|_| Value::String(raw_arguments.to_string())) } -fn build_mcp_search_text(info: &ToolInfo) -> String { +fn build_mcp_search_text(info: &ToolInfo, agent_plugin: bool) -> String { let tool_name = info.canonical_tool_name(); let mut schema_properties = info .tool @@ -583,7 +583,8 @@ fn build_mcp_search_text(info: &ToolInfo) -> String { { parts.push(connector_name.to_string()); } - if let Some(namespace_description) = info.namespace_description.as_deref().map(str::trim) + if let Some(namespace_description) = + effective_namespace_description(info, agent_plugin).map(str::trim) && !namespace_description.is_empty() { parts.push(namespace_description.to_string()); 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 e52b8c3618..50c11dfd5a 100644 --- a/codex-rs/core/src/tools/handlers/mcp_search_tests.rs +++ b/codex-rs/core/src/tools/handlers/mcp_search_tests.rs @@ -92,8 +92,9 @@ fn mcp_namespace_descriptions_are_bounded_at_512_kib() { #[test] fn agent_plugin_namespace_descriptions_use_the_stricter_bound() { let expected_description = "é".repeat(MAX_AGENT_PLUGIN_MCP_NAMESPACE_DESCRIPTION_BYTES / 2); + let full_description = format!("{expected_description}overflow"); let mut info = tool_info(); - info.namespace_description = Some(format!("{expected_description}overflow")); + info.namespace_description = Some(full_description.clone()); let handler = McpHandler::new_agent_plugin(info).expect("MCP tool spec should build"); let search_info = handler.search_info().expect("MCP search info"); @@ -108,6 +109,10 @@ fn agent_plugin_namespace_descriptions_use_the_stricter_bound() { panic!("expected namespace search output"); }; assert_eq!(namespace.description, expected_description); + assert_eq!( + handler.tool_info().namespace_description.as_deref(), + Some(full_description.as_str()) + ); } fn tool_info() -> ToolInfo {