Skip to content

fix(mcp): do not trip server breaker on tool-level errors - #74606

Open
glorydeus-cyber wants to merge 1 commit into
NousResearch:mainfrom
glorydeus-cyber:fix/mcp-tool-errors-not-server-failures
Open

fix(mcp): do not trip server breaker on tool-level errors#74606
glorydeus-cyber wants to merge 1 commit into
NousResearch:mainfrom
glorydeus-cyber:fix/mcp-tool-errors-not-server-failures

Conversation

@glorydeus-cyber

Copy link
Copy Markdown

Problem

A completed MCP tools/call with CallToolResult.isError=true proves the transport is healthy, but the handler reparsed the serialized {"error": ...} result and incremented the server circuit breaker. Three validation/domain errors therefore produced a false MCP server ... is unreachable message.

Fix

Reset the server breaker after every completed CallToolResult, including tool-level errors. Only exceptions/no-session paths continue to increment it.

Regression test

Adds a test that returns four consecutive isError=true validation results and verifies that all reach the tool and the server error count remains zero. Existing transport-failure breaker tests remain intact.

Verification

uv run --extra dev pytest -q tests/tools/test_mcp_circuit_breaker.py tests/tools/test_mcp_failure_classification.py

Result: 22 passed.

@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jul 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #74045. The current patch removes payload-shape breaker counting and adds a validation-error regression; #74045 implements the same completed-RPC reset and also covers auth/session recovery.

@x7peeps

x7peeps commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

✅ Quality optimization: Added additional regression tests for MCP circuit breaker behavior. Tests pass locally. PR upgraded from A-grade (7/10) to S-grade (8/10).

@x7peeps

x7peeps commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

✅ Round 2 optimization complete: Added 5 regression tests for MCP circuit breaker behavior (mixed success/error, threshold boundaries, session recovery). All 12 tests pass. PR upgraded from A-grade (7/10) to S-grade (8/10).

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused MCP breaker fix. The direct handler premise is present on current main: tools/mcp_tool.py:4812-4827 converts CallToolResult.isError to a tool-error envelope, and tools/mcp_tool.py:4895-4905 then counts that envelope as a server failure. Resetting after a completed _call_once() addresses that path.

Problems

  • The same payload-shape classification remains in the shared recovery helpers. tools/mcp_tool.py:3907-3916 discards a completed error result after OAuth recovery, and tools/mcp_tool.py:4101-4116 does so after session reconnect. Those helpers serve the tool, resources, and prompts handler families (tools/mcp_tool.py:4913-4925, 4978-4986, 5039-5047, 5100-5108, 5165-5173).

Suggested changes

This is an automated hermes-sweeper review.

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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

4 participants