fix(mcp): preserve original tool name for external MCP servers - #6803
fix(mcp): preserve original tool name for external MCP servers#6803Neuromediator wants to merge 1 commit into
Conversation
Tool names are sanitized during discovery over streamable HTTP, and the sanitized name was then sent back to the server as the name to invoke, so any server-side tool containing a hyphen became unreachable. Context7's `resolve-library-id` was requested as `resolve_library_id`. `_discover_mcp_tools` keyed its schema dict by `sanitize_tool_name(tool.name)` and discarded the original, so `_resolve_external` had nothing else to pass as `MCPToolWrapper(tool_name=...)` -- a parameter whose own docstring described it as "Original name of the tool on the MCP server" and which `_run` forwards verbatim to `session.call_tool`. The native path already solved this: `_resolve_native` reads `original_name` from the tool definition and passes `original_tool_name=` to `MCPNativeTool`. This applies the same pattern to the external path, so the two stay consistent. Sanitized names are still what agents see, so LLM function-calling constraints are unaffected; only the name used to invoke the tool changes. `original_tool_name` defaults to `tool_name`, so existing callers and any schemas cached by a previous version keep working. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Per Flagging it explicitly here so it isn't mistaken for an undisclosed AI contribution. Could a maintainer add the label? Happy to make any changes you'd like. |
📝 WalkthroughWalkthroughMCP tool discovery now stores original server names with sanitized identifiers. ChangesMCP tool name preservation
Sequence Diagram(s)sequenceDiagram
participant MCPServer
participant ToolResolver
participant MCPToolWrapper
MCPServer->>ToolResolver: return tool with original name
ToolResolver->>ToolResolver: sanitize name for schema lookup
ToolResolver->>MCPToolWrapper: pass sanitized and original names
MCPToolWrapper->>MCPServer: invoke original tool name
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/tests/mcp/test_tool_resolver_external.py`:
- Around line 109-120: Extend test_hyphenated_name_is_used_for_server_calls to
execute the resolved tool through the fake transport, capture the name supplied
to ClientSession.call_tool, and assert it is "resolve-library-id". Keep the
existing wrapper-state assertion, but verify the RPC uses the original
hyphenated name rather than a sanitized name.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9acf1fc0-005b-45ce-95f8-d0addc9e1073
📒 Files selected for processing (3)
lib/crewai/src/crewai/mcp/tool_resolver.pylib/crewai/src/crewai/tools/mcp_tool_wrapper.pylib/crewai/tests/mcp/test_tool_resolver_external.py
| def test_hyphenated_name_is_used_for_server_calls(self, resolver): | ||
| tools = self._resolve_one( | ||
| resolver, | ||
| { | ||
| "description": "Resolve a library id", | ||
| "args_schema": None, | ||
| "original_name": "resolve-library-id", | ||
| }, | ||
| ) | ||
|
|
||
| assert len(tools) == 1 | ||
| assert tools[0].original_tool_name == "resolve-library-id" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Verify the MCP RPC argument.
Line 120 checks wrapper state. It does not verify the name passed to ClientSession.call_tool.
Execute the resolved wrapper with the fake transport. Record the call_tool name. Assert that it equals "resolve-library-id". This will detect a regression that sends the sanitized name to the MCP server.
As per coding guidelines, “Write unit tests for new functionality that focus on behavior rather than implementation details.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai/tests/mcp/test_tool_resolver_external.py` around lines 109 - 120,
Extend test_hyphenated_name_is_used_for_server_calls to execute the resolved
tool through the fake transport, capture the name supplied to
ClientSession.call_tool, and assert it is "resolve-library-id". Keep the
existing wrapper-state assertion, but verify the RPC uses the original
hyphenated name rather than a sanitized name.
Source: Coding guidelines
1 participant
Problem
Tool names are sanitized during discovery over streamable HTTP, and the sanitized name is
then sent back to the server as the name to invoke. Any server-side tool whose name contains a
hyphen is therefore unreachable.
Concretely, with Context7:
resolve-library-idis discovered, then invoked asresolve_library_id, and the serverresponds with an unknown-tool error.
Reproduced against the live server on
main:Cause
_discover_mcp_toolskeys its schema dict bysanitize_tool_name(tool.name)and discards theoriginal:
By the time
_resolve_externalbuilds the wrapper, the original name no longer exists, so thesanitized key is passed as
MCPToolWrapper(tool_name=...). That parameter's own docstringdescribes it as "Original name of the tool on the MCP server", and
_runforwards it verbatimto
session.call_tool.Fix
The native path already solves this —
_resolve_nativereadsoriginal_nameoff the tooldefinition and passes
original_tool_name=toMCPNativeTool. This applies the same pattern tothe external path so the two are consistent:
_discover_mcp_toolskeepsoriginal_namealongside the sanitized key.MCPToolWrapper.__init__gainsoriginal_tool_name: str | None = None, mirroringMCPNativeTool.__init__, and storesoriginal_tool_name or tool_name._resolve_externalpasses it through.Eight added lines of source.
Compatibility
function calling; only the name used to invoke the tool changes. There is a test asserting
no hyphen leaks into
tool.name.original_tool_namedefaults totool_name, so existing callers and schemas cached by aprevious version behave exactly as before.
url#toolselection still compares sanitized names, covered by a test.Tests
New
lib/crewai/tests/mcp/test_tool_resolver_external.py— 11 tests covering discovery,resolution, wrapper construction and the warning path. There was previously no external-path
test module.
Verified the tests are meaningful by reverting only the source changes:
The 6 that pass either way are the invariants the fix must not break.
Checks run locally: