Skip to content

fix(mcp): surface the real cause on MCP loss and recover parked servers faster - #80815

Open
Revelation-Agency wants to merge 1 commit into
NousResearch:mainfrom
Revelation-Agency:fix/mcp-resilience-0030
Open

fix(mcp): surface the real cause on MCP loss and recover parked servers faster#80815
Revelation-Agency wants to merge 1 commit into
NousResearch:mainfrom
Revelation-Agency:fix/mcp-resilience-0030

Conversation

@Revelation-Agency

Copy link
Copy Markdown

Hi — a small reliability patch to tools/mcp_tool.py, offered from production experience.

What happened to us. A client-facing agent ran ~53 hours with zero MCP tools and nothing alerted. The MCP server had dropped, its tools were deregistered, and the agent carried on answering from memory rather than saying it had lost the ability to act. When we went looking, the log said unhandled errors in a TaskGroup (1 sub-exception), which didn't tell us what actually broke.

What this changes (all in tools/mcp_tool.py):

  1. Surface the real cause. _flatten_exc() unwraps ExceptionGroup / TaskGroup sub-exceptions — bounded at depth 4 and the first 5 sub-exceptions — so the underlying failure reaches the log instead of the wrapper message.

  2. Make total tool loss loud. Parking now logs at ERROR and names the server plus how many tools were lost, and _deregister_tools(reason=...) emits an ERROR on the transition to zero tools. It stays quiet on ordinary shutdown, so clean exits don't get noisy. An agent that can no longer act felt worth an ERROR rather than a WARNING.

  3. Recover faster. _PARKED_RETRY_INTERVAL 300s → 30s. At five minutes a parked server turns a blip into an outage; 30s took our recovery from ~9 minutes down to ~60–90s.

Beyond log levels/content and that retry cadence there's no behaviour change, and no public API change.

Happy to adjust any of this to your conventions — split it into separate commits, reword the log lines or dial the levels back, make the retry interval configurable instead of a constant, or drop any part you'd rather not carry. Just say which and I'll rework it.

…rs faster

When an MCP server drops, the agent silently loses its tools and the operator is
not told. A client-facing agent ran 53 hours with zero tools and nothing alerted.

1. _flatten_exc() unwraps ExceptionGroup/TaskGroup sub-exceptions (bounded depth
   4, first 5) so the real failure reaches the log instead of "unhandled errors
   in a TaskGroup (1 sub-exception)".
2. Parking is logged at ERROR naming the server and the number of tools lost;
   _deregister_tools(reason=...) emits ERROR on the transition to ZERO tools,
   suppressed on ordinary shutdown so clean exits stay quiet.
3. _PARKED_RETRY_INTERVAL 300s -> 30s so recovery is not itself an outage.
@alt-glitch alt-glitch added type/bug Something isn't working tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists labels Aug 7, 2026
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…esearch#25437)

Followup to PR NousResearch#24182 — caught when scanning OpenClaw for recent codex
fixes we hadn't considered. OpenClaw learned the hard way (NousResearch#80815) that
migrating plugins which codex itself reports as unavailable produces
config that fails at activation time.

Our /codex-runtime codex_app_server enable path queries codex's
plugin/list and migrates everything where installed=true. We were
trusting codex's installation state and ignoring its availability
field. So a plugin that's installed=true but availability=UNAVAILABLE
(broken local install) or REQUIRES_AUTH (OAuth expired or never
completed) would get an [plugins."<n>@openai-curated"] entry in
~/.codex/config.toml — and the user's first codex turn after enabling
the runtime would fail because codex refuses to activate it.

Fix: filter on availability in _query_codex_plugins(). Only emit
plugins where availability is empty (older codex versions without the
field — preserve backward compat) or explicitly AVAILABLE.

Tests:
  test_plugin_discovery_skips_unavailable_plugins — verifies 4 cases:
    - good-plugin (installed=True, availability=AVAILABLE) → migrated
    - broken-plugin (installed=True, availability=UNAVAILABLE) → skipped
    - auth-pending (installed=True, availability=REQUIRES_AUTH) → skipped
    - legacy-plugin (installed=True, no availability field) → migrated
      (older codex versions; preserve backward compat)

Docs:
  Added bullet to 'What's NOT migrated' list in the docs page calling
  out the availability filter and why.

Other OpenClaw codex PRs I reviewed but did NOT apply (with reasoning):
  - NousResearch#81591 (load Codex for selectable models): we resolve runtime
    per-call already, no startup-time gating to fix
  - NousResearch#81510 (cron compatibility): we documented cron as untested; their
    fix is for OpenClaw-specific cron orchestration shape
  - NousResearch#81223 (rotate incompatible context-engine threads): we don't
    have a Lossless context engine equivalent
  - NousResearch#80688 (constrain sandbox): we don't have an outer-sandbox concept
  - NousResearch#80616 (release on turn_aborted): we already handle status=
    interrupted in turn/completed correctly
  - NousResearch#80278 (expose activeModel in plugin SDK): not our surface
  - NousResearch#80792 (default destructive_actions on): we don't expose that knob

56 codex-runtime migration tests still green (+1 new).
@spfcraze

spfcraze commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
The premise of change 1 is already handled on main: tools/mcp_tool.py:3238 unwraps TaskGroup errors before the log lines this PR edits, so the one-sub-exception log the description cites already shows its root cause.

Problems:

  • tools/mcp_tool.py:3238 already runs root = _unwrap_exception_group(exc) — helper defined at tools/mcp_tool.py:980, whose docstring quotes the same "unhandled errors in a TaskGroup" string — before the parking and reconnect logs this PR changes, and those logs already print the unwrapped root cause. For the cited one-sub-exception case _flatten_exc(exc) re-adds the wrapper text around the same cause; its real delta is the multi-leaf rendering (up to 5 sub-exceptions, depth 4). The same string is reported on the initial-connect path in open issue [Bug] /reload-mcp drops ALL stdio MCP servers — every initial connection fails with "unhandled errors in a TaskGroup" and servers park permanently #78426, which this PR does not touch.
  • The motivation for change 2 — an agent running with zero MCP tools while nothing alerts — is the subject of open issue [Feature/Bug]: No signal exposed to indicate whether a completed run's tool calls actually succeeded - a session with zero working tools can still return a confident, well-formed disposition #73389 (zero working tools still yield a confident, well-formed disposition, with no signal exposed), which the description does not reference; the ERROR log is a diagnostic step toward it, not the run-visible signal that issue asks for. The alert it adds fires only on the degraded-to-parked path: the permanent-error park (tools/mcp_tool.py:3354-3367) deregisters a previously-working server's tools via a bare self._deregister_tools() call, so the transition item 2 says is worth an ERROR remains a WARNING there.
  • The description names one retry-cadence change (_PARKED_RETRY_INTERVAL, item 3) and states there is "no behaviour change" beyond it; the diff also shortens _CIRCUIT_BREAKER_COOLDOWN_SEC 60.0 to 15.0 (tools/mcp_tool.py:3623), which shrinks the model-facing "Auto-retry available in ~Ns" window (tools/mcp_tool.py:4904) — a second behavior change the body does not name.

Solution:
Keep the existing type(root).__name__, root lines and drop _flatten_exc, or scope it to the multi-sub-exception case; name _CIRCUIT_BREAKER_COOLDOWN_SEC in the description or revert it; reference #73389 if change 2 targets that issue; and pass a reason at the permanent-error park if every zero-tool transition is meant to alert.


Checked against 4acbc10 — the tip of fix/mcp-resilience-0030 when this was written — and 20e01f9, main at the same moment.

igangz pushed a commit to igangz/hermes-agent that referenced this pull request Aug 10, 2026
…esearch#25437)

Followup to PR NousResearch#24182 — caught when scanning OpenClaw for recent codex
fixes we hadn't considered. OpenClaw learned the hard way (NousResearch#80815) that
migrating plugins which codex itself reports as unavailable produces
config that fails at activation time.

Our /codex-runtime codex_app_server enable path queries codex's
plugin/list and migrates everything where installed=true. We were
trusting codex's installation state and ignoring its availability
field. So a plugin that's installed=true but availability=UNAVAILABLE
(broken local install) or REQUIRES_AUTH (OAuth expired or never
completed) would get an [plugins."<n>@openai-curated"] entry in
~/.codex/config.toml — and the user's first codex turn after enabling
the runtime would fail because codex refuses to activate it.

Fix: filter on availability in _query_codex_plugins(). Only emit
plugins where availability is empty (older codex versions without the
field — preserve backward compat) or explicitly AVAILABLE.

Tests:
  test_plugin_discovery_skips_unavailable_plugins — verifies 4 cases:
    - good-plugin (installed=True, availability=AVAILABLE) → migrated
    - broken-plugin (installed=True, availability=UNAVAILABLE) → skipped
    - auth-pending (installed=True, availability=REQUIRES_AUTH) → skipped
    - legacy-plugin (installed=True, no availability field) → migrated
      (older codex versions; preserve backward compat)

Docs:
  Added bullet to 'What's NOT migrated' list in the docs page calling
  out the availability filter and why.

Other OpenClaw codex PRs I reviewed but did NOT apply (with reasoning):
  - NousResearch#81591 (load Codex for selectable models): we resolve runtime
    per-call already, no startup-time gating to fix
  - NousResearch#81510 (cron compatibility): we documented cron as untested; their
    fix is for OpenClaw-specific cron orchestration shape
  - NousResearch#81223 (rotate incompatible context-engine threads): we don't
    have a Lossless context engine equivalent
  - NousResearch#80688 (constrain sandbox): we don't have an outer-sandbox concept
  - NousResearch#80616 (release on turn_aborted): we already handle status=
    interrupted in turn/completed correctly
  - NousResearch#80278 (expose activeModel in plugin SDK): not our surface
  - NousResearch#80792 (default destructive_actions on): we don't expose that knob

56 codex-runtime migration tests still green (+1 new).
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…esearch#25437)

Followup to PR NousResearch#24182 — caught when scanning OpenClaw for recent codex
fixes we hadn't considered. OpenClaw learned the hard way (NousResearch#80815) that
migrating plugins which codex itself reports as unavailable produces
config that fails at activation time.

Our /codex-runtime codex_app_server enable path queries codex's
plugin/list and migrates everything where installed=true. We were
trusting codex's installation state and ignoring its availability
field. So a plugin that's installed=true but availability=UNAVAILABLE
(broken local install) or REQUIRES_AUTH (OAuth expired or never
completed) would get an [plugins."<n>@openai-curated"] entry in
~/.codex/config.toml — and the user's first codex turn after enabling
the runtime would fail because codex refuses to activate it.

Fix: filter on availability in _query_codex_plugins(). Only emit
plugins where availability is empty (older codex versions without the
field — preserve backward compat) or explicitly AVAILABLE.

Tests:
  test_plugin_discovery_skips_unavailable_plugins — verifies 4 cases:
    - good-plugin (installed=True, availability=AVAILABLE) → migrated
    - broken-plugin (installed=True, availability=UNAVAILABLE) → skipped
    - auth-pending (installed=True, availability=REQUIRES_AUTH) → skipped
    - legacy-plugin (installed=True, no availability field) → migrated
      (older codex versions; preserve backward compat)

Docs:
  Added bullet to 'What's NOT migrated' list in the docs page calling
  out the availability filter and why.

Other OpenClaw codex PRs I reviewed but did NOT apply (with reasoning):
  - NousResearch#81591 (load Codex for selectable models): we resolve runtime
    per-call already, no startup-time gating to fix
  - NousResearch#81510 (cron compatibility): we documented cron as untested; their
    fix is for OpenClaw-specific cron orchestration shape
  - NousResearch#81223 (rotate incompatible context-engine threads): we don't
    have a Lossless context engine equivalent
  - NousResearch#80688 (constrain sandbox): we don't have an outer-sandbox concept
  - NousResearch#80616 (release on turn_aborted): we already handle status=
    interrupted in turn/completed correctly
  - NousResearch#80278 (expose activeModel in plugin SDK): not our surface
  - NousResearch#80792 (default destructive_actions on): we don't expose that knob

56 codex-runtime migration tests still green (+1 new).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

4 participants