morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #6744] [CLOSED] fix(objdump): name the rejected field in inspect-with-objdump input errors #7404

New issue

Closed

opened 2026-11-01 21:47:36 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6744
Author: @ezrabrilliant
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/objdump-rejected-field

📝 Commits (1)

2c13171 fix(objdump): name the rejected field in inspect-with-objdump input errors

📊 Changes

2 files changed (+13 additions, -3 deletions)

View changed files

📝 src/application/reverse/ReverseEngineeringService.ts (+8 -1)
📝 tests/composition/objdumpSelectedDisassembly.test.ts (+5 -2)

📄 Description

Summary

Fixes #6473.

inspect-with-objdump rejects a selector that its operation does not accept (for example symbols --symbol main). The rejection carried no issues, but its recovery told callers to "correct the listed fields". It now lists symbol as an unknown_argument.

Problem and expected behavior

ReverseEngineeringService.inspectWithObjdump turned a failed objdumpInputSchema parse into new AnalysisInputError("inspect_with_objdump"), with no issues and no cause. The shared input-issue projector already maps this schema failure to {"path":["symbol"],"reason":"unknown_argument"}, and the sibling resolveRizinCommandRequest in the same file passes its issues through analysisInputErrorFromIssues. Expected behavior: keep the rejection, and identify the rejected field before any artifact read or objdump launch.

Change and scope

src/application/reverse/ReverseEngineeringService.ts: build the objdump input error with analysisInputErrorFromIssues(operation, parsed.error.issues, rawInput, { cause }), the same helper and arguments the Rizin path uses. The helper was already imported.
tests/composition/objdumpSelectedDisassembly.test.ts: the existing rejection test, which covers all seven operations that do not accept symbol, now also asserts issues: [{ path: ["symbol"], reason: "unknown_argument" }].

Not included: src/cutter/CutterBridgeService.ts:149 has the same pattern (a failed schema parse returned as a bare AnalysisInputError("cutter_command")). It is a separate provider, so I left it for its own issue or PR rather than widening this fix.

Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision required.

Contract and boundary impact

Semantic owner and earliest changed stage: inspectWithObjdump input validation in the shared application service, before artifact admission.
Contribution scope fit and linked maintainer decision: not applicable (ordinary fix).
CLI and MCP/tool-catalog contract: the existing invalid_request error now includes details.issues. Code, message, exit status, and retryability are unchanged. No schema or catalog change.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. The rejection still happens before reading the artifact or launching objdump, and the test still asserts that digest and run are never called.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: rejects and names symbol selection for %s before reading or launching (7 cases) in tests/composition/objdumpSelectedDisassembly.test.ts.
Base reproduction: with only the test change applied on base bc411c24b, all 7 cases failed with "issues": [] where [{ path: ["symbol"], reason: "unknown_argument" }] was expected. The other 14 tests in the file passed.

User-visible CLI output after the fix, from the issue's reproduction (inspect-with-objdump <fixture> symbols --symbol main --json --full-output; fields extracted, exit 1, empty stderr):

{
 "ok": false,
 "code": "invalid_request",
 "message": "Analysis input is invalid. Check the arguments and try again.",
 "details": {
  "operation": "inspect_with_objdump",
  "issues": [{ "path": ["symbol"], "reason": "unknown_argument" }]
 },
 "retryable": false
}

Remaining proof gaps: not run on Linux or macOS locally (CI covers Linux). No real objdump run is needed, because the request is rejected before provider admission.

For evidence-bearing changes: not applicable.

Validation performed

All runs on Windows 11 x64 with Node v24.22.1 (the .nvmrc pin), from a source checkout with native/windows/build/ restored from the rea-agents@6.7.0 tarball (ignored files, not committed). Builds used GOMAXPROCS=2 GOMEMLIMIT=2GiB (both passed through by Turbo) because the host was low on commit memory.

npm run test:local -- tests/composition/objdumpSelectedDisassembly.test.ts: 7 failed and 14 passed before the fix; 21/21 passed after.
npm run test:focused -- on the 13 test files that exercise objdump (contracts, CLI and MCP boundaries, composition): 13 files, 222/222 passed.
npm run check:fast (typecheck, lint, module boundaries): passed, 2/2 Turbo tasks.
The issue's CLI reproduction: output as shown above.

Compatibility, safety, and release

Breaking changes or migration steps: none. The error gains details.issues; existing fields are unchanged.
Real Hopper/Ghidra, browser, or OS coverage: Windows 11 x64; no provider needed.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none. rawInput is passed to the shared projector exactly as on the Rizin path.

Author readiness

Human owner: @ezrabrilliant

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/6744 **Author:** [@ezrabrilliant](https://github.com/ezrabrilliant) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/objdump-rejected-field` --- ### 📝 Commits (1) - [`2c13171`](https://github.com/morluto/rea/commit/2c13171ce58016ce32ff63ba9ea10c264a01419a) fix(objdump): name the rejected field in inspect-with-objdump input errors ### 📊 Changes **2 files changed** (+13 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/application/reverse/ReverseEngineeringService.ts` (+8 -1) 📝 `tests/composition/objdumpSelectedDisassembly.test.ts` (+5 -2) </details> ### 📄 Description ## Summary Fixes #6473. `inspect-with-objdump` rejects a selector that its operation does not accept (for example `symbols --symbol main`). The rejection carried no issues, but its recovery told callers to "correct the listed fields". It now lists `symbol` as an `unknown_argument`. ## Problem and expected behavior `ReverseEngineeringService.inspectWithObjdump` turned a failed `objdumpInputSchema` parse into `new AnalysisInputError("inspect_with_objdump")`, with no issues and no cause. The shared input-issue projector already maps this schema failure to `{"path":["symbol"],"reason":"unknown_argument"}`, and the sibling `resolveRizinCommandRequest` in the same file passes its issues through `analysisInputErrorFromIssues`. Expected behavior: keep the rejection, and identify the rejected field before any artifact read or objdump launch. ## Change and scope - `src/application/reverse/ReverseEngineeringService.ts`: build the objdump input error with `analysisInputErrorFromIssues(operation, parsed.error.issues, rawInput, { cause })`, the same helper and arguments the Rizin path uses. The helper was already imported. - `tests/composition/objdumpSelectedDisassembly.test.ts`: the existing rejection test, which covers all seven operations that do not accept `symbol`, now also asserts `issues: [{ path: ["symbol"], reason: "unknown_argument" }]`. Not included: `src/cutter/CutterBridgeService.ts:149` has the same pattern (a failed schema parse returned as a bare `AnalysisInputError("cutter_command")`). It is a separate provider, so I left it for its own issue or PR rather than widening this fix. Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision required. ## Contract and boundary impact - Semantic owner and earliest changed stage: `inspectWithObjdump` input validation in the shared application service, before artifact admission. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix). - CLI and MCP/tool-catalog contract: the existing `invalid_request` error now includes `details.issues`. Code, message, exit status, and retryability are unchanged. No schema or catalog change. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. The rejection still happens before reading the artifact or launching objdump, and the test still asserts that `digest` and `run` are never called. - Generated metadata, package, or installation impact: none. - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [x] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `rejects and names symbol selection for %s before reading or launching` (7 cases) in `tests/composition/objdumpSelectedDisassembly.test.ts`. - Base reproduction: with only the test change applied on base `bc411c24b`, all 7 cases failed with `"issues": []` where `[{ path: ["symbol"], reason: "unknown_argument" }]` was expected. The other 14 tests in the file passed. - User-visible CLI output after the fix, from the issue's reproduction (`inspect-with-objdump <fixture> symbols --symbol main --json --full-output`; fields extracted, exit 1, empty stderr): ```json { "ok": false, "code": "invalid_request", "message": "Analysis input is invalid. Check the arguments and try again.", "details": { "operation": "inspect_with_objdump", "issues": [{ "path": ["symbol"], "reason": "unknown_argument" }] }, "retryable": false } ``` - Remaining proof gaps: not run on Linux or macOS locally (CI covers Linux). No real objdump run is needed, because the request is rejected before provider admission. For evidence-bearing changes: not applicable. ## Validation performed All runs on Windows 11 x64 with Node v24.22.1 (the `.nvmrc` pin), from a source checkout with `native/windows/build/` restored from the `rea-agents@6.7.0` tarball (ignored files, not committed). Builds used `GOMAXPROCS=2 GOMEMLIMIT=2GiB` (both passed through by Turbo) because the host was low on commit memory. - `npm run test:local -- tests/composition/objdumpSelectedDisassembly.test.ts`: 7 failed and 14 passed before the fix; 21/21 passed after. - `npm run test:focused --` on the 13 test files that exercise objdump (contracts, CLI and MCP boundaries, composition): 13 files, 222/222 passed. - `npm run check:fast` (typecheck, lint, module boundaries): passed, 2/2 Turbo tasks. - The issue's CLI reproduction: output as shown above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The error gains `details.issues`; existing fields are unchanged. - Real Hopper/Ghidra, browser, or OS coverage: Windows 11 x64; no provider needed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. `rawInput` is passed to the shared projector exactly as on the Rizin path. ## Author readiness Human owner: @ezrabrilliant - [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 - [ ] 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](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:47:36 +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#7404

No description provided.

Rows
Columns