Repository navigation
Retry a tool call once after a HeaderMismatch rejection #3627
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 1 commit
fe11e39
98d9dd0
a57f2e4
64fab09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
…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
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -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`). | ||
|
|
@@ -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
+824
to
+837
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): 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 Why this was flaggedThe instruction guards the 2.x compatibility contract. Concretely: callers that caught 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." |
||
|
|
||
|
|
||
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) Tool handlers that surface a
-32020error 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 noMcp-Param-*validation exists) any-32020must 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-32020propagates 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 thenreturn await send(r, s)(line 832). src/mcp/shared/inbound.py:452 only emits HEADER_MISMATCHif headers is not None, so on stdio/in-process modern connections every-32020originates in the handler. Base behavior: the handler ran once.