From cb7d8f45a1393d71b333aea64123227028ae535f Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Fri, 13 Mar 2026 14:50:16 -0700 Subject: [PATCH] Normalize MCP tool names to code-mode safe form (#14605) Code mode doesn't allow `-` in names and it's better if function names and code-mode names are the same. --- .../tests/suite/v2/mcp_server_elicitation.rs | 2 +- codex-rs/core/src/mcp_connection_manager.rs | 6 +++-- .../core/src/mcp_connection_manager_tests.rs | 6 ++--- .../src/tools/handlers/tool_search_tests.rs | 24 +++++++++---------- codex-rs/core/src/tools/spec_tests.rs | 14 +++++------ codex-rs/core/tests/suite/plugins.rs | 4 ++-- codex-rs/core/tests/suite/search_tool.rs | 6 ++--- 7 files changed, 32 insertions(+), 30 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs b/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs index 584cd1a52..a784b3490 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs @@ -66,7 +66,7 @@ const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs const CONNECTOR_ID: &str = "calendar"; const CONNECTOR_NAME: &str = "Calendar"; const TOOL_NAME: &str = "calendar_confirm_action"; -const QUALIFIED_TOOL_NAME: &str = "mcp__codex_apps__calendar-confirm-action"; +const QUALIFIED_TOOL_NAME: &str = "mcp__codex_apps__calendar_confirm_action"; const TOOL_CALL_ID: &str = "call-calendar-confirm"; const ELICITATION_MESSAGE: &str = "Allow this request?"; diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 19997f039..6e2174c4d 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -110,7 +110,7 @@ const MCP_TOOLS_CACHE_WRITE_DURATION_METRIC: &str = "codex.mcp.tools.cache_write fn sanitize_responses_api_tool_name(name: &str) -> String { let mut sanitized = String::with_capacity(name.len()); for c in name.chars() { - if c.is_ascii_alphanumeric() || c == '_' || c == '-' { + if c.is_ascii_alphanumeric() || c == '_' { sanitized.push(c); } else { sanitized.push('_'); @@ -1230,11 +1230,12 @@ fn normalize_codex_apps_tool_name( return tool_name.to_string(); } - let tool_name = sanitize_name(tool_name); + let tool_name = sanitize_name(tool_name).replace('-', "_"); if let Some(connector_name) = connector_name .map(str::trim) .map(sanitize_name) + .map(|name| name.replace('-', "_")) .filter(|name| !name.is_empty()) && let Some(stripped) = tool_name.strip_prefix(&connector_name) && !stripped.is_empty() @@ -1245,6 +1246,7 @@ fn normalize_codex_apps_tool_name( if let Some(connector_id) = connector_id .map(str::trim) .map(sanitize_name) + .map(|name| name.replace('-', "_")) .filter(|name| !name.is_empty()) && let Some(stripped) = tool_name.strip_prefix(&connector_id) && !stripped.is_empty() diff --git a/codex-rs/core/src/mcp_connection_manager_tests.rs b/codex-rs/core/src/mcp_connection_manager_tests.rs index f584e5947..c5f7fc4a4 100644 --- a/codex-rs/core/src/mcp_connection_manager_tests.rs +++ b/codex-rs/core/src/mcp_connection_manager_tests.rs @@ -161,17 +161,17 @@ fn test_qualify_tools_long_names_same_server() { #[test] fn test_qualify_tools_sanitizes_invalid_characters() { - let tools = vec![create_test_tool("server.one", "tool.two")]; + let tools = vec![create_test_tool("server.one", "tool.two-three")]; let qualified_tools = qualify_tools(tools); assert_eq!(qualified_tools.len(), 1); let (qualified_name, tool) = qualified_tools.into_iter().next().expect("one tool"); - assert_eq!(qualified_name, "mcp__server_one__tool_two"); + assert_eq!(qualified_name, "mcp__server_one__tool_two_three"); // The key is sanitized for OpenAI, but we keep original parts for the actual MCP call. assert_eq!(tool.server_name, "server.one"); - assert_eq!(tool.tool_name, "tool.two"); + assert_eq!(tool.tool_name, "tool.two-three"); assert!( qualified_name diff --git a/codex-rs/core/src/tools/handlers/tool_search_tests.rs b/codex-rs/core/src/tools/handlers/tool_search_tests.rs index fc7ef5970..704653c21 100644 --- a/codex-rs/core/src/tools/handlers/tool_search_tests.rs +++ b/codex-rs/core/src/tools/handlers/tool_search_tests.rs @@ -10,10 +10,10 @@ use std::sync::Arc; fn serialize_tool_search_output_tools_groups_results_by_namespace() { let entries = [ ( - "mcp__codex_apps__calendar-create-event".to_string(), + "mcp__codex_apps__calendar_create_event".to_string(), ToolInfo { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "-create-event".to_string(), + tool_name: "_create_event".to_string(), tool_namespace: "mcp__codex_apps__calendar".to_string(), tool: Tool { name: "calendar-create-event".to_string().into(), @@ -36,10 +36,10 @@ fn serialize_tool_search_output_tools_groups_results_by_namespace() { }, ), ( - "mcp__codex_apps__gmail-read-email".to_string(), + "mcp__codex_apps__gmail_read_email".to_string(), ToolInfo { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "-read-email".to_string(), + tool_name: "_read_email".to_string(), tool_namespace: "mcp__codex_apps__gmail".to_string(), tool: Tool { name: "gmail-read-email".to_string().into(), @@ -62,10 +62,10 @@ fn serialize_tool_search_output_tools_groups_results_by_namespace() { }, ), ( - "mcp__codex_apps__calendar-list-events".to_string(), + "mcp__codex_apps__calendar_list_events".to_string(), ToolInfo { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "-list-events".to_string(), + tool_name: "_list_events".to_string(), tool_namespace: "mcp__codex_apps__calendar".to_string(), tool: Tool { name: "calendar-list-events".to_string().into(), @@ -100,7 +100,7 @@ fn serialize_tool_search_output_tools_groups_results_by_namespace() { description: "Plan events".to_string(), tools: vec![ ResponsesApiNamespaceTool::Function(ResponsesApiTool { - name: "-create-event".to_string(), + name: "_create_event".to_string(), description: "Create a calendar event.".to_string(), strict: false, defer_loading: Some(true), @@ -112,7 +112,7 @@ fn serialize_tool_search_output_tools_groups_results_by_namespace() { output_schema: None, }), ResponsesApiNamespaceTool::Function(ResponsesApiTool { - name: "-list-events".to_string(), + name: "_list_events".to_string(), description: "List calendar events.".to_string(), strict: false, defer_loading: Some(true), @@ -129,7 +129,7 @@ fn serialize_tool_search_output_tools_groups_results_by_namespace() { name: "mcp__codex_apps__gmail".to_string(), description: "Read mail".to_string(), tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool { - name: "-read-email".to_string(), + name: "_read_email".to_string(), description: "Read an email.".to_string(), strict: false, defer_loading: Some(true), @@ -148,10 +148,10 @@ fn serialize_tool_search_output_tools_groups_results_by_namespace() { #[test] fn serialize_tool_search_output_tools_falls_back_to_connector_name_description() { let entries = [( - "mcp__codex_apps__gmail-batch-read-email".to_string(), + "mcp__codex_apps__gmail_batch_read_email".to_string(), ToolInfo { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "-batch-read-email".to_string(), + tool_name: "_batch_read_email".to_string(), tool_namespace: "mcp__codex_apps__gmail".to_string(), tool: Tool { name: "gmail-batch-read-email".to_string().into(), @@ -182,7 +182,7 @@ fn serialize_tool_search_output_tools_falls_back_to_connector_name_description() name: "mcp__codex_apps__gmail".to_string(), description: "Tools for working with Gmail.".to_string(), tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool { - name: "-batch-read-email".to_string(), + name: "_batch_read_email".to_string(), description: "Read multiple emails.".to_string(), strict: false, defer_loading: Some(true), diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 2f2256813..258911c36 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -1686,10 +1686,10 @@ fn search_tool_description_includes_only_codex_apps_connector_names() { ])), Some(HashMap::from([ ( - "mcp__codex_apps__calendar-create-event".to_string(), + "mcp__codex_apps__calendar_create_event".to_string(), ToolInfo { server_name: crate::mcp::CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "-create-event".to_string(), + tool_name: "_create_event".to_string(), tool_namespace: "mcp__codex_apps__calendar".to_string(), tool: mcp_tool( "calendar-create-event", @@ -1862,10 +1862,10 @@ fn search_tool_registers_namespaced_app_tool_aliases() { None, Some(HashMap::from([ ( - "mcp__codex_apps__calendar-create-event".to_string(), + "mcp__codex_apps__calendar_create_event".to_string(), ToolInfo { server_name: crate::mcp::CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "-create-event".to_string(), + tool_name: "_create_event".to_string(), tool_namespace: "mcp__codex_apps__calendar".to_string(), tool: mcp_tool( "calendar-create-event", @@ -1879,10 +1879,10 @@ fn search_tool_registers_namespaced_app_tool_aliases() { }, ), ( - "mcp__codex_apps__calendar-list-events".to_string(), + "mcp__codex_apps__calendar_list_events".to_string(), ToolInfo { server_name: crate::mcp::CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "-list-events".to_string(), + tool_name: "_list_events".to_string(), tool_namespace: "mcp__codex_apps__calendar".to_string(), tool: mcp_tool( "calendar-list-events", @@ -1900,7 +1900,7 @@ fn search_tool_registers_namespaced_app_tool_aliases() { ) .build(); - let alias = tool_handler_key("-create-event", Some("mcp__codex_apps__calendar")); + let alias = tool_handler_key("_create_event", Some("mcp__codex_apps__calendar")); assert!(registry.has_handler(TOOL_SEARCH_TOOL_NAME, None)); assert!(registry.has_handler(alias.as_str(), None)); diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index a68f75a01..4641f6672 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -307,7 +307,7 @@ async fn explicit_plugin_mentions_inject_plugin_guidance() -> Result<()> { assert!( request_tools .iter() - .any(|name| name == "mcp__codex_apps__google-calendar-create-event"), + .any(|name| name == "mcp__codex_apps__google_calendar_create_event"), "expected plugin app tools to become visible for this turn: {request_tools:?}" ); let echo_description = tool_description(&request_body, "mcp__sample__echo") @@ -318,7 +318,7 @@ async fn explicit_plugin_mentions_inject_plugin_guidance() -> Result<()> { ); let calendar_description = tool_description( &request_body, - "mcp__codex_apps__google-calendar-create-event", + "mcp__codex_apps__google_calendar_create_event", ) .expect("plugin app tool description should be present"); assert!( diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index e7f0a60cb..d23d2f266 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -34,10 +34,10 @@ const SEARCH_TOOL_DESCRIPTION_SNIPPETS: [&str; 1] = [ "Tools of the apps (Calendar) are hidden until you search for them with this tool (`tool_search`).", ]; const TOOL_SEARCH_TOOL_NAME: &str = "tool_search"; -const CALENDAR_CREATE_TOOL: &str = "mcp__codex_apps__calendar-create-event"; -const CALENDAR_LIST_TOOL: &str = "mcp__codex_apps__calendar-list-events"; +const CALENDAR_CREATE_TOOL: &str = "mcp__codex_apps__calendar_create_event"; +const CALENDAR_LIST_TOOL: &str = "mcp__codex_apps__calendar_list_events"; const SEARCH_CALENDAR_NAMESPACE: &str = "mcp__codex_apps__calendar"; -const SEARCH_CALENDAR_CREATE_TOOL: &str = "-create-event"; +const SEARCH_CALENDAR_CREATE_TOOL: &str = "_create_event"; fn tool_names(body: &Value) -> Vec { body.get("tools")