-
Notifications
You must be signed in to change notification settings - Fork 4k
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 all commits
fe11e39
98d9dd0
a57f2e4
64fab09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,12 +8,13 @@ | |
| from collections.abc import Awaitable, Callable, Mapping, Sequence | ||
| from contextlib import AbstractAsyncContextManager, AsyncExitStack | ||
| from dataclasses import KW_ONLY, dataclass, field | ||
| from typing import Any, Literal, TypeVar, cast | ||
| from typing import Any, Final, Literal, TypeVar, cast | ||
|
|
||
| import anyio | ||
| import anyio.lowlevel | ||
| import mcp_types as types | ||
| from mcp_types import ( | ||
| HEADER_MISMATCH, | ||
| INVALID_PARAMS, | ||
| CacheableResult, | ||
| CallToolResult, | ||
|
|
@@ -40,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 | ||
|
|
@@ -79,6 +81,9 @@ | |
| initialize), or a modern protocol-version string (adopt directly). The ``str`` arm is for | ||
| forward-compat; ``Client.__post_init__`` rejects anything outside that set at construction.""" | ||
|
|
||
| _RELIST_PAGE_CAP: Final = 100 | ||
| """Page cap for the tools/list walk that follows a `HEADER_MISMATCH`: a paginator that never ends cannot hang a call.""" | ||
|
|
||
| _T = TypeVar("_T") | ||
| _ResultT = TypeVar("_ResultT") | ||
| _CacheableT = TypeVar("_CacheableT", bound=CacheableResult) | ||
|
|
@@ -775,10 +780,17 @@ async def call_tool( | |
| exceptions propagate as-is. To receive the claimed shape yourself, use | ||
| `client.session.call_tool(..., allow_claimed=True)`. | ||
|
|
||
| On a 2026-07-28 connection, a call the server rejects with `HEADER_MISMATCH` | ||
| (this client has not listed the tool, or its input schema changed since) is | ||
| resent once after refetching the tool listing. A second rejection is raised, | ||
| and so is the first when the listing cannot be refetched. | ||
|
|
||
| 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`. Defaults to this | ||
| client's `read_timeout_seconds`. | ||
| progress_callback: Callback for progress updates. | ||
| input_responses: Responses to seed the first call with (e.g. when | ||
| resuming from a persisted `InputRequiredResult`). | ||
|
|
@@ -795,7 +807,7 @@ async def call_tool( | |
| conform to the negotiated protocol version. | ||
| """ | ||
|
|
||
| async def retry(r: InputResponses | None, s: str | None) -> CallToolResult | InputRequiredResult | Result: | ||
| async def send(r: InputResponses | None, s: str | None) -> CallToolResult | InputRequiredResult | Result: | ||
| return await self.session.call_tool( | ||
| name, | ||
| arguments, | ||
|
|
@@ -809,6 +821,21 @@ async def retry(r: InputResponses | None, s: str | None) -> CallToolResult | Inp | |
| allow_claimed=True, | ||
| ) | ||
|
|
||
| async def retry(r: InputResponses | None, s: str | None) -> CallToolResult | InputRequiredResult | Result: | ||
| try: | ||
| return await send(r, s) | ||
| except MCPError as mismatch: | ||
| if mismatch.code != HEADER_MISMATCH or self.protocol_version not in MODERN_PROTOCOL_VERSIONS: | ||
| raise | ||
| # The spec's recovery: the tool's listed schema is missing or stale, so re-list and resend once. | ||
| timeout = read_timeout_seconds if read_timeout_seconds is not None else self.read_timeout_seconds | ||
| try: | ||
| with anyio.fail_after(timeout): | ||
| 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
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. 🟡 (optional) Tool handlers that surface a Why this was flaggedA 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
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." |
||
|
|
||
| result = await self._drive_input_required(await retry(input_responses, request_state), retry) | ||
| if isinstance(result, CallToolResult): | ||
| return result | ||
|
|
@@ -943,6 +970,15 @@ async def list_tools( | |
| ), | ||
| ) | ||
|
|
||
| async def _relist_tool(self, name: str) -> None: | ||
| """Refetch the tool listing from the server, page by page, until a page lists `name`.""" | ||
| cursor: str | None = None | ||
| for _ in range(_RELIST_PAGE_CAP): | ||
| page = await self.list_tools(cursor=cursor, cache_mode="refresh") | ||
|
claude[bot] marked this conversation as resolved.
|
||
| cursor = page.next_cursor | ||
| if cursor is None or any(tool.name == name for tool in page.tools): | ||
|
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. P2: A paginated refresh that exhausts without Prompt for AI agents |
||
| return | ||
|
|
||
| @deprecated("The roots capability is deprecated as of 2026-07-28 (SEP-2577).", category=MCPDeprecationWarning) | ||
| async def send_roots_list_changed(self) -> None: | ||
| """Send a notification that the roots list has changed.""" | ||
|
|
||
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.
🟡 nit (optional): readers lose the only docs for the server-side
Mcp-Param-*validation and the publicmcp.shared.inboundvalidators, and no page describes the new re-list-and-retry. The deleted section at docs/migration.md:2868 was the sole place in docs/ namingvalidate_mcp_param_headers,decode_header_value, the skip-when-no-handler rule, duplicate-header rejection and strict base64-sentinel decoding. docs/advanced/header-parameters.md:15 still says only "a client that has listed the tool" sends the header. Fix: move the server-side facts onto docs/advanced/header-parameters.md and add the client recovery there (re-list, one resend, the 100-page cap), so the deletion is a relocation rather than a loss. [also at: docs/whats-new.md:202 - nit: after merging, readers of docs/ find no page describing the new re-list-and-retry behaviour nor the server-sideMcp-Param-*validation rules. docs/whats-new.md:202 drops the "has the rules" link and docs/migration.md deletes the only section that stated those rules, while no page gains the client recovery that this PR adds toClient.call_tool.; src/mcp/client/client.py:785 - nit: AGENTS.md requires the relevant docs/ page to be updated in the same PR when user-visible behaviour changes.]Why this was flagged
The diff removes the whole "Servers validate
Mcp-Param-*headers against the request body (SEP-2243)" section from docs/migration.md (old lines 2868-2874).validate_mcp_param_headers,decode_header_value, "supplied more than once" rejection and the=?base64?...?=strictness now appear only in src/ and tests/, in no page under docs/. docs/advanced/header-parameters.md is untouched and at line 15 still describes only the listed-tool case, so the new behaviour added at src/mcp/client/client.py:821-832 (an extratools/listper page and a secondtools/callafter a-32020) is documented only in a docstring. AGENTS.md requires that a change affecting user-visible behaviour update the relevant docs page in the same PR and that docs/migration.md only be corrected or clarified, not have content removed. On the base branch a reader finds both the server-side rules and the "list the tool first" guidance; after merge they find neither, and nothing tells them a call now silently issues up to 100tools/listrequests before being resent.Verification: Triggering condition: any reader of docs/ looking for how
Client.call_toolbehaves on-32020or for the server-sideMcp-Param-*validation rules. The only docs edits are the deletion of the migration.md section (old lines 2868-2874) and removal of the whats-new.md link to it. docs/advanced/header-parameters.md is untouched. Harm is to documentation only; nothing fails at runtime.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.
The new bullet on docs/advanced/header-parameters.md covers the client's re-list-and-resend, which closes half of this. The server-side facts that the deleted docs/migration.md section carried are still absent from every page under docs/:
grep -rn "validate_mcp_param_headers\|decode_header_value" docs/returns nothing, and nothing names the skip-when-no-tools/list-handler rule, rejection of a recognizedMcp-Param-*header supplied more than once, or the strict=?base64?...?=decoding (also applied toMcp-Name). Either restore the section at docs/migration.md:2868 or add a short "On the server" paragraph to docs/advanced/header-parameters.md stating those rules and pointing low-levelServerauthors atmcp.shared.inbound.validate_mcp_param_headers/decode_header_value, so the deletion becomes a relocation.