fix: handle duplicate tool names across MCP servers by auto-renaming#2954
fix: handle duplicate tool names across MCP servers by auto-renaming#2954houziershi wants to merge 1 commit intoopenai:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b6fef1c17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for tool in server_tools: | ||
| if tool.name in duplicates: | ||
| original_name = tool.name | ||
| new_name = f"{server.name}__{original_name}" |
There was a problem hiding this comment.
Normalize server names before building renamed MCP tool names
This rename path uses server.name verbatim (f"{server.name}__{original_name}"), but built-in MCP server defaults include spaces/colons/URLs (for example "stdio: ..." and "sse: ..." in src/agents/mcp/server.py). When duplicates occur, this can generate tool names like "stdio: cmd__search", which are not valid function tool identifiers for OpenAI tool payloads and can cause runtime request failures instead of resolving the collision. The prefix should be sanitized (or otherwise encoded) before constructing the public tool name.
Useful? React with 👍 / 👎.
| while new_name in tool_names: | ||
| new_name = f"{new_name}_" |
There was a problem hiding this comment.
Reserve current-server names when choosing duplicate aliases
The alias collision loop only checks tool_names, which does not yet include later tools from the same server_tools list. If the current server already has a tool named like the generated alias (e.g., existing server2__search plus duplicated search), the duplicated tool can be renamed to that same name and then the original tool is appended unchanged, leaving duplicate public names and ambiguous dispatch. Alias generation needs to account for all names in the current server, not just previously seen servers.
Useful? React with 👍 / 👎.
1b6fef1 to
7c372d4
Compare
When multiple MCP servers have tools with the same name, instead of raising a UserError, the SDK now automatically renames duplicate tools with a server prefix (e.g., 'server_name__tool_name'). This allows users to use multiple MCP servers even when they have overlapping tool names. Changes: - Modified MCPUtil.get_all_function_tools() to detect duplicates and rename them instead of raising an error - Added MCPUtil._rename_tool() helper to properly copy and rename FunctionTool instances while preserving all internal metadata (_tool_origin, _mcp_title, etc.) - Added warning logs to inform users about the renaming - Updated existing tests to reflect the new behavior - Added new tests for duplicate name handling scenarios Fixes openai#1167 Fixes openai#464
7c372d4 to
86683e0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86683e01ac
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| new_name = cls._build_renamed_tool_name( | ||
| server.name, | ||
| original_name, | ||
| tool_names, |
There was a problem hiding this comment.
Avoid aliasing renamed MCP tools onto existing agent tools
When a duplicate MCP tool is renamed here, the uniqueness check only considers tool_names from MCP servers, so the generated alias can still collide with a user-defined tool name (for example a local function tool already named server2__search). AgentBase.get_all_tools() later concatenates MCP tools with local tools without deduplicating (src/agents/agent.py), and build_function_tool_lookup_map() resolves name collisions with last-wins semantics (src/agents/_tool_identity.py), so a model call to the renamed alias can execute the wrong tool. This regression is introduced by the new auto-rename path, because the previous behavior failed fast with UserError on MCP duplicates.
Useful? React with 👍 / 👎.
seratch
left a comment
There was a problem hiding this comment.
I don't think this PR is sufficient. Please take a look at my comment at #2788 (review) for more details.
Also, while fixing this issue may look relatively simple, I don't think it actually is. We'd like to review it carefully to prevent any breaking changes. There may be some other things to consider as well.
When multiple MCP servers have tools with the same name, instead of raising a UserError, the SDK now automatically renames duplicate tools with a server prefix (e.g., 'server_name__tool_name'). This allows users to use multiple MCP servers even when they have overlapping tool names.
Changes:
Fixes #1167
Fixes #464