morluto/rea

Fork 0

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

[GH-ISSUE #5093] [Bug] start-adb-activity rejects valid empty-string intent extras before provider admission #3130

New issue

Closed

opened 2026-10-31 00:36:59 +00:00 by bsakaguchi · 4 comments

bsakaguchi commented

2026-10-31 00:36:59 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 18, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5093

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

CLI

Observed behavior

An in-process invocation of the actual CLI parser and shared application service rejects --extra string:k: with invalid_request. Changing only the value to string:k:payload reaches provider admission and returns capability_unavailable for a deliberately missing adb binary.

A separate shared-service probe with a mocked provider confirms that the empty string fails at input.extras[0].value with a minimum-length validation issue and zero provider calls. The nonempty control calls the mock once. Neither probe launches an Android activity or contacts a device.

Reproduction

From the source checkout at the revision below, with locked dependencies already installed, run this in an isolated subprocess. It invokes the source CLI through its production createCli entry point; no runtime build or adb installation is needed. The Node/tsx process environment should have only the shown variables plus PATH, clearing inherited REA and client configuration overrides.

WORK="$(mktemp -d)"
mkdir -p "$WORK/home" "$WORK/config" "$WORK/cache" "$WORK/data"
env -i PATH="$PATH" HOME="$WORK/home" \
  XDG_CONFIG_HOME="$WORK/config" XDG_CACHE_HOME="$WORK/cache" \
  XDG_DATA_HOME="$WORK/data" \
  node --import tsx --input-type=module <<'JS'
import { createCli } from './src/cli.ts';
for (const extra of ['string:k:', 'string:k:payload']) {
  const cli = createCli({ REA_ADB_PATH: `${process.env.XDG_CONFIG_HOME}/missing-adb` });
  let stdout = '';
  await cli.serve(['start-adb-activity', 'device', 'android.intent.action.VIEW',
    '--extra', extra, '--json'], {
    env: {}, exit: () => {}, stdout: (text) => { stdout += text; }
  });
  console.log(JSON.stringify({ extra, code: JSON.parse(stdout).code }));
}
process.exitCode = 0;
JS

This is an in-process CLI probe, not an executable exit-status test. The final assignment clears the CLI's global exit status after both observations. The same JavaScript and environment isolation were executed during verification.

Expected behavior

A typed string extra should preserve the present empty string and reach provider admission. Empty and absent values are distinct.

REA advertises string extras for this operation, and its tool contract guidance requires preserving meaningful caller choices and grounding limits in real constraints.

Android's Intent.parseCommandArgs, case --es passes the value to putExtra without a nonempty restriction. The argument reader returns supplied strings unchanged, and getNextArgRequired rejects absence, not an empty string. Android acceptance is source-derived; no device run was performed.

Impact and scope

The REA typed-extra interface cannot express a present empty string. The reproduced failure is bounded to the empty-string value of start_adb_activity extras; it does not establish a broader launch failure or an affected third-party application. Replacing the empty value with nonempty text or omitting the extra changes the requested intent. Direct adb use lies outside this REA interface and was not exercised.

The MCP contract references the same input schema, so its exclusion is source-derived. No MCP transport was run.

REA version

Source main at 3537e58c1ab29309f06eabf50bc52adba9817a0c, package version 6.7.0.

Runtime and platform

Node.js v26.12.0; macOS 27.0.1; arm64, measured during verification. The repository-pinned Node 24.18.0 was not exercised. Android runtime version: NOT_ENOUGH_INFO; no device was used.

Target and artifact context

Synthetic serial device, action android.intent.action.VIEW, key k. The serial identifies no real device in this probe. Provider admission deliberately selects a missing adb executable under the isolated configuration directory. No binary or artifact is needed.

Interface

CLI

Earliest failure stage

Request parsing or validation

Relevant output or evidence

Executed CLI source probe:

{"extra":"string:k:","code":"invalid_request"}
{"extra":"string:k:payload","code":"capability_unavailable"}

The shared-service mocked-boundary probe observed too_small, minimum: 1, and path ["input","extras",0,"value"] for the empty string, with zero provider calls. The nonempty control reached the mock once. These observations establish host-side admission behavior, not successful provider execution.

Blanket minimum length on extra values.
CLI parsing preserves the trailing empty field.
Shared admission rejects before provider execution.
Provider builds typed-extra operands.
Remote-shell quoting handles empty operands. Exercising this helper encoded an empty operand as ''.

Typed outcome or recovery expected

Admit a present empty string for type string and preserve it as the --es value. Keep the numeric and boolean type checks. A valid string value should not require replacement with different text to reach provider admission.

Analysis or suspected cause

The shared extra-value schema applies min(1) to all types. The CLI already distinguishes a missing delimiter from a present empty trailing field. This blanket minimum causes the observed rejection before the provider's type-specific handling or remote-shell quoting can run.

Additional context

The domain schema, CLI parser, MCP contract and shared admission have no source differences between rea-agents-6.7.0 and the tested main revision. Stable-release runtime behavior remains NOT_ENOUGH_INFO; source equality is not a runtime reproduction.

Related closed issues concern malformed delimiter error projection, Activity Manager timeout interpretation, and advertised regex compatibility. They have different triggers and expected corrections.

Originally created by @coygeek on GitHub (Oct 18, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5093 ## Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ## Area CLI ## Observed behavior An in-process invocation of the actual CLI parser and shared application service rejects `--extra string:k:` with `invalid_request`. Changing only the value to `string:k:payload` reaches provider admission and returns `capability_unavailable` for a deliberately missing adb binary. A separate shared-service probe with a mocked provider confirms that the empty string fails at `input.extras[0].value` with a minimum-length validation issue and zero provider calls. The nonempty control calls the mock once. Neither probe launches an Android activity or contacts a device. ## Reproduction From the source checkout at the revision below, with locked dependencies already installed, run this in an isolated subprocess. It invokes the source CLI through its production `createCli` entry point; no runtime build or adb installation is needed. The Node/tsx process environment should have only the shown variables plus PATH, clearing inherited REA and client configuration overrides. ```sh WORK="$(mktemp -d)" mkdir -p "$WORK/home" "$WORK/config" "$WORK/cache" "$WORK/data" env -i PATH="$PATH" HOME="$WORK/home" \ XDG_CONFIG_HOME="$WORK/config" XDG_CACHE_HOME="$WORK/cache" \ XDG_DATA_HOME="$WORK/data" \ node --import tsx --input-type=module <<'JS' import { createCli } from './src/cli.ts'; for (const extra of ['string:k:', 'string:k:payload']) { const cli = createCli({ REA_ADB_PATH: `${process.env.XDG_CONFIG_HOME}/missing-adb` }); let stdout = ''; await cli.serve(['start-adb-activity', 'device', 'android.intent.action.VIEW', '--extra', extra, '--json'], { env: {}, exit: () => {}, stdout: (text) => { stdout += text; } }); console.log(JSON.stringify({ extra, code: JSON.parse(stdout).code })); } process.exitCode = 0; JS ``` This is an in-process CLI probe, not an executable exit-status test. The final assignment clears the CLI's global exit status after both observations. The same JavaScript and environment isolation were executed during verification. ## Expected behavior A typed string extra should preserve the present empty string and reach provider admission. Empty and absent values are distinct. REA [advertises string extras for this operation](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/contracts/adb/adbToolContracts.ts#L341-L346), and its [tool contract guidance](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/docs/tool-design.md#L46-L69) requires preserving meaningful caller choices and grounding limits in real constraints. Android's [`Intent.parseCommandArgs`, case `--es`](https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-13.0.0_r32/core/java/android/content/Intent.java) passes the value to `putExtra` without a nonempty restriction. The [argument reader](https://android.googlesource.com/platform/prebuilts/fullsdk/sources/+/dc3f885ebe8ddc75bd9cf2d567eef4d1ed433a09/android-35/com/android/modules/utils/BasicShellCommandHandler.java#247) returns supplied strings unchanged, and [`getNextArgRequired`](https://android.googlesource.com/platform/prebuilts/fullsdk/sources/+/dc3f885ebe8ddc75bd9cf2d567eef4d1ed433a09/android-35/com/android/modules/utils/BasicShellCommandHandler.java#295) rejects absence, not an empty string. Android acceptance is source-derived; no device run was performed. ## Impact and scope The REA typed-extra interface cannot express a present empty string. The reproduced failure is bounded to the empty-string value of `start_adb_activity` extras; it does not establish a broader launch failure or an affected third-party application. Replacing the empty value with nonempty text or omitting the extra changes the requested intent. Direct adb use lies outside this REA interface and was not exercised. The MCP contract references the same input schema, so its exclusion is source-derived. No MCP transport was run. ## REA version Source main at `3537e58c1ab29309f06eabf50bc52adba9817a0c`, package version 6.7.0. ## Runtime and platform Node.js v26.12.0; macOS 27.0.1; arm64, measured during verification. The repository-pinned Node 24.18.0 was not exercised. Android runtime version: NOT_ENOUGH_INFO; no device was used. ## Target and artifact context Synthetic serial `device`, action `android.intent.action.VIEW`, key `k`. The serial identifies no real device in this probe. Provider admission deliberately selects a missing adb executable under the isolated configuration directory. No binary or artifact is needed. ## Interface CLI ## Earliest failure stage Request parsing or validation ## Relevant output or evidence Executed CLI source probe: ```json {"extra":"string:k:","code":"invalid_request"} {"extra":"string:k:payload","code":"capability_unavailable"} ``` The shared-service mocked-boundary probe observed `too_small`, `minimum: 1`, and path `["input","extras",0,"value"]` for the empty string, with zero provider calls. The nonempty control reached the mock once. These observations establish host-side admission behavior, not successful provider execution. - [Blanket minimum length on extra values](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/domain/adb/adbDeviceAnalysis.ts#L284-L303). - [CLI parsing preserves the trailing empty field](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/cli/adbCommands.ts#L357-L395). - [Shared admission rejects before provider execution](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/application/ProviderOperationExecution.ts#L53-L56). - [Provider builds typed-extra operands](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/adb/AdbProvider.ts#L2791-L2800). - [Remote-shell quoting handles empty operands](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/adb/AdbCommand.ts#L69-L84). Exercising this helper encoded an empty operand as `''`. ## Typed outcome or recovery expected Admit a present empty string for type `string` and preserve it as the `--es` value. Keep the numeric and boolean type checks. A valid string value should not require replacement with different text to reach provider admission. ## Analysis or suspected cause The shared extra-value schema applies `min(1)` to all types. The CLI already distinguishes a missing delimiter from a present empty trailing field. This blanket minimum causes the observed rejection before the provider's type-specific handling or remote-shell quoting can run. ## Additional context The domain schema, CLI parser, MCP contract and shared admission have no source differences between `rea-agents-6.7.0` and the tested main revision. Stable-release runtime behavior remains NOT_ENOUGH_INFO; source equality is not a runtime reproduction. Related closed issues concern [malformed delimiter error projection](https://github.com/morluto/rea/issues/3210), [Activity Manager timeout interpretation](https://github.com/morluto/rea/issues/2225), and [advertised regex compatibility](https://github.com/morluto/rea/issues/1581). They have different triggers and expected corrections.

bsakaguchi closed this issue

2026-10-31 00:36:59 +00:00

bsakaguchi commented

2026-10-31 00:37:00 +00:00

Author

Owner

@chordpli commented on GitHub (Oct 18, 2026):

I'd like to take this one.

I checked current main (d1adc6079) and the cause is still there. The shared start_adb_activity schema puts .min(1) on every extra value, whatever its type (src/domain/adb/adbDeviceAnalysis.ts). The CLI parser already keeps an empty trailing field, and the provider only type-checks boolean, int, long and float values, so this schema is the only place that rejects an empty string extra.

Dropping .min(1) alone would let an empty int or boolean pass admission and fail later in the provider, which changes how that error is reported. So I plan to allow an empty value only for type: "string" in the domain schema and keep the nonempty check for the other types, so those still come back as invalid_request. I'll add a regression test at the CLI and shared-service boundary: string:k: should reach provider admission, and int:k: should still be rejected.

@chordpli commented on GitHub (Oct 18, 2026): I'd like to take this one. I checked current main (`d1adc6079`) and the cause is still there. The shared `start_adb_activity` schema puts `.min(1)` on every extra value, whatever its type (`src/domain/adb/adbDeviceAnalysis.ts`). The CLI parser already keeps an empty trailing field, and the provider only type-checks boolean, int, long and float values, so this schema is the only place that rejects an empty string extra. Dropping `.min(1)` alone would let an empty int or boolean pass admission and fail later in the provider, which changes how that error is reported. So I plan to allow an empty value only for `type: "string"` in the domain schema and keep the nonempty check for the other types, so those still come back as `invalid_request`. I'll add a regression test at the CLI and shared-service boundary: `string:k:` should reach provider admission, and `int:k:` should still be rejected.

bsakaguchi commented

2026-10-31 00:37:00 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 19, 2026):

Agreed with the plan — an empty string is a legitimate value for an Android string extra (e.g. passing an intentionally empty value to the activity), and keeping the nonempty check for boolean/int/long/float types preserves the invalid_request boundary instead of deferring those type errors to the provider.

One suggestion: it may be worth adding a short comment on the schema explaining why string is exempt from .min(1), and a regression case for the explicitly quoted empty value (string:k:"" ) to confirm the parser also preserves it end-to-end.

@XinwangTao commented on GitHub (Oct 19, 2026): Agreed with the plan — an empty string is a legitimate value for an Android string extra (e.g. passing an intentionally empty value to the activity), and keeping the nonempty check for boolean/int/long/float types preserves the invalid_request boundary instead of deferring those type errors to the provider. One suggestion: it may be worth adding a short comment on the schema explaining why string is exempt from .min(1), and a regression case for the explicitly quoted empty value (string:k:"" ) to confirm the parser also preserves it end-to-end.

bsakaguchi commented

2026-10-31 00:37:00 +00:00

Author

Owner

@matbrgz commented on GitHub (Oct 19, 2026):

Cross-referencing with sibling ADB tooling and activity automation reports in Cluster AH: #3369 ([Bug] ADB push reports checksum capture overflow as device unavailability) and #5190. Both isolate input validation boundaries and verifier cleanup lifecycle in the ADB provider.

Consolidated under Cluster AH (Android ADB Automation & Activity Execution Boundaries) in tracker #4330.

@matbrgz commented on GitHub (Oct 19, 2026): Cross-referencing with sibling ADB tooling and activity automation reports in **Cluster AH**: #3369 (`[Bug] ADB push reports checksum capture overflow as device unavailability`) and #5190. Both isolate input validation boundaries and verifier cleanup lifecycle in the ADB provider. Consolidated under **Cluster AH (Android ADB Automation & Activity Execution Boundaries)** in tracker #4330.

bsakaguchi commented

2026-10-31 00:37:00 +00:00

Author

Owner

@coygeek commented on GitHub (Oct 24, 2026):

Checked the reported fix on main at d87e73f2db.

The shared activity-extra schema is now discriminated by type: string preserves an explicitly empty value, while numeric/boolean variants require nonempty and valid typed text. Domain tests check empty-string admission and invalid typed text at extras[index].value; CLI matrix checks string🔑 reaches missing-provider admission; MCP fixture includes empty value and checks remote-shell operand --es empty ''. This resolves host admission without claiming a real Android launch.

Implementation: adbDeviceAnalysis.ts, adbDeviceAnalysis.ts.

The relevant regression checks in adbDeviceAnalysis.test.ts, adbCli.test.ts, adbStartMcp.test.ts passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked the reported fix on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. The shared activity-extra schema is now discriminated by type: string preserves an explicitly empty value, while numeric/boolean variants require nonempty and valid typed text. Domain tests check empty-string admission and invalid typed text at extras[index].value; CLI matrix checks string:key: reaches missing-provider admission; MCP fixture includes empty value and checks remote-shell operand --es empty ''. This resolves host admission without claiming a real Android launch. Implementation: [adbDeviceAnalysis.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/domain/adb/adbDeviceAnalysis.ts#L57), [adbDeviceAnalysis.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/domain/adb/adbDeviceAnalysis.ts#L66). The relevant regression checks in [adbDeviceAnalysis.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/domain/adb/adbDeviceAnalysis.test.ts), [adbCli.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/cli/adbCli.test.ts), [adbStartMcp.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/mcp/adbStartMcp.test.ts) passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

bsakaguchi referenced this issue

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

[PR #3232] [MERGED] test(perf): evaluate larger and non-HTTP packet workflows #5661

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

No description provided.

Rows
Columns