[codex] Remove string-keyed MCP tool maps

codex-rs/codex-mcp/src/codex_apps.rs

@@ -5,7 +5,6 @@
 //! connector allow-list filtering, and the normalization that turns app
 //! connector/tool metadata into model-visible MCP callable names.
 
-use std::collections::HashMap;
 use std::path::PathBuf;
 use std::time::Instant;
 
@@ -38,16 +37,6 @@ pub fn codex_apps_tools_cache_key(auth: Option<&CodexAuth>) -> CodexAppsToolsCac
     }
 }
 
-pub fn filter_non_codex_apps_mcp_tools_only(
-    mcp_tools: &HashMap<String, ToolInfo>,
-) -> HashMap<String, ToolInfo> {
-    mcp_tools
-        .iter()
-        .filter(|(_, tool)| tool.server_name != CODEX_APPS_MCP_SERVER_NAME)
-        .map(|(name, tool)| (name.clone(), tool.clone()))
-        .collect()
-}
-
 #[derive(Clone)]
 pub(crate) struct CodexAppsToolsCacheContext {
     pub(crate) codex_home: PathBuf,

codex-rs/codex-mcp/src/connection_manager.rs

@@ -31,7 +31,7 @@ use crate::runtime::McpRuntimeEnvironment;
 use crate::runtime::emit_duration;
 use crate::tools::ToolInfo;
 use crate::tools::filter_tools;
-use crate::tools::qualify_tools;
+use crate::tools::normalize_tools_for_model;
 use crate::tools::tool_with_model_visible_input_schema;
 use anyhow::Context;
 use anyhow::Result;
@@ -337,26 +337,25 @@ impl McpConnectionManager {
         failures
     }
 
-    /// Returns a single map that contains all tools. Each key is the
-    /// fully-qualified name for the tool.
+    /// Returns all tools with model-visible names normalized.
     #[instrument(level = "trace", skip_all)]
-    pub async fn list_all_tools(&self) -> HashMap<String, ToolInfo> {
+    pub async fn list_all_tools(&self) -> Vec<ToolInfo> {
         let mut tools = Vec::new();
         for managed_client in self.clients.values() {
             let Some(server_tools) = managed_client.listed_tools().await else {
                 continue;
             };
             tools.extend(server_tools);
         }
-        qualify_tools(tools)
+        normalize_tools_for_model(tools)
     }
 
     /// Force-refresh codex apps tools by bypassing the in-process cache.
     ///
     /// On success, the refreshed tools replace the cache contents and the
-    /// latest filtered tool map is returned directly to the caller. On
+    /// latest filtered tools are returned directly to the caller. On
     /// failure, the existing cache remains unchanged.
-    pub async fn hard_refresh_codex_apps_tools_cache(&self) -> Result<HashMap<String, ToolInfo>> {
+    pub async fn hard_refresh_codex_apps_tools_cache(&self) -> Result<Vec<ToolInfo>> {
         let managed_client = self
             .clients
             .get(CODEX_APPS_MCP_SERVER_NAME)
@@ -399,7 +398,7 @@ impl McpConnectionManager {
                 tool.tool = tool_with_model_visible_input_schema(&tool.tool);
                 tool
             });
-        Ok(qualify_tools(tools))
+        Ok(normalize_tools_for_model(tools))
     }
 
     /// Returns a single map that contains all resources. Each key is the
@@ -638,7 +637,7 @@ impl McpConnectionManager {
     pub async fn resolve_tool_info(&self, tool_name: &ToolName) -> Option<ToolInfo> {
         let all_tools = self.list_all_tools().await;
         all_tools
-            .into_values()
+            .into_iter()
             .find(|tool| tool.canonical_tool_name() == *tool_name)
     }
 

codex-rs/codex-mcp/src/connection_manager_tests.rs

@@ -14,7 +14,7 @@ use crate::rmcp_client::elicitation_capability_for_server;
 use crate::tools::ToolFilter;
 use crate::tools::ToolInfo;
 use crate::tools::filter_tools;
-use crate::tools::qualify_tools;
+use crate::tools::normalize_tools_for_model;
 use crate::tools::tool_with_model_visible_input_schema;
 use codex_config::Constrained;
 use codex_protocol::ToolName;
@@ -85,6 +85,13 @@ fn create_codex_apps_tools_cache_context(
     }
 }
 
+fn model_tool_names(tools: &[ToolInfo]) -> HashSet<ToolName> {
+    tools
+        .iter()
+        .map(ToolInfo::canonical_tool_name)
+        .collect::<HashSet<_>>()
+}
+
 #[test]
 fn declared_openai_file_fields_treat_names_literally() {
     let meta = serde_json::json!({
@@ -272,35 +279,41 @@ async fn disabled_permissions_do_not_auto_accept_elicitation_with_requested_fiel
 }
 
 #[test]
-fn test_qualify_tools_short_non_duplicated_names() {
+fn test_normalize_tools_short_non_duplicated_names() {
     let tools = vec![
         create_test_tool("server1", "tool1"),
         create_test_tool("server1", "tool2"),
     ];
 
-    let qualified_tools = qualify_tools(tools);
+    let model_tools = normalize_tools_for_model(tools);
 
-    assert_eq!(qualified_tools.len(), 2);
-    assert!(qualified_tools.contains_key("mcp__server1__tool1"));
-    assert!(qualified_tools.contains_key("mcp__server1__tool2"));
+    assert_eq!(
+        model_tool_names(&model_tools),
+        HashSet::from([
+            ToolName::namespaced("mcp__server1__", "tool1"),
+            ToolName::namespaced("mcp__server1__", "tool2")
+        ])
+    );
 }
 
 #[test]
-fn test_qualify_tools_duplicated_names_skipped() {
+fn test_normalize_tools_duplicated_names_skipped() {
     let tools = vec![
         create_test_tool("server1", "duplicate_tool"),
         create_test_tool("server1", "duplicate_tool"),
     ];
 
-    let qualified_tools = qualify_tools(tools);
+    let model_tools = normalize_tools_for_model(tools);
 
     // Only the first tool should remain, the second is skipped
-    assert_eq!(qualified_tools.len(), 1);
-    assert!(qualified_tools.contains_key("mcp__server1__duplicate_tool"));
+    assert_eq!(
+        model_tool_names(&model_tools),
+        HashSet::from([ToolName::namespaced("mcp__server1__", "duplicate_tool")])
+    );
 }
 
 #[test]
-fn test_qualify_tools_long_names_same_server() {
+fn test_normalize_tools_long_names_same_server() {
     let server_name = "my_server";
 
     let tools = vec![
@@ -314,116 +327,131 @@ fn test_qualify_tools_long_names_same_server() {
         ),
     ];
 
-    let qualified_tools = qualify_tools(tools);
+    let model_tools = normalize_tools_for_model(tools);
 
-    assert_eq!(qualified_tools.len(), 2);
+    assert_eq!(model_tools.len(), 2);
 
-    let mut keys: Vec<_> = qualified_tools.keys().cloned().collect();
-    keys.sort();
+    let names = model_tool_names(&model_tools);
 
-    assert!(keys.iter().all(|key| key.len() == 64));
-    assert!(keys.iter().all(|key| key.starts_with("mcp__my_server__")));
+    assert!(names.iter().all(|name| name.display().len() == 64));
+    assert!(
+        names
+            .iter()
+            .all(|name| name.namespace.as_deref() == Some("mcp__my_server__"))
+    );
     assert!(
-        keys.iter()
-            .all(|key| key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')),
-        "qualified names must be code-mode compatible: {keys:?}"
+        names.iter().all(|name| name
+            .display()
+            .chars()
+            .all(|c| c.is_ascii_alphanumeric() || c == '_')),
+        "model-visible names must be code-mode compatible: {names:?}"
     );
 }
 
 #[test]
-fn test_qualify_tools_sanitizes_invalid_characters() {
+fn test_normalize_tools_sanitizes_invalid_characters() {
     let tools = vec![create_test_tool("server.one", "tool.two-three")];
 
-    let qualified_tools = qualify_tools(tools);
+    let model_tools = normalize_tools_for_model(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_three");
+    assert_eq!(model_tools.len(), 1);
+    let tool = model_tools.into_iter().next().expect("one tool");
+    let model_name = tool.canonical_tool_name();
+    assert_eq!(
+        model_name,
+        ToolName::namespaced("mcp__server_one__", "tool_two_three")
+    );
     assert_eq!(
         format!("{}{}", tool.callable_namespace, tool.callable_name),
-        qualified_name
+        model_name.display()
     );
 
-    // The key and callable parts are sanitized for model-visible tool calls, but
-    // the raw MCP name is preserved for the actual MCP call.
+    // The callable parts are sanitized for model-visible tool calls, but the raw
+    // MCP name is preserved for the actual MCP call.
     assert_eq!(tool.server_name, "server.one");
     assert_eq!(tool.callable_namespace, "mcp__server_one__");
     assert_eq!(tool.callable_name, "tool_two_three");
     assert_eq!(tool.tool.name, "tool.two-three");
 
     assert!(
-        qualified_name
+        model_name
+            .display()
             .chars()
             .all(|c| c.is_ascii_alphanumeric() || c == '_'),
-        "qualified name must be code-mode compatible: {qualified_name:?}"
+        "model-visible name must be code-mode compatible: {model_name:?}"
     );
 }
 
 #[test]
-fn test_qualify_tools_keeps_hyphenated_mcp_tools_callable() {
+fn test_normalize_tools_keeps_hyphenated_mcp_tools_callable() {
     let tools = vec![create_test_tool("music-studio", "get-strudel-guide")];
 
-    let qualified_tools = qualify_tools(tools);
+    let model_tools = normalize_tools_for_model(tools);
 
-    assert_eq!(qualified_tools.len(), 1);
-    let (qualified_name, tool) = qualified_tools.into_iter().next().expect("one tool");
-    assert_eq!(qualified_name, "mcp__music_studio__get_strudel_guide");
+    assert_eq!(model_tools.len(), 1);
+    let tool = model_tools.into_iter().next().expect("one tool");
+    assert_eq!(
+        tool.canonical_tool_name(),
+        ToolName::namespaced("mcp__music_studio__", "get_strudel_guide")
+    );
     assert_eq!(tool.callable_namespace, "mcp__music_studio__");
     assert_eq!(tool.callable_name, "get_strudel_guide");
     assert_eq!(tool.tool.name, "get-strudel-guide");
 }
 
 #[test]
-fn test_qualify_tools_disambiguates_sanitized_namespace_collisions() {
+fn test_normalize_tools_disambiguates_sanitized_namespace_collisions() {
     let tools = vec![
         create_test_tool("basic-server", "lookup"),
         create_test_tool("basic_server", "query"),
     ];
 
-    let qualified_tools = qualify_tools(tools);
+    let model_tools = normalize_tools_for_model(tools);
 
-    assert_eq!(qualified_tools.len(), 2);
-    let mut namespaces = qualified_tools
-        .values()
+    assert_eq!(model_tools.len(), 2);
+    let mut namespaces = model_tools
+        .iter()
         .map(|tool| tool.callable_namespace.as_str())
         .collect::<Vec<_>>();
     namespaces.sort();
     namespaces.dedup();
     assert_eq!(namespaces.len(), 2);
 
-    let raw_servers = qualified_tools
-        .values()
+    let raw_servers = model_tools
+        .iter()
         .map(|tool| tool.server_name.as_str())
         .collect::<HashSet<_>>();
     assert_eq!(raw_servers, HashSet::from(["basic-server", "basic_server"]));
+    let model_names = model_tool_names(&model_tools);
     assert!(
-        qualified_tools
-            .keys()
-            .all(|key| key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')),
-        "qualified names must be code-mode compatible: {qualified_tools:?}"
+        model_names.iter().all(|name| name
+            .display()
+            .chars()
+            .all(|c| c.is_ascii_alphanumeric() || c == '_')),
+        "model-visible names must be code-mode compatible: {model_names:?}"
     );
 }
 
 #[test]
-fn test_qualify_tools_disambiguates_sanitized_tool_name_collisions() {
+fn test_normalize_tools_disambiguates_sanitized_tool_name_collisions() {
     let tools = vec![
         create_test_tool("server", "tool-name"),
         create_test_tool("server", "tool_name"),
     ];
 
-    let qualified_tools = qualify_tools(tools);
+    let model_tools = normalize_tools_for_model(tools);
 
-    assert_eq!(qualified_tools.len(), 2);
-    let raw_tool_names = qualified_tools
-        .values()
+    assert_eq!(model_tools.len(), 2);
+    let raw_tool_names = model_tools
+        .iter()
         .map(|tool| tool.tool.name.to_string())
         .collect::<HashSet<_>>();
     assert_eq!(
         raw_tool_names,
         HashSet::from(["tool-name".to_string(), "tool_name".to_string()])
     );
-    let callable_tool_names = qualified_tools
-        .values()
+    let callable_tool_names = model_tools
+        .iter()
         .map(|tool| tool.callable_name.as_str())
         .collect::<HashSet<_>>();
     assert_eq!(callable_tool_names.len(), 2);
@@ -672,7 +700,11 @@ async fn list_all_tools_uses_startup_snapshot_while_client_is_pending() {
 
     let tools = manager.list_all_tools().await;
     let tool = tools
-        .get("mcp__codex_apps__calendar_create_event")
+        .iter()
+        .find(|tool| {
+            tool.canonical_tool_name()
+                == ToolName::namespaced("mcp__codex_apps__", "calendar_create_event")
+        })
         .expect("tool from startup cache");
     assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME);
     assert_eq!(tool.callable_name, "calendar_create_event");
@@ -798,7 +830,11 @@ async fn list_all_tools_uses_startup_snapshot_when_client_startup_fails() {
 
     let tools = manager.list_all_tools().await;
     let tool = tools
-        .get("mcp__codex_apps__calendar_create_event")
+        .iter()
+        .find(|tool| {
+            tool.canonical_tool_name()
+                == ToolName::namespaced("mcp__codex_apps__", "calendar_create_event")
+        })
         .expect("tool from startup cache");
     assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME);
     assert_eq!(tool.callable_name, "calendar_create_event");

codex-rs/codex-mcp/src/lib.rs

@@ -48,7 +48,6 @@ pub use mcp::oauth_login_support;
 pub use mcp::resolve_oauth_scopes;
 pub use mcp::should_retry_without_scopes;
 
-pub use codex_apps::filter_non_codex_apps_mcp_tools_only;
 pub use mcp::McpPermissionPromptAutoApproveContext;
 pub use mcp::mcp_permission_prompt_is_auto_approved;
 pub use mcp::qualified_mcp_tool_name_prefix;

codex-rs/codex-mcp/src/mcp/mod.rs

@@ -564,7 +564,7 @@ async fn collect_mcp_server_status_snapshot_from_manager(
     );
 
     let mut tools_by_server = HashMap::<String, HashMap<String, Tool>>::new();
-    for (_qualified_name, tool_info) in tools {
+    for tool_info in tools {
         let raw_tool_name = tool_info.tool.name.to_string();
         let Some(tool) = protocol_tool_from_rmcp_tool(&raw_tool_name, &tool_info.tool) else {
             continue;