fix(client): validate protocol_version_override and fix in-proc auto-mode connect

cubic's review found two real gaps in the protocol_version_override feature:

- Client(server, mode="auto", protocol_version_override=...) against an
  in-process Server/MCPServer failed to connect. negotiate_auto's override
  path calls session.initialize() directly, but _connect_inproc picked the
  handshake-less DirectDispatcher for every non-"legacy" mode, so there was
  no JSON-RPC dispatcher for initialize() to run on. _build_session now
  collapses this combination to the legacy connector.
- protocol_version_override accepted any string uncritically: a modern
  version silently misbehaved instead of being rejected, and setting it
  alongside a version-pinned mode was silently ignored rather than erroring.
  Both are now validated in __post_init__, mirroring the existing mode check.

Also pinned the untested default (None) branch's exact initialize() call
shape in test_session_group.py, per the same review.

Author: STiFLeR7

src/mcp/client/client.py

@@ -367,7 +367,12 @@ async def main():
 
     _entered: bool = field(init=False, default=False)
     protocol_version_override: str | None = None
-    """The protocol version to request during initialization. Defaults to the latest version."""
+    """Pin the legacy `initialize` handshake to a specific handshake-era version.
+
+    Only meaningful with `mode='legacy'` or `mode='auto'` (where it skips `server/discover`
+    and negotiates directly); raises at construction with any other `mode`, since a version
+    pin already fixes the negotiated version. Must be a member of `HANDSHAKE_PROTOCOL_VERSIONS`.
+    `None` (the default) negotiates the latest version each `mode` would otherwise pick."""
     _session: ClientSession | None = field(init=False, default=None)
     _exit_stack: AsyncExitStack | None = field(init=False, default=None)
     _connect: _Connector = field(init=False, repr=False, compare=False)
@@ -385,6 +390,23 @@ def __post_init__(self) -> None:
                 f"mode must be 'legacy', 'auto', or one of {list(MODERN_PROTOCOL_VERSIONS)}; got {self.mode!r}{hint}"
             )
 
+        if self.protocol_version_override is not None:
+            if self.protocol_version_override not in HANDSHAKE_PROTOCOL_VERSIONS:
+                hint = (
+                    f" ({self.protocol_version_override!r} is a modern version; mode='auto' already negotiates it)"
+                    if self.protocol_version_override in MODERN_PROTOCOL_VERSIONS
+                    else ""
+                )
+                raise ValueError(
+                    "protocol_version_override must be one of "
+                    f"{list(HANDSHAKE_PROTOCOL_VERSIONS)}; got {self.protocol_version_override!r}{hint}"
+                )
+            if self.mode not in ("legacy", "auto"):
+                raise ValueError(
+                    f"protocol_version_override has no effect with mode={self.mode!r} "
+                    "(a version pin already fixes the negotiated version); use mode='legacy' or mode='auto'"
+                )
+
         self._folded_extensions = _fold_extensions(self.extensions)
 
         srv = self.server
@@ -426,7 +448,12 @@ def __post_init__(self) -> None:
 
     async def _build_session(self, exit_stack: AsyncExitStack) -> ClientSession:
         """Enter the resolved connector and return an un-entered ClientSession."""
-        dispatcher = await self._connect(exit_stack, self.mode, self.raise_exceptions)
+        # An override on mode='auto' skips discovery and drives `initialize()` directly
+        # (see `negotiate_auto`), so the in-proc connector must hand back the legacy,
+        # stream-backed dispatcher for this combination too, not the handshake-less
+        # DirectDispatcher it otherwise picks for every non-'legacy' mode.
+        connect_mode = "legacy" if self.mode == "auto" and self.protocol_version_override is not None else self.mode
+        dispatcher = await self._connect(exit_stack, connect_mode, self.raise_exceptions)
         message_handler = self.message_handler
         if self._response_cache is not None:
             message_handler = _evicting_message_handler(self._response_cache, self.message_handler)

tests/client/test_client.py

@@ -33,7 +33,7 @@
     Tool,
     ToolsCapability,
 )
-from mcp_types.version import LATEST_HANDSHAKE_VERSION
+from mcp_types.version import LATEST_HANDSHAKE_VERSION, LATEST_MODERN_VERSION
 from pydantic import FileUrl
 
 from mcp import MCPDeprecationWarning, MCPError, StdioServerParameters
@@ -138,6 +138,32 @@ async def test_client_custom_protocol_version(app: MCPServer):
         assert client.server_info.name == "test"
 
 
+async def test_client_auto_mode_with_override_against_in_process_server(app: MCPServer):
+    """Regression: `mode='auto'` with `protocol_version_override` against an in-process
+    `Server`/`MCPServer` used to always get the handshake-less `DirectDispatcher` (every
+    non-'legacy' mode picked it), so `negotiate_auto`'s direct `initialize()` call for the
+    override case had no JSON-RPC dispatcher to run on and the connect failed.
+    """
+    async with Client(app, mode="auto", protocol_version_override="2024-11-05") as client:
+        assert client.protocol_version == "2024-11-05"
+        assert client.server_info is not None
+        assert client.server_info.name == "test"
+
+
+def test_client_rejects_modern_protocol_version_override(app: MCPServer):
+    """`protocol_version_override` only pins the legacy handshake; a modern version string
+    is a construction-time error rather than a confusing failure once connected."""
+    with pytest.raises(ValueError, match="protocol_version_override must be one of"):
+        Client(app, mode="auto", protocol_version_override=LATEST_MODERN_VERSION)
+
+
+def test_client_rejects_protocol_version_override_with_a_version_pin_mode(app: MCPServer):
+    """`protocol_version_override` has no effect once `mode` already pins a version, so it's
+    rejected at construction instead of being silently ignored."""
+    with pytest.raises(ValueError, match="protocol_version_override has no effect with mode="):
+        Client(app, mode=LATEST_MODERN_VERSION, protocol_version_override="2024-11-05")
+
+
 async def test_client_with_simple_server(simple_server: Server):
     """Test that from_server works with a basic Server instance."""
     async with Client(simple_server) as client:

tests/client/test_session_group.py

@@ -397,7 +397,7 @@ async def test_client_session_group_establish_session_parameterized(
                 client_info=None,
             )
             mock_raw_session_cm.__aenter__.assert_awaited_once()
-            mock_entered_session.initialize.assert_awaited_once()
+            mock_entered_session.initialize.assert_awaited_once_with()
 
             # 3. Assert returned values
             assert returned_server_info is mock_initialize_result.server_info