fix(mcp): gate utility stubs on server-advertised capabilities - #18052
fix(mcp): gate utility stubs on server-advertised capabilities#18052nikolay-bratanov wants to merge 1 commit into
Conversation
The four MCP utility schemas (`list_resources` / `read_resource` / `list_prompts` / `get_prompt`) were registered for every connected MCP server because `_select_utility_schemas` checked `hasattr(server.session, required_method)` — which is always True, since `mcp.ClientSession` defines all four methods on the class regardless of what the remote server supports. Servers that advertise only the `tools` capability (e.g. Context7, `@upstash/context7-mcp` v2.2.3) therefore exposed 4 dead schemas to the LLM. Calling those stubs returns JSON-RPC -32601 "Method not found", which leads the model to report the server as broken even when the real `tools/*` methods work fine. Fix: * Cache the `InitializeResult` returned from `session.initialize()` on `MCPServerTask.initialize_result` (added to `__slots__`, initialised to None, populated at all three `session.initialize()` call sites). * In `_select_utility_schemas`, inspect `initialize_result.capabilities.resources` / `.capabilities.prompts` and skip the corresponding stubs when those sub-objects are None (per the MCP spec, they are non-None only when the server actually implements those request types). * Keep the previous `hasattr` check as a fallback for the rare case where `initialize_result` is missing (e.g. older test fixtures), so pre-existing behaviour is unchanged on that code path. Closes NousResearch#18051
trevorgordon981
left a comment
There was a problem hiding this comment.
LGTM. The fix is minimal, surgical, and addresses the root cause: the was being discarded, so we couldn’t gate utility stubs on actual server capabilities.
Verified:
- ✅ Unit test (inline) passes: tools-only server → , full-caps → all 4 utilities, legacy fallback → all 4
- ✅ All 182 MCP tool tests pass
- ✅ No breaking changes: defaults to , fallback preserves old behavior
- ✅ Low risk: one new slot, 3 assignments, one gated if/else block
The change correctly filters out , , , and for servers like that only advertise . Well done.
trevorgordon981
left a comment
There was a problem hiding this comment.
LGTM. The fix is minimal, surgical, and addresses the root cause: the initialize_result was being discarded, so we could not gate utility stubs on actual server capabilities.
Verified:
- Unit test (inline) passes: tools-only server -> [], full-caps -> all 4 utilities, legacy fallback -> all 4
- All 182 MCP tool tests pass
- No breaking changes: initialize_result defaults to None, fallback preserves old behavior
- Low risk: one new slot, 3 assignments, one gated if/else block
The change correctly filters out list_resources, read_resource, list_prompts, and get_prompt for servers like @upstash/context7-mcp that only advertise tools. Well done.
|
Automated hermes-sweeper review found this fix is already implemented on current Evidence:
Thanks for the original repro and root-cause analysis; it matches what landed on main. |
Summary
Fixes #18051. The four MCP utility schemas (
list_resources/read_resource/list_prompts/get_prompt) are registered for every connected MCP server, even when the server advertises only thetoolscapability. Today's gate useshasattr(server.session, required_method), which is alwaysTruebecausemcp.ClientSessiondefines all four methods on the class regardless of what the remote server actually supports.The InitializeResult returned from
await session.initialize()is the source of truth —result.capabilities.resourcesandresult.capabilities.promptsare non-Noneonly when the server implements those request types — but it was being discarded.Changes
MCPServerTask.__slots__: add"initialize_result".MCPServerTask.__init__: initialiseself.initialize_result: Optional[Any] = None.await session.initialize()call sites (_run_http,_run_httpreconnect path,_run_stdio): assign the result intoself.initialize_result._select_utility_schemas: wheninitialize_resultis present, skiplist_resources/read_resourceifcapabilities.resources is None, and skiplist_prompts/get_promptifcapabilities.prompts is None. Wheninitialize_resultisNone(older test fixtures, no real connect), fall back to the existinghasattrcheck so behaviour on that path is unchanged.Verification
Direct unit-style test against a freshly patched tree (
venv/bin/pythonagainsttools.mcp_tool):End-to-end: I have this running locally against
@upstash/context7-mcp(which is the canonical "tools-only" server). With the patch applied,_register_server_toolsregisters exactly two LLM-visible tools (mcp_context7_resolve_library_id,mcp_context7_query_docs) instead of the previous six.Risk
initialize_resultslot isNoneuntil the first successfulinitialize(). The fallback path keeps existing behaviour for code paths that haven't reached that point yet.Notes
I haven't added a unit test in this PR because the existing tests in
tests/tools/test_mcp_tool.pymockClientSessionrather than the full connect/initialize flow, and adding a capabilities-gating test would require either a new fixture forInitializeResultor relaxing the existing fixtures. Happy to follow up with one in a separate commit if you'd like — let me know which fixture style you prefer.