[PR #6353] [MERGED] fix(mcp): escalate transient failures before asking for a restart #7302

Closed
opened 2026-11-01 21:46:53 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6353
Author: @morluto
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: cursor/agent-recovery-ladder-f397


📝 Commits (5)

  • b371f28 fix(mcp): escalate transient failures before asking for a restart
  • cfdbf6a fix(process): stop unexpected command failures from recommending a retry
  • 999378f fix(mcp): limit retries to safe calls and recommended settings
  • 1c55530 test(mcp): match recovery guidance across wrapped lines
  • 367b004 merge(main): take the shared cancellation contract

📊 Changes

15 files changed (+233 additions, -44 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/SKILL.md (+9 -1)
📝 .agents/skills/reverse-engineer-anything/references/connection-and-recovery.md (+16 -10)
📝 docs/mcp-contracts.md (+9 -5)
📝 docs/user-visible-errors.csv (+5 -5)
📝 scripts/lib/test-projects.mjs (+1 -1)
📝 src/application/process/ProcessCli.ts (+8 -0)
📝 src/domain/analysisErrorProjection.test.ts (+1 -1)
📝 src/generatedPackageMetadata.ts (+1 -1)
➕ src/main/messages.test.ts (+150 -0)
📝 src/main/messages.ts (+3 -3)
📝 src/server/createServer.ts (+1 -1)
📝 tests/boundary/filesystem/processCliFileInput.test.ts (+6 -0)
📝 tests/boundary/mcp/deferredRuntime.test.ts (+1 -3)
📝 tests/boundary/mcp/javascriptAnalysisResources.test.ts (+11 -0)
📝 tests/boundary/mcp/mainErrors.test.ts (+11 -13)

📄 Description

Summary

Transient MCP failures tell an agent when one retry is safe, then to tell the user and ask them to restart the MCP client. A dropped connection is retried only when the call is safe to repeat or known not to have started. Startup that can reject the same bytes every time does not ask for a retry. A setting changes only when the returned remediation explicitly recommends that change.

No linked issue. This aligns agent-facing recovery copy with the current settings; it does not change a reported defect's reproduction.

Problem and expected behavior

Dropped MCP connections and startup failures only said to restart REA or run rea doctor. Agents had no bounded retry and no scripted handoff to the user. The timeout catalog still said to reduce work without assuming adjustable limits, even where a producer names analysis_timeout_ms or a REA_* setting.

A dropped connection can return after the tool has already changed the target, so the retry is limited to a call that is safe to repeat or known not to have started. Otherwise the agent tells the user REA lost its MCP client connection and asks them to restart the MCP client. A connection that never started still retries once, then uses that handoff and rea doctor. Server startup, including a catalog the same bytes will reject again, does not repeat: tell the user REA could not start, restart the MCP client, then run rea doctor.

An unexpected process command does not use that retry. The catch can already have launched the target, and the projection stays retryable: false. Its remediation tells the user the command could not complete, asks them to check the input files, and names rea doctor.

Timeout guidance changes a request field or REA_* setting only when the remediation explicitly recommends that change. A mention that a control does not raise the deadline, such as the Wakaru recovery note about analysis_timeout_ms and max_heap_mb, is not a recommendation.

Change and scope

The ladder lives in the stderr constants, MCP initialization instructions, and the shipped skill. rea doctor is included only when startup can be diagnosed. A lost connection does not name doctor, because doctor cannot restore that connection.

Shutdown and HTTP listen failures keep their existing named fixes. A failed close must end the process before another start, and a port conflict is fixed by choosing another port. Hopper close_binary recoveries and ceiling dial catalogs (captureDecoderHeapRecovery, mcpPhaseBudgetRecovery, and the other effective-value helpers) are unchanged.

Generic AnalysisTimeoutError text still does not invent a setting. Producer remediations that already name one, such as JavaScript analysis_timeout_ms, stay in place. The skill version moves from 44 to 45 so an installed skill can pick up the recovery guide.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

  • Semantic owner and earliest changed stage: MCP transport stderr and server initialization instructions own the connection ladder. Process CLI owns the unexpected-command remediation. Domain timeout projection is unchanged except tests that forbid the removed phrase.
  • Contribution scope fit and linked maintainer decision: ordinary recovery-copy fix inside existing boundaries. Maintainer scope decision not applicable.
  • CLI and MCP/tool-catalog contract: no tool schema or argument change. Initialization instructions and stderr text change. Unexpected process CLI errors gain a remediation.action that does not recommend a retry; retryable stays false.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): none.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): authored skill version 45. src/generatedPackageMetadata.ts skillVersion is 45. Ignored skills/ output is regenerated by CI.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: src/main/messages.test.ts locks the safe-retry ladder, keeps startup off a retry, keeps shutdown and HTTP listen on their named fixes, keeps ceiling dial recoveries free of the user handoff, rejects the old non-adjustable timeout phrases, and rejects the word retry in the unexpected process remediation. It also requires guidance to change a setting only when the remediation explicitly recommends that change, and keeps the Wakaru sentence that raising analysis_timeout_ms or max_heap_mb does not raise that deadline. Boundary tests cover stderr emission, deferred startup, process CLI remediation, and initialization instructions.
  • Base reproduction or other evidence: source inspection of the previous strings. Connection loss was REA lost its MCP client connection. Restart REA from your MCP client. Startup was REA could not start its MCP connection. Restart REA from your MCP client; run rea doctor if it fails again. The timeout catalog said without assuming adjustable limits.
  • User-visible CLI/MCP output (if applicable): connection loss is Retry the tool call once only when it is safe to repeat or known not to have started. If it may already have changed the target, or it fails again, tell the user REA lost its MCP client connection and ask them to restart the MCP client. A connection that never started still begins Retry once. Server startup is Do not repeat this startup. Tell the user REA could not start and ask them to restart the MCP client, then run rea doctor. Unexpected process commands keep the sanitized cause in message. Remediation is Do not repeat this command. Tell the user the process command could not complete and ask them to check the input files, then run rea doctor.
  • Remaining proof gaps: token cost of the longer initialization paragraph is unmeasured. No real MCP client session was driven outside the in-memory and stdio tests. Ceiling default, floor, and maximum values were not exercised as behavior changes. GitHub CI for 367b0043a had not finished when this body was updated.

Validation performed

  • npm run test:local -- src/main/messages.test.ts — 3 passed on 1c5553032.
  • npm run check:fast — typecheck and lint succeeded on 1c5553032.
  • npx oxfmt --check on the edited recovery sources — passed on 1c5553032.
  • npm run test:focused -- tests/boundary/mcp/mainErrors.test.ts tests/boundary/mcp/deferredRuntime.test.ts tests/boundary/mcp/javascriptAnalysisResources.test.ts — 39 passed on 1c5553032.
  • npm run test:focused -- tests/boundary/filesystem/processCliFileInput.test.ts — 19 passed on cfdbf6abb. That remediation was not edited again.
  • npm run metadata:check — succeeded during the focused-test build with skill version 45.
  • GitHub CI on 1c5553032: Portable tests and Test shard (6/8) failed on reports file progress and honors cancellation before the next source. The run received ArtifactOperationError where that older test required AnalysisCancelledError. CI required failed because those jobs failed. Greptile Review passed on that commit.
  • 367b0043a merges current main, which already asserts the shared cancellation projection (code: cancelled, caller abort reason, partial observation) instead of one error tag.
  • npm run test:focused -- tests/boundary/filesystem/javascript-applications/javascriptArtifactReconstruction.test.ts — 44 passed on 367b0043a, including that cancellation test.

Compatibility, safety, and release

  • Breaking changes or migration steps: no CLI or MCP schema break. Installed skills at version 44 are stale relative to skillVersion 45; setup or skill install refreshes the recovery guide.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable. Copy and remediation only.
  • Package or release metadata impact: skillVersion 45 in generated package metadata. Package version unchanged.
  • Security, privacy, process, or containment review: stderr still omits the transport cause. Unexpected process messages still include the sanitized cause and not a stack. The unexpected-command remediation does not ask the agent to launch the target again. A dropped-connection retry is limited to a call that is safe to repeat or known not to have started. No new process or network authority.

Author readiness

Human owner: @morluto

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.
Open in Web Open in Cursor 

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6353 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `cursor/agent-recovery-ladder-f397` --- ### 📝 Commits (5) - [`b371f28`](https://github.com/morluto/rea/commit/b371f2847a9e29715d417aefd1496ac51aec1d38) fix(mcp): escalate transient failures before asking for a restart - [`cfdbf6a`](https://github.com/morluto/rea/commit/cfdbf6abbc5cb35e3557231ba15aa5fca207dfcb) fix(process): stop unexpected command failures from recommending a retry - [`999378f`](https://github.com/morluto/rea/commit/999378f9e6379f339f23e5ee55728da64fe9d0f2) fix(mcp): limit retries to safe calls and recommended settings - [`1c55530`](https://github.com/morluto/rea/commit/1c555303239dcebeeeaf80f8c53e49b35cfe1e9e) test(mcp): match recovery guidance across wrapped lines - [`367b004`](https://github.com/morluto/rea/commit/367b0043a56ca24b043133befc1ce465c3c1bf46) merge(main): take the shared cancellation contract ### 📊 Changes **15 files changed** (+233 additions, -44 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/SKILL.md` (+9 -1) 📝 `.agents/skills/reverse-engineer-anything/references/connection-and-recovery.md` (+16 -10) 📝 `docs/mcp-contracts.md` (+9 -5) 📝 `docs/user-visible-errors.csv` (+5 -5) 📝 `scripts/lib/test-projects.mjs` (+1 -1) 📝 `src/application/process/ProcessCli.ts` (+8 -0) 📝 `src/domain/analysisErrorProjection.test.ts` (+1 -1) 📝 `src/generatedPackageMetadata.ts` (+1 -1) ➕ `src/main/messages.test.ts` (+150 -0) 📝 `src/main/messages.ts` (+3 -3) 📝 `src/server/createServer.ts` (+1 -1) 📝 `tests/boundary/filesystem/processCliFileInput.test.ts` (+6 -0) 📝 `tests/boundary/mcp/deferredRuntime.test.ts` (+1 -3) 📝 `tests/boundary/mcp/javascriptAnalysisResources.test.ts` (+11 -0) 📝 `tests/boundary/mcp/mainErrors.test.ts` (+11 -13) </details> ### 📄 Description <!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Summary Transient MCP failures tell an agent when one retry is safe, then to tell the user and ask them to restart the MCP client. A dropped connection is retried only when the call is safe to repeat or known not to have started. Startup that can reject the same bytes every time does not ask for a retry. A setting changes only when the returned remediation explicitly recommends that change. No linked issue. This aligns agent-facing recovery copy with the current settings; it does not change a reported defect's reproduction. ## Problem and expected behavior Dropped MCP connections and startup failures only said to restart REA or run `rea doctor`. Agents had no bounded retry and no scripted handoff to the user. The timeout catalog still said to reduce work without assuming adjustable limits, even where a producer names `analysis_timeout_ms` or a `REA_*` setting. A dropped connection can return after the tool has already changed the target, so the retry is limited to a call that is safe to repeat or known not to have started. Otherwise the agent tells the user REA lost its MCP client connection and asks them to restart the MCP client. A connection that never started still retries once, then uses that handoff and `rea doctor`. Server startup, including a catalog the same bytes will reject again, does not repeat: tell the user REA could not start, restart the MCP client, then run `rea doctor`. An unexpected process command does not use that retry. The catch can already have launched the target, and the projection stays `retryable: false`. Its remediation tells the user the command could not complete, asks them to check the input files, and names `rea doctor`. Timeout guidance changes a request field or `REA_*` setting only when the remediation explicitly recommends that change. A mention that a control does not raise the deadline, such as the Wakaru recovery note about `analysis_timeout_ms` and `max_heap_mb`, is not a recommendation. ## Change and scope The ladder lives in the stderr constants, MCP initialization instructions, and the shipped skill. `rea doctor` is included only when startup can be diagnosed. A lost connection does not name doctor, because doctor cannot restore that connection. Shutdown and HTTP listen failures keep their existing named fixes. A failed close must end the process before another start, and a port conflict is fixed by choosing another port. Hopper `close_binary` recoveries and ceiling dial catalogs (`captureDecoderHeapRecovery`, `mcpPhaseBudgetRecovery`, and the other effective-value helpers) are unchanged. Generic `AnalysisTimeoutError` text still does not invent a setting. Producer remediations that already name one, such as JavaScript `analysis_timeout_ms`, stay in place. The skill version moves from 44 to 45 so an installed skill can pick up the recovery guide. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: MCP transport stderr and server initialization instructions own the connection ladder. Process CLI owns the unexpected-command remediation. Domain timeout projection is unchanged except tests that forbid the removed phrase. - Contribution scope fit and linked maintainer decision: ordinary recovery-copy fix inside existing boundaries. Maintainer scope decision not applicable. - CLI and MCP/tool-catalog contract: no tool schema or argument change. Initialization instructions and stderr text change. Unexpected process CLI errors gain a `remediation.action` that does not recommend a retry; `retryable` stays false. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): none. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): authored skill version 45. `src/generatedPackageMetadata.ts` `skillVersion` is 45. Ignored `skills/` output is regenerated by CI. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [x] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `src/main/messages.test.ts` locks the safe-retry ladder, keeps startup off a retry, keeps shutdown and HTTP listen on their named fixes, keeps ceiling dial recoveries free of the user handoff, rejects the old non-adjustable timeout phrases, and rejects the word retry in the unexpected process remediation. It also requires guidance to change a setting only when the remediation explicitly recommends that change, and keeps the Wakaru sentence that raising `analysis_timeout_ms` or `max_heap_mb` does not raise that deadline. Boundary tests cover stderr emission, deferred startup, process CLI remediation, and initialization instructions. - Base reproduction or other evidence: source inspection of the previous strings. Connection loss was `REA lost its MCP client connection. Restart REA from your MCP client.` Startup was `REA could not start its MCP connection. Restart REA from your MCP client; run `rea doctor` if it fails again.` The timeout catalog said `without assuming adjustable limits`. - User-visible CLI/MCP output (if applicable): connection loss is `Retry the tool call once only when it is safe to repeat or known not to have started. If it may already have changed the target, or it fails again, tell the user REA lost its MCP client connection and ask them to restart the MCP client.` A connection that never started still begins `Retry once.` Server startup is `Do not repeat this startup. Tell the user REA could not start and ask them to restart the MCP client, then run `rea doctor`.` Unexpected process commands keep the sanitized cause in `message`. Remediation is `Do not repeat this command. Tell the user the process command could not complete and ask them to check the input files, then run `rea doctor`.` - Remaining proof gaps: token cost of the longer initialization paragraph is unmeasured. No real MCP client session was driven outside the in-memory and stdio tests. Ceiling default, floor, and maximum values were not exercised as behavior changes. GitHub CI for `367b0043a` had not finished when this body was updated. ## Validation performed - `npm run test:local -- src/main/messages.test.ts` — 3 passed on `1c5553032`. - `npm run check:fast` — typecheck and lint succeeded on `1c5553032`. - `npx oxfmt --check` on the edited recovery sources — passed on `1c5553032`. - `npm run test:focused -- tests/boundary/mcp/mainErrors.test.ts tests/boundary/mcp/deferredRuntime.test.ts tests/boundary/mcp/javascriptAnalysisResources.test.ts` — 39 passed on `1c5553032`. - `npm run test:focused -- tests/boundary/filesystem/processCliFileInput.test.ts` — 19 passed on `cfdbf6abb`. That remediation was not edited again. - `npm run metadata:check` — succeeded during the focused-test build with skill version 45. - GitHub CI on `1c5553032`: Portable tests and Test shard (6/8) failed on `reports file progress and honors cancellation before the next source`. The run received `ArtifactOperationError` where that older test required `AnalysisCancelledError`. `CI required` failed because those jobs failed. Greptile Review passed on that commit. - `367b0043a` merges current `main`, which already asserts the shared cancellation projection (`code: cancelled`, caller abort reason, partial observation) instead of one error tag. - `npm run test:focused -- tests/boundary/filesystem/javascript-applications/javascriptArtifactReconstruction.test.ts` — 44 passed on `367b0043a`, including that cancellation test. ## Compatibility, safety, and release - Breaking changes or migration steps: no CLI or MCP schema break. Installed skills at version 44 are stale relative to `skillVersion` 45; setup or skill install refreshes the recovery guide. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Copy and remediation only. - Package or release metadata impact: `skillVersion` 45 in generated package metadata. Package version unchanged. - Security, privacy, process, or containment review: stderr still omits the transport cause. Unexpected process messages still include the sanitized cause and not a stack. The unexpected-command remediation does not ask the agent to launch the target again. A dropped-connection retry is limited to a call that is safe to repeat or known not to have started. No new process or network authority. ## Author readiness Human owner: @morluto - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-1be02f83-efc4-557d-beb4-6f6e523cf397?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a href="https://cursor.com/background-agent?bcId=bc-1be02f83-efc4-557d-beb4-6f6e523cf397&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:46:53 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7302
No description provided.