mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
Hubert Gancarczyk
20b5e8b4f0
## Problem
`keyboard` accepted `text` and `key` in one call, and that shape has no
meaning a caller can rely on. The same request reads two ways:
- `{ text: "hello", key: "enter" }` — "type, then submit"
- `{ text: "abc", key: "backspace" }` — "delete, then type" reads just
as naturally
Whichever order a backend picks, the other reading silently corrupts the
field. The backends did not even agree: `typeAndroidPhone` pressed
`params.key` **before** `params.text`, while simulator-server, chromium
and vega all did the reverse and the tool's own description promised
"the text is typed first and the key is pressed after it".
## Change
**`keyboard` takes `text` or `key`, never both.** A combined call is
rejected in `execute`, above the platform dispatch, so it resolves no
`ARGENT_SECRET_*` value and reaches no device — no backend has to defend
against the shape.
**To mix text and a key, use `run-sequence`.** That is what the error
message, both parameter descriptions, the tool description and the skill
docs now say:
```json
{ "udid": "<UDID>", "steps": [
{ "tool": "keyboard", "args": { "text": "hello world" } },
{ "tool": "keyboard", "args": { "key": "enter" } }
]}
```
Two separate calls work too, but cost an extra round-trip — and are
**not** equivalent when the text carries a secret (see below).
### Why the rule is prose, not JSON Schema
An earlier revision hand-wrote `inputSchema` with `not: { required:
["text", "key"] }`. That is no longer legal: #782 banned every top-level
combinator repo-wide, `not` included, because the Messages API rejects a
request whose tool schemas declare one — and that 400 fails **every tool
in the request**, not just this one.
`tool-input-schema-contract.test.ts` enforces it across the catalog.
So the constraint is restated in **both** fields' `.describe()` (the
`boot-device` house rule) and in the tool description.
`keyboard-text-key-exclusive.test.ts` pins both texts, and pins that the
advertised schema stays combinator-free.
### What the error message carries
Three things a caller needs and cannot get elsewhere:
1. **The literal retry**, not an ellipsis the Android backend cannot
type.
2. **The TV caveat.** A TV target rejects `key` outright
(`platforms/tv.ts`), so the prescribed `{ key: "enter" }` step cannot
succeed there. The guard runs above the dispatch on purpose, so it would
otherwise swallow that diagnosis; the caveat is carried statically
rather than by an async TV probe.
3. **The secret steer**, only when `text` holds a `{{secret:…}}`
placeholder. The auto-screenshot skip is decided per tool call from the
whole request, so:
| call shape | auto-screenshot |
| --- | --- |
| one `run-sequence` carrying both steps | skipped |
| `{ text: "{{secret:…}}" }` alone | skipped |
| a separate `{ key: "enter" }` | **not** skipped — captured after the
key lands, and can hand the still-visible secret back as pixels |
The check is the syntactic `.includes` `flow-utils.ts` already uses, so
the guard still resolves nothing. A plain combined call does not carry
the sentence, with a positive control pinning that.
### Simplifications the rule makes possible
With at most one of the two ever reaching a backend, the
resolve-then-inject splits that existed only to order a combined call
fold back into their injectors (`android-input.ts`, `vega-input.ts`),
and each backend collapses to two exclusive branches.
### Fixed alongside
`run-sequence`'s description claimed "No screenshot is captured
automatically". `shouldAutoScreenshot("run-sequence")` is `true` with a
15000 ms settle cap, so that line was already false — and this change
makes it load-bearing, since the secret guidance rests on it.
## Compatibility
A stored flow or scripted `run-sequence` whose step is `{ tool:
keyboard, args: { text: "…", key: "enter" } }` now errors on replay, and
the steps after it are skipped. The recorder never emitted the combined
shape (`flow-add-step` executes the call before it appends the step), so
the blast radius is hand-authored flows and scripted `run-sequence`
calls — which is exactly what the old worked example taught. The fix is
to split the step in two.
## Test coverage
- **`keyboard-text-key-exclusive.test.ts`** (new, replaces
`keyboard-key-order.test.ts`) — the rejection on all four backends with
nothing injected and no service resolved, plus a positive control per
backend that text alone and a key alone still work; shape-not-truthiness
(`{ text: "", key: "enter" }` rejects); at-most-one rather than
exactly-one (`{}` stays a no-op); rejection before secret resolution;
the run-sequence retry, TV and secret sentences, with a positive control
against an unconditional secret steer; and the two channels the
constraint actually reaches a client through.
- **`keyboard-backend-fidelity.test.ts`** — absorbs what key-order
uniquely pinned. The unknown-key 400 now names the offending key on iOS
and chromium here, on android in `keyboard-android.test.ts`, and on vega
in `vega-injection.test.ts` (tightened from the bare `/Unknown Vega
key/` prefix), so stripping the name is red on every backend.
- **`keyboard-android.test.ts`** — the combined-call tests are gone.
What they uniquely covered survives as single-parameter tests: the
multi-segment `%` codepoint count, the pressed key's identity against a
second literal keycode, and un-typeable text rejected with no on-device
side effect. The exhaustive 24-keycode loop now runs through
`injectAndroidNamedKey` and asserts the emitted `input keyevent <code>`,
so it pins the wire and not just the lookup.
- **`interaction-messages.test.ts`** — the two formatters see different
shapes now. `startedMsg` renders before the rejection, so it still words
four (including the combined one); `completedMsg` runs only after
success, so it words three. Both empty-request branches stay pinned.
- **`keyboard-tv.test.ts`** — the TV `key` rejection is independent of
this guard, which only ever sees requests carrying both. Its header says
so, since the exclusivity message's TV clause depends on it.
- **`scripts/e2e-full/phases/30-android.sh`** — the combined call
becomes an `assert_reject`, and the prescribed `run-sequence` remedy is
driven end to end.
## Live verification
Isolated tool-server built from this branch (own port, shared MCP server
untouched), against the Android settings search field on an API 36 AVD,
read back out of the view hierarchy rather than off a screenshot:
| Call | HTTP | Field / result |
| --- | --- | --- |
| seed `{ text: "XYZ" }` | 200 | `XYZ` |
| `{ text: "abc", key: "backspace" }` | **400** | field still `XYZ` —
byte-identical, nothing typed |
| `run-sequence` [`{text:"abc"}`, `{key:"backspace"}`] | 200,
`completed: 2` | `XYZab` — exactly what the combined call used to
produce |
| `{ text: "hello" }` | 200 | `{ typed: "hello", keys: 5 }` |
| `{ key: "enter" }` | 200 | `{ typed: "enter", keys: 1 }` |
| `{}` | 200 | `{ typed: "", keys: 0 }` — still a no-op |
| `{ text: "", key: "enter" }` | **400** | shape, not truthiness |
| `{ text: "{{secret:APP_PASSWORD}}", key: "enter" }` | **400** |
message carries the ONE-`run-sequence` steer and the
still-visible-secret reason |
| `{ text: "hi", key: "enter" }` | **400** | message does **not** carry
the secret sentence |
`GET /tools` reports `["properties", "required", "type"]` for `keyboard`
— no top-level combinator.
Suite: `packages/tool-server` 336 files / 4041 passed,
`packages/registry` 90 passed, `packages/argent-mcp` 79 passed. `tsc
--build`, `tsc --noEmit -p tsconfig.test.json`, eslint (0 warnings),
prettier and knip all clean.
---------
Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
Full Argent E2E harness
A release-gating end-to-end test that starts from nothing but a
swmansion-argent-*.tgz bundle and exercises the whole product: the install
flow, every CLI command, all 70 tools' argument validation, and a happy-path run
of every tool that applies against real devices.
Run it on the real Linux box and the real Mac before a release.
Quick start
# from the repo root, with a swmansion-argent-*.tgz present there:
bash scripts/e2e-full/run-e2e.sh
# a subset of phases:
bash scripts/e2e-full/run-e2e.sh --phase install,introspection,validation
# offline core only, driving the unpacked bundle (no npm install, fast):
bash scripts/e2e-full/run-e2e.sh --skip-install --phase introspection,validation
A markdown report is written to scripts/e2e-full/results/report-<ts>.md and the
raw per-case log to results/e2e-<ts>.jsonl. The process exits non-zero if any
hard assertion failed (skips do not fail the run).
What it does (phases)
| phase | needs | covers |
|---|---|---|
install |
npm + network | npm i -g <tgz>, bundled binaries, init (global + --local), update, uninstall, telemetry, MCP-config generation |
introspection |
— | --version/--help, tools, tools describe for all 70 tools, feature flags, server start/status/logs/stop, link/unlink |
validation |
— | for every tool: missing-required / bad-enum / bad-type rejection (deterministic, no hardware) |
android |
Android emulator | happy-path of every touch/gesture/screenshot/app-lifecycle tool |
chromium |
Electron (bundled optional dep) + a display | boots a generated Electron app; drives CDP tools (scroll/drag/tabs/cookies/storage) |
rn |
~/dev/bluesky + Android device |
debugger + react/native profiler + network chain against the real Bluesky app |
Tiers auto-skip (with a recorded reason) when their prerequisites are missing, so a partial run still produces a meaningful report. iOS / tvOS / Vega tiers are intentionally out of scope.
Isolation
Everything runs under a throwaway HOME and npm prefix ($(mktemp -d)), so the
real machine's ~/.argent, editor MCP configs, and global packages are never
touched — safe to run on a shared box. Add --keep to inspect the sandbox after.
Providing a device
The device tiers need a booted device. Two ways:
- Inject an already-booted one (recommended on shared/CI machines):
--android-serial emulator-5554(the harness attaches, doesn't boot/teardown it). - Let the harness boot it:
--android-avd Pixel_9a(usesboot-device).
The Chromium tier needs no device — it generates and boots its own Electron app
(requires DISPLAY, or xvfb-run on a headless Linux box).
RN (Bluesky) tier
E2E_RN_DIR=~/dev/bluesky E2E_RN_PKG=xyz.blueskyweb.app \
bash scripts/e2e-full/run-e2e.sh --phase rn --android-serial <serial>
Assumes the Bluesky dev-client is already built and installed on the device
(E2E_RN_PREBUILT, the default). Pass E2E_RN_BUILD=1 to let it run
expo run:android first (slow). It starts Metro itself and tears it down.
Flags
--tgz PATH tarball to test (default: newest swmansion-argent-*.tgz at repo root)
--phase a,b,c subset of: install introspection validation android chromium rn
--skip-install drive the unpacked bundle directly (offline phases only; skips `install`)
--system install to the REAL global prefix (dedicated release machine only)
--android-serial S use an already-booted Android device
--android-avd NAME boot this AVD via boot-device
--keep leave the sandbox dir for inspection
Layout
run-e2e.sh orchestrator: env setup, phase dispatch, report, exit code
lib/common.sh logging, argent_cli/run_tool, assert_* helpers, server + screenshot helpers
lib/discover-tools.sh parses `argent tools describe` into per-tool arg models
lib/report.py JSONL -> markdown (per-phase + per-tool coverage matrix + failures)
phases/*.sh one run_phase() per phase
results/ generated JSONL + report (gitignored)