Repository navigation
Carry capabilities.extensions on legacy initialize handshakes
#3776
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -934,6 +934,23 @@ def test_serialize_server_result_drops_2026_only_keys_at_pre_2026_versions(metho | |
| assert "cacheScope" not in sieved | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("version", PRE_2026) | ||
| def test_initialize_round_trips_capability_extensions_at_pre_2026_versions(version: str): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 nit (optional): the new test Why this was flaggedNo runtime input triggers this; it is a convention check on the added test at tests/types/test_methods.py:938. The signature is Verification: AGENTS.md line 50 reads Finding ID:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 nit (optional): The new parametrized test has no docstring, so a later failure gives no hint whether it is a spec regression or an SDK choice. Why this was flaggedThe trigger is a future failure of Verification: nit — triggered whenever this test later fails and someone has to triage it; nothing breaks at runtime. The skill rule is at .claude/skills/test-quality/SKILL.md:16-19. The added test violates it: tests/types/test_methods.py:937-951 is new in this diff, and line 938 is followed directly at line 939 by Finding ID: |
||
| extensions = {"io.modelcontextprotocol/ui": {"mimeTypes": ["text/html;profile=mcp-app"]}} | ||
| capabilities = {"tools": {}, "extensions": extensions} | ||
| result = {"protocolVersion": version, "capabilities": capabilities, "serverInfo": {"name": "S", "version": "1"}} | ||
| sieved = methods.serialize_server_result("initialize", version, result) | ||
| assert sieved["capabilities"] == capabilities | ||
| parsed = methods.parse_server_result("initialize", version, sieved) | ||
| assert isinstance(parsed, types.InitializeResult) | ||
| assert parsed.capabilities.extensions == extensions | ||
| client_info = {"name": "C", "version": "1"} | ||
| params = {"protocolVersion": version, "capabilities": {"extensions": extensions}, "clientInfo": client_info} | ||
| request = methods.parse_client_request("initialize", version, params) | ||
| assert isinstance(request, types.InitializeRequest) | ||
| assert request.params.capabilities.extensions == extensions | ||
|
Comment on lines
+947
to
+951
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 nit (optional): The client-request half of the new test passes without this PR, so nothing proves the Why this was flaggedThe condition is always present: Verification: The request half of the new test (tests/types/test_methods.py:947-951) would pass on the base commit, so it does not exercise the Finding ID: |
||
|
|
||
|
|
||
| def test_serialize_server_result_keeps_2026_only_keys_at_2026_07_28(): | ||
| dumped = MONOLITH_RESULT_FIXTURES["tools/list"].model_dump(by_alias=True, mode="json", exclude_none=True) | ||
| sieved = methods.serialize_server_result("tools/list", "2026-07-28", dumped) | ||
|
|
||
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.
🟡 (optional) Legacy clients are now told a server supports an extension whose only methods that server answers with METHOD_NOT_FOUND on the same connection. The legacy
initializeresult advertises every entry ofself.extensionswhatever the version, while_version_gated(src/mcp/server/mcpserver/server.py:1389) still refuses methods pinned withprotocol_versions. Fix: on the legacyinitializepath, omit identifiers whose contributions are all gated away from handshake versions, mirroring the client's_build_capabilities(src/mcp/client/session.py:621-628). The base hid this because the 2025 sieve dropped the map. [also at: scripts/gen_surface_types.py:80 - Legacy clients are now told a server supports an extension whose methods it refuses on that connection. Theinitializeresult now carries every entry ofServer.extensions, whatever version was negotiated.]Why this was flagged
Trigger: an
MCPServerwhose extension binds only version-pinned methods, as docs_src/extensions/tutorial004.py:35-40 does withprotocol_versions=frozenset({"2026-07-28"}), reached by a client withmode="legacy"._apply_extensionwritesself._lowlevel_server.extensions[extension.identifier]unconditionally (src/mcp/server/mcpserver/server.py:352), andget_capabilitiescopies the whole map with no version filter (src/mcp/server/lowlevel/server.py:634). After this change the legacyinitializeresult therefore carriesextensions: {"com.example/search": {}}, yetgatedraisesMETHOD_NOT_FOUNDfor that method (src/mcp/server/mcpserver/server.py:1389-1390). A client that feature-detects onserver_capabilities.extensionsnow passes its check and gets -32601; on the base branch the map was dropped by the 2025 sieve, so the client sawNoneand skipped the call. No safeguard applies: the client side drops inactive identifiers on legacy, but the server has no counterpart.Verification: Triggers when an
MCPServerextension binds only methods pinned to a modern version and a client connects with a legacyinitializehandshake.src/mcp/server/mcpserver/server.py:1389-1390:if ctx.protocol_version not in versions: raise MCPError(code=METHOD_NOT_FOUND, ...). On the base, the 2025 wire model had noextensionsfield, so theinitializesieve dropped the map.Finding ID:
f-f019fd81