Skip to content

Commit f55e795

Browse files
committed
fix: honor protocol_version_override on the auto-mode success path
negotiate_auto only consulted protocol_version in its initialize() fallback calls, so mode="auto" (the default) silently dropped the override whenever the server/discover probe succeeded first - the override only ever took effect when the probe failed. Since the whole point of protocol_version_override is to let a caller pin an older or custom protocol version, an override must always win: when set, skip the discover probe entirely and go straight to the legacy handshake at that version. Regression tests: a unit test on negotiate_auto proving the probe is skipped even when the stub's discover script would otherwise succeed, and an e2e test over a real streamable-HTTP server (mode="auto" + override) proving only `initialize` is sent and `server/discover` never is. Reported by a static review pass on this PR; verified independently by tracing the actual control flow before applying this fix.
1 parent abb1d5b commit f55e795

4 files changed

Lines changed: 58 additions & 17 deletions

File tree

src/mcp/client/_probe.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,21 @@ async def negotiate_auto(session: ClientSession, protocol_version: str | None =
5353
``session.discover_result`` / ``session.initialize_result`` is set on
5454
return.
5555
56+
``protocol_version`` pins the legacy handshake to a specific version. A
57+
caller supplying it wants that exact version, so this skips the
58+
``server/discover`` probe entirely and goes straight to the handshake —
59+
otherwise a server with modern support would win discovery and the pin
60+
would be silently ignored.
61+
5662
Raises:
5763
MCPError: The server is modern-only and shares no version with this
5864
client (-32022 with a disjoint ``supported`` list), or the
5965
fallback handshake failed and one corrective re-probe did too.
6066
Exception: Any transport/network error from the probe propagates as-is.
6167
"""
68+
if protocol_version is not None:
69+
await session.initialize(protocol_version=protocol_version)
70+
return
6271
version = LATEST_MODERN_VERSION
6372
for attempt in range(2):
6473
try:
@@ -73,10 +82,7 @@ async def negotiate_auto(session: ClientSession, protocol_version: str | None =
7382
if supported is not None and not any(v in HANDSHAKE_PROTOCOL_VERSIONS for v in supported):
7483
raise # server is modern-only and disjoint — real incompatibility
7584
try:
76-
if protocol_version is not None:
77-
await session.initialize(protocol_version=protocol_version)
78-
else:
79-
await session.initialize() # every other rpc-error → legacy (the denylist)
85+
await session.initialize() # every other rpc-error → legacy (the denylist)
8086
except MCPError as handshake_exc:
8187
if handshake_exc.code != UNSUPPORTED_PROTOCOL_VERSION or attempt != 0:
8288
raise
@@ -97,10 +103,7 @@ async def negotiate_auto(session: ClientSession, protocol_version: str | None =
97103
try:
98104
result = types.DiscoverResult.model_validate(raw)
99105
except ValidationError:
100-
if protocol_version is not None:
101-
await session.initialize(protocol_version=protocol_version)
102-
else:
103-
await session.initialize() # unparseable result → not modern evidence
106+
await session.initialize() # unparseable result → not modern evidence
104107
return
105108
session.adopt(result)
106109
return

tests/client/test_probe.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -323,17 +323,18 @@ def test_parse_supported_returns_none_for_anything_not_shaped_like_the_spec_erro
323323
assert _parse_supported(data) == expected
324324

325325

326-
async def test_negotiate_auto_mcp_error_with_custom_protocol_version() -> None:
327-
"""Test that negotiate_auto initializes with a custom protocol version when discover returns an MCPError."""
328-
session = _StubSession(MCPError(code=METHOD_NOT_FOUND, message="nope"))
329-
await _negotiate(session, protocol_version="2024-11-05")
330-
assert session.initialized
331-
assert session.initialize_version == "2024-11-05"
326+
# --- protocol_version override forces the legacy handshake, unconditionally ---
332327

333328

334-
async def test_negotiate_auto_validation_error_with_custom_protocol_version() -> None:
335-
"""Test that negotiate_auto initializes with a custom protocol version when discover returns unparseable result."""
336-
session = _StubSession({"not": "a discover result"})
329+
async def test_a_protocol_version_override_skips_discovery_and_forces_the_legacy_handshake() -> None:
330+
"""`protocol_version` pins an explicit legacy version, so the caller wants exactly that
331+
version - the probe is skipped entirely and the handshake runs unconditionally, even
332+
though the stub's discover script would otherwise return a valid modern result (regression:
333+
the override used to only reach `initialize()` via the fallback paths, so a successful
334+
discover silently dropped it)."""
335+
session = _StubSession(_discover_dict())
337336
await _negotiate(session, protocol_version="2024-11-05")
337+
assert session.probed_at == []
338338
assert session.initialized
339339
assert session.initialize_version == "2024-11-05"
340+
assert session.adopted is None

tests/interaction/_requirements.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,15 @@ def __post_init__(self) -> None:
464464
),
465465
added_in="2026-07-28",
466466
),
467+
"lifecycle:mode:auto-override-skips-discover": Requirement(
468+
source="sdk",
469+
behavior=(
470+
"A Client constructed with mode='auto' and protocol_version_override=<version> sends "
471+
"initialize at that version as its first request and never sends server/discover, even "
472+
"when the server would answer discover successfully."
473+
),
474+
added_in="2026-07-28",
475+
),
467476
# ═══════════════════════════════════════════════════════════════════════════
468477
# Protocol primitives: cancellation, timeout, progress, errors, _meta
469478
# ═══════════════════════════════════════════════════════════════════════════

tests/interaction/lowlevel/test_client_connect.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,34 @@ async def test_auto_mode_probes_server_discover_and_adopts_the_result() -> None:
175175
assert "initialize" not in [b["method"] for b in bodies]
176176

177177

178+
@requirement("lifecycle:mode:auto-override-skips-discover")
179+
async def test_auto_mode_with_a_protocol_version_override_skips_discover_and_initializes() -> None:
180+
"""`Client(..., mode='auto', protocol_version_override=...)` sends `initialize` at the
181+
override version and never probes `server/discover`, even though the mounted server answers
182+
discover successfully. Regression: the override used to only reach `negotiate_auto`'s
183+
`initialize()` fallback calls, so a successful discover silently dropped it and the client
184+
ended up modern-negotiated at the server's latest version instead of the pinned one.
185+
"""
186+
requests, on_request = _request_recorder()
187+
server = _tools_server("discoverable")
188+
189+
with anyio.fail_after(5):
190+
async with (
191+
mounted_app(server, on_request=on_request) as (http, _),
192+
Client(
193+
streamable_http_client(f"{BASE_URL}/mcp", http_client=http),
194+
mode="auto",
195+
protocol_version_override="2024-11-05",
196+
) as client,
197+
):
198+
assert client.protocol_version == "2024-11-05"
199+
assert client.server_info.name == "discoverable"
200+
201+
bodies = [json.loads(r.content)["method"] for r in requests if r.method == "POST"]
202+
assert bodies[0] == "initialize"
203+
assert "server/discover" not in bodies
204+
205+
178206
@requirement("lifecycle:discover:retry-on-32022")
179207
async def test_auto_mode_retries_discover_once_on_unsupported_protocol_version() -> None:
180208
"""A -32022 from `server/discover` triggers exactly one retry at the highest mutual modern version.

0 commit comments

Comments
 (0)