Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Bound the re-list by the caller's timeout and chain every re-list fai…
…lure

`read_timeout_seconds` now covers the whole re-list after a
`HeaderMismatch`, which otherwise ran on the session default. When it
elapses, or a `tools/list` page fails validation, the original `-32020`
is raised with that failure as its cause.
  • Loading branch information
maxisbey committed Oct 2, 2026
commit 98d9dd04476fc0bff686f7aa28424ceb7f9e9313
9 changes: 6 additions & 3 deletions src/mcp/client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
ServerCapabilities,
)
from mcp_types.version import HANDSHAKE_PROTOCOL_VERSIONS, MODERN_PROTOCOL_VERSIONS
from pydantic import ValidationError
from typing_extensions import deprecated

from mcp.client._input_required import DEFAULT_INPUT_REQUIRED_MAX_ROUNDS, run_input_required_driver
Expand Down Expand Up @@ -787,7 +788,8 @@ async def call_tool(
Args:
name: The name of the tool to call.
arguments: Arguments to pass to the tool.
read_timeout_seconds: Timeout for each underlying `tools/call` round.
read_timeout_seconds: Timeout for each underlying `tools/call` round, and
for the whole re-list after a `HEADER_MISMATCH`.
progress_callback: Callback for progress updates.
input_responses: Responses to seed the first call with (e.g. when
resuming from a persisted `InputRequiredResult`).
Expand Down Expand Up @@ -826,8 +828,9 @@ async def retry(r: InputResponses | None, s: str | None) -> CallToolResult | Inp
raise
# The spec's recovery: the tool's listed schema is missing or stale, so re-list and resend once.
try:
await self._relist_tool(name)
except MCPError as relist_error:
with anyio.fail_after(read_timeout_seconds):
await self._relist_tool(name)
except (MCPError, TimeoutError, ValidationError) as relist_error:
raise mismatch from relist_error
return await send(r, s)
Comment on lines +828 to +837

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Tool handlers that surface a -32020 error are now executed twice per call, where the base ran them once and raised. The gate at src/mcp/client/client.py:825 checks only the negotiated version, not whether the transport is HTTP, so on stdio or in-memory 2026-07-28 connections (where no Mcp-Param-* validation exists) any -32020 must come from the handler itself, yet the client still re-lists and resends at src/mcp/client/client.py:832. Fix: only recover when the rejection can be a pre-dispatch header rejection, e.g. gate on the connection being Streamable HTTP (Client already knows a URL server at client.py:396-398) or on the error being a transport-level rejection, so handler-originated -32020 propagates unchanged.

Why this was flagged

A Server on a 2026-07-28 stdio or in-memory connection whose tool handler raises MCPError with code -32020 (for example a proxy/aggregator tool that forwards an upstream HTTP server's HeaderMismatch verbatim). The client's call_tool enters retry at src/mcp/client/client.py:821-832; the guard at client.py:825 passes because self.protocol_version is modern, so _relist_tool issues tools/list and send runs the handler a second time at client.py:832. On the base branch the first -32020 was raised to the caller and the handler ran once. The SDK server emits HEADER_MISMATCH only from the HTTP ladder (src/mcp/shared/inbound.py:448-471); classify_inbound_request skips the header rung when headers is None, so on non-HTTP transports every -32020 is handler-originated and the double run is unconditional. The PR text calls this accepted, but nothing in Client distinguishes the transport even though the constructor knows a URL server from a stdio/in-memory one (client.py:396-402).

Verification: The gate at src/mcp/client/client.py:825 inspects only the negotiated version, never the transport; on passing it calls self._relist_tool(name) (line 829) and then return await send(r, s) (line 832). src/mcp/shared/inbound.py:452 only emits HEADER_MISMATCH if headers is not None, so on stdio/in-process modern connections every -32020 originates in the handler. Base behavior: the handler ran once.

Comment on lines +824 to +837

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit (optional): AGENTS.md says any change to an existing public API's observable behaviour is an explicit maintainer design decision and should generally be avoided. The new retry wrapper changes Client.call_tool's observable behaviour on 2026-07-28 connections: a -32020 that used to be raised immediately now silently issues up to 100 tools/list requests and resends the tools/call, and this also wraps every input-required resumption. Fix: have a maintainer explicitly sign off on the behaviour change on the linked issue (#3483) before merge, or gate the recovery behind an opt-in so existing callers' observable behaviour is unchanged.

Why this was flagged

The instruction guards the 2.x compatibility contract. Concretely: callers that caught -32020 themselves (the migration guide told them 'the spec's recovery is to re-list and retry') now see extra wire traffic and a second tools/call before any error; the PR notes a tool handler that itself raises -32020 is run twice on a 2026-07-28 connection. Mitigating context the maintainer may weigh: the spec's Client Behavior section says a client SHOULD do this, the TypeScript SDK already does, the PR fixes an assigned issue, signatures are unchanged, and legacy connections are untouched.

Verification: AGENTS.md at base (Branching Model): "v2 is released; its public API is a compatibility contract for the 2.x line. Removals, renames, or any change to an existing API's signature or observable behaviour ... is a design decision a maintainer makes explicitly, and should generally be avoided."


Expand Down
108 changes: 108 additions & 0 deletions tests/interaction/transports/test_hosting_http_modern.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,26 @@
Tool,
)
from mcp_types.version import LATEST_MODERN_VERSION
from pydantic import ValidationError
from trio.testing import MockClock

from mcp import MCPError
from mcp.client.client import Client
from mcp.client.session import ClientSession
from mcp.client.streamable_http import streamable_http_client
from mcp.server import Server, ServerRequestContext
from mcp.server.context import CallNext, HandlerResult
from tests.interaction._connect import BASE_URL, base_headers, initialize_via_http, mounted_app
from tests.interaction._requirements import requirement

pytestmark = pytest.mark.anyio


@pytest.fixture(autouse=True)
def _module_runner_lease() -> None:
"""Opt out of the shared per-module event loop: this module parametrizes `anyio_backend`."""


def _modern_headers(*, method: str, name: str | None = None) -> dict[str, str]:
"""Request headers for a 2026-07-28 POST.

Expand Down Expand Up @@ -778,6 +786,106 @@ async def rewrite_mcp_name(request: httpx2.Request) -> None:
assert methods == ["tools/call", "tools/list"]


@requirement("client-transport:http:header-mismatch-recovery")
async def test_modern_client_raises_the_header_mismatch_when_the_re_list_returns_a_malformed_page() -> None:
"""A `tools/list` page that fails validation leaves the caller with the server's `HeaderMismatch`.

SDK-defined: a caller's `except MCPError` still sees the rejection, with the `ValidationError` as its
cause, and the call is not resent. The page comes from a middleware that answers without `call_next`,
the one place the SDK server does not validate an outgoing result.
"""

async def malformed_listing(ctx: ServerRequestContext, call_next: CallNext) -> HandlerResult:
assert ctx.method == "tools/list"
return {"tools": "not a list"}

async def call_tool(ctx: ServerRequestContext, params: CallToolRequestParams) -> CallToolResult:
raise NotImplementedError

server = Server("malformed", on_call_tool=call_tool)
server.middleware.append(malformed_listing)

methods: list[str] = []

async def rewrite_mcp_name(request: httpx2.Request) -> None:
method = json.loads(request.content)["method"]
methods.append(method)
if method == "tools/call":
request.headers["mcp-name"] = "another-tool"

discover = DiscoverResult(
supported_versions=[LATEST_MODERN_VERSION],
capabilities=ServerCapabilities(),
)
async with (
mounted_app(server, on_request=rewrite_mcp_name) as (http, _),
Client(
streamable_http_client(f"{BASE_URL}/mcp", http_client=http),
mode=LATEST_MODERN_VERSION,
prior_discover=discover,
) as client,
):
with anyio.fail_after(5), pytest.raises(MCPError) as excinfo:
await client.call_tool("run", {"region": "us-west1"})

assert excinfo.value.error.code == HEADER_MISMATCH
assert isinstance(excinfo.value.__cause__, ValidationError)
assert methods == ["tools/call", "tools/list"]


# The timeout also governs the rejected `tools/call`, which must be answered before the re-list can
# wait it out, so any real-clock value is a bet against CI scheduler stalls. On trio's autojumping
# clock time advances only when every task is blocked: the answered call cannot time out however slow
# the runner, and once the re-list blocks the clock jumps straight to the deadline, with no real wait.
@requirement("client-transport:http:header-mismatch-recovery")
@pytest.mark.parametrize(
"anyio_backend",
[pytest.param(("trio", {"clock": MockClock(autojump_threshold=0)}), id="trio-mockclock")],
)
async def test_modern_client_raises_the_header_mismatch_when_the_re_list_outlasts_the_read_timeout() -> None:
"""The caller's `read_timeout_seconds` bounds the re-list, which otherwise has no timeout of its own.

SDK-defined: the server rejects the call and then never answers `tools/list`. When the timeout elapses
the rejection is raised with the `TimeoutError` as its cause, and the call is not resent.
"""

async def list_tools(ctx: ServerRequestContext, params: PaginatedRequestParams | None) -> ListToolsResult:
await anyio.Event().wait() # blocks until the abandoned request's disconnect interrupts it
raise NotImplementedError # unreachable

async def call_tool(ctx: ServerRequestContext, params: CallToolRequestParams) -> CallToolResult:
raise NotImplementedError

server = Server("stalled", on_list_tools=list_tools, on_call_tool=call_tool)

methods: list[str] = []

async def rewrite_mcp_name(request: httpx2.Request) -> None:
method = json.loads(request.content)["method"]
methods.append(method)
if method == "tools/call":
request.headers["mcp-name"] = "another-tool"

discover = DiscoverResult(
supported_versions=[LATEST_MODERN_VERSION],
capabilities=ServerCapabilities(),
)
async with (
mounted_app(server, on_request=rewrite_mcp_name) as (http, _),
Client(
streamable_http_client(f"{BASE_URL}/mcp", http_client=http),
mode=LATEST_MODERN_VERSION,
prior_discover=discover,
) as client,
):
with anyio.fail_after(5), pytest.raises(MCPError) as excinfo:
await client.call_tool("run", {"region": "us-west1"}, read_timeout_seconds=0.05)

assert excinfo.value.error.code == HEADER_MISMATCH
assert isinstance(excinfo.value.__cause__, TimeoutError)
assert methods == ["tools/call", "tools/list"]


@requirement("client-transport:http:header-mismatch-recovery")
async def test_legacy_client_raises_a_header_mismatch_error_without_re_listing_or_retrying() -> None:
"""On a pre-2026 connection a `-32020` error from a tool call is raised as it arrives.
Expand Down
Loading