morluto/rea

Fork 0

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

[PR #5596] [CLOSED] fix(cli): report malformed output values in the selected structured format #6985

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5596
Author: @saiprasanth-git
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: fix/structured-invalid-output-values

📝 Commits (3)

345595e fix(cli): report malformed output values in the selected structured format
f2d6e5d docs(cli): clarify malformed format handoff comment
d1d267d test(cli): cover malformed equals-style output values

📊 Changes

3 files changed (+129 additions, -7 deletions)

View changed files

📝 docs/cli.md (+3 -1)
📝 src/cliOutput.ts (+39 -6)
📝 tests/boundary/cli/cliOutput.test.ts (+87 -0)

📄 Description

Summary

Fixes #5531. Once --json or a structured --format (json, jsonl, yaml) is selected, a malformed --format, --token-limit, or --token-offset value now fails before command execution with a structured INVALID_OUTPUT_VALUE error in the selected format, instead of a TOON diagnostic that JSON/JSONL consumers cannot decode.

Problem and expected behavior

capabilities --json --token-limit not-a-number exited 1 with stdout:

code: UNKNOWN
message: "Invalid value for --token-limit: \"not-a-number\""

parseCliOutputArguments set parseError and deferred to Incur, which reports in TOON. The existing MISSING_OUTPUT_VALUE path already shows that argument-validation failures can preserve the selected structured format.

Change and scope

parseCliOutputArguments records invalidValue (flag + value) when a malformed value appears after a structured format was selected; otherwise it keeps the current parseError handoff to Incur, so TOON/md behavior and flag ordering semantics are unchanged (a later --json after the bad value is not honored, mirroring Incur stopping at the malformed flag).
validateCliOutputArguments returns INVALID_OUTPUT_VALUE naming the flag and value (and the accepted formats for --format), rendered by the existing renderCliOutputArgumentError.
docs/cli.md documents the new code next to MISSING_OUTPUT_VALUE.
Not included: negative/fractional token-window bounds (#5532, covered by #5544/#5578). The two may touch nearby lines.

Contribution scope: ordinary fix within existing CLI boundaries; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: src/cliOutput.ts CLI output-argument validation, before command execution.
Contribution scope fit and linked maintainer decision: not applicable (bug fix).
CLI and MCP/tool-catalog contract: CLI only; new error code INVALID_OUTPUT_VALUE for structured formats. MCP unaffected.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Documentation or usability only; no target interaction.

Evidence and regression coverage

Tests added or updated: tests/boundary/cli/cliOutput.test.ts: unit cases for all five issue variants plus YAML, a negative case confirming non-structured formats still defer to Incur, and compiled-CLI integration cases that decode JSON and each JSONL line.
Base reproduction: on the base revision, the reproduction above prints TOON; with only src/cliOutput.ts reverted, the new tests fail (7 failed / 38 passed).
User-visible CLI output after the change:

$ rea capabilities --json --token-limit not-a-number
{"ok":false,"error":{"code":"INVALID_OUTPUT_VALUE","message":"Invalid --token-limit value \"not-a-number\". Use a number."}}
$ rea capabilities --json --format bogus
{"ok":false,"error":{"code":"INVALID_OUTPUT_VALUE","message":"Invalid --format value \"bogus\". Use one of: toon, json, yaml, md, jsonl."}}

All exit 1.

Remaining proof gaps: Linux x64, Node 24.18.0 only; TTY output not checked; full repository suite and real-provider workflows not run (not exercised by this CLI argument change).

Validation performed

Rebased on main at 0ba4d6f; head d1d267d. Linux x64, Node 24.18.0.

npm run test:prepare: passed
npx vitest run tests/boundary/cli tests/boundary/process/runtimeBundle.test.ts: 106 files passed, 11 skipped; 676 tests passed, 55 skipped
npx vitest run tests/boundary/cli/cliOutput.test.ts: 45 passed at d1d267d (adds equals-style malformed cases); 7 failed with src/cliOutput.ts reverted to main
npx tsc --noEmit -p tsconfig.json: passed
npm run lint: passed (pre-existing warnings only)
npm run verify:test-discovery: all 1818 test files discovered exactly once
npx prettier --check on changed files: passed
scripts/ci/pr-readiness.mjs evaluateReadiness on this body: success

Compatibility, safety, and release

Breaking changes or migration steps: none; error code changes from Incur's UNKNOWN to INVALID_OUTPUT_VALUE only for structured formats.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none.

Author readiness

Human owner: @saiprasanth-git

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/5596 **Author:** [@saiprasanth-git](https://github.com/saiprasanth-git) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/structured-invalid-output-values` --- ### 📝 Commits (3) - [`345595e`](https://github.com/morluto/rea/commit/345595e3a2c27fadeff5959420e6ed6b333f006b) fix(cli): report malformed output values in the selected structured format - [`f2d6e5d`](https://github.com/morluto/rea/commit/f2d6e5d563b5fc5162f320b4bf345b41c0503a70) docs(cli): clarify malformed format handoff comment - [`d1d267d`](https://github.com/morluto/rea/commit/d1d267d618a5059484a31646e95dc4a045ecc9a5) test(cli): cover malformed equals-style output values ### 📊 Changes **3 files changed** (+129 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `docs/cli.md` (+3 -1) 📝 `src/cliOutput.ts` (+39 -6) 📝 `tests/boundary/cli/cliOutput.test.ts` (+87 -0) </details> ### 📄 Description ## Summary Fixes #5531. Once `--json` or a structured `--format` (json, jsonl, yaml) is selected, a malformed `--format`, `--token-limit`, or `--token-offset` value now fails before command execution with a structured `INVALID_OUTPUT_VALUE` error in the selected format, instead of a TOON diagnostic that JSON/JSONL consumers cannot decode. ## Problem and expected behavior `capabilities --json --token-limit not-a-number` exited 1 with stdout: ``` code: UNKNOWN message: "Invalid value for --token-limit: \"not-a-number\"" ``` `parseCliOutputArguments` set `parseError` and deferred to Incur, which reports in TOON. The existing `MISSING_OUTPUT_VALUE` path already shows that argument-validation failures can preserve the selected structured format. ## Change and scope - `parseCliOutputArguments` records `invalidValue` (flag + value) when a malformed value appears after a structured format was selected; otherwise it keeps the current `parseError` handoff to Incur, so TOON/md behavior and flag ordering semantics are unchanged (a later `--json` after the bad value is not honored, mirroring Incur stopping at the malformed flag). - `validateCliOutputArguments` returns `INVALID_OUTPUT_VALUE` naming the flag and value (and the accepted formats for `--format`), rendered by the existing `renderCliOutputArgumentError`. - `docs/cli.md` documents the new code next to `MISSING_OUTPUT_VALUE`. - Not included: negative/fractional token-window bounds (#5532, covered by #5544/#5578). The two may touch nearby lines. Contribution scope: ordinary fix within existing CLI boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/cliOutput.ts` CLI output-argument validation, before command execution. - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: CLI only; new error code `INVALID_OUTPUT_VALUE` for structured formats. MCP unaffected. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/cli/cliOutput.test.ts`: unit cases for all five issue variants plus YAML, a negative case confirming non-structured formats still defer to Incur, and compiled-CLI integration cases that decode JSON and each JSONL line. - Base reproduction: on the base revision, the reproduction above prints TOON; with only `src/cliOutput.ts` reverted, the new tests fail (7 failed / 38 passed). - User-visible CLI output after the change: ``` $ rea capabilities --json --token-limit not-a-number {"ok":false,"error":{"code":"INVALID_OUTPUT_VALUE","message":"Invalid --token-limit value \"not-a-number\". Use a number."}} $ rea capabilities --json --format bogus {"ok":false,"error":{"code":"INVALID_OUTPUT_VALUE","message":"Invalid --format value \"bogus\". Use one of: toon, json, yaml, md, jsonl."}} ``` All exit 1. - Remaining proof gaps: Linux x64, Node 24.18.0 only; TTY output not checked; full repository suite and real-provider workflows not run (not exercised by this CLI argument change). ## Validation performed Rebased on `main` at 0ba4d6f; head d1d267d. Linux x64, Node 24.18.0. - `npm run test:prepare`: passed - `npx vitest run tests/boundary/cli tests/boundary/process/runtimeBundle.test.ts`: 106 files passed, 11 skipped; 676 tests passed, 55 skipped - `npx vitest run tests/boundary/cli/cliOutput.test.ts`: 45 passed at d1d267d (adds equals-style malformed cases); 7 failed with `src/cliOutput.ts` reverted to `main` - `npx tsc --noEmit -p tsconfig.json`: passed - `npm run lint`: passed (pre-existing warnings only) - `npm run verify:test-discovery`: all 1818 test files discovered exactly once - `npx prettier --check` on changed files: passed - `scripts/ci/pr-readiness.mjs` `evaluateReadiness` on this body: success ## Compatibility, safety, and release - Breaking changes or migration steps: none; error code changes from Incur's `UNKNOWN` to `INVALID_OUTPUT_VALUE` only for structured formats. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. ## Author readiness Human owner: @saiprasanth-git - [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. --- <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:30 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6985] [MERGED] fix(browser): preserve media type parameter spelling #7550

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

No description provided.

Rows
Columns