-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat(mcp): optional server-name prefix for colliding tool names #2442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6b9328f53
ℹ️ 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".
|
This is a well-thought-out change. Introducing server-based namespacing removes ambiguity when duplicate tool names exist across MCP servers and improves overall agent reliability. The sanitization, logging updates, and documentation make the feature complete. Nice work! |
|
I haven't done any e2e tests with this branch yet. Please do manual testing with examples/* in addition to unit tests. |
|
Addressed both requested changes in commit 8f48920:\n\n1. Renamed MCP config key from to across implementation and tests.\n2. Removed all changes from this PR so it only contains code/test updates.\n\nLocal verification:\n- All checks passed!\n- (38 passed)\n\nI will run example-level manual checks next and report back in this PR thread. |
|
Quick update on the requested changes:
Validation done locally:
Commit: 8f48920 |
|
Pushed follow-up format fix for CI lint in commit f47fe53. The full test workflow has been retriggered and is running now. |
Summary
This PR adds an opt-in way to avoid MCP tool name collisions across multiple servers.
When
Agent.mcp_config["prefix_tool_names_with_server_name"] = True, MCP tools are exposed to the model as:<sanitized_server_name>_<tool_name>This keeps current behavior unchanged by default (duplicate names still raise an error), but gives users a first-class way to run multiple MCP servers that expose the same tool names.
Fixes #464
Changes
prefix_tool_names_with_server_nametoMCPConfigAgent.get_mcp_tools()->MCPUtil.get_all_function_tools()"server_")Testing
uv run ruff check src/agents/agent.py src/agents/mcp/util.py tests/mcp/test_mcp_util.pyuv run mypy . --exclude siteuv run pytest tests/mcp/test_mcp_util.py -quv run coverage run -m pytest tests/mcp/test_mcp_util.py -quv run coverage report -m src/agents/mcp/util.pyCoverage
tests/mcp/test_mcp_util.py: 37 passedsrc/agents/mcp/util.py: 90% (existing uncovered branches remain; new collision/prefix paths are covered by added tests)