[PR #2230] [MERGED] fix(cli): retain browser and Inspector input correction details #5099

New issue

Closed

opened 2026-10-31 01:03:55 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:03:55 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/cli-input-diagnostics-2180

📝 Commits (1)

05ba40e fix(cli): retain browser and Inspector input correction details

📊 Changes

3 files changed (+247 additions, -23 deletions)

View changed files

📝 src/cli/browserCommands.ts (+39 -13)
📝 src/cliJavaScriptRuntimeCommands.ts (+23 -10)
➕ tests/boundary/cli/browserInputDiagnostics.test.ts (+185 -0)

📄 Description

Summary

Fixes #2180.

Preserve safe correction details when supplied arguments fail the named input contracts for list-browser-targets, inspect-web-page, analyze-web-bundle, observe-web-session, list-javascript-runtime-targets, and observe-javascript-runtime.

Problem and expected behavior

For example, rea list-browser-targets 'not a url' --json correctly exits 1 with invalid_request, but its remediation asks callers to correct listed arguments without returning any issues. The handler discards issues already produced by its input contract.

The same rejection should identify the field and authored correction or bound without echoing rejected values. This patch adds details.operation and details.issues through the existing shared projection. Classification, retryability, message, and exit status remain unchanged.

Change and scope

Retain each canonical input object and pass its Zod issues to analysisInputErrorFromIssues.
Remove the obsolete no-issues Inspector wrapper.
Add compiled CLI regressions for all six reported inputs, two endpoint-credential non-echo cases including debug stderr, authored cross-field guidance, and omitted positional input.
No admission-schema, provider, or success-path changes.

Contract and boundary impact

Semantic owner and earliest changed stage: CLI named-contract validation; the existing domain projector owns safe correction metadata.
CLI and MCP/tool-catalog contract: restore the existing typed invalid-input details in six CLI handlers; MCP and advertised schemas are unchanged.
Provider, bridge, target-format, or platform compatibility: unchanged.
Evidence, artifact, provenance, or reconstruction contract: unchanged.
Process execution, authorization, cleanup, or containment impact: no new runtime authority; regression processes and their temporary workspaces are fixture-owned.
Generated metadata, package, or installation impact: none; no generated outputs or dependency changes included.

Evidence and regression coverage

Executed locally on Linux x64 using pinned Node.js 24.18.0 and npm 11.16.0, against fork base bfa805decf83744bd3ecb0d1402667cf3ed5c3b2.

Before the production fix, the new test file had 9 failures on missing details and 1 passing omitted-endpoint parser control. This reproduced all six reported command failures.
After the fix, all ten new cases pass. Existing successful browser and Inspector CLI workflows and sibling scenario/screenshot diagnostics also pass in the focused selection below.
A malformed endpoint now includes:
{"operation":"list_browser_targets","issues":[{"path":["cdp_endpoint"],"reason":"invalid_value","message":"Invalid CDP endpoint URL"}]}
An empty target identifier includes {"path":["target_id"],"reason":"out_of_range","minimum":1}.
Successful provider coverage uses local protocol fixtures. No real browser, Electron, Inspector, Hopper, or Ghidra execution is claimed. The full deterministic suite and coverage were not run locally.

Validation performed

npm run test:focused -- \
  tests/boundary/cli/browserInputDiagnostics.test.ts \
  tests/boundary/cli/browserCli.test.ts \
  tests/boundary/cli/javascriptRuntimeObservationCli.test.ts \
  tests/boundary/cli/scenarioCliInputDiagnostics.test.ts \
  tests/boundary/cli/browserCompareJsonFiles.test.ts

Focused command above: 19 tests passed across 5 files; required compiled artifacts prepared by the runner.
npm run check: passed, including typecheck, formatting, metadata/dependency checks, lint, module boundaries, and architecture guards. Repository-wide lint reported 911 warnings and 0 errors; no warnings named the changed files.
npm run verify:test-discovery: all 933 test files discovered exactly once; this is discovery validation, not full-suite execution.
Targeted oxlint on the three changed files and git diff --check: passed.
Forward and reverse patch application verified; clean base application reproduces the tested files byte-for-byte.

Dependencies were installed with lifecycle scripts disabled. Turbo telemetry was disabled during local checks. docs:check, package checks, and real-provider lanes were not run; this patch changes no schema, package, or generated-metadata source.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real OS coverage: Linux x64 compiled CLI processes; other platforms and real providers unverified.
Package or release metadata impact: none.
Security/privacy review: reuse the canonical safe projection; tests verify rejected endpoint values and synthetic credentials are absent from stdout and debug stderr.

Review checklist

Concrete issue linked; one focused conventional-title fix.
Changed observable behavior and meaningful failures covered.
No owning schema, documentation, or generated-metadata changes required.
Representative output and verification limits included.
Final diff reviewed for unrelated changes, real secrets, and unsupported claims.

AI assistance

The implementation, regression tests, and PR description were developed with OpenAI Codex assistance. The local validation listed above was executed, and a separate AI-assisted source review found no actionable findings. Maintainer review is pending.

_{🔄 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/2230 **Author:** [@369Apps](https://github.com/369Apps) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/cli-input-diagnostics-2180` --- ### 📝 Commits (1) - [`05ba40e`](https://github.com/morluto/rea/commit/05ba40e3cb283eab215ce9a8b988844793af2bf6) fix(cli): retain browser and Inspector input correction details ### 📊 Changes **3 files changed** (+247 additions, -23 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/browserCommands.ts` (+39 -13) 📝 `src/cliJavaScriptRuntimeCommands.ts` (+23 -10) ➕ `tests/boundary/cli/browserInputDiagnostics.test.ts` (+185 -0) </details> ### 📄 Description ## Summary Fixes #2180. Preserve safe correction details when supplied arguments fail the named input contracts for `list-browser-targets`, `inspect-web-page`, `analyze-web-bundle`, `observe-web-session`, `list-javascript-runtime-targets`, and `observe-javascript-runtime`. ## Problem and expected behavior For example, `rea list-browser-targets 'not a url' --json` correctly exits 1 with `invalid_request`, but its remediation asks callers to correct listed arguments without returning any issues. The handler discards issues already produced by its input contract. The same rejection should identify the field and authored correction or bound without echoing rejected values. This patch adds `details.operation` and `details.issues` through the existing shared projection. Classification, retryability, message, and exit status remain unchanged. ## Change and scope - Retain each canonical input object and pass its Zod issues to `analysisInputErrorFromIssues`. - Remove the obsolete no-issues Inspector wrapper. - Add compiled CLI regressions for all six reported inputs, two endpoint-credential non-echo cases including debug stderr, authored cross-field guidance, and omitted positional input. - No admission-schema, provider, or success-path changes. ## Contract and boundary impact - Semantic owner and earliest changed stage: CLI named-contract validation; the existing domain projector owns safe correction metadata. - CLI and MCP/tool-catalog contract: restore the existing typed invalid-input details in six CLI handlers; MCP and advertised schemas are unchanged. - Provider, bridge, target-format, or platform compatibility: unchanged. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Process execution, authorization, cleanup, or containment impact: no new runtime authority; regression processes and their temporary workspaces are fixture-owned. - Generated metadata, package, or installation impact: none; no generated outputs or dependency changes included. ## Evidence and regression coverage Executed locally on Linux x64 using pinned Node.js 24.18.0 and npm 11.16.0, against fork base `bfa805decf83744bd3ecb0d1402667cf3ed5c3b2`. - Before the production fix, the new test file had 9 failures on missing details and 1 passing omitted-endpoint parser control. This reproduced all six reported command failures. - After the fix, all ten new cases pass. Existing successful browser and Inspector CLI workflows and sibling scenario/screenshot diagnostics also pass in the focused selection below. - A malformed endpoint now includes: `{"operation":"list_browser_targets","issues":[{"path":["cdp_endpoint"],"reason":"invalid_value","message":"Invalid CDP endpoint URL"}]}` - An empty target identifier includes `{"path":["target_id"],"reason":"out_of_range","minimum":1}`. - Successful provider coverage uses local protocol fixtures. No real browser, Electron, Inspector, Hopper, or Ghidra execution is claimed. The full deterministic suite and coverage were not run locally. ## Validation performed ```sh npm run test:focused -- \ tests/boundary/cli/browserInputDiagnostics.test.ts \ tests/boundary/cli/browserCli.test.ts \ tests/boundary/cli/javascriptRuntimeObservationCli.test.ts \ tests/boundary/cli/scenarioCliInputDiagnostics.test.ts \ tests/boundary/cli/browserCompareJsonFiles.test.ts ``` - Focused command above: 19 tests passed across 5 files; required compiled artifacts prepared by the runner. - `npm run check`: passed, including typecheck, formatting, metadata/dependency checks, lint, module boundaries, and architecture guards. Repository-wide lint reported 911 warnings and 0 errors; no warnings named the changed files. - `npm run verify:test-discovery`: all 933 test files discovered exactly once; this is discovery validation, not full-suite execution. - Targeted `oxlint` on the three changed files and `git diff --check`: passed. - Forward and reverse patch application verified; clean base application reproduces the tested files byte-for-byte. Dependencies were installed with lifecycle scripts disabled. Turbo telemetry was disabled during local checks. `docs:check`, package checks, and real-provider lanes were not run; this patch changes no schema, package, or generated-metadata source. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real OS coverage: Linux x64 compiled CLI processes; other platforms and real providers unverified. - Package or release metadata impact: none. - Security/privacy review: reuse the canonical safe projection; tests verify rejected endpoint values and synthetic credentials are absent from stdout and debug stderr. ## Review checklist - [x] Concrete issue linked; one focused conventional-title fix. - [x] Changed observable behavior and meaningful failures covered. - [x] No owning schema, documentation, or generated-metadata changes required. - [x] Representative output and verification limits included. - [x] Final diff reviewed for unrelated changes, real secrets, and unsupported claims. ## AI assistance The implementation, regression tests, and PR description were developed with OpenAI Codex assistance. The local validation listed above was executed, and a separate AI-assisted source review found no actionable findings. Maintainer review is pending. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:03:55 +00:00