[PR #4829] fix(errors): separate recovery actions from failure messages #6433

New issue

Open

opened 2026-10-31 01:14:25 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:25 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4829
Author: @prabhuramkarki
Created: 10/17/2026
Status: 🔄 Open

Base: main ← Head: fix/actionable-error-remediation

📝 Commits (3)

8f8242f fix(errors): separate recovery actions from failure messages
61fe76b fix(errors): preserve producer recovery guidance
373c3b7 test(errors): preserve retained cleanup recovery guidance

📊 Changes

5 files changed (+86 additions, -4 deletions)

View changed files

📝 src/domain/analysisErrorCleanup.test.ts (+10 -0)
📝 src/domain/analysisErrorPresentation.ts (+10 -0)
📝 src/domain/analysisErrorProjection.test.ts (+57 -0)
📝 tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts (+1 -1)
📝 tests/boundary/providers/kuna/kunaProvider.test.ts (+8 -3)

📄 Description

Summary

Addresses the default error-remediation portion of #4815, item 8. Protocol/output failures, adapter failures, cancellations, and timeouts now return an imperative default recovery action instead of repeating their failure message. Producer-authored messages and explicit remediation retain their existing precedence.

Problem and expected behavior

A plain AnalysisOutputError previously repeated “Analysis returned an unreadable result…” in remediation.action. Its default action now starts “Run rea doctor…”. Custom Ghidra and Kuna messages still direct callers to reopen the target when the current provider or executable identity is lost.

Change and scope

Reuse the error-tag groups in the shared domain presentation owner. Apply generic defaults only after explicit remediation, specialized handlers, and custom producer messages. This preserves existing producer guidance without provider-specific repairs or text parsing. Update the MCP batch-decompile expectation and add projection and Kuna boundary regressions.

Synced the branch with upstream main 39275420e, which includes the existing French README formatting and composed Electron cleanup assertion fixes from #4824.

Contract and boundary impact

Semantic owner: analysisErrorRemediationAction; shared CLI/MCP and doctor consumers receive the same defaults.
Contribution scope: ordinary usability/correctness fix within existing boundaries; new-capability maintainer decision not applicable.
Changes recovery wording for seven default error classes. Schemas, codes, messages, retryability, diagnostic details, authority, and target side effects are unchanged.
Provider-specific messages and explicit remediation remain verbatim.

Expected effects:

Documentation or usability only; no target interaction.

Evidence and regression coverage

Original default-action controls failed before the default change.
The earlier PR head reproduced two Ghidra recovery regressions. Review also identified Kuna identity-change recovery supplied only through a custom message.
Before the preservation guard, new adapter/output/timeout custom-message controls and a Kuna executable-replacement boundary assertion failed. Existing Ghidra lost-provider assertions also failed.
With the shared guard, all 258 focused tests passed across Ghidra behavior, domain projections, projection conformance, MCP batches, Electron launch rejections, and Kuna provider boundaries.
Cleanup review follow-up: ProviderCleanupError inherits the adapter tag but supplies its own cleanup-specific message. The same shared guard preserves that message before the generic adapter default. A regression verifies cleanup_incomplete, non-retryability, retained resource ownership, and unchanged cleanup advice; it failed when the guard was removed and passed when restored. No additional producer repair or cleanup remains for this finding.
Independent final source review found no remaining findings.

Validation performed

Node 24.18.0 / npm 11.16.0 through npm exec:

npm run test:focused -- src/ghidra/GhidraProvider.behavior.test.ts src/domain/analysisErrorProjection.test.ts tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts tests/conformance/contracts/analysisErrorProjection.test.ts tests/boundary/process/electronLaunchRejections.test.ts tests/boundary/providers/kuna/kunaProvider.test.ts — 258 passed; runtime artifacts prepared.
npm run check — passed, including typecheck, compilation, lint/module boundaries, repository formatting, knip, metadata, and architecture guards.
npm run test:local -- src/domain/analysisErrorCleanup.test.ts src/domain/analysisErrorProjection.test.ts — 82 passed after adding the cleanup review regression.
git diff --check — passed.
npm run test:fast — all 643 source test files passed at production-fix head 61fe76b46: 10,532 tests passed and 8 skipped.

Compatibility, safety, and release

No schema migration, new dependencies, target side effects, or release metadata changes. Ghidra and Kuna provider boundaries used simulated providers; no native Ghidra/Kuna target session or Windows qualification was performed. The complete built/portable repository suite and performance profiling were not run.

GitHub validation status

Upstream CI and readiness are checked separately from these local results. This contributor account cannot approve fork workflow runs or rerun upstream readiness workflows (HTTP 403: repository admin rights required). Upstream CI is action_required with no jobs executed. Readiness remains pending because GitHub reports no upstream commit-to-PR association for the fork head. These statuses must pass before merge; local validation does not replace them.

Author readiness

Human owner: @prabhuramkarki

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.

Keep this AI-assisted PR in draft until the human owner completes these declarations.

_{🔄 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/4829 **Author:** [@prabhuramkarki](https://github.com/prabhuramkarki) **Created:** 10/17/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/actionable-error-remediation` --- ### 📝 Commits (3) - [`8f8242f`](https://github.com/morluto/rea/commit/8f8242f48fd2f826fa39889ebe9ba8d4335da787) fix(errors): separate recovery actions from failure messages - [`61fe76b`](https://github.com/morluto/rea/commit/61fe76b461cea8e52c18448a25ff18d4f2479e46) fix(errors): preserve producer recovery guidance - [`373c3b7`](https://github.com/morluto/rea/commit/373c3b72aae64440b469bb9056595737b618baf9) test(errors): preserve retained cleanup recovery guidance ### 📊 Changes **5 files changed** (+86 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/analysisErrorCleanup.test.ts` (+10 -0) 📝 `src/domain/analysisErrorPresentation.ts` (+10 -0) 📝 `src/domain/analysisErrorProjection.test.ts` (+57 -0) 📝 `tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts` (+1 -1) 📝 `tests/boundary/providers/kuna/kunaProvider.test.ts` (+8 -3) </details> ### 📄 Description ## Summary Addresses the default error-remediation portion of #4815, item 8. Protocol/output failures, adapter failures, cancellations, and timeouts now return an imperative default recovery action instead of repeating their failure message. Producer-authored messages and explicit remediation retain their existing precedence. ## Problem and expected behavior A plain `AnalysisOutputError` previously repeated “Analysis returned an unreadable result…” in `remediation.action`. Its default action now starts “Run `rea doctor`…”. Custom Ghidra and Kuna messages still direct callers to reopen the target when the current provider or executable identity is lost. ## Change and scope Reuse the error-tag groups in the shared domain presentation owner. Apply generic defaults only after explicit remediation, specialized handlers, and custom producer messages. This preserves existing producer guidance without provider-specific repairs or text parsing. Update the MCP batch-decompile expectation and add projection and Kuna boundary regressions. Synced the branch with upstream main `39275420e`, which includes the existing French README formatting and composed Electron cleanup assertion fixes from #4824. ## Contract and boundary impact - Semantic owner: `analysisErrorRemediationAction`; shared CLI/MCP and doctor consumers receive the same defaults. - Contribution scope: ordinary usability/correctness fix within existing boundaries; new-capability maintainer decision not applicable. - Changes recovery wording for seven default error classes. Schemas, codes, messages, retryability, diagnostic details, authority, and target side effects are unchanged. - Provider-specific messages and explicit remediation remain verbatim. Expected effects: - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - Original default-action controls failed before the default change. - The earlier PR head reproduced two Ghidra recovery regressions. Review also identified Kuna identity-change recovery supplied only through a custom message. - Before the preservation guard, new adapter/output/timeout custom-message controls and a Kuna executable-replacement boundary assertion failed. Existing Ghidra lost-provider assertions also failed. - With the shared guard, all 258 focused tests passed across Ghidra behavior, domain projections, projection conformance, MCP batches, Electron launch rejections, and Kuna provider boundaries. - Cleanup review follow-up: `ProviderCleanupError` inherits the adapter tag but supplies its own cleanup-specific message. The same shared guard preserves that message before the generic adapter default. A regression verifies `cleanup_incomplete`, non-retryability, retained resource ownership, and unchanged cleanup advice; it failed when the guard was removed and passed when restored. No additional producer repair or cleanup remains for this finding. - Independent final source review found no remaining findings. ## Validation performed Node 24.18.0 / npm 11.16.0 through `npm exec`: - `npm run test:focused -- src/ghidra/GhidraProvider.behavior.test.ts src/domain/analysisErrorProjection.test.ts tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts tests/conformance/contracts/analysisErrorProjection.test.ts tests/boundary/process/electronLaunchRejections.test.ts tests/boundary/providers/kuna/kunaProvider.test.ts` — 258 passed; runtime artifacts prepared. - `npm run check` — passed, including typecheck, compilation, lint/module boundaries, repository formatting, knip, metadata, and architecture guards. - `npm run test:local -- src/domain/analysisErrorCleanup.test.ts src/domain/analysisErrorProjection.test.ts` — 82 passed after adding the cleanup review regression. - `git diff --check` — passed. - `npm run test:fast` — all 643 source test files passed at production-fix head `61fe76b46`: 10,532 tests passed and 8 skipped. ## Compatibility, safety, and release No schema migration, new dependencies, target side effects, or release metadata changes. Ghidra and Kuna provider boundaries used simulated providers; no native Ghidra/Kuna target session or Windows qualification was performed. The complete built/portable repository suite and performance profiling were not run. ## GitHub validation status Upstream CI and readiness are checked separately from these local results. This contributor account cannot approve fork workflow runs or rerun upstream readiness workflows (HTTP 403: repository admin rights required). Upstream CI is `action_required` with no jobs executed. Readiness remains pending because GitHub reports no upstream commit-to-PR association for the fork head. These statuses must pass before merge; local validation does not replace them. ## Author readiness Human owner: @prabhuramkarki - [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. Keep this AI-assisted PR in draft until the human owner completes these declarations. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:14:25 +00:00

bsakaguchi referenced this issue

2026-11-01 21:47:41 +00:00

[PR #6784] [CLOSED] fix(pcap): refuse a directory-selected tshark as a tool configuration error #7413