From 0170860ef2a55a964dbbfd7c7f8746ac32ffa024 Mon Sep 17 00:00:00 2001 From: Gabriel Peal Date: Sun, 19 Oct 2025 17:41:55 -0700 Subject: [PATCH] [MCP] Prefix MCP tools names with `mcp__` (#5309) This should make it more clear that specific tools come from MCP servers. #4806 requested that we add the server name but we already do that. Fixes #4806 --- codex-rs/.gitignore | 1 + codex-rs/core/src/mcp_connection_manager.rs | 14 +++++++------- codex-rs/core/tests/suite/rmcp_client.rs | 8 ++++---- codex-rs/tui/src/history_cell.rs | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/codex-rs/.gitignore b/codex-rs/.gitignore index e9962537..e3156604 100644 --- a/codex-rs/.gitignore +++ b/codex-rs/.gitignore @@ -1,4 +1,5 @@ /target/ +/target-*/ # Recommended value of CARGO_TARGET_DIR when using Docker as explained in .devcontainer/README.md. /target-amd64/ diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 99c2e626..9ea63442 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -65,8 +65,8 @@ fn qualify_tools(tools: Vec) -> HashMap { let mut qualified_tools = HashMap::new(); for tool in tools { let mut qualified_name = format!( - "{}{}{}", - tool.server_name, MCP_TOOL_NAME_DELIMITER, tool.tool_name + "mcp{}{}{}{}", + MCP_TOOL_NAME_DELIMITER, tool.server_name, MCP_TOOL_NAME_DELIMITER, tool.tool_name ); if qualified_name.len() > MAX_TOOL_NAME_LENGTH { let mut hasher = Sha1::new(); @@ -741,8 +741,8 @@ mod tests { let qualified_tools = qualify_tools(tools); assert_eq!(qualified_tools.len(), 2); - assert!(qualified_tools.contains_key("server1__tool1")); - assert!(qualified_tools.contains_key("server1__tool2")); + assert!(qualified_tools.contains_key("mcp__server1__tool1")); + assert!(qualified_tools.contains_key("mcp__server1__tool2")); } #[test] @@ -756,7 +756,7 @@ mod tests { // Only the first tool should remain, the second is skipped assert_eq!(qualified_tools.len(), 1); - assert!(qualified_tools.contains_key("server1__duplicate_tool")); + assert!(qualified_tools.contains_key("mcp__server1__duplicate_tool")); } #[test] @@ -784,13 +784,13 @@ mod tests { assert_eq!(keys[0].len(), 64); assert_eq!( keys[0], - "my_server__extremely_lena02e507efc5a9de88637e436690364fd4219e4ef" + "mcp__my_server__extremel119a2b97664e41363932dc84de21e2ff1b93b3e9" ); assert_eq!(keys[1].len(), 64); assert_eq!( keys[1], - "my_server__yet_another_e1c3987bd9c50b826cbe1687966f79f0c602d19ca" + "mcp__my_server__yet_anot419a82a89325c1b477274a41f8c65ea5f3a7f341" ); } } diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 1a2815b0..ab702b5d 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -44,7 +44,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { let call_id = "call-123"; let server_name = "rmcp"; - let tool_name = format!("{server_name}__echo"); + let tool_name = format!("mcp__{server_name}__echo"); mount_sse_once_match( &server, @@ -182,7 +182,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { let call_id = "call-1234"; let server_name = "rmcp_whitelist"; - let tool_name = format!("{server_name}__echo"); + let tool_name = format!("mcp__{server_name}__echo"); mount_sse_once_match( &server, @@ -317,7 +317,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { let call_id = "call-456"; let server_name = "rmcp_http"; - let tool_name = format!("{server_name}__echo"); + let tool_name = format!("mcp__{server_name}__echo"); mount_sse_once_match( &server, @@ -485,7 +485,7 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { let call_id = "call-789"; let server_name = "rmcp_http_oauth"; - let tool_name = format!("{server_name}__echo"); + let tool_name = format!("mcp__{server_name}__echo"); mount_sse_once_match( &server, diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 0a358257..5df687b9 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -1001,7 +1001,7 @@ pub(crate) fn new_mcp_tools_output( } for (server, cfg) in config.mcp_servers.iter() { - let prefix = format!("{server}__"); + let prefix = format!("mcp__{server}__"); let mut names: Vec = tools .keys() .filter(|k| k.starts_with(&prefix))