[PR #1987] [MERGED] fix(targets): remove path advice for format refusals #4951

Closed
opened 2026-10-31 00:57:19 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1987
Author: @Ha1baraA11
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/12/2026
Merged by: @N0zoM1z0

Base: main ← Head: pr/rea-1979


📝 Commits (4)

  • 420e956 fix(targets): remove path advice for format refusals
  • 2d2860f chore: sync main into target format guidance
  • a21d828 test(targets): group executable format refusals
  • 4fce039 chore: sync verified MIPS16 support into target diagnostics

📊 Changes

4 files changed (+91 additions, -9 deletions)

View changed files

📝 src/application/BinaryTargetResolver.ts (+19 -3)
📝 src/domain/analysisErrorPresentation.ts (+10 -2)
📝 src/domain/configurationErrors.ts (+2 -1)
📝 tests/boundary/filesystem/targetDiagnostics.test.ts (+60 -3)

📄 Description

Summary

A readable executable rejected by format parsing now reports its format reason without misleading path advice. CLI/MCP and rea doctor classify the refusal as an unsupported target; actual missing-path and permission diagnostics retain their own guidance.

Fixes #1979

Refs #1852

Problem and expected behavior

A readable MZ image with an NE, LE, or LX header is correctly refused, but the current message and remediation.action also advise checking whether the path exists and is readable. The path is valid, so that advice sends users toward the wrong cause. Format refusals should explain that the selected file is unsupported or malformed; path and access errors should retain their path-specific guidance.

Change and scope

  • Mark format-parser, raw-binary admission, and DOS COM admission failures with the typed file_format_rejected constraint after the file has been read.
  • Render the parser reason without generic path advice and give a format-specific remediation action.
  • Classify these results as unsupported_target in the shared CLI/MCP projection and rea doctor; preserve the existing directory, missing-path, permission and WASM static-inspection routes.
  • Add synthetic NE, LE, and LX fixture tests through target parsing, error projection, and Doctor diagnostics.

Contract and boundary impact

  • Semantic owner and earliest changed stage: BinaryTargetResolver identifies a readable file whose format is rejected; shared error presentation serves CLI and MCP, and Doctor reports the typed target classification.
  • CLI and MCP/tool-catalog contract: No tool or schema registration changes. code and details.reason remain stable; details.constraint gains the additive value file_format_rejected. Example: message: "Cannot open artifact: unsupported NE executable in MZ image."; remediation.action: "Review the reported file-format reason and select a supported, well-formed target.".
  • Provider, bridge, target-format, or platform compatibility: Applies to parser refusals and explicit raw-binary/DOS COM admission failures. Supported format admission and provider selection are unchanged.
  • Evidence, artifact, provenance, or reconstruction contract: No artifact bytes, identity, evidence, or provenance are changed; the original reason and selected path remain available in details.
  • Process execution, authorization, cleanup, or containment impact: None; format rejection still occurs before provider startup.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: None; no generated output or package files change.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/filesystem/targetDiagnostics.test.ts writes readable MZ images with NE, LE, and LX headers and checks the projected message, remediation, reason, constraint, and Doctor classification; existing directory and permission cases remain covered.
  • Base reproduction or other evidence: All three new format-refusal assertions failed on the exact unmodified PR base because the message and remediation included generic path advice and no typed format constraint; both existing directory and permission tests passed. On the patch, all five focused boundary tests pass.
  • User-visible CLI/MCP output (if applicable): message is Cannot open artifact: unsupported NE executable in MZ image.; remediation.action is Review the reported file-format reason and select a supported, well-formed target.; details.reason remains unsupported NE executable in MZ image.
  • Remaining proof gaps: Actual compiled CLI and fresh stdio MCP replay verified all three refusals, exact canonical projection parity, unchanged file bytes, service responsiveness and a missing-path control. A real native-provider session is unnecessary for these pre-startup refusals; Windows execution of the follow-up remains unverified.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Follow-up verification

  • Integrated current main, preserving the merged WASM recovery route and source metadata inspection.
  • Added strict shared-error category and non-retryable assertions; the NE/LE/LX category checks failed before the follow-up and passed afterward.
  • Preserved malformed-header, missing-path and real filesystem permission checks; separated the format test group to keep the existing lint gate.
  • Focused parser/projection/Doctor/raw-binary/DOS COM/WASM checks: 103 tests across nine files passed on the final integrated head.
  • Real Linux x64 public CLI/MCP replay: three NE/LE/LX controls and a missing-path control passed with exact canonical error parity and unchanged source bytes.
  • npm run check:ci passed all eight static tasks and architecture guards on the integrated head.
  • npm run docs:check passed on the final integrated head.

Validation performed

  • npm run test:focused -- tests/boundary/filesystem/targetDiagnostics.test.ts — passed (5 tests).
  • npm run check:fast — passed (typecheck and lint).

Compatibility, safety, and release

  • Breaking changes or migration steps: None; target_unavailable and details.reason remain stable, with an additive constraint value and clearer text.
  • Real Hopper/Ghidra, browser, or OS coverage: No engine or browser was started; macOS arm64 boundary tests passed.
  • Package or release metadata impact: None.
  • Security, privacy, process, or containment review: No new authority, external process, or persistent file access; regression fixtures are created in a temporary test directory.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • 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.

🔄 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/1987 **Author:** [@Ha1baraA11](https://github.com/Ha1baraA11) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `pr/rea-1979` --- ### 📝 Commits (4) - [`420e956`](https://github.com/morluto/rea/commit/420e9567f9d52ed1e55b8609ed62db06ecdf9e32) fix(targets): remove path advice for format refusals - [`2d2860f`](https://github.com/morluto/rea/commit/2d2860f6c4f781380be1e623541e236b83c51de9) chore: sync main into target format guidance - [`a21d828`](https://github.com/morluto/rea/commit/a21d828b06f5d94835110b40632ac56b99b36403) test(targets): group executable format refusals - [`4fce039`](https://github.com/morluto/rea/commit/4fce03901f4b940a0f190d052981adfa865fc7bb) chore: sync verified MIPS16 support into target diagnostics ### 📊 Changes **4 files changed** (+91 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/application/BinaryTargetResolver.ts` (+19 -3) 📝 `src/domain/analysisErrorPresentation.ts` (+10 -2) 📝 `src/domain/configurationErrors.ts` (+2 -1) 📝 `tests/boundary/filesystem/targetDiagnostics.test.ts` (+60 -3) </details> ### 📄 Description ## Summary A readable executable rejected by format parsing now reports its format reason without misleading path advice. CLI/MCP and `rea doctor` classify the refusal as an unsupported target; actual missing-path and permission diagnostics retain their own guidance. Fixes #1979 Refs #1852 ## Problem and expected behavior A readable MZ image with an NE, LE, or LX header is correctly refused, but the current `message` and `remediation.action` also advise checking whether the path exists and is readable. The path is valid, so that advice sends users toward the wrong cause. Format refusals should explain that the selected file is unsupported or malformed; path and access errors should retain their path-specific guidance. ## Change and scope - Mark format-parser, raw-binary admission, and DOS COM admission failures with the typed `file_format_rejected` constraint after the file has been read. - Render the parser reason without generic path advice and give a format-specific remediation action. - Classify these results as `unsupported_target` in the shared CLI/MCP projection and `rea doctor`; preserve the existing directory, missing-path, permission and WASM static-inspection routes. - Add synthetic NE, LE, and LX fixture tests through target parsing, error projection, and Doctor diagnostics. ## Contract and boundary impact - Semantic owner and earliest changed stage: `BinaryTargetResolver` identifies a readable file whose format is rejected; shared error presentation serves CLI and MCP, and Doctor reports the typed target classification. - CLI and MCP/tool-catalog contract: No tool or schema registration changes. `code` and `details.reason` remain stable; `details.constraint` gains the additive value `file_format_rejected`. Example: `message: "Cannot open artifact: unsupported NE executable in MZ image."`; `remediation.action: "Review the reported file-format reason and select a supported, well-formed target."`. - Provider, bridge, target-format, or platform compatibility: Applies to parser refusals and explicit raw-binary/DOS COM admission failures. Supported format admission and provider selection are unchanged. - Evidence, artifact, provenance, or reconstruction contract: No artifact bytes, identity, evidence, or provenance are changed; the original reason and selected path remain available in details. - Process execution, authorization, cleanup, or containment impact: None; format rejection still occurs before provider startup. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: None; no generated output or package files change. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/filesystem/targetDiagnostics.test.ts` writes readable MZ images with NE, LE, and LX headers and checks the projected message, remediation, reason, constraint, and Doctor classification; existing directory and permission cases remain covered. - Base reproduction or other evidence: All three new format-refusal assertions failed on the exact unmodified PR base because the message and remediation included generic path advice and no typed format constraint; both existing directory and permission tests passed. On the patch, all five focused boundary tests pass. - User-visible CLI/MCP output (if applicable): `message` is `Cannot open artifact: unsupported NE executable in MZ image.`; `remediation.action` is `Review the reported file-format reason and select a supported, well-formed target.`; `details.reason` remains `unsupported NE executable in MZ image`. - Remaining proof gaps: Actual compiled CLI and fresh stdio MCP replay verified all three refusals, exact canonical projection parity, unchanged file bytes, service responsiveness and a missing-path control. A real native-provider session is unnecessary for these pre-startup refusals; Windows execution of the follow-up remains unverified. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Follow-up verification - Integrated current main, preserving the merged WASM recovery route and source metadata inspection. - Added strict shared-error category and non-retryable assertions; the NE/LE/LX category checks failed before the follow-up and passed afterward. - Preserved malformed-header, missing-path and real filesystem permission checks; separated the format test group to keep the existing lint gate. - Focused parser/projection/Doctor/raw-binary/DOS COM/WASM checks: 103 tests across nine files passed on the final integrated head. - Real Linux x64 public CLI/MCP replay: three NE/LE/LX controls and a missing-path control passed with exact canonical error parity and unchanged source bytes. - `npm run check:ci` passed all eight static tasks and architecture guards on the integrated head. - `npm run docs:check` passed on the final integrated head. ## Validation performed - `npm run test:focused -- tests/boundary/filesystem/targetDiagnostics.test.ts` — passed (5 tests). - `npm run check:fast` — passed (typecheck and lint). ## Compatibility, safety, and release - Breaking changes or migration steps: None; `target_unavailable` and `details.reason` remain stable, with an additive constraint value and clearer text. - Real Hopper/Ghidra, browser, or OS coverage: No engine or browser was started; macOS arm64 boundary tests passed. - Package or release metadata impact: None. - Security, privacy, process, or containment review: No new authority, external process, or persistent file access; regression fixtures are created in a temporary test directory. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 00:57:19 +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#4951
No description provided.