[PR #6008] [MERGED] fix(firmware): keep a missing producer report distinct after exit 0 #7115

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6008
Author: @kaoru0822-kitauji
Created: 10/21/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/5876-binwalk-missing-report


📝 Commits (2)

  • c733381 fix(firmware): keep a missing producer report distinct after exit 0
  • b735b40 fix(firmware): keep missing-report recovery from replaying removed paths

📊 Changes

5 files changed (+210 additions, -51 deletions)

View changed files

📝 docs/firmware-analysis.md (+3 -0)
📝 src/firmware/FirmwareCommand.ts (+36 -21)
📝 src/firmware/FirmwareFiles.ts (+49 -10)
📝 tests/boundary/firmware/firmwareIntegration.test.ts (+46 -20)
📝 tests/boundary/firmware/firmwareReportFiles.test.ts (+76 -0)

📄 Description

What broke: A firmware engine (observed: real Binwalk 3.1.0) can exit 0 without writing its requested report; REA returned a generic unreadable_output whose message told the caller to run rea doctor, with stdout/stderr folded into a free-text reason.
Fix: The report owner (FirmwareFileAccess.report) now records the observed report_state (missing, empty, invalid_json, oversized, not_regular_file); readFirmwareCommandReport keeps it with the command, args, limiter, exit status, signal and captured output, and gives report-specific recovery (recorded args are provenance; their private-workspace paths are removed, so recovery says to replace them rather than replay). No automatic retry, no empty result.
Risk: Low. Same error code (unreadable_output, not retryable) and success path; details.reason text changes and stdout/stderr move into details.diagnostics.

Summary

Fixes #5876. Before: Binwalk exiting 0 with no --log report gave unreadable_output with the reason Tool did not produce the expected complete report: <path>; stdout (0 bytes): ; stderr (0 bytes): and generic rea doctor advice. After: the failure says the engine exited 0 without a usable report, names the report state, keeps the command facts in structured diagnostics, and gives recovery steps for this report failure. A successful exit status is still not treated as an empty or complete scan.

Problem and expected behavior

Trigger: the producer exits 0 and the requested report is absent (observed intermittently with Binwalk 3.1.0 in the issue), empty, or malformed. Invariant: exit 0 without a complete report is not scan Evidence, and the failure must stay explicit. The caller should see what was observed (report state, exit status, output) and checks for this case, not generic doctor advice presented as a repair.

Change and scope

  • src/firmware/FirmwareFiles.ts: the report reader classifies its own refusals with diagnostics.report_state. It adds missing (ENOENT at open) and empty (0 bytes, which used to fall into the JSON parse error). Its existing oversized, invalid-JSON and non-regular-file refusals now carry their states too.
  • src/firmware/FirmwareCommand.ts: readFirmwareCommandReport combines the reader's facts with the execution facts: report_path, report_failure, command, args, resource_limiter, exit_code, signal, stdout/stderr and byte counts. These are the same execution facts a successful run keeps in raw_result. The accepted Unblob exit-1 path keeps ProviderAdapterError and the same diagnostic fields. The exit-0 path is an AnalysisOutputError with its own userMessage and remediationAction. Removed: the stdout/stderr text folded into reason.
  • Not included: root cause of Binwalk's missing report (still unknown per the issue), automatic retry, and any change to resource limits or report validation.
  • Contribution scope: ordinary fix within an existing boundary; maintainer scope decision not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: FirmwareFileAccess.report (report-state classification), then readFirmwareCommandReport (failure composition).

  • Contribution scope fit and linked maintainer decision: not applicable (failure classification fix).

  • CLI and MCP/tool-catalog contract: no schema or catalog change. The error code is still unreadable_output, retryable: false. details.reason, message and remediation.action text changed, and details.diagnostics was added.

  • Provider, bridge, target-format, or platform compatibility: unchanged (Linux-only firmware lane).

  • Evidence, artifact, provenance, or reconstruction contract: unchanged success Evidence. Failures keep the command and report facts.

  • Target selection, authority, upstream behavior, and cleanup details: unchanged. No retry and no new process launches.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added or updated:

    • tests/boundary/firmware/firmwareReportFiles.test.ts (cross-platform): an exit-0 run with a missing, empty, truncated-JSON, oversized or directory report projects unreadable_output with report_state, report path, command/args/limiter, exit and output facts, without rea doctor in the message; recovery must not present the removed private-workspace arguments as replayable.
    • tests/boundary/firmware/firmwareIntegration.test.ts (Linux): real owned subprocess workflow for both inspect_firmware_regions (Binwalk lane) and extract_firmware (Unblob lane), through the service projection and MCP structuredContent. The exit-1 startup test now asserts report_state: "missing".
  • Base reproduction: the new boundary test failed on the base revision for the original 3 states (missing/empty/invalid_json; source restored with git stash); oversized/not_regular_file cases were added after review. Real Binwalk is not installed on this host, so the runtime intermittency was not reproduced here.

  • User-visible output (from the projection asserted in tests, missing report):

    • message: binwalk exited successfully but left no usable report (Report was not created), so inspect_firmware_regions produced no Evidence. A successful exit status is not an empty or complete result.
    • details.diagnostics.report_state: "missing"
  • Remaining proof gaps: the Linux-only integration cases were not run locally (macOS host); CI ran them green on the first push. Real Binwalk 3.1.0 was not run. The issue's root cause is still NOT_ENOUGH_INFO.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:local -- src/firmware/{FirmwareCommand,FirmwareDeadline,FirmwareReports,FirmwareVersion}.test.ts src/domain/analysisErrorProjection.test.ts: 101 passed, 8 skipped (Linux-only).
  • npm run test:focused -- tests/boundary/firmware/{firmwareReportFiles,firmwareValidationProjection,firmwareWorkspaceRecovery}.test.ts tests/process-global/firmware{FileLifetime,PublicationDeadline}.test.ts tests/acceptance/tools/localToolContracts.test.ts: 48 passed, 23 skipped (Linux-only).
  • npm run test:focused -- tests/boundary/firmware/firmwareIntegration.test.ts: 36 skipped (macOS host; Linux-only).
  • After review fixes: npm run test:local -- src/firmware/FirmwareCommand.test.ts src/domain/analysisErrorProjection.test.ts — 62 passed, 8 skipped; npm run test:focused -- tests/boundary/firmware/{firmwareReportFiles,firmwareIntegration,firmwareWorkspaceRecovery}.test.ts — 11 passed, 38 skipped (Linux-only).
  • npm run check:fast: passed.
  • npm run format:check: passed.
  • npm run docs:check: passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Error code and retryability are unchanged. Consumers that parsed stdout/stderr out of details.reason should read details.diagnostics.stdout/stderr instead.
  • Real Hopper/Ghidra, browser, or OS coverage: macOS local only; Linux through CI.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: diagnostics include the configured engine path and private workspace paths, which successful raw results already expose. Captured output stays within the existing diagnostic byte budget.

Author readiness

Human owner: @kaoru0822-kitauji

  • 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.

🤖 Generated with Claude Code


🔄 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/6008 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/5876-binwalk-missing-report` --- ### 📝 Commits (2) - [`c733381`](https://github.com/morluto/rea/commit/c73338147c04a0d0392d472d6d70552ace4b7ba2) fix(firmware): keep a missing producer report distinct after exit 0 - [`b735b40`](https://github.com/morluto/rea/commit/b735b405706a017be91e6d8be38813a5ae47441f) fix(firmware): keep missing-report recovery from replaying removed paths ### 📊 Changes **5 files changed** (+210 additions, -51 deletions) <details> <summary>View changed files</summary> 📝 `docs/firmware-analysis.md` (+3 -0) 📝 `src/firmware/FirmwareCommand.ts` (+36 -21) 📝 `src/firmware/FirmwareFiles.ts` (+49 -10) 📝 `tests/boundary/firmware/firmwareIntegration.test.ts` (+46 -20) 📝 `tests/boundary/firmware/firmwareReportFiles.test.ts` (+76 -0) </details> ### 📄 Description **What broke:** A firmware engine (observed: real Binwalk 3.1.0) can exit 0 without writing its requested report; REA returned a generic `unreadable_output` whose message told the caller to run `rea doctor`, with stdout/stderr folded into a free-text reason. **Fix:** The report owner (`FirmwareFileAccess.report`) now records the observed `report_state` (`missing`, `empty`, `invalid_json`, `oversized`, `not_regular_file`); `readFirmwareCommandReport` keeps it with the command, args, limiter, exit status, signal and captured output, and gives report-specific recovery (recorded args are provenance; their private-workspace paths are removed, so recovery says to replace them rather than replay). No automatic retry, no empty result. **Risk:** Low. Same error code (`unreadable_output`, not retryable) and success path; `details.reason` text changes and stdout/stderr move into `details.diagnostics`. ## Summary Fixes #5876. Before: Binwalk exiting 0 with no `--log` report gave `unreadable_output` with the reason `Tool did not produce the expected complete report: <path>; stdout (0 bytes): ; stderr (0 bytes): ` and generic `rea doctor` advice. After: the failure says the engine exited 0 without a usable report, names the report state, keeps the command facts in structured diagnostics, and gives recovery steps for this report failure. A successful exit status is still not treated as an empty or complete scan. ## Problem and expected behavior Trigger: the producer exits 0 and the requested report is absent (observed intermittently with Binwalk 3.1.0 in the issue), empty, or malformed. Invariant: exit 0 without a complete report is not scan Evidence, and the failure must stay explicit. The caller should see what was observed (report state, exit status, output) and checks for this case, not generic doctor advice presented as a repair. ## Change and scope - `src/firmware/FirmwareFiles.ts`: the report reader classifies its own refusals with `diagnostics.report_state`. It adds `missing` (ENOENT at open) and `empty` (0 bytes, which used to fall into the JSON parse error). Its existing oversized, invalid-JSON and non-regular-file refusals now carry their states too. - `src/firmware/FirmwareCommand.ts`: `readFirmwareCommandReport` combines the reader's facts with the execution facts: `report_path`, `report_failure`, `command`, `args`, `resource_limiter`, `exit_code`, `signal`, stdout/stderr and byte counts. These are the same execution facts a successful run keeps in `raw_result`. The accepted Unblob exit-1 path keeps `ProviderAdapterError` and the same diagnostic fields. The exit-0 path is an `AnalysisOutputError` with its own `userMessage` and `remediationAction`. Removed: the stdout/stderr text folded into `reason`. - Not included: root cause of Binwalk's missing report (still unknown per the issue), automatic retry, and any change to resource limits or report validation. - Contribution scope: ordinary fix within an existing boundary; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `FirmwareFileAccess.report` (report-state classification), then `readFirmwareCommandReport` (failure composition). - Contribution scope fit and linked maintainer decision: not applicable (failure classification fix). - CLI and MCP/tool-catalog contract: no schema or catalog change. The error code is still `unreadable_output`, `retryable: false`. `details.reason`, `message` and `remediation.action` text changed, and `details.diagnostics` was added. - Provider, bridge, target-format, or platform compatibility: unchanged (Linux-only firmware lane). - Evidence, artifact, provenance, or reconstruction contract: unchanged success Evidence. Failures keep the command and report facts. - Target selection, authority, upstream behavior, and cleanup details: unchanged. No retry and no new process launches. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: - `tests/boundary/firmware/firmwareReportFiles.test.ts` (cross-platform): an exit-0 run with a missing, empty, truncated-JSON, oversized or directory report projects `unreadable_output` with `report_state`, report path, command/args/limiter, exit and output facts, without `rea doctor` in the message; recovery must not present the removed private-workspace arguments as replayable. - `tests/boundary/firmware/firmwareIntegration.test.ts` (Linux): real owned subprocess workflow for both `inspect_firmware_regions` (Binwalk lane) and `extract_firmware` (Unblob lane), through the service projection and MCP `structuredContent`. The exit-1 startup test now asserts `report_state: "missing"`. - Base reproduction: the new boundary test failed on the base revision for the original 3 states (missing/empty/invalid_json; source restored with `git stash`); oversized/not_regular_file cases were added after review. Real Binwalk is not installed on this host, so the runtime intermittency was not reproduced here. - User-visible output (from the projection asserted in tests, missing report): - message: `binwalk exited successfully but left no usable report (Report was not created), so inspect_firmware_regions produced no Evidence. A successful exit status is not an empty or complete result.` - `details.diagnostics.report_state`: `"missing"` - Remaining proof gaps: the Linux-only integration cases were not run locally (macOS host); CI ran them green on the first push. Real Binwalk 3.1.0 was not run. The issue's root cause is still NOT_ENOUGH_INFO. - [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. ## Validation performed - `npm run test:local -- src/firmware/{FirmwareCommand,FirmwareDeadline,FirmwareReports,FirmwareVersion}.test.ts src/domain/analysisErrorProjection.test.ts`: 101 passed, 8 skipped (Linux-only). - `npm run test:focused -- tests/boundary/firmware/{firmwareReportFiles,firmwareValidationProjection,firmwareWorkspaceRecovery}.test.ts tests/process-global/firmware{FileLifetime,PublicationDeadline}.test.ts tests/acceptance/tools/localToolContracts.test.ts`: 48 passed, 23 skipped (Linux-only). - `npm run test:focused -- tests/boundary/firmware/firmwareIntegration.test.ts`: 36 skipped (macOS host; Linux-only). - After review fixes: `npm run test:local -- src/firmware/FirmwareCommand.test.ts src/domain/analysisErrorProjection.test.ts` — 62 passed, 8 skipped; `npm run test:focused -- tests/boundary/firmware/{firmwareReportFiles,firmwareIntegration,firmwareWorkspaceRecovery}.test.ts` — 11 passed, 38 skipped (Linux-only). - `npm run check:fast`: passed. - `npm run format:check`: passed. - `npm run docs:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Error code and retryability are unchanged. Consumers that parsed stdout/stderr out of `details.reason` should read `details.diagnostics.stdout`/`stderr` instead. - Real Hopper/Ghidra, browser, or OS coverage: macOS local only; Linux through CI. - Package or release metadata impact: none. - Security, privacy, process, or containment review: diagnostics include the configured engine path and private workspace paths, which successful raw results already expose. Captured output stays within the existing diagnostic byte budget. ## Author readiness Human owner: @kaoru0822-kitauji - [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 - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:45:30 +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#7115
No description provided.