morluto/rea

Fork 0

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

[PR #7360] fix(cli): admit native data type selectors before opening a target #7834

New issue

Open

opened 2026-11-05 01:48:00 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-05 01:48:00 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7360
Author: @viniciusdsandrade
Created: 10/27/2026
Status: 🔄 Open

Base: main ← Head: fix/7276-native-data-type-selector-admission

📝 Commits (6)

6d9b454 fix(cli): admit native data type selectors before opening a target
44a1926 fix(cli): advertise the native data type selector choice
2312e41 test(cli): reject invalid native data type selectors before target access
a10c333 test(cli): cover native data type selector discovery
3662bd1 Merge branch 'main' of morluto/rea into fix/7276-native-data-type-selector-admission
a28a531 fix(cli): reject an empty native data type target before resolution

📊 Changes

4 files changed (+103 additions, -14 deletions)

View changed files

📝 src/cli/coreNativeCommands.ts (+39 -14)
📝 tests/boundary/cli/analysisSelectorCli.test.ts (+43 -0)
📝 tests/boundary/cli/cliInputSchemaRequired.test.ts (+20 -0)
📝 tests/boundary/cli/emptyLocalTargetAdmission.test.ts (+1 -0)

📄 Description

Summary

Fixes #7276. rea inspect-native-data-type forwarded its optional --type and --address selectors straight to runDirectAnalysis, which prepares the target before the provider validates the operation input. A request with no selector, both selectors, or an empty selector therefore reported target_unavailable for a missing or unsupported target, and for a valid target it opened the target before rejecting the request. The command now admits the canonical selector choice before opening the target, and its help names that choice like the other CLI commands with selector alternatives. It also declares its positional target with localPathStringSchema, which #7956 applied to its sibling local-file commands but not to this one, so an empty target is rejected at validation instead of resolving to the working directory.

Current main (d48e472fa) is merged into the branch, resolving the import conflict with #7723 and #7956; the resulting tree is identical to a rebase onto that commit.

Problem and expected behavior

Smallest trigger: rea inspect-native-data-type does-not-exist --format json, or the same command with --type /MyStruct --address 0x1000, --type "", or --address "".

Base behavior: code: "target_unavailable". #7276 reproduces the six malformed cases through the production CLI builder and dispatcher. By source, runAnalysis in src/application/DirectAnalysis.ts calls prepareSnapshot before session.execute, and the selector rule is enforced after that, for Ghidra by its input schema in src/ghidra/GhidraFunctionValues.ts.

Violated invariant: nativeDataTypeInputSchema in src/domain/native/nativeDataType.ts requires exactly one nonempty type or address. The CLI translations section of docs/tool-design.md requires the help to state a required choice that the CLI schema cannot express, and admission of that choice before target access.

Expected: an input error before target access, whether or not the target exists or is supported.

Change and scope

src/cli/coreNativeCommands.ts: parse the selectors with nativeDataTypeInputSchema before calling runDirectAnalysis, and return failures through analysisInputErrorFromIssues and projectAnalysisError, the admission shape that inspect-gpu-code-objects and inspect-model-artifact already use. Valid requests pass the same input object to runDirectAnalysis as before.
Same file: the command description and both option descriptions state "Supply exactly one of --type or --address.", following the instrument-with-frida script source guidance.
Same file: the positional path uses localPathStringSchema, the shared nonempty, NUL-free schema that #7956 adopted for the other local-file commands in this group. docs/cli.md already states that local inspection commands reject empty paths before resolution.
Not included: no change to the domain or Ghidra schemas, to MCP, or to runDirectAnalysis ordering for other commands. The commands already in the required selector discovery table admit their choice before target access (resolveCliAnalysisSelector and the instrument-with-frida script check).

Contract and boundary impact

Semantic owner and earliest changed stage: the selector rule stays owned by nativeDataTypeInputSchema; the CLI adapter now applies it at admission, before runDirectAnalysis.
Contribution scope fit and linked maintainer decision: not applicable; CLI admission fix within an existing command.
CLI and MCP/tool-catalog contract: CLI only. An invalid selector returns invalid_request for operation inspect_native_data_type before target access, and the command help, schema, and full manifest name the exactly-one choice. Valid requests and MCP are unchanged.
Provider, bridge, target-format, or platform compatibility: none; an invalid selector no longer reaches a provider.
Evidence, artifact, provenance, or reconstruction contract: unchanged.
Target selection, authority, upstream behavior, and cleanup details (if applicable): an invalid selector no longer opens, analyzes, or releases a target.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: no generated file is committed; the advertised change is the command and option description text.

Expected effects:

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:
- tests/boundary/cli/analysisSelectorCli.test.ts: runs the compiled CLI against does-not-exist with no selector, both selectors, --type "", and --address "", and expects invalid_request for operation inspect_native_data_type with the canonical issue. A single --type or --address still reaches the target and reports target_unavailable. On the base revision the four invalid cases report target_unavailable, so the test fails there.
- tests/boundary/cli/cliInputSchemaRequired.test.ts: adds inspect-native-data-type to the required selector discovery table, which checks the guidance in the command schema, help, and full manifest, keeps both options optional in the advertised schema, and validates representable and rejected option shapes with Ajv. On the base revision the descriptions lack the guidance, so the entry fails there.
- tests/boundary/cli/emptyLocalTargetAdmission.test.ts: adds inspect-native-data-type --type /MyStruct to the empty-target table, which expects VALIDATION_ERROR on path with too_small before runDirectAnalysis is called and admits an explicit .. On main the command still accepts the empty target, so the row fails there.
Base reproduction or other evidence: the #7276 reproduction, and the source path above (coreNativeCommands.ts, DirectAnalysis.ts runAnalysis, GhidraFunctionValues.ts).
User-visible CLI/MCP output (if applicable): the details asserted by the new test, which the compiled CLI produced in the local run below:
```
{"operation":"inspect_native_data_type","issues":[{"path":[],"reason":"invalid_value","message":"Supply exactly one of type or address"}]}
```
An empty selector reports {"path":["type"],"reason":"out_of_range","minimum":1} (or address).
Remaining proof gaps: no real provider run; none is needed for the rejected cases because they no longer reach a provider, and valid requests take the unchanged path. The base-revision failures described above are source-derived and were not re-executed on the old base.

Covered boundary regressions: missing, empty, and unknown values (omitted, empty, and conflicting selectors against a single valid selector) and public success and error delivery (the compiled CLI path and its named invalid_request contract). Lifecycle regressions: not applicable; no resource ownership or finalization changes, and invalid requests no longer acquire a target.

Validation performed

On the updated branch (merged with main at d48e472fa; Node 24.22.1, macOS arm64):

npm run test:focused -- tests/boundary/cli/analysisSelectorCli.test.ts tests/boundary/cli/cliInputSchemaRequired.test.ts tests/boundary/cli/emptyLocalTargetAdmission.test.ts: 3 files, 73 tests passed, including the compiled-CLI selector cases and the new empty-target row.
npm run check:fast: typecheck and lint passed.
Not run: the full check:pr gate and any real provider workflow.

Compatibility, safety, and release

Breaking changes or migration steps: none. A request that was already invalid now fails before target access with the canonical input error.
Real Hopper/Ghidra, browser, or OS coverage: platform-independent; no provider runs for the rejected cases.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new process, file, or network access; invalid requests now avoid target access.

Author readiness

Human owner:

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.

_{🔄 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/7360 **Author:** [@viniciusdsandrade](https://github.com/viniciusdsandrade) **Created:** 10/27/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/7276-native-data-type-selector-admission` --- ### 📝 Commits (6) - [`6d9b454`](https://github.com/morluto/rea/commit/6d9b4548b8aa066e7acbe0a34d5089c682f8b82a) fix(cli): admit native data type selectors before opening a target - [`44a1926`](https://github.com/morluto/rea/commit/44a192608a963bb0da64f3f493e9523090ac7cbe) fix(cli): advertise the native data type selector choice - [`2312e41`](https://github.com/morluto/rea/commit/2312e41447017f628a2df1ec0ba208d5888e163e) test(cli): reject invalid native data type selectors before target access - [`a10c333`](https://github.com/morluto/rea/commit/a10c3338ffaa908bfab8a0dfd22e2b1870edcc4f) test(cli): cover native data type selector discovery - [`3662bd1`](https://github.com/morluto/rea/commit/3662bd18b4b1ea004e424b6f096e18675d0c0368) Merge branch 'main' of morluto/rea into fix/7276-native-data-type-selector-admission - [`a28a531`](https://github.com/morluto/rea/commit/a28a531fdc9671b429555ccfab7c464576e17fa3) fix(cli): reject an empty native data type target before resolution ### 📊 Changes **4 files changed** (+103 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/coreNativeCommands.ts` (+39 -14) 📝 `tests/boundary/cli/analysisSelectorCli.test.ts` (+43 -0) 📝 `tests/boundary/cli/cliInputSchemaRequired.test.ts` (+20 -0) 📝 `tests/boundary/cli/emptyLocalTargetAdmission.test.ts` (+1 -0) </details> ### 📄 Description ## Summary Fixes #7276. `rea inspect-native-data-type` forwarded its optional `--type` and `--address` selectors straight to `runDirectAnalysis`, which prepares the target before the provider validates the operation input. A request with no selector, both selectors, or an empty selector therefore reported `target_unavailable` for a missing or unsupported target, and for a valid target it opened the target before rejecting the request. The command now admits the canonical selector choice before opening the target, and its help names that choice like the other CLI commands with selector alternatives. It also declares its positional target with `localPathStringSchema`, which #7956 applied to its sibling local-file commands but not to this one, so an empty target is rejected at validation instead of resolving to the working directory. Current `main` (`d48e472fa`) is merged into the branch, resolving the import conflict with #7723 and #7956; the resulting tree is identical to a rebase onto that commit. ## Problem and expected behavior Smallest trigger: `rea inspect-native-data-type does-not-exist --format json`, or the same command with `--type /MyStruct --address 0x1000`, `--type ""`, or `--address ""`. Base behavior: `code: "target_unavailable"`. #7276 reproduces the six malformed cases through the production CLI builder and dispatcher. By source, `runAnalysis` in `src/application/DirectAnalysis.ts` calls `prepareSnapshot` before `session.execute`, and the selector rule is enforced after that, for Ghidra by its input schema in `src/ghidra/GhidraFunctionValues.ts`. Violated invariant: `nativeDataTypeInputSchema` in `src/domain/native/nativeDataType.ts` requires exactly one nonempty `type` or `address`. The CLI translations section of `docs/tool-design.md` requires the help to state a required choice that the CLI schema cannot express, and admission of that choice before target access. Expected: an input error before target access, whether or not the target exists or is supported. ## Change and scope - `src/cli/coreNativeCommands.ts`: parse the selectors with `nativeDataTypeInputSchema` before calling `runDirectAnalysis`, and return failures through `analysisInputErrorFromIssues` and `projectAnalysisError`, the admission shape that `inspect-gpu-code-objects` and `inspect-model-artifact` already use. Valid requests pass the same input object to `runDirectAnalysis` as before. - Same file: the command description and both option descriptions state "Supply exactly one of --type or --address.", following the `instrument-with-frida` script source guidance. - Same file: the positional `path` uses `localPathStringSchema`, the shared nonempty, NUL-free schema that #7956 adopted for the other local-file commands in this group. `docs/cli.md` already states that local inspection commands reject empty paths before resolution. - Not included: no change to the domain or Ghidra schemas, to MCP, or to `runDirectAnalysis` ordering for other commands. The commands already in the required selector discovery table admit their choice before target access (`resolveCliAnalysisSelector` and the `instrument-with-frida` script check). ## Contract and boundary impact - Semantic owner and earliest changed stage: the selector rule stays owned by `nativeDataTypeInputSchema`; the CLI adapter now applies it at admission, before `runDirectAnalysis`. - Contribution scope fit and linked maintainer decision: not applicable; CLI admission fix within an existing command. - CLI and MCP/tool-catalog contract: CLI only. An invalid selector returns `invalid_request` for operation `inspect_native_data_type` before target access, and the command help, schema, and full manifest name the exactly-one choice. Valid requests and MCP are unchanged. - Provider, bridge, target-format, or platform compatibility: none; an invalid selector no longer reaches a provider. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Target selection, authority, upstream behavior, and cleanup details (if applicable): an invalid selector no longer opens, analyzes, or releases a target. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: no generated file is committed; the advertised change is the command and option description text. Expected effects: - [ ] 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: - `tests/boundary/cli/analysisSelectorCli.test.ts`: runs the compiled CLI against `does-not-exist` with no selector, both selectors, `--type ""`, and `--address ""`, and expects `invalid_request` for operation `inspect_native_data_type` with the canonical issue. A single `--type` or `--address` still reaches the target and reports `target_unavailable`. On the base revision the four invalid cases report `target_unavailable`, so the test fails there. - `tests/boundary/cli/cliInputSchemaRequired.test.ts`: adds `inspect-native-data-type` to the required selector discovery table, which checks the guidance in the command schema, help, and full manifest, keeps both options optional in the advertised schema, and validates representable and rejected option shapes with Ajv. On the base revision the descriptions lack the guidance, so the entry fails there. - `tests/boundary/cli/emptyLocalTargetAdmission.test.ts`: adds `inspect-native-data-type --type /MyStruct` to the empty-target table, which expects `VALIDATION_ERROR` on `path` with `too_small` before `runDirectAnalysis` is called and admits an explicit `.`. On `main` the command still accepts the empty target, so the row fails there. - Base reproduction or other evidence: the #7276 reproduction, and the source path above (`coreNativeCommands.ts`, `DirectAnalysis.ts` `runAnalysis`, `GhidraFunctionValues.ts`). - User-visible CLI/MCP output (if applicable): the `details` asserted by the new test, which the compiled CLI produced in the local run below: ```json {"operation":"inspect_native_data_type","issues":[{"path":[],"reason":"invalid_value","message":"Supply exactly one of type or address"}]} ``` An empty selector reports `{"path":["type"],"reason":"out_of_range","minimum":1}` (or `address`). - Remaining proof gaps: no real provider run; none is needed for the rejected cases because they no longer reach a provider, and valid requests take the unchanged path. The base-revision failures described above are source-derived and were not re-executed on the old base. Covered [boundary regressions](https://github.com/morluto/rea/blob/main/docs/testing.md#boundary-regressions): missing, empty, and unknown values (omitted, empty, and conflicting selectors against a single valid selector) and public success and error delivery (the compiled CLI path and its named `invalid_request` contract). [Lifecycle regressions](https://github.com/morluto/rea/blob/main/docs/testing.md#lifecycle-regressions): not applicable; no resource ownership or finalization changes, and invalid requests no longer acquire a target. ## Validation performed On the updated branch (merged with `main` at `d48e472fa`; Node 24.22.1, macOS arm64): - `npm run test:focused -- tests/boundary/cli/analysisSelectorCli.test.ts tests/boundary/cli/cliInputSchemaRequired.test.ts tests/boundary/cli/emptyLocalTargetAdmission.test.ts`: 3 files, 73 tests passed, including the compiled-CLI selector cases and the new empty-target row. - `npm run check:fast`: typecheck and lint passed. - Not run: the full `check:pr` gate and any real provider workflow. ## Compatibility, safety, and release - Breaking changes or migration steps: none. A request that was already invalid now fails before target access with the canonical input error. - Real Hopper/Ghidra, browser, or OS coverage: platform-independent; no provider runs for the rejected cases. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new process, file, or network access; invalid requests now avoid target access. ## Author readiness Human owner:  - [ ] 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. --- <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-11-05 01:48:00 +00:00

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

No description provided.

Rows
Columns