morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2004] [MERGED] fix(targets): project invalid raw-binary loads as caller input #4960

New issue

Closed

opened 2026-10-31 00:57:24 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:57:24 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/raw-binary-load-input-classification

📝 Commits (2)

21cc7c5 fix(targets): project invalid raw-binary loads as caller input
6deebe0 fix(targets): name real load options and cover DOS COM kind pairing

📊 Changes

8 files changed (+230 additions, -44 deletions)

View changed files

📝 src/application/BinaryTargetResolver.ts (+17 -10)
📝 src/domain/analysisErrorPresentation.ts (+12 -1)
📝 src/domain/analysisErrorProjection.ts (+5 -3)
📝 src/domain/configurationErrors.ts (+2 -1)
📝 src/domain/rawBinary.test.ts (+33 -11)
📝 src/domain/rawBinary.ts (+39 -15)
📝 tests/boundary/cli/rawBinaryCli.test.ts (+2 -2)
📝 tests/boundary/filesystem/targetDiagnostics.test.ts (+120 -1)

📄 Description

Summary

Fixes #2002. Invalid caller-supplied raw-binary load choices (--target-format/--architecture/--entry-point) were projected as problems with the target file: target_unavailable/unsupported_target (or unavailable) with remediation that told the caller to review the file. They now project as caller input — invalid_request/invalid_input, retryable, with remediation naming the load options by their real spellings on both surfaces. An empty file remains a target refusal, and the DOS COM target-kind pairing is reclassified with the same caller-input treatment.

Problem and expected behavior

Smallest trigger: rea analyze f.bin --target-format raw-binary --architecture arm --entry-point 0x1 on any readable non-empty file reported unsupported_target with "Review the reported file-format reason and select a supported, well-formed target." — the file was fine; only the entry alignment was wrong.
Violated invariant: identity and failure classification belong to the producer. BinaryTargetResolver labeled every admitRawBinary rejection file_format_rejected, although only the empty-file extent is a property of the target; the format-pairing rejection carried no constraint at all and fell to the generic "Check that the path exists…" advice.
New behavior: caller-input rejections (missing architecture, malformed/misaligned/out-of-width entry, format pairing, target-kind pairing for both raw-binary and DOS COM) carry a new load_selection_rejected constraint and project as invalid_request/invalid_input with "Correct the reported load argument (--target-format, --architecture, or --entry-point on the CLI; format, architecture, or entry_point through MCP) and retry."; the empty-file extent keeps file_format_rejected → target_unavailable/unsupported_target.

Change and scope

src/domain/rawBinary.ts: admitRawBinary rejections are now structured (reason + owning classification: load_selection_rejected for caller input, file_format_rejected for the empty-file target refusal), so the classification is derived at the owner instead of being re-inferred downstream.
src/application/BinaryTargetResolver.ts: raw-binary rejections carry the admission's constraint; both target-kind pairings (raw-binary and DOS COM) use it too, so the analogous DOS COM conflict no longer advises selecting a different file; the architecture/entry pairing now reuses rawBinarySelectionError (its existing message owner, already used by the MCP contract layer) instead of a duplicated inline string.
src/domain/analysisErrorPresentation.ts + analysisErrorProjection.ts: load_selection_rejected maps to category invalid_input, code invalid_request, its own remediation (option names verified against src/cli/options.ts and src/contracts/sessionLifecycleInputs.ts, following the cross-adapter naming the WASM remediation uses), and drops the path-checking advice from the user message.
The constraint is named load_selection_rejected rather than raw-binary-specific so one value covers both headerless formats' caller-input rejections honestly.
Not included: DOS COM length validation keeps file_format_rejected (it is a property of the file); Doctor's inspectTarget passes no load arguments and cannot observe these rejections, so it is unchanged.

Contract and boundary impact

Semantic owner and earliest changed stage: raw-binary admission classification in src/domain/rawBinary.ts, consumed by target resolution before any provider selection.
CLI and MCP/tool-catalog contract: no tool, parameter, or schema change. invalid_request/invalid_input are existing projection values; the new load_selection_rejected string appears in details.constraint (a free-form detail field).
Provider, bridge, target-format, or platform compatibility: none; admission happens before provider selection.
Evidence, artifact, provenance, or reconstruction contract: none.
Process execution, authorization, cleanup, or containment impact: none.
Generated metadata, package, or installation impact: none (docs:check green; no code enumeration changed).

Evidence and regression coverage

Tests added or updated: src/domain/rawBinary.test.ts asserts the owning classification for the unaligned, width-overflow, malformed/missing-argument, and empty-file rejections; tests/boundary/filesystem/targetDiagnostics.test.ts projects the misaligned arm entry, the x86 width overflow, the pairing-without-format, the raw-binary and DOS COM target-kind pairings, and the empty file through the public parseBinaryTarget → projectAnalysisError boundary, including message text, remediation, and absence of path advice; tests/boundary/cli/rawBinaryCli.test.ts now pins invalid_request for the missing-architecture and unaligned-arm64 cases through the real CLI.
Base reproduction: with the source changes stashed, the new filesystem-boundary tests fail on e19879328 (they receive target_unavailable/unsupported_target/unavailable); the empty-file test passes on base by design, pinning the behavior that must not change. The CLI boundary expectations fail on base with the new code and pass with it.

Real public CLI before/after on a 64-byte fixture (macOS 27 arm64, Node 24.18.0), matching the issue's table:

Invocation	Before	After
`--target-format raw-binary --architecture arm --entry-point 0x1`	`target_unavailable`/`unsupported_target`, "Review the reported file-format reason…"	`invalid_request`/`invalid_input`, retryable, remediation naming the CLI/MCP option spellings
`--target-format raw-binary --architecture x86 --entry-point 0xffffffffff`	same as above	`invalid_request`/`invalid_input`
`--architecture arm --entry-point 0x0` (no format)	`target_unavailable`/`unavailable` + "Check that the path exists…"	`invalid_request`/`invalid_input`, no path advice
empty file, valid load arguments	`target_unavailable`/`unsupported_target`	unchanged

Remaining proof gaps: MCP-path behavior is covered through the shared projection, not a separate stdio MCP session run; non-macOS hosts were not exercised (path is platform-neutral).
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:focused -- src/domain/rawBinary.test.ts tests/boundary/filesystem/targetDiagnostics.test.ts tests/boundary/cli/rawBinaryCli.test.ts — 41 passed (including real-CLI boundary runs)
npm run check:fast — green
npm run check — green (typecheck, lint, format, knip, metadata, architecture guards)
npm run docs:check — green
Real rea analyze runs before/after as tabled above
Base-failure check via git stash of the source changes

Compatibility, safety, and release

Breaking changes or migration steps: none required. The error code/category for these rejections changes from target-shaped to input-shaped, which is the requested behavior; callers keying on details.constraint === "file_format_rejected" for raw-binary argument errors (introduced in #1979, four days old) see the new load_selection_rejected value instead.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (admission precedes provider selection); real CLI on macOS 27 arm64.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none; no new subprocess, path, or write behavior.

Review checklist

This PR addresses a concrete problem, not an unscoped refactor.
The PR has one focused outcome, and the title follows type(scope): outcome.
Related issue is linked.
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 (tabled above).
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/2004 **Author:** [@jimpablo](https://github.com/jimpablo) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/raw-binary-load-input-classification` --- ### 📝 Commits (2) - [`21cc7c5`](https://github.com/morluto/rea/commit/21cc7c5b110e29e07f00f44c6198e77e96a12bad) fix(targets): project invalid raw-binary loads as caller input - [`6deebe0`](https://github.com/morluto/rea/commit/6deebe0f7a37df29144579df7b228ecd2edc82f3) fix(targets): name real load options and cover DOS COM kind pairing ### 📊 Changes **8 files changed** (+230 additions, -44 deletions) <details> <summary>View changed files</summary> 📝 `src/application/BinaryTargetResolver.ts` (+17 -10) 📝 `src/domain/analysisErrorPresentation.ts` (+12 -1) 📝 `src/domain/analysisErrorProjection.ts` (+5 -3) 📝 `src/domain/configurationErrors.ts` (+2 -1) 📝 `src/domain/rawBinary.test.ts` (+33 -11) 📝 `src/domain/rawBinary.ts` (+39 -15) 📝 `tests/boundary/cli/rawBinaryCli.test.ts` (+2 -2) 📝 `tests/boundary/filesystem/targetDiagnostics.test.ts` (+120 -1) </details> ### 📄 Description ## Summary Fixes #2002. Invalid caller-supplied raw-binary load choices (`--target-format`/`--architecture`/`--entry-point`) were projected as problems with the target file: `target_unavailable`/`unsupported_target` (or `unavailable`) with remediation that told the caller to review the file. They now project as caller input — `invalid_request`/`invalid_input`, retryable, with remediation naming the load options by their real spellings on both surfaces. An empty file remains a target refusal, and the DOS COM target-kind pairing is reclassified with the same caller-input treatment. ## Problem and expected behavior - Smallest trigger: `rea analyze f.bin --target-format raw-binary --architecture arm --entry-point 0x1` on any readable non-empty file reported `unsupported_target` with "Review the reported file-format reason and select a supported, well-formed target." — the file was fine; only the entry alignment was wrong. - Violated invariant: identity and failure classification belong to the producer. `BinaryTargetResolver` labeled every `admitRawBinary` rejection `file_format_rejected`, although only the empty-file extent is a property of the target; the format-pairing rejection carried no constraint at all and fell to the generic "Check that the path exists…" advice. - New behavior: caller-input rejections (missing architecture, malformed/misaligned/out-of-width entry, format pairing, target-kind pairing for both raw-binary and DOS COM) carry a new `load_selection_rejected` constraint and project as `invalid_request`/`invalid_input` with "Correct the reported load argument (--target-format, --architecture, or --entry-point on the CLI; format, architecture, or entry_point through MCP) and retry."; the empty-file extent keeps `file_format_rejected` → `target_unavailable`/`unsupported_target`. ## Change and scope - `src/domain/rawBinary.ts`: `admitRawBinary` rejections are now structured (`reason` + owning classification: `load_selection_rejected` for caller input, `file_format_rejected` for the empty-file target refusal), so the classification is derived at the owner instead of being re-inferred downstream. - `src/application/BinaryTargetResolver.ts`: raw-binary rejections carry the admission's constraint; both target-kind pairings (raw-binary and DOS COM) use it too, so the analogous DOS COM conflict no longer advises selecting a different file; the architecture/entry pairing now reuses `rawBinarySelectionError` (its existing message owner, already used by the MCP contract layer) instead of a duplicated inline string. - `src/domain/analysisErrorPresentation.ts` + `analysisErrorProjection.ts`: `load_selection_rejected` maps to category `invalid_input`, code `invalid_request`, its own remediation (option names verified against `src/cli/options.ts` and `src/contracts/sessionLifecycleInputs.ts`, following the cross-adapter naming the WASM remediation uses), and drops the path-checking advice from the user message. - The constraint is named `load_selection_rejected` rather than raw-binary-specific so one value covers both headerless formats' caller-input rejections honestly. - Not included: DOS COM length validation keeps `file_format_rejected` (it is a property of the file); Doctor's `inspectTarget` passes no load arguments and cannot observe these rejections, so it is unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: raw-binary admission classification in `src/domain/rawBinary.ts`, consumed by target resolution before any provider selection. - CLI and MCP/tool-catalog contract: no tool, parameter, or schema change. `invalid_request`/`invalid_input` are existing projection values; the new `load_selection_rejected` string appears in `details.constraint` (a free-form detail field). - Provider, bridge, target-format, or platform compatibility: none; admission happens before provider selection. - Evidence, artifact, provenance, or reconstruction contract: none. - Process execution, authorization, cleanup, or containment impact: none. - Generated metadata, package, or installation impact: none (`docs:check` green; no code enumeration changed). ## Evidence and regression coverage - Tests added or updated: `src/domain/rawBinary.test.ts` asserts the owning classification for the unaligned, width-overflow, malformed/missing-argument, and empty-file rejections; `tests/boundary/filesystem/targetDiagnostics.test.ts` projects the misaligned arm entry, the x86 width overflow, the pairing-without-format, the raw-binary and DOS COM target-kind pairings, and the empty file through the public `parseBinaryTarget` → `projectAnalysisError` boundary, including message text, remediation, and absence of path advice; `tests/boundary/cli/rawBinaryCli.test.ts` now pins `invalid_request` for the missing-architecture and unaligned-arm64 cases through the real CLI. - Base reproduction: with the source changes stashed, the new filesystem-boundary tests fail on `e19879328` (they receive `target_unavailable`/`unsupported_target`/`unavailable`); the empty-file test passes on base by design, pinning the behavior that must not change. The CLI boundary expectations fail on base with the new code and pass with it. - Real public CLI before/after on a 64-byte fixture (macOS 27 arm64, Node 24.18.0), matching the issue's table: | Invocation | Before | After | | --- | --- | --- | | `--target-format raw-binary --architecture arm --entry-point 0x1` | `target_unavailable`/`unsupported_target`, "Review the reported file-format reason…" | `invalid_request`/`invalid_input`, retryable, remediation naming the CLI/MCP option spellings | | `--target-format raw-binary --architecture x86 --entry-point 0xffffffffff` | same as above | `invalid_request`/`invalid_input` | | `--architecture arm --entry-point 0x0` (no format) | `target_unavailable`/`unavailable` + "Check that the path exists…" | `invalid_request`/`invalid_input`, no path advice | | empty file, valid load arguments | `target_unavailable`/`unsupported_target` | unchanged | - Remaining proof gaps: MCP-path behavior is covered through the shared projection, not a separate stdio MCP session run; non-macOS hosts were not exercised (path is platform-neutral). - [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:focused -- src/domain/rawBinary.test.ts tests/boundary/filesystem/targetDiagnostics.test.ts tests/boundary/cli/rawBinaryCli.test.ts` — 41 passed (including real-CLI boundary runs) - `npm run check:fast` — green - `npm run check` — green (typecheck, lint, format, knip, metadata, architecture guards) - `npm run docs:check` — green - Real `rea analyze` runs before/after as tabled above - Base-failure check via `git stash` of the source changes ## Compatibility, safety, and release - Breaking changes or migration steps: none required. The error code/category for these rejections changes from target-shaped to input-shaped, which is the requested behavior; callers keying on `details.constraint === "file_format_rejected"` for raw-binary argument errors (introduced in #1979, four days old) see the new `load_selection_rejected` value instead. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (admission precedes provider selection); real CLI on macOS 27 arm64. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; no new subprocess, path, or write behavior. ## Review checklist - [x] This PR addresses a concrete problem, not an unscoped refactor. - [x] The PR has one focused outcome, and the title follows `type(scope): outcome`. - [x] Related issue is linked. - [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 (tabled above). - [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:24 +00:00

closed this issue
added the
pull-request
label

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#4960

No description provided.

Rows
Columns