Skip to content

Commit 333250b

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 9ca38c1 commit 333250b

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
@@ -58,12 +58,21 @@ async def negotiate_auto(session: ClientSession, protocol_version: str | None =
5858
``session.discover_result`` / ``session.initialize_result`` is set on
5959
return.
6060
61+
``protocol_version`` pins the legacy handshake to a specific version. A
62+
caller supplying it wants that exact version, so this skips the
63+
``server/discover`` probe entirely and goes straight to the handshake —
64+
otherwise a server with modern support would win discovery and the pin
65+
would be silently ignored.
66+
6167
Raises:
6268
MCPError: The server is modern-only and shares no version with this
6369
client (-32022 with a disjoint ``supported`` list), or the
6470
fallback handshake failed and one corrective re-probe did too.
6571
Exception: Any transport/network error from the probe propagates as-is.
6672
"""
73+
if protocol_version is not None:
74+
await session.initialize(protocol_version=protocol_version)
75+
return
6776
version = LATEST_MODERN_VERSION
6877
for attempt in range(2):
6978
try:
@@ -78,10 +87,7 @@ async def negotiate_auto(session: ClientSession, protocol_version: str | None =
7887
if supported is not None and not any(v in HANDSHAKE_PROTOCOL_VERSIONS for v in supported):
7988
raise # server is modern-only and disjoint — real incompatibility
8089
try:
81-
if protocol_version is not None:
82-
await session.initialize(protocol_version=protocol_version)
83-
else:
84-
await session.initialize() # every other rpc-error → legacy (the denylist)
90+
await session.initialize() # every other rpc-error → legacy (the denylist)
8591
except MCPError as handshake_exc:
8692
if handshake_exc.code != UNSUPPORTED_PROTOCOL_VERSION or attempt != 0:
8793
raise
@@ -102,10 +108,7 @@ async def negotiate_auto(session: ClientSession, protocol_version: str | None =
102108
try:
103109
result = types.DiscoverResult.model_validate(raw)
104110
except ValidationError:
105-
if protocol_version is not None:
106-
await session.initialize(protocol_version=protocol_version)
107-
else:
108-
await session.initialize() # unparseable result → not modern evidence
111+
await session.initialize() # unparseable result → not modern evidence
109112
return
110113
if not any(v in result.supported_versions for v in MODERN_PROTOCOL_VERSIONS):
111114
# A discover-answering server that advertises no modern version

tests/client/test_probe.py

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

337337

338-
async def test_negotiate_auto_mcp_error_with_custom_protocol_version() -> None:
339-
"""Test that negotiate_auto initializes with a custom protocol version when discover returns an MCPError."""
340-
session = _StubSession(MCPError(code=METHOD_NOT_FOUND, message="nope"))
341-
await _negotiate(session, protocol_version="2024-11-05")
342-
assert session.initialized
343-
assert session.initialize_version == "2024-11-05"
338+
# --- protocol_version override forces the legacy handshake, unconditionally ---
344339

345340

346-
async def test_negotiate_auto_validation_error_with_custom_protocol_version() -> None:
347-
"""Test that negotiate_auto initializes with a custom protocol version when discover returns unparseable result."""
348-
session = _StubSession({"not": "a discover result"})
341+
async def test_a_protocol_version_override_skips_discovery_and_forces_the_legacy_handshake() -> None:
342+
"""`protocol_version` pins an explicit legacy version, so the caller wants exactly that
343+
version - the probe is skipped entirely and the handshake runs unconditionally, even
344+
though the stub's discover script would otherwise return a valid modern result (regression:
345+
the override used to only reach `initialize()` via the fallback paths, so a successful
346+
discover silently dropped it)."""
347+
session = _StubSession(_discover_dict())
349348
await _negotiate(session, protocol_version="2024-11-05")
349+
assert session.probed_at == []
350350
assert session.initialized
351351
assert session.initialize_version == "2024-11-05"
352+
assert session.adopted is None

tests/interaction/_requirements.py

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

tests/interaction/lowlevel/test_client_connect.py

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

180180

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

0 commit comments

Comments
 (0)