Skip to content

fix(mcp): recovered retry keeps the tool's real answer, breaker strike intact (salvage #106753) - #109180

Merged
teknium1 merged 3 commits into
mainfrom
fix/mcp-retry-keeps-application-result-salvage
Sep 12, 2026
Merged

teknium1 merged 3 commits into
mainfrom
fix/mcp-retry-keeps-application-result-salvage

Conversation

@teknium1

Copy link
Copy Markdown
Collaborator

After an MCP transport recovery (OAuth refresh or session reconnect), the retried call's real result now reaches the model even when the tool answered with an application error, instead of being dropped for a generic "needs reauth" / "call failed" message.

Salvages #106753 from @Hermsi1337 (earliest fix on this path).

What was wrong

tools/mcp_tool_handlers.py::_retry_once returned None when the retried call came back with an {"error": ...} payload, so the caller fell through to _strike(... needs_reauth=True) or the generic failure path. The tool had actually answered; the model just never saw it. Code-path reachable on every recovery path (_handle_auth_error_and_retry, _handle_session_expired_and_retry).

Changes

  • _retry_once returns the retried result whenever the RPC completed (@Hermsi1337's commit, cherry-picked verbatim).
  • Improvement during salvage: the result is routed through _record_call_outcome so an application error still counts as one breaker strike. The strike is the feature: 3ff18ff (fix: add circuit breaker to MCP tool handler to prevent retry burn loops #10776, closes Gateway hangs when Google Workspace MCP skill returns error — infinite retry loop #10447) added the breaker precisely because a server answering errors made the model hammer it 8× in 10 s. The contributor's version reset the counter on error answers; this variant keeps the answer and the semantics.
  • Test: the existing test_call_tool_handler_rebuilds_configured_server_transport is parametrized over application_error (contributor) and asserts the counter is 1 after an error answer, 0 after a success (mine). Conflict with main's newer stdio outcome_uncertain branch resolved keep-both.
  • contributors/emails/d.hermsmeier@mittwald.de → Hermsi1337.

Not salvaged

Validation

Check Result
scripts/run_tests.sh tests/tools/test_mcp_tool_session_expired.py tests/tools/test_mcp_circuit_breaker.py 22 passed, 0 failed
Red on base (origin/main handlers swapped in) application-error-http param fails: {'error': 'MCP call failed: ClosedResourceError…'} != {'error': 'reconnected'}
ruff / windows-footguns / compat-pointers / git diff --check clean

Infographic

infographic

Hermsi1337 and others added 3 commits September 12, 2026 07:18
The pick returns the real result after a transport recovery instead of
dropping it, but it also skipped the breaker bookkeeping. Application
errors counting as strikes is the point of the breaker (3ff18ff,
#10447: a server answering errors made the model hammer it 8x in 10s).
Route the recovered result through _record_call_outcome so the caller
sees the tool's answer and the counter still moves the right way.
@github-actions

github-actions Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on d89b7bb — chore: map contributor email d.hermsmeier@mittwald.de -> Her

⚠️ Warnings

OSV vulnerability scan · View job

80 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.


debug info

CI timings

CI timings · View report · View job

Wall time 4m57s vs 4m33s (+8.8%). 9 job(s) slower, 3 faster, 2 unchanged.

  • Python tests / Run tests: +29.0s
  • OS-specific tests / Windows-only tests: +17.0s
  • OSV scan / Emit review status: +7.0s
  • OS-specific tests / macOS-only tests: -7.0s
  • Python tests / e2e: -5.0s

@teknium1
teknium1 merged commit dfa84e4 into main Sep 12, 2026
37 checks passed
@teknium1
teknium1 deleted the fix/mcp-retry-keeps-application-result-salvage branch September 12, 2026 15:23
teknium1 added a commit that referenced this pull request Sep 12, 2026
…able"

Application errors (isError payloads) keep counting as breaker strikes:
that is #10447's point (a server answering errors made the model hammer
it 8x in 10s) and #109180 just reasserted it. What #11113 actually hit is
the open-breaker MESSAGE: after three rejected fetches the model was told
the server was "unreachable" and went to the user instead of fixing its
URL. Track whether the streak was all application errors and word the
pause accordingly; one transport strike restores the unreachable text.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway hangs when Google Workspace MCP skill returns error — infinite retry loop

2 participants