feat: #1167 add opt-in server-prefixed MCP tool names#3019
feat: #1167 add opt-in server-prefixed MCP tool names#3019mavrickdeveloper wants to merge 4 commits intoopenai:mainfrom
Conversation
github-actions
Bot
added
bug
mavrickdeveloper
changed the title
mavrickdeveloper
changed the title
seratch
left a comment
seratch
left a comment
There was a problem hiding this comment.
One remaining collision surface seems worth covering before merge: handoff tool names are not included in reserved_tool_names.
Agent.get_mcp_tools() currently reserves existing local FunctionTool names, but handoffs are also serialized as function tools in the same model request via handoff.tool_name. During turn resolution, handoff_map is checked before function-tool lookup, so an agent with a handoff named mcp_calendar__search plus a calendar MCP server exposing search would publish a duplicate public function name, and a model call to mcp_calendar__search would be treated as a handoff instead of the MCP tool.
Could we include enabled handoff tool names in the reserved set too, or otherwise validate and fail fast on this collision? That would make the new collision-avoidance behavior cover all local function-like tools, not just entries in agent.tools.
github-actions
Bot
added
enhancement
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c11fd67dd6
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| candidate = cls._shorten_tool_name(base_name, seed) | ||
| if candidate not in reserved_names: | ||
| return candidate |
There was a problem hiding this comment.
When include_server_in_tool_names is enabled, this returns the unsuffixed base_name for whichever colliding tool is seen first, and only later tools get hashed suffixes. If a server returns tools in a different order between turns (or across resumed runs), two MCP tools whose names normalize to the same safe form (for example search and search!) can swap public aliases, so a previously emitted tool name can dispatch to a different underlying MCP tool. This violates the commit’s deterministic-name guarantee and can route model calls to the wrong tool in multi-turn runs.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Use batch-local server/tool coordinates for prefixed MCP tool name allocation so the override map is scoped to a single listing pass and does not rely on Python object identity.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Summary
This fixes the duplicate local MCP tool-name case from #1167 without changing the default behavior. By default, duplicate MCP tool names still fail fast. When
include_server_in_tool_namesis enabled, local MCP tools are exposed with deterministic server-prefixed public names, while calls still go back to the original MCP tool name on the original server.The generated names are kept safe for function-tool payloads: ASCII-only, capped at 64 characters, and collision-resistant when server names or tool names normalize to the same value. The reservation set also includes local function tool names on the agent so an MCP alias cannot shadow an existing tool.
Test plan
uv run pytest tests/mcp/test_mcp_util.py tests/mcp/test_runner_calls_mcp.py -quv run mypy src/agents/agent.py src/agents/mcp/util.py tests/mcp/test_mcp_util.py tests/mcp/test_runner_calls_mcp.pyuv run pyright --project pyrightconfig.json src/agents/agent.py src/agents/mcp/util.py tests/mcp/test_mcp_util.py tests/mcp/test_runner_calls_mcp.pyuv run ruff check src/agents/agent.py src/agents/mcp/util.py tests/mcp/test_mcp_util.py tests/mcp/test_runner_calls_mcp.pymake build-docsnpx @modelcontextprotocol/server-filesystem: verified unique server-prefixed names and invokedlist_allowed_directoriesthrough the prefixed public name.I also ran the repo verification script.
make formatandmake lintpassed, but the fullmake tests/ repo-widemake typecheckare currently blocked locally by unrelated optional dependency, runloop, voice, and sandbox failures. The focused MCP checks above pass.Issue number
Closes #1167
Checks
make lintandmake format