Skip to content

fix(mcp): stop counting app-level tool errors as server unreachability - #74042

Closed
ppazosp wants to merge 1 commit into
NousResearch:mainfrom
useomnia:ppp/mcp-breaker-app-errors
Closed

fix(mcp): stop counting app-level tool errors as server unreachability#74042
ppazosp wants to merge 1 commit into
NousResearch:mainfrom
useomnia:ppp/mcp-breaker-app-errors

Conversation

@ppazosp

@ppazosp ppazosp commented Jul 29, 2026

Copy link
Copy Markdown

Summary

The MCP circuit breaker (#10776, added for the retry burn loops in #10447) decides "server unreachable" by sniffing the tool result JSON for an "error" key:

parsed = json.loads(result)
if "error" in parsed:
    _bump_server_error(server_name)

Application-level tool errors — MCP result.isError payloads such as a "not found" lookup — produce exactly that shape, so they count as consecutive failures identically to transport failures. Three bad-argument calls in a row open the breaker, and every tool on that server fast-fails as MCP server '<name>' is unreachable after 3 consecutive failures for the full 60s cooldown.

Observed in production: an agent probed a server with three stale IDs, got three sub-second "not found" responses (i.e. the server was healthy and answering), and the harness then reported the whole server as down to the model and the user.

Fix

Classify by mechanism instead of payload shape:

  • A completed RPC round-trip proves the server is reachable — reset the breaker regardless of what the tool returned (_reset_server_error on any non-raising _call_once()).
  • Only the transport paths bump the count: the except Exception handler and the server-not-connected path, unchanged.

The auth-recovery and session-expired retry helpers had the same payload-sniffing flaw — a retry that completed but carried an app-level error payload fell through to needs_reauth / the generic error path instead of being returned. They now return the completed result as-is and reset the breaker; a raising retry still falls through exactly as before.

Genuine outages are unaffected: transport failures trip the breaker, the cooldown and half-open probe behave as before (existing tests unchanged and passing).

Tests

Two regression tests in tests/tools/test_mcp_circuit_breaker.py:

  • test_app_level_tool_errors_do_not_trip_breaker — threshold+1 consecutive isError results: every call reaches the session, the breaker stays closed.
  • test_app_level_tool_error_closes_partially_tripped_breaker — an isError result resets a below-threshold transport-failure count, like any successful response.

Ran per-file: test_mcp_circuit_breaker.py (9), test_mcp_tool.py (234), test_mcp_tool_401_handling.py (7), test_mcp_tool_session_expired.py (32) — all passing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xa5D4FJwo8A3R8skLiK2aq

The circuit breaker (NousResearch#10776, for NousResearch#10447) classifies failures by sniffing
the result JSON for an "error" key, so application-level tool errors
(result.isError, e.g. a "not found" lookup) count as consecutive
failures. Three bad-argument calls in a row open the breaker and every
tool on that server fast-fails as "unreachable" for the 60s cooldown —
observed in production against a server that was healthy and answering
in under a second throughout.

Classify by mechanism instead: a completed RPC round-trip proves the
server is reachable and closes the breaker regardless of payload; only
the transport-exception paths bump the count. The auth-recovery and
session-expired retry helpers had the same payload-sniffing flaw — a
retry that completed with an app-level error fell through to
needs_reauth / the generic error path — and now return the completed
result as-is.

Two regression tests lock it in: consecutive isError results never trip
the breaker, and an isError result closes a partially-tripped one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xa5D4FJwo8A3R8skLiK2aq
@ppazosp

ppazosp commented Jul 29, 2026

Copy link
Copy Markdown
Author

Superseded by #74045 — same change, reopened to follow the fix/ branch-naming convention from CONTRIBUTING.md and with the design rationale and platform notes the PR guidelines ask for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants