feat(mcp): per-server circuit breaker threshold via breaker_threshold config - #68455
feat(mcp): per-server circuit breaker threshold via breaker_threshold config#68455igitur wants to merge 2 commits into
Conversation
… config Adds support for per-server circuit breaker threshold override via mcp_servers.<name>.breaker_threshold in config.yaml. - _circuit_breaker_thresholds dict stores per-server overrides - _bump_server_error and the tool handler check both use it - Falls back to _CIRCUIT_BREAKER_THRESHOLD (3) when not configured - Populated during register_mcp_servers from server config Closes NousResearch#44172
teknium1
left a comment
There was a problem hiding this comment.
Thanks for adding a narrowly scoped MCP configuration path. The premise is still present on current main: both breaker transitions use the global threshold at tools/mcp_tool.py:3609 and tools/mcp_tool.py:4692.
Problems
tools/mcp_tool.py:5181only writes valid overrides into the new module-level map. It never removes an existing entry. The normal reload path callsshutdown_mcp_servers()and then fresh discovery (cli.py:11367-11371), so removing or invalidatingbreaker_thresholdwould leave the prior threshold active instead of restoring the global default.- This user-facing config key is not documented in the MCP server-key reference (
website/docs/reference/mcp-config-reference.md:44-68).
Suggested changes
- Reconcile or clear the override map on reload, and add a test covering override removal/invalid values after a prior valid override.
- Document the positive-integer constraint and default fallback in the MCP config reference.
- The PR addresses only one portion of #44172; please make the issue reference partial rather than
Closes.
Automated hermes-sweeper review.
| """ | ||
| # Populate per-server circuit breaker thresholds before any early | ||
| # returns so they are available even when MCP SDK is not loaded. | ||
| for srv_name, srv_cfg in servers.items(): |
There was a problem hiding this comment.
This map needs a removal/reconciliation path. After a valid threshold is configured, then removed or made invalid in config.yaml, MCP reload will leave the old value here and no longer fall back to _CIRCUIT_BREAKER_THRESHOLD. Please clear absent/invalid entries and add a reload regression test.
SummaryTwenty PRs address or reference four related MCP reliability and hardening issues. Their diffs cover completed-response versus transport-failure classification, idle keepalive, dead-transport recovery, breaker messaging and locking, proactive reconnects, and per-server threshold configuration; #68455 addresses only the configurable-threshold portion of #44172. Related pull requests
Duplicates#67340 is a closed duplicate of #61555; #11128 and #32728 are classifier-based alternatives, while #40951, #61555, #68669, and #74606 overlap on the direct completed-RPC reset. #74042 was superseded by #74045, #74718 was replaced by #74795, and #74795 and #75511 duplicate #74045's direct-plus-auth/session-recovery behavior. Suggested consolidationAuthor action: rebase #68455 onto main, or split out the part that can merge; reconcile removal or invalidation of Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I11113(["issue #11113 (open)"])
I44172(["issue #44172 (open)"])
P68455["PR #68455 (open)"]
P68455 -.->|partial| I11113
P68455 -->|best fix| I44172
class I11113 open
class I44172 open
class P68455 open
class P68455 best
class P68455 target
click I11113 "https://github.com/NousResearch/hermes-agent/issues/11113"
click I44172 "https://github.com/NousResearch/hermes-agent/issues/44172"
click P68455 "https://github.com/NousResearch/hermes-agent/pull/68455"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 20 pull requests and 4 issues in this complex. Each diff was read against this issue; Assessment working set: 156 kB of PR diffs, 61 kB of issue/PR text, 48 kB of discussion (62 comments), 33 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Adds per-server
breaker_thresholdconfig key undermcp_servers.<name>to override the global_CIRCUIT_BREAKER_THRESHOLD(default: 3).Problem
The MCP circuit breaker uses a hardcoded threshold of 3 consecutive failures before marking a server unreachable. For unstable-but-recoverable MCP servers (e.g. email/IMAP servers that flap on Gmail rate limiting), 3 failures is too aggressive — the server gets locked out for 60s even though it would recover on the next call.
Changes
_circuit_breaker_thresholds: Dict[str, int]module-level dict for per-server overrides_bump_server_error()and the tool handler guard check both now resolve threshold per-server, falling back to_CIRCUIT_BREAKER_THRESHOLD(3)register_mcp_servers()populates the dict fromsrv_cfg.get("breaker_threshold")when it's a positive intUsage
Omit
breaker_thresholdto keep the default of 3.Closes #44172