[PR #5608] [MERGED] fix(mcp): guide client and provider timeout recovery #6994

New issue

Closed

opened 2026-11-01 21:44:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:33 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5608
Author: @gensokyo-dev
Created: 10/20/2026
Status: ✅ Merged
Merged: 10/20/2026
Merged by: @gensokyo-dev

Base: main ← Head: feat/5598-precall-recovery

📝 Commits (2)

24e32b2 fix(mcp): teach deadline recovery before responses are lost
af29cc3 fix(errors): avoid invented controls in generic timeout recovery

📊 Changes

14 files changed (+179 additions, -37 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/SKILL.md (+3 -1)
📝 .agents/skills/reverse-engineer-anything/references/connection-and-recovery.md (+41 -0)
📝 .agents/skills/reverse-engineer-anything/references/javascript-applications.md (+5 -2)
📝 docs/mcp-contracts.md (+21 -3)
📝 docs/user-visible-errors.csv (+1 -1)
📝 src/contracts/javascript/electronToolContracts.ts (+1 -1)
📝 src/domain/analysisErrorPresentation.ts (+2 -2)
📝 src/domain/analysisErrorProjection.test.ts (+8 -1)
📝 src/generatedPackageMetadata.ts (+1 -1)
📝 src/javascript/analysis/JavaScriptAnalysisWorker.ts (+3 -0)
📝 src/server/createServer.ts (+6 -1)
📝 tests/acceptance/applications/javascriptAnalysisLimits.test.ts (+33 -0)
📝 tests/boundary/mcp/javascriptAnalysisResources.test.ts (+53 -23)
📝 tests/boundary/mcp/webBundleContainmentMcp.test.ts (+1 -1)

📄 Description

Summary

Fixes #5598. Teach recovery before a host deadline can discard REA's error. The issue records a pinned, cloned RepoPrompt CE comparison and actual Codex counterexamples.

Problem and expected behavior

With a 1-second Codex MCP deadline, the old agent repeated an identical failed analysis without identifying the client setting. Returned provider errors also blurred the two deadline owners. An unavailable partial-Evidence reader must not block a corrected static retry.

Change and scope

Concise initialization guidance plus a targeted JavaScript tool-description warning: a host-only timeout needs client tool_timeout_sec, not server analysis_timeout_ms.
Producer-authored JavaScript errors name the actual per-source control and CLI equivalent, optional exact source selection, and summary-versus-analysis distinction. Shared generic error projection neither suggests client-only changes for returned server timeouts nor assumes adjustable controls on fixed-budget operations.
Update the shipped skill's focused recovery reference and version. Reuse the canonical error/reference parsers in boundary tests instead of interpreting the diagnostic twice.

Contract and boundary impact

Owner: initialization/catalog teaching surfaces; JavaScript timeout producer; shared domain error presentation.
Scope: routine usability fix; no new provider/capability or maintainer scope decision needed.
CLI/MCP: same error/schema identities, argument types, results, and caller choices. No discovery tool or repeated full schema payload added.
Evidence/provenance/cleanup: unchanged; partial references stay usable; unknown work/cleanup stays unknown. No execution, installation, configuration, or target-authority expansion.
Generated metadata: authored skill 44 and tracked package metadata; ignored catalogs/skills regenerated, not committed.
Documentation or usability only; no target interaction.

Evidence and regression coverage

The initialization regression failed against the unmodified compiled base, then passed after the fix.
Public CLI and stdio MCP exercise actual 1ms JavaScript timeouts, precise remediation, retained partial Evidence, continued analysis, caller bounds and provenance. Existing wire-budget/untrusted-content guards retained.
Actual standalone codex-cli 0.161.0, gpt-6.1-sol, Node 24.18.0; Commander 14.0.3, static only. Fresh processes, serial bounded-heap trials. Full calls and receipts retained locally; outcome/performance summary linked in the issue after final qualification.
Actual Codex qualification: client deadlines 4/4 actionable stops without unchanged retries; server timeouts 2/2 repaired in two calls; normal controls 2/2 one-call successes. Across all iterations 28 trials were retained. Final eight: 741,148 input tokens (587,136 cached), 4,825 output; 255.62s wall sum, 112.50s CPU, sampled peak family RSS 1,255.25 MiB. Warm/uncontrolled caches; 500ms summed RSS is not cgroup peak. Immediate cleanup samples are inconclusive where a pending refresh raced; a separate post-run audit found no task-specific process matches.
Important negative evidence retained: an initial imperative to inspect partial Evidence caused two stops with hidden readers; an initialization-only candidate still made one unchanged client retry. The final version makes inspection optional and also teaches via the advertised tool description. No claim of universal model reliability or a causal benchmark.
Two independent Codex CLI source reviews: no actionable findings; no target/provider execution by reviewers.

Validation performed

npm run test:focused -- tests/boundary/mcp/javascriptAnalysisResources.test.ts tests/acceptance/applications/javascriptAnalysisLimits.test.ts tests/boundary/mcp/webBundleContainmentMcp.test.ts tests/boundary/mcp/targetContentFramingMcp.test.ts src/domain/analysisErrorProjection.test.ts src/server/EvidenceMcpServer.test.ts — 79 tests passed; the refactored resource file additionally rerun.
npm run check — passed (typecheck, lint, format, knip, metadata, architecture/module boundaries).
npm run docs:generate / npm run docs:check — passed.
Skill validator and npm pack --dry-run --json — passed.

Compatibility, safety, and release

No breaking changes or new runtime work/cache. Startup teaching costs extra context; whole-task tokens, wall/CPU time and sampled family RSS are reported, not inferred from bytes. No real Hopper/Ghidra/macOS qualification: unavailable-provider probes only test deliberate missing-install preflight; RepoPrompt research is source-only. Underlying analysis semantics are unchanged.

Author readiness

Human owner: @gensokyo-dev (task owner's explicitly selected account).

The task owner explicitly directed ready-for-review submission and delegated continuation/merge, including bypass when appropriate. Automated review and executed evidence are reported above; no separate human final-diff review is asserted by this agent.

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

Concrete problem, focused scope and Conventional Commit title.
No unresolved new authority or capability questions.
Related issue, observable output, owning docs and regression coverage included.
Meaningful verification limits recorded; generated outputs and credentials excluded.

_{🔄 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/5608 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/20/2026 **Status:** ✅ Merged **Merged:** 10/20/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `feat/5598-precall-recovery` --- ### 📝 Commits (2) - [`24e32b2`](https://github.com/morluto/rea/commit/24e32b26d5d5d57eb5dd411d0d48b2034a9e2790) fix(mcp): teach deadline recovery before responses are lost - [`af29cc3`](https://github.com/morluto/rea/commit/af29cc3ef81d3b983930ffe1b8b162827fc590f7) fix(errors): avoid invented controls in generic timeout recovery ### 📊 Changes **14 files changed** (+179 additions, -37 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/SKILL.md` (+3 -1) 📝 `.agents/skills/reverse-engineer-anything/references/connection-and-recovery.md` (+41 -0) 📝 `.agents/skills/reverse-engineer-anything/references/javascript-applications.md` (+5 -2) 📝 `docs/mcp-contracts.md` (+21 -3) 📝 `docs/user-visible-errors.csv` (+1 -1) 📝 `src/contracts/javascript/electronToolContracts.ts` (+1 -1) 📝 `src/domain/analysisErrorPresentation.ts` (+2 -2) 📝 `src/domain/analysisErrorProjection.test.ts` (+8 -1) 📝 `src/generatedPackageMetadata.ts` (+1 -1) 📝 `src/javascript/analysis/JavaScriptAnalysisWorker.ts` (+3 -0) 📝 `src/server/createServer.ts` (+6 -1) 📝 `tests/acceptance/applications/javascriptAnalysisLimits.test.ts` (+33 -0) 📝 `tests/boundary/mcp/javascriptAnalysisResources.test.ts` (+53 -23) 📝 `tests/boundary/mcp/webBundleContainmentMcp.test.ts` (+1 -1) </details> ### 📄 Description ## Summary Fixes #5598. Teach recovery **before** a host deadline can discard REA's error. The issue records a pinned, cloned RepoPrompt CE comparison and actual Codex counterexamples. ## Problem and expected behavior With a 1-second Codex MCP deadline, the old agent repeated an identical failed analysis without identifying the client setting. Returned provider errors also blurred the two deadline owners. An unavailable partial-Evidence reader must not block a corrected static retry. ## Change and scope - Concise initialization guidance plus a targeted JavaScript tool-description warning: a host-only timeout needs client `tool_timeout_sec`, not server `analysis_timeout_ms`. - Producer-authored JavaScript errors name the actual per-source control and CLI equivalent, optional exact source selection, and summary-versus-analysis distinction. Shared generic error projection neither suggests client-only changes for returned server timeouts nor assumes adjustable controls on fixed-budget operations. - Update the shipped skill's focused recovery reference and version. Reuse the canonical error/reference parsers in boundary tests instead of interpreting the diagnostic twice. ## Contract and boundary impact - Owner: initialization/catalog teaching surfaces; JavaScript timeout producer; shared domain error presentation. - Scope: routine usability fix; no new provider/capability or maintainer scope decision needed. - CLI/MCP: same error/schema identities, argument types, results, and caller choices. No discovery tool or repeated full schema payload added. - Evidence/provenance/cleanup: unchanged; partial references stay usable; unknown work/cleanup stays unknown. No execution, installation, configuration, or target-authority expansion. - Generated metadata: authored skill 44 and tracked package metadata; ignored catalogs/skills regenerated, not committed. - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - The initialization regression failed against the unmodified compiled base, then passed after the fix. - Public CLI and stdio MCP exercise actual 1ms JavaScript timeouts, precise remediation, retained partial Evidence, continued analysis, caller bounds and provenance. Existing wire-budget/untrusted-content guards retained. - Actual standalone `codex-cli 0.161.0`, `gpt-6.1-sol`, Node 24.18.0; Commander 14.0.3, static only. Fresh processes, serial bounded-heap trials. Full calls and receipts retained locally; outcome/performance summary linked in the issue after final qualification. - Actual Codex qualification: client deadlines 4/4 actionable stops without unchanged retries; server timeouts 2/2 repaired in two calls; normal controls 2/2 one-call successes. Across all iterations 28 trials were retained. Final eight: 741,148 input tokens (587,136 cached), 4,825 output; 255.62s wall sum, 112.50s CPU, sampled peak family RSS 1,255.25 MiB. Warm/uncontrolled caches; 500ms summed RSS is not cgroup peak. Immediate cleanup samples are inconclusive where a pending refresh raced; a separate post-run audit found no task-specific process matches. - Important negative evidence retained: an initial imperative to inspect partial Evidence caused two stops with hidden readers; an initialization-only candidate still made one unchanged client retry. The final version makes inspection optional and also teaches via the advertised tool description. No claim of universal model reliability or a causal benchmark. - Two independent Codex CLI source reviews: no actionable findings; no target/provider execution by reviewers. ## Validation performed - `npm run test:focused -- tests/boundary/mcp/javascriptAnalysisResources.test.ts tests/acceptance/applications/javascriptAnalysisLimits.test.ts tests/boundary/mcp/webBundleContainmentMcp.test.ts tests/boundary/mcp/targetContentFramingMcp.test.ts src/domain/analysisErrorProjection.test.ts src/server/EvidenceMcpServer.test.ts` — 79 tests passed; the refactored resource file additionally rerun. - `npm run check` — passed (typecheck, lint, format, knip, metadata, architecture/module boundaries). - `npm run docs:generate` / `npm run docs:check` — passed. - Skill validator and `npm pack --dry-run --json` — passed. ## Compatibility, safety, and release No breaking changes or new runtime work/cache. Startup teaching costs extra context; whole-task tokens, wall/CPU time and sampled family RSS are reported, not inferred from bytes. No real Hopper/Ghidra/macOS qualification: unavailable-provider probes only test deliberate missing-install preflight; RepoPrompt research is source-only. Underlying analysis semantics are unchanged. ## Author readiness Human owner: @gensokyo-dev (task owner's explicitly selected account). The task owner explicitly directed ready-for-review submission and delegated continuation/merge, including bypass when appropriate. Automated review and executed evidence are reported above; no separate human final-diff review is asserted by this agent. - [ ] 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. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] Concrete problem, focused scope and Conventional Commit title. - [x] No unresolved new authority or capability questions. - [x] Related issue, observable output, owning docs and regression coverage included. - [x] Meaningful verification limits recorded; generated outputs and credentials excluded. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:44:33 +00:00