morluto/rea

Fork 0

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

[PR #5471] [CLOSED] fix(cli): restore documented guidance for argument validation errors #6933

New issue

Closed

opened 2026-11-01 21:44:09 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:09 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5471
Author: @dark13b
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: fix/cli-argument-validation-guidance

📝 Commits (2)

436a02e fix(cli): restore documented guidance for argument validation errors
b939fa8 test(cli): cover numeric option bounds in argument validation errors

📊 Changes

3 files changed (+59 additions, -3 deletions)

View changed files

📝 docs/user-visible-errors.csv (+1 -1)
📝 src/cli.ts (+16 -1)
📝 tests/boundary/cli/cliOutput.test.ts (+42 -1)

📄 Description

Summary

Fixes #5398

Omitted or mistyped CLI arguments returned VALIDATION_ERROR with the raw Zod issue dump serialized into message. The documented replacement in docs/user-visible-errors.csv (24.95) was missing. This restores that guidance and keeps the structured fieldErrors that callers use to correct the call.

Problem and expected behavior

Smallest trigger: rea decompile --json with no path.

Before (built checkout at aa314ab68, exit 1, empty stderr):

{
  "code": "VALIDATION_ERROR",
  "message": "Invalid input: expected string, received undefined\n\nDetails: [\n  {\n    \"expected\": \"string\",\n    \"code\": \"invalid_type\",\n    \"path\": [\n      \"path\"\n    ],\n    \"message\": \"Invalid input: expected string, received undefined\"\n  }\n]",
  "fieldErrors": [{ "code": "invalid_type", "missing": true, "path": "path", "expected": "string", "received": "", "message": "Invalid input: expected string, received undefined" }]
}

--json analyze, search x --kind bogus --json, and the default TOON output showed the same raw Details: block.

Expected (24.95): "REA could not read the command arguments. Check the named argument and its expected type, then try again.", with argument names, omission information, allowed values, and exit status 1 preserved.

Root cause: Incur's Parser.zodParse throws ValidationError with the ZodError as cause, and BaseError appends Details: ${cause.message} to the message. REA used to clean this up with sanitizeCliOutput in the dispatcher (77454dc8f). The call was dropped from scripts/rea.mjs in abc4f4768, and the function was removed as dead code in 63bf91405, so the documented copy regressed.

Change and scope

createCli registers one root middleware. Incur parses arguments inside the innermost link of the middleware chain, so the middleware catches the ValidationError and rethrows it with the documented guidance and the same fieldErrors, without the cause. This fixes the message where the error is produced instead of parsing formatted stdout again, so JSON, JSONL, YAML, TOON, Markdown, and --full-output all get the same result.

Notes:

REA has no async-generator CLI commands, so Incur's streaming-with-middleware path is not exercised by this change.
Terminal (TTY) mode still uses Incur's field-specific text ("Error: missing required argument ", "See below for usage."). It reads fieldErrors, which are unchanged.
Not included: changing the fieldErrors shape. Existing tests treat it as the structured correction contract, so it stays as is. rea mcp doctor uses its own REA-authored argument messages and is not affected.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

Semantic owner and earliest changed stage: CLI command boundary in src/cli.ts (createCli), where Incur reports argument validation.
Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable.
CLI and MCP/tool-catalog contract: CLI VALIDATION_ERROR.message now matches documented copy 24.95. code, fieldErrors, and exit status 1 are unchanged. MCP is not affected.
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): not applicable.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none. Only the 24.95 verification_result cell in docs/user-visible-errors.csv changed, because it named the removed sanitizer tests.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

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/cliOutput.test.ts. The missing-argument test now asserts the exact guidance in JSON output, and the guidance plus no Details in the default, YAML, and --full-output outputs. The enum test asserts the guidance while keeping the existing kind / "strings"|"procedures" field assertions. A numeric-bound test (inspect-desktop-application <dir> --max_entries 100001 --json, from the #5398 follow-up) asserts the guidance with max_entries / too_big / <=100000 kept in fieldErrors.
Base reproduction or other evidence: with the src/cli.ts change removed and the new tests kept, the focused suite fails 2 of 38 (preserves missing-argument coordinates through every dispatcher formatter, preserves invalid option names and enum constraints in structured diagnostics). Both received the raw Invalid input: ... / Invalid option: ... messages. With the change, 38 of 38 pass.

User-visible CLI/MCP output (if applicable):

$ rea decompile --json
{
  "code": "VALIDATION_ERROR",
  "message": "REA could not read the command arguments. Check the named argument and its expected type, then try again.",
  "fieldErrors": [
    {
      "code": "invalid_type",
      "missing": true,
      "path": "path",
      "expected": "string",
      "received": "",
      "message": "Invalid input: expected string, received undefined"
    }
  ]
}
exit=1

$ rea search x --kind bogus --json
{
  "code": "VALIDATION_ERROR",
  "message": "REA could not read the command arguments. Check the named argument and its expected type, then try again.",
  "fieldErrors": [
    {
      "code": "invalid_value",
      "missing": false,
      "path": "kind",
      "expected": "",
      "received": "",
      "message": "Invalid option: expected one of \"strings\"|\"procedures\""
    }
  ]
}
exit=1

$ rea decompile
code: VALIDATION_ERROR
message: "REA could not read the command arguments. Check the named argument and its expected type, then try again."
fieldErrors[1]{code,missing,path,expected,received,message}:
  invalid_type,true,path,string,"","Invalid input: expected string, received undefined"
exit=1

Remaining proof gaps: only Windows was run locally; Linux and macOS are left to CI.

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.

Validation performed

Windows 11 x64, Node 24.21.0, npm 11.19.0 (.nvmrc pins 24.18.0, engines allow ^24.11.0). The Windows native addon and manifest were restored from the published rea-agents@6.7.0 package, matching the checkout version, as described in native/windows/README.md.

npm run build:cached — passed
npm run test:focused -- tests/boundary/cli/cliOutput.test.ts — 38 passed. One earlier run hit the 3000 ms deadline in fails promptly across output formats without invoking the command; that path exits in validateCliOutputArguments before createCli runs, the process printed the expected INVALID_OUTPUT_FILTER output, and four later runs with the change passed 38/38.
Same focused suite with only the src/cli.ts change removed — 2 failed, 36 passed, as described above
npm run check — passed (lint 0 errors, same 1893 existing warnings as main; architecture guards passed)
npm run docs:check — passed
After adding the numeric-bound test (b939fa8ca): the new test fails with only the src/cli.ts change reverted and passes with it; npm run check passed (0 errors, 1893 warnings). The suite was 38/39 on the last run, the failure being the same 3000 ms fails promptly deadline under high CPU load; it also failed without this change under that load.
The five numeric triggers from the #5398 follow-up, run by hand on this branch: all exit 1 with empty stderr, the 24.95 guidance, no Details, and the field and bound in fieldErrors; the --max_entries 100000 control exits 0
Pre-push check:fast — passed

Compatibility, safety, and release

Breaking changes or migration steps: none. The CLI message text for VALIDATION_ERROR changes to the documented copy. A caller that matched the raw Zod text in message should read fieldErrors instead, which is unchanged.
Real Hopper/Ghidra, browser, or OS coverage: not applicable to providers. CLI checked on Windows 11 x64.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new process, file, or network behavior. The change removes validator internals from caller-visible output.

Author readiness

Human owner: @dark13b

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/5471 **Author:** [@dark13b](https://github.com/dark13b) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/cli-argument-validation-guidance` --- ### 📝 Commits (2) - [`436a02e`](https://github.com/morluto/rea/commit/436a02ee7a345fa98013e73ff12f05fdc5a054e1) fix(cli): restore documented guidance for argument validation errors - [`b939fa8`](https://github.com/morluto/rea/commit/b939fa8ca741a739ac72b6b516d502a115a0f7a5) test(cli): cover numeric option bounds in argument validation errors ### 📊 Changes **3 files changed** (+59 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `docs/user-visible-errors.csv` (+1 -1) 📝 `src/cli.ts` (+16 -1) 📝 `tests/boundary/cli/cliOutput.test.ts` (+42 -1) </details> ### 📄 Description ## Summary Fixes #5398 Omitted or mistyped CLI arguments returned `VALIDATION_ERROR` with the raw Zod issue dump serialized into `message`. The documented replacement in `docs/user-visible-errors.csv` (24.95) was missing. This restores that guidance and keeps the structured `fieldErrors` that callers use to correct the call. ## Problem and expected behavior Smallest trigger: `rea decompile --json` with no path. Before (built checkout at `aa314ab68`, exit 1, empty stderr): ```json { "code": "VALIDATION_ERROR", "message": "Invalid input: expected string, received undefined\n\nDetails: [\n {\n \"expected\": \"string\",\n \"code\": \"invalid_type\",\n \"path\": [\n \"path\"\n ],\n \"message\": \"Invalid input: expected string, received undefined\"\n }\n]", "fieldErrors": [{ "code": "invalid_type", "missing": true, "path": "path", "expected": "string", "received": "", "message": "Invalid input: expected string, received undefined" }] } ``` `--json analyze`, `search x --kind bogus --json`, and the default TOON output showed the same raw `Details:` block. Expected (24.95): "REA could not read the command arguments. Check the named argument and its expected type, then try again.", with argument names, omission information, allowed values, and exit status 1 preserved. Root cause: Incur's `Parser.zodParse` throws `ValidationError` with the `ZodError` as `cause`, and `BaseError` appends `Details: ${cause.message}` to the message. REA used to clean this up with `sanitizeCliOutput` in the dispatcher (77454dc8f). The call was dropped from `scripts/rea.mjs` in abc4f4768, and the function was removed as dead code in 63bf91405, so the documented copy regressed. ## Change and scope `createCli` registers one root middleware. Incur parses arguments inside the innermost link of the middleware chain, so the middleware catches the `ValidationError` and rethrows it with the documented guidance and the same `fieldErrors`, without the cause. This fixes the message where the error is produced instead of parsing formatted stdout again, so JSON, JSONL, YAML, TOON, Markdown, and `--full-output` all get the same result. Notes: - REA has no async-generator CLI commands, so Incur's streaming-with-middleware path is not exercised by this change. - Terminal (TTY) mode still uses Incur's field-specific text ("Error: missing required argument <path>", "See below for usage."). It reads `fieldErrors`, which are unchanged. - Not included: changing the `fieldErrors` shape. Existing tests treat it as the structured correction contract, so it stays as is. `rea mcp doctor` uses its own REA-authored argument messages and is not affected. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: CLI command boundary in `src/cli.ts` (`createCli`), where Incur reports argument validation. - Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: CLI `VALIDATION_ERROR.message` now matches documented copy 24.95. `code`, `fieldErrors`, and exit status 1 are unchanged. MCP is not affected. - 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): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Only the 24.95 `verification_result` cell in `docs/user-visible-errors.csv` changed, because it named the removed sanitizer tests. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [ ] 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/cliOutput.test.ts`. The missing-argument test now asserts the exact guidance in JSON output, and the guidance plus no `Details` in the default, YAML, and `--full-output` outputs. The enum test asserts the guidance while keeping the existing `kind` / `"strings"|"procedures"` field assertions. A numeric-bound test (`inspect-desktop-application <dir> --max_entries 100001 --json`, from the #5398 follow-up) asserts the guidance with `max_entries` / `too_big` / `<=100000` kept in `fieldErrors`. - Base reproduction or other evidence: with the `src/cli.ts` change removed and the new tests kept, the focused suite fails 2 of 38 (`preserves missing-argument coordinates through every dispatcher formatter`, `preserves invalid option names and enum constraints in structured diagnostics`). Both received the raw `Invalid input: ...` / `Invalid option: ...` messages. With the change, 38 of 38 pass. - User-visible CLI/MCP output (if applicable): ```console $ rea decompile --json { "code": "VALIDATION_ERROR", "message": "REA could not read the command arguments. Check the named argument and its expected type, then try again.", "fieldErrors": [ { "code": "invalid_type", "missing": true, "path": "path", "expected": "string", "received": "", "message": "Invalid input: expected string, received undefined" } ] } exit=1 $ rea search x --kind bogus --json { "code": "VALIDATION_ERROR", "message": "REA could not read the command arguments. Check the named argument and its expected type, then try again.", "fieldErrors": [ { "code": "invalid_value", "missing": false, "path": "kind", "expected": "", "received": "", "message": "Invalid option: expected one of \"strings\"|\"procedures\"" } ] } exit=1 $ rea decompile code: VALIDATION_ERROR message: "REA could not read the command arguments. Check the named argument and its expected type, then try again." fieldErrors[1]{code,missing,path,expected,received,message}: invalid_type,true,path,string,"","Invalid input: expected string, received undefined" exit=1 ``` - Remaining proof gaps: only Windows was run locally; Linux and macOS are left to CI. 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. ## Validation performed Windows 11 x64, Node 24.21.0, npm 11.19.0 (`.nvmrc` pins 24.18.0, engines allow ^24.11.0). The Windows native addon and manifest were restored from the published `rea-agents@6.7.0` package, matching the checkout version, as described in `native/windows/README.md`. - `npm run build:cached` — passed - `npm run test:focused -- tests/boundary/cli/cliOutput.test.ts` — 38 passed. One earlier run hit the 3000 ms deadline in `fails promptly across output formats without invoking the command`; that path exits in `validateCliOutputArguments` before `createCli` runs, the process printed the expected `INVALID_OUTPUT_FILTER` output, and four later runs with the change passed 38/38. - Same focused suite with only the `src/cli.ts` change removed — 2 failed, 36 passed, as described above - `npm run check` — passed (lint 0 errors, same 1893 existing warnings as `main`; architecture guards passed) - `npm run docs:check` — passed - After adding the numeric-bound test (b939fa8ca): the new test fails with only the `src/cli.ts` change reverted and passes with it; `npm run check` passed (0 errors, 1893 warnings). The suite was 38/39 on the last run, the failure being the same 3000 ms `fails promptly` deadline under high CPU load; it also failed without this change under that load. - The five numeric triggers from the #5398 follow-up, run by hand on this branch: all exit 1 with empty stderr, the 24.95 guidance, no `Details`, and the field and bound in `fieldErrors`; the `--max_entries 100000` control exits 0 - Pre-push `check:fast` — passed ## Compatibility, safety, and release - Breaking changes or migration steps: none. The CLI `message` text for `VALIDATION_ERROR` changes to the documented copy. A caller that matched the raw Zod text in `message` should read `fieldErrors` instead, which is unchanged. - Real Hopper/Ghidra, browser, or OS coverage: not applicable to providers. CLI checked on Windows 11 x64. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new process, file, or network behavior. The change removes validator internals from caller-visible output. ## Author readiness Human owner: @dark13b - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:44:09 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 13:38:49 +00:00

[PR #7014] [MERGED] fix(ghidra): admit recordless MIPS-I legacy targets #7573

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

No description provided.

Rows
Columns