morluto/rea

Fork 0

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

[PR #3271] [CLOSED] fix(cli): classify malformed adb --extra as invalid input #5689

New issue

Closed

opened 2026-10-31 01:08:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:08:39 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3271
Author: @paoValle
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/3210-adb-extra-invalid-request

📝 Commits (1)

4bd41d3 fix(cli): classify malformed adb --extra as invalid input

📊 Changes

2 files changed (+118 additions, -31 deletions)

View changed files

📝 src/cli/adbCommands.ts (+59 -31)
📝 tests/boundary/cli/adbCli.test.ts (+59 -0)

📄 Description

Summary

Fixes #3210.

rea start-adb-activity --extra rejected a nonempty value with fewer than two
colons, but reported code: "UNKNOWN" with no category, retryable or
remediation. The command parsed the type:key:value text in its callback and
threw a plain Error before the shared logged boundary, so the failure reached
Incur's generic handler instead of the canonical invalid_request projection.

Problem and expected behavior

Smallest trigger — no device, emulator or real adb is needed:

REA_ADB_PATH=/nonexistent/adb node scripts/rea.mjs start-adb-activity \
  emulator-5554 android.intent.action.VIEW --extra string:key --json

Before:

{"code":"UNKNOWN","message":"--extra expects type:key:value, received: string:key"}

Expected per docs/error-contract.schema.json (the invalid_request variant),
src/contracts/errorSchemas.ts:34, and AGENTS.md (Architecture and Cleanup:
parse boundary values and model expected failures with the tagged algebra):
code: "invalid_request", category: "invalid_input", a nonempty message, a
boolean retryable, a nonempty remediation.action, and exit 1 before any
device operation. The unsupported-type control bogus:key:value already
produced a schema-valid invalid_request through the shared input schema and
must keep doing so; a well-formed extra must still reach provider admission.

Change and scope

src/cli/adbCommands.ts: the body formerly in the local execute helper is now
executeRequest(name, operation, buildInput); execute delegates with
() => input. start-adb-activity builds its request inside the thunk and the
malformed text raises AnalysisInputError(operation, { userMessage }) there.
That construction now happens inside logCliCommand's callback, so the existing
projection and process.exitCode = 1 apply without a new error path.

Not included: no change to the MCP surface (it takes typed extras), no schema
change, no change to the acceptance rule itself (two colons still required).
--extra "" remains rejected by Incur's argument schema before the command
runs, so it still returns VALIDATION_ERROR rather than the shared contract;
that is pre-existing CLI argument-validation behavior, the issue scopes itself
to nonempty values, and fixing it is an argument-boundary change of its own.

Contract and boundary impact

Semantic owner and earliest changed stage: the CLI request-construction boundary for start_adb_activity (src/cli/adbCommands.ts).
Contribution scope fit and linked maintainer decision: not applicable — ordinary fix inside existing boundaries; no execution, injection or mutation change.
CLI and MCP/tool-catalog contract: the CLI failure keeps the shared error contract; MCP is unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: the malformed request is still rejected before any provider or device work; the valid control still reaches provider admission.
Generated metadata/package/installation impact: none.

Expected effects: [x] Read existing files or captured data. [x] Documentation or usability only; no target interaction. The reproduction uses a nonexistent adb path and never contacts a device.

Evidence and regression coverage

Tests added: tests/boundary/cli/adbCli.test.ts — classifies a malformed --extra before any device operation, a cliTest (real compiled CLI, real exit codes) over string:key, : and string. Each payload is parsed with the repo's own analysisErrorProjectionSchema so the test enforces the contract (nonempty remediation.action, boolean retryable) instead of its shape, and asserts the details.issues entry for the rejected extra. The control asserts string:key:value still reaches capability_unavailable with exit 1.
Bite check: with src/cli/adbCommands.ts reverted and the test kept, the new test fails (1 failed | 6 passed), asserting on code: "UNKNOWN".
Remaining proof gaps: Windows not exercised; the defect is provider-independent request validation.

Validation performed

npm run test:focused -- tests/boundary/cli/adbCli.test.ts — Test Files 1 passed (1), Tests 7 passed (7).
npm run check — Tasks: 6 successful, 6 total; All matched files use the correct format.; Checked resolved module boundaries in 1862 source files.; Architecture guards passed across 1386 source files.
CLI, before/after for all five issue cases (string:key, :, string, bogus:key:value, string:key:value): the three malformed cases now exit 1 with {"code":"invalid_request","category":"invalid_input","message":"--extra expects type:key:value, received: …","retryable":true,"remediation":{"action":"Correct the listed arguments and retry."},"details":{"operation":"start-adb-activity","issues":[{"path":["extra",0],"reason":"invalid_format","message":"--extra expects type:key:value"}]}}; bogus:key:value is unchanged; string:key:value still returns capability_unavailable for the missing adb binary.

Compatibility, safety, and release

Breaking changes or migration steps: none. The malformed-input cases change from an unclassified UNKNOWN to the documented invalid_request projection with exit 1.
Real Hopper/Ghidra, browser, or OS coverage: none needed; no engine or device is involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none — the message repeats caller-supplied text that the previous error already contained.

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.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed (none required).
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

An AI coding agent drafted this change under my direction; I reviewed the diff and ran the
commands above with the output quoted here.

_{🔄 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/3271 **Author:** [@paoValle](https://github.com/paoValle) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/3210-adb-extra-invalid-request` --- ### 📝 Commits (1) - [`4bd41d3`](https://github.com/morluto/rea/commit/4bd41d3cf93f54ca563a1b487c680c10b5b9778e) fix(cli): classify malformed adb --extra as invalid input ### 📊 Changes **2 files changed** (+118 additions, -31 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/adbCommands.ts` (+59 -31) 📝 `tests/boundary/cli/adbCli.test.ts` (+59 -0) </details> ### 📄 Description ## Summary Fixes #3210. `rea start-adb-activity --extra` rejected a nonempty value with fewer than two colons, but reported `code: "UNKNOWN"` with no `category`, `retryable` or `remediation`. The command parsed the `type:key:value` text in its callback and threw a plain `Error` before the shared logged boundary, so the failure reached Incur's generic handler instead of the canonical `invalid_request` projection. ## Problem and expected behavior Smallest trigger — no device, emulator or real adb is needed: ```sh REA_ADB_PATH=/nonexistent/adb node scripts/rea.mjs start-adb-activity \ emulator-5554 android.intent.action.VIEW --extra string:key --json ``` Before: ```json {"code":"UNKNOWN","message":"--extra expects type:key:value, received: string:key"} ``` Expected per `docs/error-contract.schema.json` (the `invalid_request` variant), `src/contracts/errorSchemas.ts:34`, and `AGENTS.md` (Architecture and Cleanup: parse boundary values and model expected failures with the tagged algebra): `code: "invalid_request"`, `category: "invalid_input"`, a nonempty message, a boolean `retryable`, a nonempty `remediation.action`, and exit 1 before any device operation. The unsupported-type control `bogus:key:value` already produced a schema-valid `invalid_request` through the shared input schema and must keep doing so; a well-formed extra must still reach provider admission. ## Change and scope `src/cli/adbCommands.ts`: the body formerly in the local `execute` helper is now `executeRequest(name, operation, buildInput)`; `execute` delegates with `() => input`. `start-adb-activity` builds its request inside the thunk and the malformed text raises `AnalysisInputError(operation, { userMessage })` there. That construction now happens inside `logCliCommand`'s callback, so the existing projection and `process.exitCode = 1` apply without a new error path. Not included: no change to the MCP surface (it takes typed extras), no schema change, no change to the acceptance rule itself (two colons still required). `--extra ""` remains rejected by Incur's argument schema before the command runs, so it still returns `VALIDATION_ERROR` rather than the shared contract; that is pre-existing CLI argument-validation behavior, the issue scopes itself to nonempty values, and fixing it is an argument-boundary change of its own. ## Contract and boundary impact - Semantic owner and earliest changed stage: the CLI request-construction boundary for `start_adb_activity` (`src/cli/adbCommands.ts`). - Contribution scope fit and linked maintainer decision: not applicable — ordinary fix inside existing boundaries; no execution, injection or mutation change. - CLI and MCP/tool-catalog contract: the CLI failure keeps the shared error contract; MCP is unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: the malformed request is still rejected before any provider or device work; the valid control still reaches provider admission. - Generated metadata/package/installation impact: none. Expected effects: [x] Read existing files or captured data. [x] Documentation or usability only; no target interaction. The reproduction uses a nonexistent adb path and never contacts a device. ## Evidence and regression coverage - Tests added: `tests/boundary/cli/adbCli.test.ts` — `classifies a malformed --extra before any device operation`, a `cliTest` (real compiled CLI, real exit codes) over `string:key`, `:` and `string`. Each payload is parsed with the repo's own `analysisErrorProjectionSchema` so the test enforces the contract (nonempty `remediation.action`, boolean `retryable`) instead of its shape, and asserts the `details.issues` entry for the rejected extra. The control asserts `string:key:value` still reaches `capability_unavailable` with exit 1. - Bite check: with `src/cli/adbCommands.ts` reverted and the test kept, the new test fails (`1 failed | 6 passed`), asserting on `code: "UNKNOWN"`. - Remaining proof gaps: Windows not exercised; the defect is provider-independent request validation. ## Validation performed - `npm run test:focused -- tests/boundary/cli/adbCli.test.ts` — `Test Files 1 passed (1)`, `Tests 7 passed (7)`. - `npm run check` — `Tasks: 6 successful, 6 total`; `All matched files use the correct format.`; `Checked resolved module boundaries in 1862 source files.`; `Architecture guards passed across 1386 source files.` - CLI, before/after for all five issue cases (`string:key`, `:`, `string`, `bogus:key:value`, `string:key:value`): the three malformed cases now exit 1 with `{"code":"invalid_request","category":"invalid_input","message":"--extra expects type:key:value, received: …","retryable":true,"remediation":{"action":"Correct the listed arguments and retry."},"details":{"operation":"start-adb-activity","issues":[{"path":["extra",0],"reason":"invalid_format","message":"--extra expects type:key:value"}]}}`; `bogus:key:value` is unchanged; `string:key:value` still returns `capability_unavailable` for the missing adb binary. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The malformed-input cases change from an unclassified `UNKNOWN` to the documented `invalid_request` projection with exit 1. - Real Hopper/Ghidra, browser, or OS coverage: none needed; no engine or device is involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none — the message repeats caller-supplied text that the previous error already contained. ## 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. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed (none required). - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- An AI coding agent drafted this change under my direction; I reviewed the diff and ran the commands above with the output quoted here. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:08:39 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:46:02 +00:00

[PR #6188] fix(adb): separate unresolvable paths and host capacity in adb spawn failures #7194

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

No description provided.

Rows
Columns