MCP write-capable tools are not auto-retried after mid-flight session expiry (no duplicate side effects) (salvage #88821) - #88821
Merged
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on 45bd59a — fix: keep the write-side outcome-uncertain verdict when no s
|
alt-glitch
added
type/bug
…e MCP tools after mid-flight session expiry A 'session expired' / transport-closed failure can arrive AFTER the server already accepted and executed the request (proxy-synthesized 404s, pod rotation, ClosedResourceError firing mid-response). Auto-retrying a write-capable tool in that window risks a duplicate side effect that MCP offers no way to undo. The session-expired recovery path now consults the discovery-time readOnlyHint capture (same data the trust gate uses): only tools whose annotation is exactly True keep the reconnect+retry-once behavior. Write- capable calls still get the transport healed (reconnect, breaker reset on success) but return a structured outcome_unknown error telling the model to verify with a read before re-invoking. The OAuth 401 path keeps its retry for all tools: a 401 means the server demanded authorization before dispatch, so the call never executed — matching the upstream classifier's McpAuthRequiredError => safe rule. Fails safe: missing/unknown annotations classify as write-capable.
teknium1
force-pushed
the
cloudflare-os-port/mcp-write-retry-at-most-once
branch
from
September 15, 2026 01:42
4b3fdfd to
9e565ca
Compare
teknium1
changed the title
… reconnect _handle_session_expired_and_retry only reached the at-most-once guard when a reconnectable server record existed; without one (server torn down, MCP loop not running) a write-capable call fell through to the generic "MCP call failed" error, which invites the model to replay a write that may already have landed. The session-expired classification now runs first and a write-capable call always gets the outcome_uncertain error; the reconnect is attempted only when a server can be signalled. _track_inflight_rpc's teardown RuntimeError said "retry the request on the rebuilt session" for every op; for a write-capable tools/call it now says the request may already have been dispatched and must be verified first, so the wording matches the at-most-once contract the recoverer enforces. Docs: the readOnlyHint row explains that the same hint gates auto-retry after a mid-call session expiry, and that unannotated tools on an idle-TTL Streamable-HTTP server return outcome_uncertain on the first call after idle instead of being transparently replayed.
teknium1
deleted the
cloudflare-os-port/mcp-write-retry-at-most-once
branch
September 15, 2026 10:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A write-capable MCP tool call that fails with a session-expired/transport error is no longer auto-retried — the request may already have executed server-side, and a blind retry risks a duplicate side effect MCP offers no way to undo. Ported from cloudflare/cloudflare-os#168 ("Harden MCP connection lifecycle").
Root cause of the risk: our
_SESSION_EXPIRED_MARKERSclassifier matches both genuine pre-dispatch rejections ("session not found") AND failure shapes that routinely fire after dispatch —ClosedResourceError, "broken pipe", "connection closed" mid-response, or a proxy-synthesized session-404 after the upstream accepted the request. The previous recovery path (_handle_session_expired_and_retry) re-ran every tool call once regardless.Changes
tools/mcp_tool.py:_handle_session_expired_and_retrygains keyword-onlycall_may_have_side_effects. When True it still heals the transport (reconnect + breaker reset on success) but never re-runs the call; instead it returns a structuredoutcome_unknown=Truetool error telling the model to verify with a read before re-invoking.tools/mcp_tool.py: new_tool_is_read_only(server, tool)helper — reuses the discovery-timereadOnlyHintcapture the trust gate already maintains. Fails safe: missing/unknown/malformed annotations classify as write-capable.tools/mcp_tool.py: thetools/callhandler passescall_may_have_side_effects=not _tool_is_read_only(...). The four read-only handlers (resources/list, resources/read, prompts/list, prompts/get) keep unconditional retry — they are protocol-level reads.McpAuthRequiredError→ safe-to-retry rule).tests/tools/test_mcp_tool_session_expired.py: 4 new tests (write not retried + outcome_unknown, read-only still retries, fail-safe classification, direct helper coverage); existing retry tests updated to registerreadOnlyHint=Truesince unannotated tools no longer auto-retry.Adaptation notes
Upstream models this as
callMayHaveTakenEffect(err)over typed error classes (McpCallNotDispatchedError→ safe,McpSessionExpiredError→ unsafe for writes since a fronting proxy can synthesize it post-dispatch). Hermes' classifier is string/type-marker based with no dispatch-boundary signal, so the port keys the decision on the tool's write-capability instead — the same fail-safe direction ("anything not positively known safe is treated as possibly performed"), wired into thereadOnlyHintmetadata we already capture for trust-tier gating. No toolset/schema mutation; prompt caching unaffected.Complementary to open PR #84931 (durable tool-attempt identity in
_metaso governed servers can dedupe); this PR removes the client-side duplicate dispatch for ordinary servers.Validation
outcome_unknownerror returnedreadOnlyHint: true), session expiredTests:
tests/tools/test_mcp_tool_session_expired.py+test_mcp_trust_gating.py→ 28/28; broader MCP subset (tool, 401, circuit breaker, reconnect signal/reset, transport group, failure classification, connect/shutdown, issue-948) → 138/138.Source: cloudflare/cloudflare-os#168
Infographic