mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
stack/721-base-legacy
1119 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Hubert Gancarczyk
|
8cc2123819 | Merge branch 'fix/ui-text-invisible-characters' into fix/ios-flow-tree-depth-and-targeting | ||
|
Hubert Gancarczyk
|
65f435bbfe | Merge branch 'fix/failed-nested-step-not-recorded' into fix/ui-text-invisible-characters | ||
|
Hubert Gancarczyk
|
5b31406219 | Merge remote-tracking branch 'origin/main' into fix/failed-nested-step-not-recorded | ||
|
filip131311
|
b835de2326 |
fix(packaging): ship one shared copy of the Android screen-sharing agent (#983)
## Summary Fixes the biggest chunk of #980: the published package shipped **four byte-identical copies** of the Android screen-sharing agent (`resources/android/`: jar + per-ABI `.so`, ~12.7 MiB each) — one next to each platform's simulator-server binary, because the binary resolves the agent relative to its working directory and the spawn set cwd to the per-platform dir. This ships **one shared copy** at `bin/resources/android/` and points the spawn cwd at the bin root instead. Package size (built from this branch, same artifact set as 0.22.1): | | before | after | |---|---|---| | unpacked | 115.5 MB | **76.4 MB** | | packed | 37.6 MB | 25.4 MB | 76.4 MB is under npmmirror's 83886080-byte unpacked-size cap (the limit #980 trips — note it applies to `dist.unpackedSize`, not the tarball). Already-published oversized versions are immutable, so the mirror also needs the `cnpm/unpkg-white-list` entry to sync the backlog; this PR keeps future versions under the cap. ## Changes - `scripts/download-simulator-server.sh` extracts the agent tarball once, to `bin/resources/android/`, instead of next to each binary. - `bundle-tools.cjs` copies that single shared directory into the package. - `@argent/native-devtools-ios` replaces `simulatorServerBinaryDir()` with `simulatorServerRunDir()`: the bin root when the shared `resources/android/` exists there, falling back to the per-platform dir so an `ARGENT_SIMULATOR_SERVER_DIR` override pointing at a pre-dedup layout keeps working. - The blueprint spawns simulator-server with `cwd: RUN_DIR` (binary path stays absolute; the agent lookup is the only cwd-relative path in the simulator-server crate — audited: every other fs path there is a tempdir, `dirs::cache_dir()`, or caller-supplied). No symlinks, no postinstall scripts — plain files plus a different cwd, so nothing changes for npm/pnpm/yarn/bun or Windows. ## Testing - Unit: resolver tests updated + new shared-layout case; blueprint test asserts the spawn cwd. - Packaging: ran the modified download script + `npm run pack` end-to-end; tarball contains exactly one `bin/resources/android/` (94 files vs 112). - Live: branch tool-server with `ARGENT_SIMULATOR_SERVER_DIR` pointed at the new layout — iOS simulator (screenshot) and Android emulator (boot-device, screenshot, gesture-tap) green; `lsof` confirms both spawned simulator-servers run with cwd = bin root, where `resources/android/<jar>` and each per-ABI `.so` resolve (verified present at those exact relative paths). - Physical Android phone (Galaxy S24, the one path that actually pushes the agent): live-tested — the `android_device` controller resolved the agent from the shared layout, pushed it, and connected ("All agent connections established successfully"); screenshot and tap through the on-phone agent both green, spawn cwd confirmed as the shared bin root. No docs update needed: internal packaging/layout only — no MCP tool, CLI, config, or flow-file surface changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01UGAsAvYBJ3Jy9DKLaX9aoZ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved simulator-server resource handling by using one shared screen-sharing resource location across host platforms. * Added a warning when required shared resources are unavailable. * Ensured simulator-server processes start from the correct runtime directory. * **Documentation** * Clarified simulator-server runtime and shared resource behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Hubert Gancarczyk
|
6f1a1c479c | Merge branch 'fix/ui-text-invisible-characters' into fix/ios-flow-tree-depth-and-targeting | ||
|
Hubert Gancarczyk
|
d22f0559e4 | Merge branch 'fix/failed-nested-step-not-recorded' into fix/ui-text-invisible-characters | ||
|
Hubert Gancarczyk
|
b9d0531150 |
fix(flow): mark an abort-cancelled raw tool step as having reached the device
The swipe directive's merge brought a new abort arm into `execLeafStep`'s raw-tool `catch`: a gesture tool that consults the signal rejects when the run is cancelled mid-dispatch, and that is reported as a skip rather than the tool's own "aborted after N frames" failure. It returned that skip unmarked. `reachedAStep` reads an unmarked skip as "nothing was dispatched", so a composed or nested run cancelled inside a raw `tool: gesture-swipe` step lost its partial-mutation warning - the recorder told the agent the device was untouched by a gesture that had already gone out. Mark it, like the directive and launch arms alongside it: the tool was invoked, so an unknown resolves to reached. |
||
|
Hubert Gancarczyk
|
a855ed8090 |
Merge branch 'fix/failed-nested-step-not-recorded' into fix/ui-text-invisible-characters
# Conflicts: # packages/docs/docs/reference/flow-yaml.mdx |
||
|
Hubert Gancarczyk
|
06c7a1885f | Merge remote-tracking branch 'origin/main' into fix/failed-nested-step-not-recorded | ||
|
Ignacy Łątka
|
2cc1f5281c |
test(android-profiler): drop an assertion that cannot fail (#933)
Audit of every assertion in the repo that pins shipped agent-facing text, looking for more of the `/restart/i` shape that reddened main in #871 — an assertion satisfiable only by making the product string restate itself. **The class is essentially absent. One instance found.** ## The one `tool-server/test/android-perfetto/hang-stacks-offcpu.test.ts` ```ts expect(out).toMatch(/No usable on-CPU stack samples/); // L156 // The thread was on-CPU/executing — this is genuine CPU work, not a wait. expect(out).toMatch(/on-CPU/); // L158 ← removed expect(out).toMatch(/executing/); ``` `/on-CPU/` is a substring of the literal asserted two lines above, so `L156 ∧ L158 ≡ L156`. It cannot fail while L156 passes. Mutation-checked rather than argued: changing the pipeline's sentence from `_No usable on-CPU stack samples were captured…` to `_No usable stack samples were captured…` still fails the test — ``` × explains an empty sample set as un-unwound CPU work when the thread was executing AssertionError: expected ... to match /No usable on-CPU stack samples/ ``` — so nothing this line was catching goes uncaught. `/executing/` is **not** implied and stays; the comment above it still reads correctly. ## What the audit covered, and what it found instead | stage | count | |---|---| | assertions pinning shipped agent-facing text | 707 | | …in the `/restart/i` shape (short generic literal beside stronger siblings on the same value) | 44 | | …surviving hand-verification | **1** | The 43 rejected are doing real work. A sample, so the reasoning is auditable: - `"844x390"` / `"390x844"` — dimension-mismatch test; the point is argument order. - `"(2×)"` / `"(1×)"` — different dedup counts. - `/wait/` beside `/No on-CPU stack samples/` — the sibling literal contains no "wait"; it pins the wait-vs-CPU explanation. - `"localhost:8081"` / `"127.0.0.1:8081"` — two hosts the interceptor must filter. - metro `discovery.test.ts` `"Do not retry in a loop"` — a *different* test from the one asserting the full sentence. More importantly, the 707 is not a pile of redundancy. It is mostly the only way to test a function whose output is a string — `formatTraceFreshness(NOW - 5*60*MIN, NOW)` can only be checked by asserting `/5 hours ago/` — plus assertions that are load-bearing in their own right, e.g. `expect(description).not.toContain("hunter2")` in `configuration-core/tests/secrets.test.ts`, the sole guarantee a secret value never reaches a source description. Stripping the category wholesale would empty 74 tests into unconditional passes. ## Two adjacent findings, not changed here - `tool-server/test/native-profiler-missing-trace.test.ts:90` — `expect(result.report).toMatch(/missing|not found|unreadable/i)` is meant (per its comment) to check the wording tells the user the file is missing rather than the export empty. It passes vacuously: the fixture is named `missing_cpu.xml`, so `/missing/i` matches the filename the assertion two lines up already pins. It enforces nothing. The fix is a discriminating regex or a renamed fixture, not deletion. - `tool-server/test/capability.test.ts:58-59` — `toContain("demo-tool")` and `toContain("android")` are both substrings of `"tools/demo-tool/platforms/android.ts"` asserted at L60, so they are provably implied. Left in place deliberately: the test is named *"composes a uniform message with toolId, platform, and the file"*, and deleting them would make the name over-claim relative to what the body checks. ## Verification Branched off `366f17c61`. | gate | result | |---|---| | `vitest run` (tool-server) | **358 files, 4401 passed, 1 skipped**, exit 0 | | `tsc --build` | clean | | `typecheck:tests --workspaces` | clean | | `eslint . --max-warnings 0` | exit 0 (with `npm ci` in `packages/docs`, as CI does) | | `prettier --check` | clean | |
||
|
Juliusz Wajgelt
|
bf38e9069b |
fix(sim-remote): pass the udid before -b in simctl bootstatus (#982)
## What `simctlBootstatus` in `packages/tool-server/src/utils/sim-remote.ts` built the command as `simctl bootstatus -b <udid>`, placing the flag before the device. simctl's grammar puts the device first — `bootstatus <udid> -b`. ## Why Newer Argent Cloud deployments are more faithful to how the local `simctl` handles its arguments, so they now reject `-b` in that position as an unknown flag. The result is that **every remote boot fails**, on every device: ``` [tool: mcp__argent__boot-device] -> invalid option -- b ``` The local `xcrun` path already used the correct order (`boot-device.ts:461`), so this only affected sim-remote / Argent Cloud. ## Notes for the reviewer - I checked the other `sim-remote` wrappers for the same flag-before-udid shape. `injectDylib` is the only other one with an optional flag and it already appends after the udid, so `bootstatus` was the sole offender. - `packages/tool-server/test/boot-device.test.ts` passes (23/23), but it only exercises the local `xcrun` path — there is no existing test for the remote wrapper, so **this fix is not covered by tests**. It's worth a manual check against a Cloud device before merging. - `tsc --noEmit` reports pre-existing errors in this package from a stale `@argent/registry` build; `sim-remote.ts` itself is clean and none of them are related to this change. - No docs update needed: this is an internal argument-construction bug with no user-facing surface change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed simulator boot status checks by correcting the order of command-line arguments. * Ensured the simulator identifier is processed correctly when the optional boot flag is used. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
filip131311
|
23b9a07760 |
feat(mcp): auto-describe — element tree after interaction tools + relaxed tapping rule (#958)
## Why
The rule's `<tapping_rule>` ("Before **every** tap, you MUST call a
discovery tool") makes agents loop `tap → describe → tap → describe`:
over 35 baseline runs, `describe` was called 90 times against 86 taps —
one round-trip per tap, each a full model turn. The screen after an
action is already captured by the auto-screenshot; the element tree is
the only thing the agent still had to fetch.
## What
- **MCP** (`packages/argent-mcp`): after the action settles, call
`describe` and append `--- Elements after action (describe) ---` + the
tree to the tool result. Follows the same tool allow-list as the
auto-screenshot, skips `describe` itself, and sits inside the non-secret
branch so a `{{secret:…}}` value never re-enters context as text.
Spy-logged as `auto_describe` (duration, chars). MCP `instructions`
reworded.
- **Flag** `disable-auto-describe` (opt-out, mirrors
`disable-auto-screenshot`). Auto-describe has its own tool list
(`AUTO_DESCRIBE_TOOLS`) and its own branch, so the two features are
independent; the secret-placeholder skip and the screen-idle wait cover
both.
- **Rule + skill**: `tapping_rule` now says take coordinates from the
returned tree, call a discovery tool only when there is no fresh tree
for the screen (first action on a screen, after a wait, target not
listed); the two-failed-taps rule and the "never from pixels"
prohibition stay. `argent-device-interact` best-practice #6 and the
general-rules line updated to match.
- **Docs**: `reference/configuration.mdx` flag row,
`features/interacting-with-apps.mdx` discovery section.
- **Tests**: `autoDescribeEnabled` (global / project scope) and
`shouldAutoDescribe`.
## Evidence
A/B with `claude -p --model sonnet`, one build (variant env-gated during
the experiment), two sandbox projects differing only in rule/skill text
and the toggle; 7 tasks × 2 arms × 5 reps = 70 runs on the iPhone 17 Pro
simulator (iOS Settings ×3, Expo `my-app` ×2, Contacts create verified
in sqlite) and the Electron testbed.
| | main | this PR | Δ |
|---|---|---|---|
| Task success | 35/35 | 35/35 | = |
| Turns / run | 10.4 | 8.2 | −21% |
| Wall / run | 36.5 s | 28.0 s | −23% (new won 28/34 pairs) |
| Cost / run | $0.28 | $0.23 | −17% (27/34) |
| `describe` calls / run | 2.2 | 0.24 | −89% |
| Cache-read tokens / run | 622k | 421k | −32% |
(34 clean pairs; one pair excluded for external interference on the
simulator — with it included: wall −5%, cost −10%, turns −16%.) Every
iOS task moved 15–30% in favour of the new arm; Electron was neutral
(both arms describe once after `boot-device`, then a single
`run-sequence`). The appended tree costs ≈2.4k chars (~700 tokens) and a
median 73 ms per action; per-turn latency was unchanged, the saving is
the removed model round-trip.
Full report:
https://claude.ai/code/artifact/279528cf-ea92-4f52-9276-4f0475ba3e3c
## Caveats
- Sonnet only, short tasks, small screens. On feed-heavy apps with
10–20k-char trees the per-action overhead grows; a follow-up could cap
or filter the appended tree.
- Flags are read once at MCP start (same as `disable-auto-screenshot`),
so toggling needs an MCP restart.
## Checks
`vitest` argent-mcp (85) + configuration-core (122) green; `eslint
--max-warnings 0` on changed packages; `prettier --check .`; `npx
docusaurus build` green.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Interaction tools now return an accessibility element tree alongside
the latest screenshot.
- Agents can use fresh element data for follow-up actions without an
additional discovery step.
- **Configuration**
- Added the `disable-auto-describe` feature flag to turn off automatic
element trees.
- **Documentation**
- Updated interaction guidance and configuration references to explain
the new behavior and flag.
- **Bug Fixes**
- Secret-placeholder handling now correctly accounts for both
screenshots and element trees.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
filip131311
|
e7c6f3324c |
fix(tool-server): frame device sockets on \n only — U+2028 in a label stalled describe for 15 s (#975)
## Problem
`describe` returned an empty tree after ~15 s whenever the screen had an
accessibility label containing U+2028 (LINE SEPARATOR) — e.g. any
Element/Matrix reply preview, or rich text pasted from the web. Nothing
was wrong on the device: ax-service answered in ~170 ms with a full,
valid tree.
The tool-server framed the ax-service socket with
`readline.createInterface`, which splits on the four ECMAScript line
terminators (`\n`, `\r`, **U+2028, U+2029**). JSON permits U+2028
unescaped inside a string and `NSJSONSerialization` emits it raw, so one
valid frame became three fragments, none parsed, each was dropped by
`catch { return }`, and the pending RPC waited out its 10 s timeout —
then the native-devtools fallback waited out its 5 s. On top of that,
the timeout was reported as *"this simulator was not booted through
argent … call boot-device with force=true"*, sending the agent to reboot
a healthy simulator.
Minimal repro (no device): `JSON.stringify({label:"a
b"})+"\n"` through
`readline` → 2 lines, 0 parsed.
## Fix
- **`utils/ndjson-socket.ts`** (new): `attachNdjsonReader` frames on the
`\n` byte only (`0x0A` can never occur inside a multi-byte UTF-8
sequence, so this is exact) and reports every unparseable frame to
stderr instead of swallowing it.
- Wired into the three device-facing JSON sockets: `ax-service.ts`,
`native-devtools.ts`, `android-devtools-client.ts` (the Android helper's
`org.json` and the iOS dylib's `NSJSONSerialization` both emit U+2028
raw, so all three were exposed). The two remaining `readline` uses read
process **stdout** for readiness markers — no JSON, no user text — and
are left alone.
- **`describe/platforms/ios/index.ts`**: a throw from `axApi.describe()`
(timeout, RPC error) now carries the error message in the hint and uses
the resolved api's real `degraded` flag, instead of being reported as
the boot caveat. Separate commit so it can be reverted independently.
## Verification
- 6 new unit tests for the reader (U+2028/U+2029 in a string, chunk
split mid-multibyte char, several frames per chunk, CRLF/empty lines,
dropped-frame reporting, trailing frame on `end`) and 3 for the hint
(`describe-tool.test.ts`). Full tool-server suite: 4816 passed.
- Live A/B on iPhone 16 / iOS 18.5, Safari page with `<meta
charset=utf-8>` and a U+2028 inside a `<p>`:
- argent 0.22.1 (`main`): `describe` **12.6 s**, `source:
native-devtools`, web text missing, false "not booted through argent"
hint.
- This branch (`dist` against installed binaries): **50 ms**, `source:
ax-service`, label `"In reply to Dave
Loving it so far"` present, no
hint, 0 dropped frames logged.
## Docs
No docs update needed — no tool, CLI, config or flow surface changes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01VwqqHRhfuHXbKr6LYskxhJ
|
||
|
Ignacy Łątka
|
43b7d79a2a |
fix(boot-device): stop passing -crash-report-mode to emulators that reject it (#557)
## Problem
`boot-device` cannot launch an Android emulator on builds that do not
accept
`-crash-report-mode`. The emulator rejects the unknown option and exits
before
the device ever comes up:
```
$ argent run boot-device --args '{"avdName":"Pixel_6_API_34"}'
emulator binary exited with code 1 before the device booted
```
## Cause
`-crash-report-mode` is undocumented and missing from many emulator
builds, so
#306 added a feature-detect gate:
```ts
const crashReportArgs = (await emulatorSupportsFlag("-crash-report-mode"))
? ["-crash-report-mode", "never"]
: [];
```
That commit was a pure addition. It never removed the flag from the
shared
`LAUNCH_HARDENING_ARGS` constant, which still reads:
```ts
const LAUNCH_HARDENING_ARGS = [
"-no-boot-anim", "-netfast",
"-crash-report-mode", "never",
"-no-metrics",
] as const;
```
So the gate is dead code. Every launch gets the flag unconditionally
through
the constant, and builds that do support it are handed it twice.
## Proof
On Android emulator 36.1.9.0, whose `-help` does not list the flag:
```
$ emulator -version
Android emulator version 36.1.9.0 (build_id 13823996)
$ emulator -help | grep -c -- -crash-report-mode
0
$ emulator -help | grep -c -- -no-metrics
1
```
`-list-avds` and an unknown `-avd` both short-circuit before option
validation, so the rejection only surfaces once a real AVD resolves.
Against
one, with everything else held equal:
```
$ emulator -avd Pixel_6_API_34 -check-snapshot-loadable default_boot \
-crash-report-mode never
INFO | Found systemPath .../android-34/google_apis/arm64-v8a/
unknown option: -crash-report-mode
please use -help for a list of valid options # exits, never boots
$ emulator -avd Pixel_6_API_34 -check-snapshot-loadable default_boot
INFO | Ok: System requirements to run avd: `Pixel_6_API_34` are met
```
The launch dies at option parsing, after the AVD resolves and before any
device work. `hardeningArgs` reaches the snapshot probe, the hot boot
and the
cold boot alike, so on such a build all three spawns abort.
## Fix
- Drop `-crash-report-mode never` from `LAUNCH_HARDENING_ARGS`, leaving
the
`crashReportArgs` gate as the only source of the flag.
- Spread `crashReportArgs` into the `-check-snapshot-loadable` probe's
`extraArgs`. The probe is itself an emulator spawn, so it gets the same
crash-dialog suppression as the two boots; once the flag left the
constant
it would otherwise have been the only site missing it. This one is not a
snapshot-parity requirement - unlike `-noaudio` and `-netfast`,
`-crash-report-mode` selects crash reporting rather than qemu devices,
so a
mismatch would not invalidate the saved snapshot.
- Leave the gate where it is, after the already-running reuse fast path,
so
the `emulator -help` probe is still skipped when nothing will be
spawned.
- Fix the prose that had drifted: the comment above
`LAUNCH_HARDENING_ARGS`
still listed `-crash-report-mode` among its flags and claimed they all
need
no detection, and both it and the `emulatorSupportsFlag` docstring said
the
flag is "only present in newer emulator releases (~36.x and late 35.x)"
-
which 36.1.9.0 not having it contradicts.
`bootAndroidImpl` is the only emulator launch path in the repo, and all
three
of its spread sites are covered.
Making the gate load-bearing put weight on a probe that was only ever
advisory, so `emulator -help` detection is hardened alongside it:
- `emulatorSupportsFlag` memoized its verdict for the process lifetime
even
when the probe learned nothing - a transient spawn failure, a timeout,
or a
silent zero-exit run pinned "unsupported" until restart. An empty
capture is
now reported to stderr and left uncached, so the next boot retries.
- A killed probe is no verdict at all: the flag may sit in the part of
the
listing the child never transmitted, so partial output from a timeout is
discarded rather than read as absence.
- The three emulator probe spawns (`-help`, `-list-avds`,
`-check-snapshot-loadable`) pass `killSignal: "SIGKILL"`, matching
`runAdb`.
`execFile`'s default SIGTERM can be ignored by an emulator wedged in
Qt/graphics init, which leaves the promise unsettled past every caller's
deadline.
## Tests
A new `-crash-report-mode feature gate` block in
`packages/tool-server/test/boot-device-hotboot.test.ts` asserts the real
spawned argv and the probe's `extraArgs`:
- gate false: zero occurrences at the probe, hot-boot and cold-boot
sites;
- gate true: exactly one occurrence at each, always followed by `never`.
The
count matters - asserting mere presence passes on the double-pass code
and
would guard nothing;
- plus a test that `-help` detection is skipped entirely on the
already-running
reuse fast path, pinning why the computation sits where it does.
`emulatorSupportsFlag` is stubbed in that file's existing
`vi.mock("../src/utils/adb")` factory, because the real one memoizes its
`-help` result per binary for the process lifetime and both branches
could not
otherwise be exercised in one file. The two pre-existing
launch-hardening
assertions were updated to the four remaining flags.
`packages/tool-server/test/emulator-supports-flag.test.ts` is new and
covers
the detection hardening: which outcomes are memoized (a verdict read
from a
non-zero exit's captured listing, and a normal zero-exit run) and which
are
not (spawn failure, timeout with empty streams, zero exit with no
listing,
timeout with partial output), the stderr warning, and `SIGKILL` on all
three
spawns.
Discrimination check - both source changes reverted (flag back in
`LAUNCH_HARDENING_ARGS`, `crashReportArgs` dropped from the probe's
`extraArgs`), tests kept:
```
Tests 5 failed | 26 passed (31)
x omits the flag from the snapshot probe and the hot-boot spawn when the emulator rejects it
x omits the flag from the cold-boot spawn when the emulator rejects it
x passes the flag exactly once to the snapshot probe and the hot-boot spawn when supported
x passes the flag exactly once to the cold-boot spawn when supported
x keeps exactly one flag occurrence on the probe-reject cold fallback when supported
AssertionError: expected 1 to be +0 (x2 - the flag reaches a site that must not see it)
AssertionError: expected 2 to be 1 (x3 - the flag arrives twice)
```
Fix restored: `Tests 31 passed (31)`.
## Verification
- `npm run build` (`tsc --build`): clean.
- `npx vitest run` in `packages/tool-server`: clean.
- `npm run typecheck:tests --workspaces --if-present`: clean. This is
the CI
gate that plain `tsc` + vitest does not cover.
- `npx prettier --check .` and `npm run lint` (`eslint . --max-warnings
0`):
clean.
- End to end on a real emulator (36.1.9.0, the build that rejects the
flag).
The gate reads the binary correctly:
```
emulatorSupportsFlag(-crash-report-mode) = false
emulatorSupportsFlag(-no-metrics) = true
emulatorSupportsFlag(-no-boot-anim) = true
```
and `boot-device` against `Pixel_6_API_34` returns
`{"platform":"android","serial":"emulator-5554","booted":true}` in 4.8
s.
The live process argv shows the flag dropped and the four remaining
hardening flags intact:
```
qemu-system-aarch64 -avd Pixel_6_API_34 -force-snapshot-load
-no-snapshot-save \
-gpu auto -noaudio -no-boot-anim -netfast -no-metrics
```
`-force-snapshot-load` means the hot-boot path ran, so the snapshot
probe
accepted its flag-free argv too.
## Docs
No docs update needed: `-crash-report-mode` is an internal launch
argument
that no reference page, config key or tool parameter exposes.
|
||
|
Jakub Piasecki
|
d4a712c401 |
refactor(gesture): rename the momentum-free flag to momentum (#732)
`settle: true` collided with the runner's other `settle` — waiting for the tree to quiesce — which is the very thing a swipe step does after every dispatch, whichever way the flag is set. One sentence of the flow skill used both senses at once. This names the physics instead: `momentum: false` is the momentum-free gesture, and the natural fling is the default the prose already described. ### Renamed | surface | before | after | | --- | --- | --- | | `swipe` flow directive | `settle: true` | `momentum: false` | | `gesture-swipe` param | `settle` | `momentum` | | `gesture-drag` param | `settle` | `momentum` | The directive passes the flag straight through to the tools, so all three move together. `momentum: true` is the default and normalizes to absent, mirroring what `settle: false` did — parse/serialize stay exact inverses. ### Also - Recording summary label `(settle)` → `(momentum-free)` - Internal constants → `MOMENTUM_FREE_EASE_EXPONENT`, `MOMENTUM_FREE_MIN_STEPS` - `flow-run` tool description and `argent-create-flow` skill updated ### Testing `tsc -b`, eslint (`--max-warnings 0`) and prettier clean; `test/flows` + `test/tools` pass 671/671, including a new case covering the legacy-spelling rejection. |
||
|
Jakub Piasecki
|
e314b3ef4a |
feat(flow): swipe directive (#520)
Add one semantic finger flick for dismissing cards, paging carousels, opening drawers, and other gesture-driven actions distinct from goal-seeking scroll-to. Support bare directions plus from with exactly one of direction, to, or by, optional settle, and positive finite duration. Resolve selector or normalized-point anchors through the shared target path. Direction uses the Maestro geometry presets and now rejects anchored gestures that would collapse or travel opposite to the requested direction. Relative by deltas saturate supplied axes to [0, 1] and reject any effective zero or sign reversal. Resolved starts must remain on-screen, while explicit endpoints stay author-controlled. Keep parse and serialization invariants aligned for programmatic steps, improve unknown-key hints and selector diagnostics, surface swipe targets in run reports and recording summaries, share positive-millisecond validation, and reuse the common flow test harness. Dispatch gesture-swipe on touch platforms and gesture-drag on Chromium, reject touch directives on Vega, and document when to prefer scroll-to or a raw system-edge gesture. |
||
|
Hubert Gancarczyk
|
1e562cee8c |
perf(flow): resolve the terminate command only where the reason quotes it
`explainTargetingFailure` built the `xcrun simctl terminate` string before dispatching to a branch, so all four paid for it - including the three that never interpolate it. The cost is not free: with `ios.additionalDeviceSets` configured, `deviceSetForUdid` probes the default set and each configured one with a sequential `simctl list devices --json`, and a udid found in none caches nothing, so the sweep re-runs per call. This sits in the catch of every failed read, and a failing `await:` rebuilds its reason once per poll. Move the call into the ambiguous branch, and into the probe-failure branch's "clear the others" clause, which only appears above one connection. Measured against a booted simulator, one additional set configured, each branch driven cold through the real `queryFullHierarchyTree`: branch before after quotes it ambiguous (2 apps) 112ms 142ms yes single app not foreground 95ms 0ms no probe failed, 1 connection 98ms 0ms no probe failed, 2 connections 97ms 94ms yes no connected app 103ms 0ms no Every reason comes back byte-identical. The new case pins the coupling across both connection counts; hoisting the call back fails it. |
||
|
Hubert Gancarczyk
|
3803bb2d45 |
docs(flow): cut the depth-cap comment to what the constant cannot say
Three review threads landed on this one docblock. Two found claims in it that do not hold: - "nothing truncates it" - the recorder's cross-tree re-probe quotes a determinate reason through `cappedReason`, bounded at MAX_PROBE_REASON_CHARS (1250). Only the run/replay surfaces are unbounded. - the `compatibilityMissNote` mapping - it branches on whether the locator matched, not on the condition, so a `text` condition whose selector missed reaches the not-located arm, where `selectorMissNote` does quote tree text. Both paragraphs restated behaviour that already lives beside the code that produces it: the hoisting fallback at `evaluateCondition`, the quote bounds at `CODEPOINT_DUMP_MAX` and `typographicVariantNote`. What is left is the pair of facts the constant cannot state on its own - why 40 was too low, and why the headroom above a real tree costs nothing. |
||
|
Hubert Gancarczyk
|
ff6d67a17b |
docs: separate the two fates of an invisible-only selector field
"A field of only invisible characters matches nothing rather than everything" is true of `role`, `identifier` and `expectedText`, which reach the runtime gate, and false of `text`, which the schema refuses in both the tool and the flow parser: text must contain at least one visible character (icon-font/private-use and zero-width characters render as nothing) — select by identifier or role instead An author told their selector matches nothing goes looking for a wrong match instead of reading the validation error they actually got. Checked on a Chromium target through both surfaces. |
||
|
Hubert Gancarczyk
|
1c57f3adf5 |
docs(flow): say that any: true needs a scope where an author reads it
The sentence an author follows while authoring presented `any: true` as usable on its own. The parser refuses it: assert.visible: `any: true` matches every element on screen — pair it with a scope (within/after/next) so it selects something specific Both the tool description and the selectors section of the docs page already stated the requirement; these two sentences did not. |
||
|
Hubert Gancarczyk
|
8366764227 |
docs(flow): state what a wrapper half in a needle really does
The reference said a needle carrying half a bidi wrapper "puts the closing half mid-label and matches nothing". That is false at the label's edges: a needle of just the opening wrapper and the start of the label matches, and so does one ending exactly at the closing wrapper. An author reading the mechanism rather than the advice predicts wrong results there. The half has to meet the same half in the label at the same place, which is what makes a fragment copied out of the middle fail. Pinned in the comparator tests, and checked on a Chromium target against a bidi label. |
||
|
Hubert Gancarczyk
|
72b776924b |
test(await-ui-element): pin the zero-area filter its flow twin already pins
The miss note keeps only visible candidates for a `visible` wait and all of them for `exists`, which accepts zero-area nodes. The flow runner's copy of that rule is covered; this one was not, in either direction - dropping the filter let a `visible` timeout point at a node that is off screen, and applying it unconditionally took the explanation away from `exists`, and neither turned a test red. Both mutations now fail. |
||
|
Hubert Gancarczyk
|
564adf8342 |
test(ui-tree-match): pin the order selectorMissNote asks its two questions in
The rationale promises the codepoint answer first, because it names exact characters, but nothing enforced it: swapping the two loops left the whole selector suite green. With one candidate on screen carrying a blocking invisible and another that is a typographic variant of the same needle, the swap hands the author the vaguer answer about the wrong element. The case runs both tree orders, so the pin is about the loops rather than about which node comes first. |
||
|
Hubert Gancarczyk
|
c6c58bf6cb |
test(ui-tree-match): pin the literal grade against promotion, not only demotion
Every existing grade test catches a demotion. Raising EXACT_LITERAL to 3 survived all of them, and it moves what a tap targets: a selector whose text matches a large container literally and a small leaf only through the fold ties at 2 today, so the smallest-frame rule takes the leaf, while at 3 the container wins outright and the tap centre moves with it. The new case sits exactly on that boundary - one literal field against two folded ones - and fails under the promotion. |
||
|
Hubert Gancarczyk
|
353ca92bdf |
fix(ui-tree-match): stop the fold cache throwing away every warm entry per poll
The cache counted entries, not characters, and cleared itself wholesale. Both hurt a long-running server: - A key retains the ORIGINAL string, and one card's label has reached 11,532 characters, so an entry cap alone let the map hold megabytes of dead originals for the life of the process. - A screen emitting more distinct strings per poll than the cap - timestamped rows, a ticking relative time - wiped every stable label beside them on every pass, so the next pass refolded the whole tree cold, indefinitely. The cache is now two generations: a lookup reads both and promotes a hit out of the older one, so a string read once per cap-worth of new keys is never evicted, and a rotation drops only what nothing has read since. Both the entry count and the character count trigger the rotation. A worst-case tree of ordinary labels still fits one generation - measured warm re-fold of 24k strings: 5.18 ms min against 5.19 ms before, 0 refolds. |
||
|
Hubert Gancarczyk
|
6cf409b0bc |
fix(ui-tree-match): diagnose and defuse the control characters that never fold
Cc is not Default_Ignorable_Code_Point, so U+0000, U+001C-U+001F and DEL fell outside every question the notes ask, and quoteScreenText passed them into the tool's JSON reply as raw bytes: element matched but its text was "Sign<raw DEL> in" (wanted to equal "sign in") with no explanation of the miss. The chromium walker concatenates DOM text-node values with no Cc filter, DEL is legal in HTML text, and a copy-paste out of a log or a PDF carries the rest. The C0 controls the fold does not already absorb as whitespace join the inert ignorables the notes strip, so a codepoint note now names them, and the quote prints them as <U+007F> rather than as the byte. |
||
|
Hubert Gancarczyk
|
7e5368d3e0 |
fix(ui-tree-match): explain a miss that carries an invisible AND a typographic variant
The two diagnostic gates partitioned the misses rather than composing. The invisible gate needs the ignorable-stripped remainders to compare equal, which a rendered ellipsis defeats; the typographic gate needs NFKC equality on the originals, which a never-folded invisible defeats. A label carrying both - hyphenation inserting a soft hyphen, truncation inserting an ellipsis - got no explanation from either, on the very failure this work exists to explain: element matched but its text was "Load<SHY>ing…" (wanted to contain "Loading...") Both notes now accept a compatibility variant beside the invisible characters and name the two causes, code points first. A miss the typographic question already answers on its own keeps its single note, so a stray zero-width space next to a ligature still adds no code point the reader has no use for. |
||
|
Hubert Gancarczyk
|
dc8664945e |
fix(ui-tree-match): stop claiming ONLY invisible characters differ when case does too
The note's gate accepts a pair the comparator equates, so a case, spacing or
composition difference can stand beside the invisible one - and the codepoint
dumps in the same sentence print it. confusableTextNote("Home<CGJ>", "home")
led with "differ only in invisible characters" above [U+0048 ...] vs
[U+0068 ...].
The word ONLY is now conditional on the rest of the two strings being identical
once the inert ignorables are gone, and the note names what else differs when
it is not. Each caller asks that question through its own relation: equality
for the whole-string note, "appears in" for the substring one.
|
||
|
Hubert Gancarczyk
|
4849bf5a82 |
docs(ui-tree-match): describe the default-ignorable set as it is
The comment claimed the regex excluded the sequence builders, which lives in the helpers instead, and called every member a character whose removal leaves the same appearance - which the RENDERING_AFFECTING comment beside it denies of U+00AD and U+180E. |
||
|
Hubert Gancarczyk
|
d405b25db2 | Merge branch 'fix/ui-text-invisible-characters' into fix/ios-flow-tree-depth-and-targeting | ||
|
Hubert Gancarczyk
|
e44725305a | Merge branch 'fix/failed-nested-step-not-recorded' into fix/ui-text-invisible-characters | ||
|
Hubert Gancarczyk
|
9914c7015a | Merge remote-tracking branch 'origin/main' into fix/failed-nested-step-not-recorded | ||
|
Hubert Gancarczyk
|
0ac9e351ce |
docs(create-flow): drop the batch mechanism the cancel fix made stale
"The sequence turns a nested `success: false` into an error entry and stops there" stopped being true for one of the three causes the section lists right above it: a cancelled wait now leaves no entry, and the batch reports itself aborted instead. State the outcome the author acts on - the batch stops at the first wait that did not pass - and say what a cancel changes about the refusal it reads. Verified on a Chromium (CDP) target, an iOS simulator, an Android emulator and a Vega VVD: an unmet nested wait is refused with `message` naming the wait, a cancelled one with "run-sequence was aborted after 0 of 1 steps". Nothing is written in either case. |
||
|
Hubert Gancarczyk
|
3e028b2bd1 |
docs(flow): move the nested-wait sentence's twin, and pin the pair
live-authoring.md was corrected on this branch; flow-start-recording's
description - the surface an agent reads BEFORE it records anything - still
said "A wait nested inside a recorded run-sequence gets neither warning ... so
for those, read `toolResult`". After this branch there is no recorded
run-sequence to read one from: an unmet nested wait stops the batch, so the
recorder refuses the whole call.
Both halves verified over HTTP against a tool-server built from this branch and
a Chromium (CDP) target:
unmet nested message names the wait, `recorded` absent, stepCount 1 -> 1
held nested recorded as an opaque `tool: run-sequence` step, no warning,
where the SAME selector recorded directly does warn (an
`<input type=password aria-label="Secret code">`, which the
runner tree redacts to `[password]`)
So the sentence now names the refusal for the wait that did not hold, keeps the
silence for the one that did, and gives the remedy live-authoring.md gives.
A test in flow-skill-docs holds the two surfaces together.
|
||
|
Hubert Gancarczyk
|
08c1c3f785 |
docs(flow): stop two comments calling the append the only hand-edit absorber
This branch gave `activeFlowState` the same re-read the append has, so a return
that records nothing now absorbs a mid-recording hand edit and charges
`discardedWarnings` for it - which the `dropMovedWarnings` JSDoc rewritten here
already says ("before every append AND before every return that records
nothing"). Two comments still describe the world before that.
`appendStepToFlow` called its re-read "the only chance anyone gets to NOTICE a
hand edit", and `flow-finish-recording` explained the shortfall arithmetic on
the next line as "what the appends threw away" - so a reader reconciling a
count against it looks for appends that never happened.
Reproduced over HTTP against a tool-server built from this branch: record a wait
that raises a verdict, prepend a step to the .yaml by hand, then call
flow-add-step with `flow-add-echo` (records nothing, stepCount 2, no
`recorded`). The finish reports "1 warning raised during this recording is NOT
in `summary`" - charged entirely by the call that appended nothing.
Comment-only: the emitted JS differs on comment lines alone.
|
||
|
Hubert Gancarczyk
|
9fbde6ceee |
test(flow): give the cancel re-wording a guard that can fail
`not.toContain("failed nested step")` named a string no code path produces -
its only occurrence in the repo is the literal text of flow-add-step's own
`description`, which never reaches `result.message`. The wording it reads as
guarding against is the one `nestedOrchestratorOutcome` builds,
`run-sequence stopped at <tool> after N of M steps: ...`, and the re-wording
demotes that into the brackets rather than dropping it - so only the leading
clause separates a cancellation from a failure.
Proven by A/B against the re-wording removed (`const reason = outcome.reason`):
the shipped guard stayed green, the replacement fails, and both pass on the
unmutated tree.
The same shape at three other lines, `not.toContain("Restore the device")`,
names the pre-#719 wording of `partialMutationWarning`, which is likewise absent
from `packages/`. Dropped: each of those tests already pins the advice it cares
about positively, and the dedicated read-only case pins both halves of it.
|
||
|
Hubert Gancarczyk
|
f448b25e63 |
fix(flow): stop a cancelled wait claiming it had reached the device
`await-ui-element` dispatches nothing - it polls the tree and calls no device
tool - so marking its cancelled skip `reached` asserts what the field is defined
to deny: that the skip is no proof the device is untouched.
Its two twins already answer the other way. `case "wait"` returns the same
"run aborted during wait" reason unmarked, and the `await:` directive spelling
routes through `runDirective`, whose `reached` flips only inside
`invokeOnDevice` - which `waitForCondition` never calls.
The recorder is what reads the marker. Driven in-process against the built dist
with a real AbortController and a Chromium (CDP) target, a composed flow whose
only step is a cancelled wait:
before flow "direct-wait" was aborted - step NOT recorded. Prior composed
steps may already have changed the device ... put it back by hand ...
after flow "direct-wait" was aborted - step NOT recorded.
The same fixture with a leading `gesture-tap` still warns, and the page's own
tap counter shows why - so this narrows the warning rather than silencing it.
|
||
|
Hubert Gancarczyk
|
950b164130 |
fix(run-sequence): report a cancelled wait as the cancel it is, not a failed batch
A cancelled `await-ui-element` returns unmet like any other, so the push below
turned it into an ordinary error entry. `runSequenceOutcome` finds that entry
before it can reach its own abort arm, and the batch scores `fail` with a reason
calling a condition unmet that nothing ever read.
The same cancel of the same wait therefore had two verdicts. `tool:
await-ui-element` gives `skip` / "run aborted during wait"; `flow-execute` tests
`aborted` before `ok`, so it agrees; only the `run-sequence` spelling disagreed.
Driven in-process against the built dist with a real AbortController and a
Chromium (CDP) target, the same wait cancelled the same way:
direct skip run aborted during wait
nested fail run-sequence stopped at await-ui-element after 0 of 1 steps:
await-ui-element condition not met: wait was cancelled ...
Break without an entry instead, which is what every other cancel exit in this
loop already does - the pre-step guard and the inter-step delay both leave the
list short. The abort arm then reads `steps.length < total` and scores it
`skip` / "run-sequence was aborted after 0 of 1 steps". Both spellings now read
the cancel through `unmetUiWaitCause`, the one reader that knows which side of
its final read the cancel landed on.
|
||
|
Hubert Gancarczyk
|
895d5e7ff0 |
fix(run-sequence): mark a wait that HELD as having sent nothing, like one that did not
`await-ui-element` is the only entry in ALLOWED_TOOLS that acts on nothing - it
polls the UI tree and never dispatches. The unmet arm says so with
`dispatched: false`; the success arm said nothing, so `reachedAStep` read a
`{ tool, result }` entry as a dispatch and scored a batch whose only completed
step read the screen as one that moved the device.
The recorder is what reads that. A batch of "wait for Continue, then tap it"
with a mistyped tap arg dispatches nothing either way, yet the unmet spelling is
refused in silence while the met one sends the author to `describe` and a
hand-restore. Driven over HTTP against a tool-server built from this branch and
a Chromium (CDP) target, both calls through the real `flow-add-step`: the met
wait now returns `dispatched: false` beside its result and the refusal carries
no mutation warning, while a batch whose first step really tapped (`taps=1` on
the page) still carries it.
The marker means "provably sent no action", which is a fact about the step, not
about whether it passed - so it now sits on both arms of `StepResult`.
|
||
|
Hubert Gancarczyk
|
2740a3ec38 |
chore(knip): clear the parked dead-code backlog and drop the ceiling to 0 (#770)
> Built on top of #769, so this PR carries that commit too — the workflows only run for PRs targeting `main`, which is why it is based there rather than on the branch. #769 on its own is the minimal, low-risk fix for the currently-red job; if it merges first this shrinks to its own commit, and if this one merges first it closes #769 as well. ## What this does #572 landed the gate with 215 findings parked under `--max-issues`. That bounded the backlog but left it in place, and the first thing to touch it (#663) pushed the total to 218 and turned the job red. This clears the backlog and sets the ceiling to **0**, so the next unused export fails the job instead of consuming headroom nobody knew was there. | | before | after | | --- | ---: | ---: | | Unused exports | 47 | 0 | | Unused exported types | 156 | 0 | | Unused exported class members | 12 | 0 | | `--max-issues` | 215 | **0** | ## Scope check first Every workspace holding a finding is `private: true`. `@swmansion/argent` — the one published package — had **zero**, so nothing here moves a public API. ## Two shapes of fix **Still referenced inside its own file → drop `export`** (146 types, 36 exports). The declaration stays put; it just leaves the module's public surface. `tsc` keeps emitting these into the `.d.ts` as local declarations that the exported signatures reference. For the 146 **types** that is the whole story: they are erased, so the emitted `.js` is byte-identical. For the 36 **values** it is not. Dropping `export` from a `const`/`function`/`class` removes its `exports.X` binding from the emitted `dist/*.js` — 36 declarations across 26 files here. `packages/tool-server/dist/utils/device-info.js` no longer carries `exports.REMOTE_PREFIX` or `exports.VEGA_SERIAL_PREFIX`; `telemetry/dist/posthog.js` loses `exports.POSTHOG_PROJECT_TOKEN`; in the ESM-emitting `argent-installer`, `isTempRunnerPath` / `resolveLocalArgentDir` / `PACKAGE_ROOT` leave the module namespace object entirely. Nothing in the repo or in the `packages/argent-private` submodule at the pinned revision reads any of them by path, so nothing breaks. Flagging it because a `require()` of a built `dist/*.js` loses a dropped `export` exactly the way it lost the deleted `__resetAndroidDevtoolsInstallCache` this branch had to restore — so this half of the diff does deserve runtime scrutiny, not a skim. **Referenced nowhere → delete.** Orphaned test seams (`__test`, `_testValidators`), stale compatibility re-exports, and a few real orphans. The ones worth a reviewer's eye: - **`localSimctl`** — the `SimctlBackend` strategy is only ever constructed with `remoteSimctl`. What keeps the local iOS impl off it is the handler signature: `buildIosLaunchHandler` / `buildIosRestartHandler` read `services.nativeDevtools`, and `ios-remote` is the only platform that declares that service, so the local impl — which resolves native-devtools per device inside its own handler — has nothing to hand them and shells out to `xcrun` itself. - **`sendCharInsert`** — removing the `__sendCharInsert` test alias exposed the underlying helper as unreferenced. It is superseded, not unwired: the Chromium keyboard impl dispatches its own `type: "char"` event inline (`keyboard/platforms/chromium.ts:58`), which is exactly what this helper did. Chromium typing is unaffected. - **Error metadata fields** — `ServiceNotFoundError.serviceId`, `ToolNotFoundError.toolId` and their two siblings were never read. The `e.serviceId` in `registry-error-events.test.ts` is a field on the event object the handler pushes, not on the error class; the `err.toolId` in `http.ts` is `NotImplementedOnPlatformError`, a different class that keeps its fields. Flagging these in case they are wanted as deliberate diagnostics. Deleting `SourceMapsRegistry.toGeneratedPosition` / `findMatchingSource` cascaded into their only helper (`buildSourceCandidates`) and then into `projectRoot`, which had no remaining reader — so the constructor parameter goes too. `StubSourceMapsRegistry` existed only to pass `super("")`, so it goes with it. ## The three cross-workspace members `ArtifactStore.register`, `ArtifactStore.list` and `TypedEventEmitter.off` are all called from other workspaces — the artifact route and screenshot tools in `tool-server`, and `registry-listener` in `telemetry`. They are reported unused anyway, and the reason is the **unbuilt tree** the gate runs against: every workspace resolves `main`/`types` to a `dist/` that does not exist, so `@argent/registry` resolves to nothing and the cross-workspace edge never forms. Build the tree and all three findings disappear on their own; a control member that really is dead is still reported in the built run, so the pass is live either way. Nothing about this is specific to `classMembers` — every cross-workspace reference is invisible the same way. Each is exempted at the declaration with a `@public` JSDoc tag rather than by name in `knip.jsonc`, for one reason: **scope**. `ignoreMembers` matches a name across the whole workspace, so it would also hide a future dead `register`/`list`/`off` on any other class in `packages/registry`; `@public` binds to the member it is written on. What `@public` does **not** buy is a staleness check, and neither does `ignoreMembers`. Measured on a clean unbuilt worktree: a bogus `ignoreMembers` name and a redundant `@public` each leave the run at exit 0 with no hint, while a bogus `ignoreDependencies` name exits 1 with `Remove from ignoreDependencies`. `treatConfigHintsAsErrors` is live and simply has no producer for either exemption style — so when one of the three stops being earned, nothing says so, and the JSDoc at each member is the only record a re-audit has. Each one names its callers, and `off` records how to re-derive them (rename the member and read the `Property 'off' does not exist` errors; grepping `.off(` over-reports, since 14 of the 38 call-site hits are Node `EventEmitter`s). One correction to the commit message on that change: it says 35 hits where the exact count is 38 — 14 Node `EventEmitter`, 24 `TypedEventEmitter` (21 production, 3 in a test). The 14 and the 21 are right. `knip.jsonc`, `CONTRIBUTING.md` step 5 and the workflow's failure-explanation step all described a parked backlog and a ceiling to stay under. All three are rewritten for a gate that must simply come back empty, and all three now name the `@public` escape hatch — the remedy for a symbol whose only caller is out of knip's reach. ## Verification Run against a clean worktree with **no build output**, the way CI counts: - `npm run knip` — **exit 0, prints nothing**. Both passes empty, no config hints. - `npx tsc --build` — clean. - `npm run lint` (`eslint . --max-warnings 0`) — clean. - `npx prettier --check .` — clean. - `npm test --workspaces` — tool-server 329 files / 3857 passed; registry, telemetry, update-core, tools-client, mcp, cli green. - `npm run typecheck:tests --workspaces` — 13 workspaces define the script and all 13 are clean, but **the command itself exits 1**: npm errors `Missing script: "typecheck:tests"` for the three that do not define it rather than skipping them. Worth knowing before re-running this bullet and reading the exit code as a failure. End-to-end on a tool-server built from this branch, driven over its HTTP API: - **Chromium** (Electron smoke app) — `describe`, `screenshot`, `gesture-tap` (counter advanced 0 → 1), `POST /api/clipboard/text` read back out of the renderer, the WS `clipboardSync` command, and `debugger-connect` / `debugger-status` / `debugger-evaluate` (`sourceMapReady: true` with `StubSourceMapsRegistry` gone). - **iOS** — `boot-device`, `launch-app`, `describe` via ax-service, `screenshot`. - **tvOS** — `boot-device`, `launch-app` on an Apple TV udid, focus-driven `describe`. The registry snapshot then carries `NativeDevtools:<tvOS udid>`, confirming injection **is** resolved on tvOS through the local impl. - **Android** — `boot-device` on a dedicated AVD, `describe` via android-devtools, `gesture-tap`. - **Metro source maps** — the registry driven against a real 12.5 MB Metro `.map` over loopback, the way `Debugger.scriptParsed` drives it: allowlist passes and `waitForPending()` resolves on both, while retained heap after gc drops from 17.3 MB to 3.6 MB. ## Review follow-ups Five commits on top, one per finding. The base is back on `main`, so the workflows run again. - **`docs(launch-app)`** — the services comment explained the empty shape through `LaunchAppAndroidServices`, deleted here, and skipped `LaunchAppVegaServices`, which is still there. Same edit in `restart-app/types.ts`. - **`docs(registry)`** — the `@public` tag on `ArtifactStore.register` named 2 of 7 callers. Derived the full set the way the sibling tag on `off` prescribes: renamed the member, read the 16 `TS2339` errors `tsc --build` reports across flow-visual (5), screenshot (4), native-profiler-stop (2), screenshot-diff (2), native-profiler-analyze, react-profiler-analyze and screen-recording-stop. The re-derivation recipe is recorded too — `.register(` greps as badly as `.off(`. - **`docs(knip)`** — "the build, the tests and this gate all stay green on a wrong delete" held for argent-private and not for the cross-workspace case that justifies three of the four `@public` tags. Deleting `ArtifactStore.register` gives 16 `TS2339` errors and `9 failed | 2 passed` in `test/artifacts.test.ts`; only the gate stays green. Split in the workflow comment, the failure annotation and `CONTRIBUTING.md` step 5. - **`perf(debugger)`** — `doRegister`'s `data:` branch still base64-decoded and `JSON.parse`d a payload it dropped, inside a `catch` that swallows. Both arms returned the same `void`, so the branch collapses to an early return; `scriptUrl` / `scriptId` go with it, since their last reader was the deleted `this.maps.push(...)` and `doRegister` is private. - **`test(debugger)`** — the allowlist check moved to a top-level early return with nothing pinning it: delete the line and both SSRF files still pass 17/17. Two tests now assert `doRegister` consults it — four rejected URLs reach no `fetch`, and the loopback `*.map` Metro emits still does. Re-verified after: `npm run knip` exit 0 and empty on a `tsc --build --clean` tree, `tsc --build`, `eslint . --max-warnings 0`, `prettier --check .` all clean, and `npm test --workspaces` with tool-server at 329 files / 3854 passed. The one red test, `argent-installer`'s `globalPath returns ~/.config/opencode/opencode.json`, fails identically with these commits stashed — it reads the real `~/.config/opencode`, which holds a `.jsonc` on this machine. End-to-end on a tool-server run from this branch over its HTTP API, on every platform: - **Chromium** — `launch-app`, `describe`, `screenshot` (returned an artifact handle, `GET /artifacts/<id>` served the 137542-byte PNG back), `gesture-tap` (counter 0 → 1), `debugger-connect` / `debugger-status` (`sourceMapReady: true`) / `debugger-evaluate` (read `taps: 1` out of the page). - **iOS** — `launch-app`, `describe` via native-devtools, `restart-app`, `screenshot`. - **tvOS** — `launch-app`, focus-driven `describe` (12 focusables in TVSettings), `tv-remote`, `restart-app`. - **Android** — `launch-app`, `describe` via android-devtools, `restart-app`, `screenshot`. - **Vega** — `boot-device` on the `tv` VVD, `describe` via the automation toolkit. - **Metro** — the real `debugger-connect` / `debugger-status` / `debugger-evaluate` tools against a stand-in Metro (HTTP `/json/list` + CDP WebSocket) emitting three `Debugger.scriptParsed` events: a loopback `*.map` on a counting server, a 1.4 MB inline `data:` map, and `http://169.254.169.254/latest.map`. `sourceMapReady: true`, `loadedScripts: 3`, evaluate 42, the loopback map fetched exactly once and the metadata URL never. A malformed inline payload gives byte-identical output, which is the finding. ### Merge with `main` Putting the base back on `main` made CI resolve the merge, and the Unit Tests job went red on a conflict no side could see on its own: `#771` fixed the same red gate this branch's first commit did, the other way round — it made `component-names.test.ts` import `StrippedName`, `ComponentAnnotation` and `ComponentNameResolution` instead of dropping their `export`. Merged `main` in, reproduced it (`tsc --noEmit -p tsconfig.test.json` → three `TS2459`s) and took main's resolution: the three exports are back. The gate stays green because knip treats test files as entry points, so a type a test imports is not an unused export. All nine workflows are green on the head commit. |
||
|
Ignacy Łątka
|
2e19c97de0 |
fix(cli): accept --help after a telemetry subcommand (#959)
Found while differentially regression-testing the 0.22.1 release against a bundle built from `v0.22.0`. ## The problem `argent telemetry status --help` exits 2 with `Error: Unknown flag: --help`. So do `-h`, and the same after `enable` / `disable`. On 0.22.0 all of them exited 0. ``` $ argent telemetry status --help 0.22.0 telemetry: / state: enabled / device id: not created (rc=0) 0.22.1 Error: Unknown flag: --help (rc=2) ``` `telemetry` is the only subcommand that behaves this way. Checked against every sibling on both versions — `server status|logs|start`, `config list|get|set`, `flow list|run`, `secrets`, `flags`, `enable`, `link`, `unlink` all return 0 on both. And the top-level help tells the user to run exactly this: "Run `argent <command> --help` for command-specific help." ## Cause `TELEMETRY_OPTIONS` declares only `scope`, and `parseCommandArgs(args.slice(1), …)` reads everything after the subcommand — so `--help` arrives as an unknown flag. The `case "--help"` arm in the switch only fires when help is `args[0]`, which is why bare `argent telemetry --help` still works. Introduced when `telemetry` was converted to the declarative parser (#952, extended by #953). ## The fix Hoist the help check above the parser, matching what `config.ts:220` and `flags.ts:92` already do; `server`, `link` and `lens` instead declare `help` in their option spec. Telemetry did neither. Ordering of `telemetryInit` / `telemetryShutdown` around the help path is unchanged, so a help invocation behaves exactly as `argent telemetry --help` did before. ## Tests Six cases added to `telemetry-command.test.ts` covering `--help` / `-h`, bare and after each subcommand, asserting usage is printed and no config file is written. Mutation-checked: with the source change reverted, 4 of the 6 fail (the 2 bare-help cases pass either way, since those already worked) — so the test pins the actual defect rather than the behaviour around it. `@argent/cli`: 498 passed, 3 skipped. `tsc --build` clean, prettier clean. ## Docs No docs update needed — this restores previously documented behaviour rather than changing a documented surface. Co-authored-by: Ignacy Łątka <f.kaminski2000@gmail.com>v0.22.1 |
||
|
filip131311
|
efb87f1917 |
chore: bump version to 0.22.1 (#956)
Bumps every workspace package, `package-lock.json`, and both version fields in `server.json` from 0.22.0 to 0.22.1. - `npm version 0.22.1 --workspaces --no-git-tag-version --workspaces-update=false` - lockfile regenerated on Node 24 / npm 11 (version-only diff, 16 lines each way) - `npm run check:versions` passes No docs update needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01EjrDqoeJa5X8Ry2ND1P714 |
||
|
Hubert Gancarczyk
|
af0264b50d |
docs: cut this branch's comments back to what the code cannot say
Delete the comments this branch added that narrate, explain or restate what the code beside them already says. What stays is the handful that encode a constraint a reader cannot recover from the code: the `aborted`-only scope of DirectiveOutcome.reached, the one-line definitions of NestedOutcome.reached and the `dispatched: false` marker, the flow-lock window activeFlowState reads its step count in, and why a cancelled wait is scored from the wait's own cause rather than the run's signal. Comment-only: every changed line is a comment line. |
||
|
filip131311
|
26405e68c7 |
refactor(cli): one declarative option parser for every hand-written subcommand (#953)
Follows #952 (merged); rebased onto main. ## Why `argent-cli` had **seven** copies of the same argv loop — `server start`, `link`/`unlink`, `flow run`, `lens`, `config`, `enable`/`disable`, and (from #952) `telemetry` — each hand-rolling `--x value` / `--x=value` / `-x` / unknown-flag / missing-value handling with slightly different messages and gaps (e.g. `lens` silently ignored unknown flags). Adding an option meant another `if (tok === "--x") … else if (tok.startsWith("--x="))` pair. ## What `packages/argent-cli/src/command-args.ts` (introduced in #952) now covers the whole subcommand surface, and every command declares its options as a spec: ```ts const START_OPTIONS = { help: { kind: "boolean", alias: "h" }, port: { kind: "value", alias: "p" }, … } as const satisfies OptionSpecs; ``` Parser features: `--name value`, `--name=value`, single-letter aliases, boolean switches, `--` end-of-options, `choices` validation, "don't swallow a following flag as a value", and the guard against a stray `true`/`false` word after a switch (previously duplicated in `flow` and `lens`). Value *semantics* (port ranges, host wildcards, flag-name charset, link targets) stay in each command — the parser only shapes argv. `flag-parser.ts` is untouched: it is the schema-driven `argent run <tool>` payload builder (JSON Schema → tool args, `--args`/stdin hatches), a different job. ## Behavior changes (bad-input paths only) | Command | Before | After | |---|---|---| | `lens --bogus` / `lens extra` | silently ignored | `lens: Unknown flag: --bogus`, exit 2 | | `lens --terminal x` | `--terminal expects "iterm" or "terminal", got "x"` | `--terminal must be "iterm" or "terminal", got "x"` | | `flow run --platfrom=ios` | `unknown flag --platfrom=ios` | `Unknown flag: --platfrom=ios` | | `unlink foo` | `Unknown flag: foo` | `Unexpected argument "foo"` | | `config … --scope x` | `--scope must be "global" or "project", got "x"` | unchanged wording | | `enable/disable`, `server start`, `link` | — | unchanged wording (tests pinned) | Happy paths are byte-identical; `server`/`link` still throw `StartFlagError`, `flow` still throws `FlagParseException` (wrapped from `UsageError`), so callers and tests keep their contracts. ## Tests `argent-cli` 489/489 (26 files): rewritten `command-args.test.ts` (aliases, `-` positional, empty-token value, true/false guard, choices message), pinned-string updates only where the table above says the wording changed (`flow.test.ts` ×4, `parse-link-flags.test.ts` ×1). `tsc --build`, `typecheck:tests`, eslint, prettier, knip green. Smoke-tested every command's error path through the built `packages/argent/dist/cli.js` (see the "Behavior changes" table — all exit 2 with usage where the command prints one). ## Docs No docs update needed: the pages document flags and their meaning, not error strings; no flag was added, removed or renamed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01BkkAihA8Kp26hHS4uhkNGu |
||
|
Jakub Piasecki
|
2e182f7971 |
fix(flows): pin iOS tree reads to the launched app instead of auto-resolving every connection (#772)
## The problem Flow runs on CI iOS fail with `ViewInspector RPC timed out: Application.getState` at seemingly random steps - mid-flow after typing, on the first read after a fresh launch, in a probe flow 25 s after the original failure - while the app under test is provably healthy: spindumps taken at failure time show its main thread idle in the runloop, display links firing, and the injected devtools queue empty. The same flows pass locally. The chain: argent's iOS injection is simulator-wide (`launchctl setenv DYLD_INSERT_LIBRARIES` inside the simulator's launchd), so every process the simulator spawns afterwards loads the devtools dylib, not just the app under test. The dylib's constructor connects unconditionally (no UIKit or `com.apple.*` guard) and the tool-server registers whatever bundle id shakes hands, so Spotlight workers (`mdworker_shared`), background-launched system apps doing first-boot work (`MobileCal`), and bare daemons (nil bundle id, registered as `"unknown"`) all become connections indistinguishable from the app under test. Flow tree reads auto-resolved the target on every read: `queryFullHierarchyTree` passed `undefined` to `resolveNativeTargetApp`, which `Promise.all`s `Application.getState` over every connection. The dylib serves `getState` with a `dispatch_sync` onto the main thread, and a background-launched or suspended process never services its main queue, so its `getState` parks until the 5 s RPC timeout and the `Promise.all` rejects. One poisoned sibling connection fails every tree read in the run; the app under test is never even asked. That one mechanism explains everything the failures did. CI spindumps caught the poisoner red-handed (`MobileCal` parked in `HandleViewInspectorRequest -> CurrentApplicationState`) next to a healthy app. "Permanent wedge" vs "recovery" was just poisoner lifetime: a Spotlight worker exits and its connection drops with it, so reads recover; Calendar lingers and the whole job stays poisoned, fresh app launches and probe flows included. And it never reproduced locally because a warm simulator did its first-boot indexing and migrations long ago, while CI boots a pristine device and does exactly that work during the test window - which also made the failures correlate with typing and tapping purely by timing. ## The fix Pin the target instead of resolving it per read. `runLaunch` records the launched app on the run state once the tree-source gate passes, as `ActionEnv.treeTarget` - a `FlowTreeTarget` carrying the `bundleId`, whether the runner still vouches for it (`pinned`), and whether its probe has ever answered in this run (`probeAnswered`). `fetchFlowTree` threads that target down to `queryFullHierarchyTree`, the only platform source that consumes it. A pinned read talks to the pinned app and nobody else: it never calls `resolveNativeTargetApp` at all, and runs four gates of its own instead (`flow-ios-tree.ts`). A `com.apple.*` id is refused first, terminally and before anything is spent - a system app is never the app under test. Then `isConnected`, a map lookup, so a dead pin reports the dropped connection instead of driving env repair from 300 ms read polls, which could otherwise latch the device's process-wide give-up. Then a probe of the pinned app's own `getAppState`, and only that app's: a timeout before this pin has ever answered is ridden out as the cold start it usually is, one after an earlier answer is refused as an app that stopped servicing its main queue. Finally a foreground check on the answered state, as lenient over one app as auto-resolve is, so a flow that left the app fails with the crisp diagnosis instead of describing a screen that is not on screen. A poisoned sibling connection is never touched. The pin is only trusted while the runner can vouch for what is on screen, and it comes off in two tiers. The five `FOREGROUND_CHANGING_TOOLS` (`launch-app`, `restart-app`, `reinstall-app`, `open-url`, `button`) drop the target outright, because the launched app may no longer be on screen at all; `launch-app` and `restart-app` then re-set it from their own `bundleId` arg, unpinned, since they name the app they switched to - which is how a recorded flow switches apps, the recorder keeping them as raw `tool:` steps. Every other raw `tool:` step demotes the pin to an unpinned hint, its effect on the foreground being opaque to the runner; a launch attempt drops the target outright until it succeeds. Either tier also retires a proven tree-outage memo: a verdict proven against the pinned branch's four gates says nothing about the path the next read runs. An unpinned read is the pre-pin behavior with one arbiter added. `resolveNativeTargetApp` fans out over every connection, and only when that fan-out times out - the CI condition this change exists for - does the hint take the read, and only after passing the same terminal `com.apple.*` policy gate, the connections check, and a foreground-checked probe of its own. A resolution that answers always wins, including its deliberate "single app but backgrounded" refusal, so the arbiter never overrides a guard that fired: it only rides out a probe a stalled sibling made unanswerable. Nested `run:` fragments share the run state, so they inherit the pin, its demotion and its clearing. E2e flows begin with a `launch` by definition, so every selector read in a CI run happens pinned. Only iOS consumes the target; the other platforms' tree sources are per-device and never auto-resolve. The review rounds also surfaced that the `isInjectableBundleId` "library validation, the dylib can never load" rationale was disproven by this PR's own spindump evidence, so that text - and every other home of the same claim, `unreadableHierarchyReason` and `treeSourceGate` included - is rewritten as the policy it actually is (system apps are never the app under test; their observed connections are background processes whose reads hang or lie) with the gates' behavior unchanged. ## What this doesn't cover Auto-resolve's own fan-out is still all-or-nothing, so every read that reaches it without a hint the gates accept stays poisonable: top-level fragment runs, `describe` without an explicit bundle id, and the stretch after a raw `tool:` step whose hint is refused or cannot answer. Recording (`captureTapSelector`) now passes the session's last `launch` id as an unpinned hint, so it arbitrates the same timed-out fan-out, but it never pins - the recorder cannot vouch for the foreground app the way a `launch:` step can. Closing the rest means hardening auto-resolve itself (exclude connections that don't answer instead of failing the fan-out) and refusing the connections at the source (dylib constructor bail for non-UIKit and `com.apple.*` processes, plus a server-side handshake filter to cover already-shipped dylibs). Both are follow-ups. ## Tests Three files, and every behavior below is mutation-proven: disabling the seam, dropping the target from any single call site, removing an invalidation, or removing any one of the four gates each fails at least one dedicated test. The reviewers' repros were turned into tests rather than argued away. `flow-ios-tree-pinned-target.test.ts` works at the tree source. It reproduces the failure shape - a healthy app plus a poisoned `com.apple.mobilecal` sibling whose `getState` only times out - and proves a pinned read probes only the pinned app while an unpinned read still fails. It then covers the states auto-resolve could never reach: the terminal `com.apple.*` refusal, including its position ahead of the connections gate so a disconnected system app is not sent into a restart loop; a dead pin, where the dropped connection is reported and no env repair runs; both sides of the `probeAnswered` split; and the foreground guard over the states a real device produces (`inactive` under a permission dialog, `inactive` with no scenes, `background` still holding a foreground-inactive scene, and `background` with none). A second block covers the unpinned arbiter: the hint read on a timeout, refused when its own probe answers from the background, and rethrowing the fan-out timeout when the hint is disconnected, cannot answer, or when the resolution failed for any other reason. `flow-launch-pins-tree-target.test.ts` proves the runner wiring through the real `fetchFlowTree` seam: every read after a `launch` carries the pin across all five read paths (assert, tap, type, rotate, idle), retry polls included; fragments inherit it; a foreground-neutral `tool:` step demotes it and a foreground-changing one that names no app drops it outright; a later launch re-pins on a fresh target with the ride-out re-armed; and a launch-free run keeps the fallback. `flow-tool-step-tree-target-e2e.test.ts` drives `createRunFlowTool` with only `resolveService` stubbed, so the real `fetchFlowTree` and `queryFullHierarchyTree` run. It pins what the two tiers cost against a wedged sibling: the launched app still arbitrating across a foreground-neutral `tool:` step, the outage memo being retired when that step demotes the pin, the `com.apple.*` and foreground refusals staying in force through the demote, and a `hidden:` assert failing after the demote instead of going green against a backgrounded app. |
||
|
Hubert Gancarczyk
|
62253d9a26 |
docs(flow): reflow the reached doc after the directive clause changed
|
||
|
Hubert Gancarczyk
|
29b751d4c9 |
docs(flow): say which orchestrator the clean-finish carve-out covers
"A cancel in the trailing delay after the last declared step is clean" holds for `run-sequence` alone. `flow-execute` has no trailing-delay exit - its per-step delay sleeps before the sub-tool - and `summarize` samples the cancel flag once the steps are done whatever they scored, so a composed run whose every declared step PASSED still comes back `aborted: true` and is refused. Stated where the carve-out is written, and pinned from both sides: a real `run-sequence` cancelled in its trailing delay records, and a real composed run cancelled after its last step passed does not. |
||
|
Hubert Gancarczyk
|
40e3644df5 |
fix(flow): mark a cancelled directive reached only when its gesture went out
A directive cancelled mid-step was reported `reached` unconditionally, on the grounds that its outcome could not say which side of the dispatch the cancel landed on. It can. A `tap` resolves its target off the tree first, and a coordinate `tap` and a first-round `scroll-to` settle before their first increment - all three return their aborted outcome from the settle, before `invokeOnDevice` runs. Settles dominate a step's latency, so that is where a cancel usually lands, and the recorder turned every one of them into "Prior composed steps may already have changed the device" - the opposite of the tool description's promise to stay silent on a cancel that landed before the first dispatch. `invokeOnDevice` is the single point through which a directive acts, so it is what answers: `runDirective` installs a per-step dispatch scope, the dispatch sets it before the call goes out, and an aborted outcome carries the result. Everything downstream of a dispatch - a cancelled `type` after its focus tap, a `scroll-to` after an increment, a `pinch` between sub-gestures - still reports `reached`. |
||
|
Hubert Gancarczyk
|
32547366b9 |
fix(flow): score a cancelled wait from the wait's own cause, not the run's signal
The guard that stops a cancelled `await-ui-element` from being blamed on the app read this run's signal. A GENUINE miss whose result resolves in the same tick as an unrelated cancel satisfies that check too, so it was scored `skip` / "run aborted during wait" instead of the failure it earned - against the principle the guard's own comment states, that a tool which finished and answered before the cancel arrived keeps its verdict. The wait already reports which it was. Only its poll loop knows whether the cancel landed before or after the read that judged the condition, and it records `cause: "cancelled"` exactly where it gave up. Read that instead. |
||
|
Hubert Gancarczyk
|
474c374948 |
fix(run-sequence): mark an unmet wait as having sent nothing to the device
`dispatched: false` marks the entries a `run-sequence` step produces without
acting on the device, and the flow recorder reads it to decide whether to warn
that a refused batch may have moved the screen. Three of the four such exits
carried it; the fourth did not.
An `await-ui-element` whose condition never held polls the UI tree and stops
there. Its entry looked like a step that acted and then failed, so a sequence
gated on a wait as its FIRST step was refused with "after 0 of 2 steps" and the
mutation warning in the same sentence - the pairing the marker exists to
prevent, and the opposite of what the tool description promises ("stays silent
when the refusal provably reached none").
The marker now means "sent no action to the device" rather than "was rejected
before the call": three of the four are rejected ahead of `execute`, and the
wait is the one that runs and only reads.
|