skills/software-mansion__argent
1
0
Fork 0
mirror of https://github.com/software-mansion/argent.git synced 2026-09-14 19:27:14 +08:00
Code Issues Packages Projects Releases Wiki Activity
708 Commits 225 Branches 38 Tags
feat/flow-script-drop-logs
Commit Graph

609 Commits

Author SHA1 Message Date
Hubert Gancarczyk fdd1774ca4 fix(flows): stop reporting what a script prints, on both sides 
A `script:` step used to bring its stdout and stderr back with it. The
recorder returned `log`/`logTruncated`, the step report carried
`scriptLog`/`scriptLogTruncated`, and the CLI and the MCP content builder
each rendered them. Both surfaces land in an agent's context, and nothing
in the flow file declares what a script prints, so nothing can decide in
advance whether that text is safe to forward. The channel goes rather than
being gated behind a parameter the agent would set itself.

A script step now answers with its verdict and its `reason` only. Deleting
the capture removes what existed to bound it: the 64 KiB per-step cap, the
256 KiB run-wide budget, the V8 frame collapser, the partial-marker guard
and the streaming half of the secret scrub.

Both pipes are still drained, and stderr is still scanned for V8's
heap-exhaustion banner and for a watchdog that never armed.
 2026-09-09 16:23:04 +02:00
filip131311 02fd22e9d6 chore: bump all versions to 0.25.0 (#1083) 
Bumps every workspace package, `package-lock.json`, and both version
fields in `server.json` from 0.24.0 to 0.25.0.

- `npm version 0.25.0 --workspaces --no-git-tag-version
--workspaces-update=false`
- lockfile regenerated with `npm install --package-lock-only
--ignore-scripts` on Node 24 / npm 11 (version-only diff)
- `npm run check:versions` passes

No docs update needed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01XdSDvFDvuU1iyz7HPjcHMV

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
 2026-09-09 15:00:24 +02:00
Arkadiusz 9d512ce933 optimization: simplify flow-run description (#954) 
# Why

It is reported that benchmark results are getting worse and the
`flow-execute` description has the largest optimization potential. The
description field of MCP tools should describe what the tool does and
when to call it, not reveal implementation details.

<img width="1195" height="382" alt="Screenshot 2026-08-25 at 12 45 30"
src="https://github.com/user-attachments/assets/d83ba9c5-5e4f-4c65-ba3a-4af0588e5faa"
/>

# Warning

Not tested. I'd have to run this change twice against the benchmark repo
but that feels like overkill. I am not sure about behavioral
regressions, relying on CI here.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Documentation**
- Updated the Flow YAML reference to streamline directive documentation.
- Revised the flow execution tool’s description with a concise overview
of supported flow sources and per-step reports.
- Clarified documentation around flow behavior, including warning
handling and project-root configuration.
- Removed outdated or overly detailed statements from the published
guidance to keep the reference accurate and easier to scan.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-09 14:21:30 +02:00
Ignacy Łątka 16ad44bb0e test(telemetry): pin the wire format and what a dead collector costs (#736) 
Two test files that look at what #570's exporter actually *does*, plus
the configuration holes they exposed.

The suite on #570 proves the exporter is **configured** right - the url,
the headers, the timeouts, all read off the constructor. Nothing looked
at the request that comes out of it, or at what happens when the
collector does not answer. Both gaps are invisible from inside Argent by
design: a failed export never reaches the caller, and an OTLP exporter
counts any 2xx as delivered.

| | |
|---|---|
| **`otel-wire.test.ts`** | Captures the real exporter's request off a
loopback socket: uncompressed OTLP/JSON on `POST /v1/logs`, where each
field the ingestion schema reads actually sits, the batch-size cap, and
connection reuse. |
| **`otel-unreachable.test.ts`** | Drives the same exporter at a silent
socket, a refused port, a 503, a 401 and a 204, and asserts each drain
finishes inside its deadline and what the collector really received. |
| **`src/otel.ts`** | Two holes the tests exposed: `compression` and the
six OTLP certificate variables are now settled in code instead of left
to the environment. |

**No docs update needed** - no user-facing capability, CLI surface,
config key, flow file or MCP tool changed.

<details>
<summary><b>The two source fixes</b></summary>

**`compression` was the one OTLP knob still left to the environment.**
With `OTEL_EXPORTER_OTLP_COMPRESSION=gzip` (or its `_LOGS_` variant) set
- and a machine already running OpenTelemetry has it set for its own
collector - argent gzips its batches. Nothing downstream notices: a
gzipped 2xx is a delivered batch, while the request this suite and the
deploy-time smoke test are written against quietly stops being the
request argent sends. `createExporter` now passes `compression`
explicitly, which beats both variables. Unlike the header channel, where
the SDK *merges* whatever the code does not set and the variables have
to be cleared outright, one explicit value settles this.

**The certificate variables could hang the command outright.** The SDK
resolves
`OTEL_EXPORTER_OTLP_{,LOGS_}{CERTIFICATE,CLIENT_CERTIFICATE,CLIENT_KEY}`
with a synchronous `fs.readFileSync` while the exporter is constructed,
and only then discards the https agent it built from them in favour of
the explicit `httpAgentOptions`. The file is read for nothing - and a
path that never answers hangs the command that emitted the event,
uninterruptibly, on `getClient()`'s path. Measured with a fifo that has
no writer: `createExporter` never returned (killed at 12s); with the
variables cleared, 1.0 ms. They join the header pair in the list cleared
across the constructor and restored on the way out.

Some source prose was also wrong about the SDK and is corrected: it does
**not** reject a null attribute - it serializes one as an empty OTLP
value, `{"key":"cloud_agent","value":{}}` with `droppedAttributesCount:
0`, stored as and unrecoverable from a property that really was empty.
That is the actual reason `toAttributes` drops the key.

</details>

<details>
<summary><b>What the tests pin, and what they deliberately do
not</b></summary>

The wire test **mirrors** `OtelClient.emit` rather than calling it:
`getClient()` resolves its endpoint from the hard-coded
`OTLP_LOGS_ENDPOINT`, and that being unredirectable is the
anti-exfiltration property the client is meant to have, so there is
deliberately no seam to aim it at a test server. `otel-endpoint.test.ts`
covers the other half against the real emit path - `SERVICE_NAME`, the
logger name, event-name-as-body, the dropping of null/undefined
properties - so a rename is caught there rather than in the mirror.

What enforces the drain budgets is the exporter's `timeoutMillis`, *not*
the processor's `exportTimeoutMillis`, which `forceFlush()` awaits
straight past. Measured against a silent socket: `exportTimeout` 200 /
`timeoutMillis` 5000 drains in 5012 ms, the reverse in 202 ms.

503 is the one collector state where the batch is sent twice, and the
request count is the only thing that tells a retried failure from a
rejected one - the SDK reports the class, not the code. The retry itself
is only reachable while the next backoff fits in what is left of
`timeoutMillis`, so the jitter is pinned to its low draw and the
assertion turns on the round trip alone.

`httpAgentOptions.timeout` is the one option with no behavioural
coverage: it covers a socket stuck in *connect*, which needs an address
whose packets are dropped and is not portably reproducible here. It
stays pinned at the constructor and the docstring says so. Its
`keepAlive` half **is** covered, on the two batches of a single drain.

Real deadlines cost real time: `otel-unreachable.test.ts` waits ~5.1 s,
taking the package suite from ~0.9 s to ~5.3 s. Inherent to driving real
deadlines against real sockets.

</details>

<details>
<summary><b>Verification</b></summary>

- `npm test -w @argent/telemetry` - **336 passed**, 19 files
- `typecheck:tests`, `eslint --max-warnings 0`, `prettier --check`,
`knip`, `tsc --build` all clean; CI green
- **Mutation-tested**, whole-suite counts rather than one test each:

  | mutation in `src/otel.ts` | tests red |
  |---|---|
  | `compression` pin dropped | 2 |
  | `compression` pinned to `"gzip"` | 5 |
  | any one of the 8 cleared env-var names dropped | 1 each |
  | env-clearing loop deleted | 2 |
  | the `finally` that restores it deleted | 1 |
  | `timeoutMillis` dropped | 5 |
  | `httpAgentOptions` dropped, or just its `timeout` | 1 |
  | `keepAlive` flipped to `false` | 2 |
  | `installDiagLogger`'s `!isDebugEnabled()` guard removed | 1 |
| `maxExportBatchSize` 20 → 19 or 25; `scheduledDelayMillis` changed | 2
|
  | `SERVICE_NAME` renamed | 2 |
  | `LOGGER_NAME` changed | 1 |
  | record body no longer the event name | 4 |
  | `severityText` INFO → WARN | 1 |
  | null/undefined property skip removed | 2 |
  | `url` given a suffix the config did not set | 9 |

- **Absence assertions carry positive controls.** The certificate test
reads its answer out of a `Failed to read ` prefix that lives in a
transitive dependency, and the 204 case asserts an empty diag channel;
both would pass just as well against a dead observer. Each now proves
its own observer live first.
- **Flake-tested.** The 503 case's exact two-request assertion used to
turn on the jitter draw: standing a 500 ms collector delay in for a
loaded runner made it fail 20/20 (13 on the request count, 7 on
`Timeout` masking the error class). With the jitter pinned, 20/20 pass.

</details>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
- Improved telemetry export reliability by preventing unintended
compression and certificate-related environment settings from affecting
exporter configuration.
- Ensured telemetry attributes with null or undefined values are omitted
rather than serialized as misleading empty values.
- Improved handling of unreachable or unresponsive telemetry collectors
with bounded failures and appropriate diagnostic reporting.
- Preserved connection reuse and batching behavior for more efficient
OTLP delivery.

- **Tests**
- Added comprehensive coverage for OTLP formatting, authorization,
batching, compression, certificates, connection handling, and failure
scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-09 12:39:35 +02:00
Ignacy Łątka 82db9e2f9f test: stop tests leaving files in the shared system temp directory (#1057) 
Three issues, one theme: tests and the shared system temp directory.

- **#858 / #854** were live leaks — three call sites captured a real
temp path and never removed it. One reaches the cleanup its file already
had; the other two get a cleanup the file lacked.
- **#836** turned out to be **already fixed on `main`** by #714, which
scoped those three tests' `TMPDIR` to a directory they own. This PR
verifies that rather than changing it — reproduction and mutation
evidence below.
- Sweeping the same shape across the two packages turned up **six more
test files** doing it, worth **44** leftover entries per run.
- A third shape, in **two** more files: `resolveFileInputs` hands its
caller a `cleanup` for whatever it materialized — the dispatcher's job
in production. Five call sites kept the result and never called it,
stranding 5 dirs per run.

A scoped-`TMPDIR` run of all 12 affected files leaves **one** entry
behind: an empty `argent-chromium-media/`, production's own fixed
staging dir. 173 tests, no other file or directory.

Fixes #858
Fixes #854
Closes #836

<details>
<summary>#858 — before/after temp scope</summary>

```
rm -rf /tmp/scope && mkdir -p /tmp/scope
TMPDIR=/tmp/scope TMP=/tmp/scope TEMP=/tmp/scope \
  npx vitest run --root packages/tool-server test/chromium-server-screenshot.test.ts
```

| site | before | after |
|---|---|---|
| `chromium-server-screenshot.test.ts` |
`argent-chromium-media/argent-screenshot-test-1788803062089-38318.png` |
*(no PNG)* |
| `chromium-cdp-blueprint.test.ts` |
`argent-chromium-media/argent-screenshot-chromium-cdp-52204-1788803091335-39401.png`
| *(no PNG)* |

Both files keep an empty `argent-chromium-media/` — that is production's
fixed staging dir (`src/chromium-server/cdp-session.ts:218`), shared
with any live chromium-server on the machine and not per-run garbage. A
test `rm`ing it would be the machine-global-resource defect #714 removed
elsewhere, so it is left alone deliberately.
</details>

<details>
<summary>#854 — before/after temp scope</summary>

```
TMPDIR=/tmp/scope … npx vitest run --root packages/argent-mcp test/content.test.ts
```

Before (`34 passed`, two directories left):
```
argent-mcp-content-kPfgYa/context.diff.png
argent-artifacts/test-temp-dir-hygiene-037c31/20260907-174453-39458/home-diff.png
```

After: **empty**. The `mkdtemp` moved onto a `beforeEach`/`afterEach`
pair mirroring the sibling describe, and `flowRunToMcpContent` now pins
`ARGENT_ARTIFACTS_DIR` the way the artifact describe above it already
did.

The unconditional `delete process.env.ARGENT_ARTIFACTS_DIR` is kept, not
converted to a save/restore: it is what keeps any future unpinned write
landing in `tmpdir` rather than a developer's configured artifacts
directory.

The pin is not hygiene-only — it is asserted. Removing
`process.env.ARGENT_ARTIFACTS_DIR = root` fails the test:
```
- diff: /tmp/scopeM4/content-flow-artifacts-LZKPAl
+   diff: /tmp/scopeM4/argent-artifacts/test-temp-dir-hygiene-037c31/20260908-100837-89701/home-diff.png
❯ test/content.test.ts:627  expect(artifactText?.text).toContain(`diff: ${root}`)
```
</details>

<details>
<summary>#836 — why no code changed, and the evidence</summary>

(This PR does change `file-inputs.test.ts`, but for the unrelated third
shape above — not for the sites #836 cites.)

The issue cites `readdir(os.tmpdir())` at `file-inputs.test.ts:180-197`
and `flow-visual.test.ts:586-602`. On current `main` neither exists:
#714 replaced both (and the `http-upload.test.ts:154-158` sibling the
issue also flags) with a per-test scratch directory the test creates,
points `os.tmpdir()` at via `redirectTmpdir`, and removes. There is now
**no** `readdir` of `os.tmpdir()` anywhere in either package.

The per-path assertion the issue asks for is not reachable at these
three sites — every name is a `mkdtemp` random suffix or a server-side
`randomUUID`, so the test cannot know the path. Scoping the tmpdir to a
directory the test owns is the same guarantee, and is what landed.

**Reproduced the reported failure** by reverting each site to the
global-listing shape, then running two simultaneous instances in a loop:

| file | pre-#714 shape | current |
|---|---|---|
| `file-inputs.test.ts` | **1 / 12** — `expected [
'argent-file-input-00mPU7', …(1371) ] to deeply equal [ …(1370) ]` | **0
/ 12** |
| `flows/flow-visual.test.ts` | **12 / 12** — `expected [
'argent-flow-crop-00R9XZ', …(4853) ] to deeply equal []` | **0 / 12** |
| `http-upload.test.ts` | — | **0 / 12** |

**Mutation-tested all three assertions** — each still catches a real
leak; production cleanup broken, test run, source restored:

| assertion | mutation | result |
|---|---|---|
| `file-inputs.test.ts:249` | drop `await cleanup()` from
`resolveFileInputs`' catch path | ✗ `+ [ "argent-file-input-amPJuK" ]` |
| `flow-visual.test.ts:610` | drop `cleanupDiffDir(cropDir, …)` from the
outer `finally` | ✗ `+ [ "argent-flow-crop-rWT9Bd" ]` |
| `http-upload.test.ts:187` | drop `rm(tarPath)` from the
`req.on("close")` handler | ✗ `+ [ "argent-upload-43d82021-….tar.gz" ]`
|
</details>

<details>
<summary>Sibling sweep — the same two shapes across both
packages</summary>

**Shape "assertion diffing a listing of `os.tmpdir()`": zero
remaining.** No `readdir`/`readdirSync`/`glob` over `os.tmpdir()`,
`tmpdir()` or a literal `/tmp` in either test tree. Every literal `/tmp`
string left is a fixture handed to a mocked API; the three that do touch
disk were checked by hand and are clean
(`native-devtools-socket-bind.test.ts` mkdtemps and removes,
`flow-utils.test.ts` is pure path math,
`native-profiler-external-device.test.ts` writes only inside its own
per-test `mkdtemp`).

**Shape "a captured temp path never cleaned up": six files fixed.** Each
gets one recording choke point plus an `afterEach`.

Running those six under a scoped `TMPDIR` (`48 passed` both times):

```
before: 44 entries      after: 1 entry
```

| file | sites |
|---|---|
| `screenshot-diff-tool.test.ts` | 9 |
| `react-profiler/ast-index.test.ts` | 6 |
| `event-log.test.ts` | 1 helper (4 dirs/run) |
| `screenshot-diff.test.ts` | 1 helper |
| `react-profiler/component-source-duplicates.test.ts` | 2 |
| `ios-instruments/combined-report-frozen-anchor.test.ts` | 1 |
| `flows/flow-visual.test.ts` | production's crop + diff dirs (5/run) |

**Left deliberately (1 entry):** an **empty** `argent-chromium-media/` —
production's fixed staging dir
(`src/chromium-server/cdp-session.ts:218`), shared with any live
chromium-server on the machine. A test `rm`ing it would be the
machine-global-resource defect #714 removed elsewhere.

Two further shapes turned up while checking that claim, both
*production* dirs a test drove and then abandoned:

| site | per run | why the test could not just `rm` it |
|---|---|---|
| `screenshot-diff-tool.test.ts` | 2 dirs + 2 PNGs | `resolveOutputDir`
mints `argent-screenshot-diff/<hex>` before anything validates the call,
so the path exists before the test can learn it — and on a throw it
never learns it at all |
| `flows/flow-visual.test.ts` | 5 dirs + 5 PNGs | `runSnapshot` leaves
the file it registered as an artifact inside its `argent-flow-crop-*` /
`argent-flow-diff-*` scratch dir; from there the dir belongs to whoever
consumes the artifact, which here is the test |

Both are fixed the same way, with `redirectTmpdir` (already used by
seven other test files): point `os.tmpdir()` at a directory the test
already owns and removes, so whatever production mints lands inside it
whether the call returns or throws. The `<hex>` and `mkdtemp` suffixes
are per-call and never shared, so nothing a concurrent tool-server owns
is touched — verified with a decoy file planted under the shared root,
which survives the run.

Measured under a scoped `TMPDIR`, three consecutive runs each:
`screenshot-diff-tool` 6 dirs + 6 PNGs → **0 + 0**; `flow-visual` 15 +
15 → **0 + 0**. The underlying production leak behind the first is still
#790.

`argent-mcp` is now clean end to end: every `mkdtemp` in the package has
a matching `rm`.
</details>

<details>
<summary>Checks</summary>

Run from the worktree root, exit codes read unpiped:

- `npm run format` → 0, no files rewritten
- `npx tsc --noEmit -p packages/tool-server/tsconfig.test.json` → 0
- `npx tsc --noEmit -p packages/argent-mcp/tsconfig.test.json` → 0
- `npm run lint` → 0
- `npm run knip` → 0
- no `it.only` / `describe.only` in the diff

Suites: `argent-mcp` **90 passed**; `tool-server` run as six shards
under a scoped `TMPDIR`.

`lens-tools-platform-gate.test.ts` times out at the 5 s default under
full-suite load and passes in isolation (`2 passed`, 3.14 s). It is not
in this diff and #714 recorded the same behaviour before it.
</details>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Tests**
- Improved test isolation by consistently cleaning up temporary files
and directories after each test.
- Added coverage verifying generated artifacts are stored under the
configured artifacts directory.
- Ensured screenshot, upload, event-log, profiling, visual-flow, and
diff-test artifacts are removed reliably after execution.
- Standardized temporary-directory handling and cleanup across test
scenarios to reduce leftover test data and improve repeatability.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-09 12:39:02 +02:00
Ignacy Łątka 86a2668fb0 test: stop the suite and the process environment leaking into each other (#1058) 
Test env leakage, both directions.

**Out** — six `argent-tools-client` suites pointed `HOME`/`USERPROFILE`
at a temp dir, deleted the dir, and never restored, so a `--no-isolate`
run handed every later file a home that no longer exists. Lifted the
redirect into a shared `test/helpers/home-redirect.ts`; the three suites
that already hand-rolled the save/restore moved onto it too.

**In** — `packages/telemetry` had no `setupFiles` at all, so its unit
tests read the developer's shell. Added
`test/setup/clear-telemetry-env.ts` (ARGENT_ prefix sweep +
`DO_NOT_TRACK` + the cloud-agent detector's markers), wired via
`setupFiles`, following the `@argent/tool-server` precedent — and
dropped the per-file guards it replaces.

**Pinned, not just fixed** — a `test/setup/assert-env-restored.ts` in
each of the three packages fails any suite that leaves a watched
variable changed, so dropping a restore can no longer pass. Each watches
`HOME` and `USERPROFILE`, plus `PATH` (`argent-tools-client`,
`tool-server`) or `CI` and the five `OTEL_EXPORTER_OTLP_*` (`telemetry`)
— the ones the sweep deliberately leaves ambient and individual tests
then set by hand. Each of the three `clear-*-env` setup files has a pin
test that re-imports it against planted sentinels;
`packages/tool-server`'s existing copy gains the mixed-case one it was
missing.

`packages/tool-server` is in the diff because it has the same two gaps:
it originated the prefix sweep (both pre-existing copies were
case-sensitive, so they are fixed together) and it has the largest
redirect surface of the three — 12 files hand-rolling the redirect plus
17 going through `scopeTempHome` — with no guard. It is already green
under one; deleting `scopeTempHome`'s restore turns 17 files red. Two of
its suites restored `HOME` by assigning back a saved value that can be
`undefined`, writing the string `"undefined"`; the guard caught both on
a host with no ambient `HOME`, and they are fixed here.

`windows-e2e.yml` runs seven `tool-server` files through the config this
branch edits, but neither that config nor `test/setup/**` was in its
`paths:` filter — so the new setup file would have reached
windows-latest unverified. Added both.

No docs change: test infrastructure only, no user-facing behaviour.

Fixes #1028
Fixes #839

Same-class problems found while reviewing this and filed rather than
folded in: #1068 (`argent-cli` fails under `DO_NOT_TRACK=1`), #1069 (the
`ios-device-runner` suites pin only `HOME`, so the redirect is inert on
Windows — its second half, the restore that wrote the string
`"undefined"`, is fixed here because the new guard caught it), #1070
(`useTempHome` shares one saved-`HOME` slot, so a nested `scopeHome()`
restores a deleted dir), #1071 (`tool-server`'s sweep is prefix-only, so
an exported `ADB_SERVER_SOCKET` reddens its suite), #1072
(`external-devices.test.ts` reads the developer's real
`~/.argent/providers`) and #1073 (`flow-script-environment.test.ts`
fails with no ambient `HOME`).

<details>
<summary>#1028 — probe results, six files</summary>

`test/zz-home-probe.test.ts` from the issue body, run per suite.
Vitest's sequencer orders by cached duration and falls back to file
size, so the cache is cleared before each run to keep the tiny probe
last:

```
rm -rf node_modules/.vite/vitest ../../node_modules/.vite/vitest
npx vitest run test/<suite>.test.ts test/zz-home-probe.test.ts --no-isolate --no-file-parallelism
```

| suite | before | after |
| --- | --- | --- |
| `launcher-state` | 1 failed \| 23 passed — `argent-state-test-…
exists=false` | 24 passed |
| `launcher-spawn` | 1 failed \| 8 passed — `argent-spawn-test-…
exists=false` | 9 passed |
| `launcher-version-gate` | 1 failed \| 7 passed —
`argent-version-gate-test-… exists=false` | 8 passed |
| `launcher-duplicate-spawn` | 1 failed \| 10 passed —
`argent-dup-test-… exists=false` | 11 passed |
| `launcher-exit` | 1 failed \| 1 passed — `argent-exit-test-…
exists=false` | 2 passed |
| `link-config` | 1 failed \| 43 passed — `argent-link-config-test-…
exists=false` | 44 passed |
| `launcher-sweep` (control, fixed in #713) | passed | passed |
| `launcher-ps-guard` (control, fixed in #713) | passed | passed |

The probe itself is a diagnostic and is not part of the diff —
`test/setup/assert-env-restored.ts` is the durable guard, and it needs
no `--no-isolate` to fire.
</details>

<details>
<summary>#839 — the stated repro is stale; here is what is actually
left</summary>

The issue was filed 2026-08-18. #714 landed 2026-08-19 and
hand-enumerated the variables inside `index.test.ts` (a
`CONSENT_ENV_KEYS` `beforeEach`) and `base-props.test.ts` (eight
`delete`s). On `aa90873b5` all three named repros are green:

```
$ DO_NOT_TRACK=1            npx vitest run --root packages/telemetry   → 319 passed
$ ARGENT_TELEMETRY=0        npx vitest run --root packages/telemetry   → 319 passed
$ CURSOR_AGENT_WORKER_ID=x  npx vitest run --root packages/telemetry   → 319 passed
```

What remains is the structural gap the issue names: the protection is
per-file and per-variable, nothing pins it, and `vitest.config.ts` still
has no `setupFiles`. This PR moves the property to the setup file and
deletes the two hand-rolled guards, so the setup file is what holds it —
see the mutation section, where removing the `setupFiles` line
reproduces the issue's 17 and 1 exactly.

Post-change sweep, one hostile value per run, 329 tests each:
`DO_NOT_TRACK=1`, `ARGENT_TELEMETRY=0`, `ARGENT_TELEMETRY_DEBUG=1`,
`CLAUDE_CODE_ENVIRONMENT_KIND=byoc`, `CLAUDE_CODE_ENTRYPOINT=remote`,
`CLAUDE_CODE_REMOTE_SESSION_ID=x`, `CURSOR_AGENT_WORKER_ID=x`,
`CURSOR_WORKER_POOL_NAME=x`, `GITHUB_ACTIONS=true`,
`GITHUB_ACTOR=copilot`,
`GITHUB_WORKFLOW_REF=…copilot-swe-agent.yml@refs/heads/main`,
`REPLIT_AGENT=1`, `ARGENT_HOST`/`ARGENT_PORT`/`ARGENT_AUTH_TOKEN`,
`CI=1`, `CI=false`, `OTEL_EXPORTER_OTLP_ENDPOINT`,
`OTEL_EXPORTER_OTLP_HEADERS`, the real CI combination, and all eleven at
once — 329 passed every time.

`GITHUB_ACTIONS` is a ci-info vendor input as well as a copilot-agent
marker, so clearing it does move `isCi()` — but only its GitHub Actions
branch, and an Actions run also sets `CI`, which `isCi()` answers ahead
of any vendor check. `GITHUB_ACTOR` and `GITHUB_WORKFLOW_REF` appear in
neither `ci-info/vendors.json` nor `ci-detect.ts`'s generic list, so
they cost nothing. The rest of the ci-info surface and `OTEL_*` are
deliberately left alone; the setup file says so.
</details>

<details>
<summary>Mutation testing — every guard, applied by hand, verified to
fail, reverted</summary>

| mutation | result |
| --- | --- |
| drop `setupFiles` from `packages/telemetry/vitest.config.ts` |
`DO_NOT_TRACK=1` → 19 failed (**17 in `index.test.ts`**, matching the
issue, + the two registration pins) |
| drop `CURSOR_WORKER_POOL_NAME` from `CLEARED_ENV_VARS` | drift test
red |
| move an `env.` read into `src/detect/vendor.ts` | drift test red,
naming `detect/vendor.ts` |
| rewrite a read as `env["X"]`, `const { X } = env`, `const { X }:
NodeJS.ProcessEnv = env` or `const { X = "d" } = env` | drift test red |
| add `cloud-agent-detect.ts` to `UNSWEPT_SRC_FILES` | drift test red |
| revert `toUpperCase()` in any of the three sweeps | that package's
mixed-case case red |
| drop `restoreHome()` from one suite / from all nine | 1 file / 9 files
red, each naming its deleted dir |
| drop `scopeTempHome()`'s restore in `tool-server` | 17 files red |
| drop the hand-rolled `CI` restore in `base-props.test.ts` | red,
naming that file (green before; under `--no-isolate`, 7 unrelated
failures in `otel-endpoint.test.ts`) |
| narrow either guard's filter to variables that were already set |
`assert-env-restored.test.ts` red |
| replace the leaked half of the guard's message with a constant |
`assert-env-restored.test.ts` red |
| rename one function's `env` parameter in `cloud-agent-detect.ts` |
drift test red — the read is found under the new binding |
| narrow any guard's filter to a subset of `SCOPED_ENV_VARS` |
`assert-env-restored.test.ts` red — the pin plants every entry |
| drop the hand-rolled OTLP restore in `otel-endpoint-live.test.ts` |
red, naming all four leaked variables |
| move the guard's `AMBIENT` snapshot inside `assertEnvRestored` |
`assert-env-restored.test.ts` red in all three packages |
| delete the guard's `afterAll(...)` registration |
`assert-env-restored.test.ts` red in all three packages |
| drop `restoreHome()` from `artifacts.test.ts`'s two `afterEach` | red
— the file-level guard catches a per-test omission |
| drop ps-guard's `PATH` restore | red, naming the file |
| empty `leakedEnvVars`, or drop a name from `SCOPED_ENV_VARS` |
`assert-env-restored.test.ts` red |
| replace the helper's delete branch with a plain assignment |
`home-redirect.test.ts` red |
| drop `USERPROFILE` from the helper | `home-redirect.test.ts` red |

</details>

<details>
<summary>Local CI parity</summary>

```
npx vitest run --root packages/argent-tools-client               213 passed | 1 skipped
npx vitest run --root packages/argent-tools-client --no-isolate  213 passed | 1 skipped
npx vitest run --root packages/telemetry                         329 passed
npx vitest run --root packages/telemetry --no-isolate            329 passed
npx vitest run --root packages/tool-server                       6166 passed | 1 skipped
npx tsc --noEmit -p packages/{argent-tools-client,telemetry,tool-server}/tsconfig.test.json   clean
npm run format / npm run lint / npm run knip                                                 clean
```
</details>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Tests**
- Improved test isolation by consistently restoring temporary
environment settings after test runs.
- Added safeguards that detect leaked environment settings and flag
incomplete cleanup.
- Added coverage to ensure telemetry-related environment variables are
cleared before tests and do not affect results.
- Added cross-platform checks for environment variable handling,
including mixed-case names.
- Expanded validation for home-directory redirection and restoration
behavior.
- Added checks confirming environment cleanup runs in the correct order
across test suites.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-09 12:38:51 +02:00
Ignacy Łątka 5bc9c7be4c test: make four non-discriminating test sites fail on what they claim to pin (#1059) 
Four tests could not fail on the thing they name: an assertion implied
by the one below
it, an assertion satisfied by the fixture's own filename, a clearance
poll with nothing
to clear, and `it.each` rows whose rendered name described only argv[0]
- two of them
stating the opposite of what the row asserts.

Each site now fails when its claim is broken, proven by mutation. Tests
only, no source
change, so no docs update.

Fixes #943
Fixes #942
Fixes #841
Fixes #861

<details>
<summary>#943 &mdash; capability: toolId/platform were substrings of the
path assertion</summary>

`toContain("demo-tool")` + `toContain("android")` are both substrings of
`tools/demo-tool/platforms/android.ts`, asserted on the next line.
Replaced by the prose
sentence, which names both outside the path.

Mutation - drop the sentence naming the tool and the platform:

```diff
-      `Tool '${opts.toolId}' is not yet implemented on ${opts.platform}. ` +
+      `Tool is not yet implemented. ` +
```

| | result |
|---|---|
| before | `Tests 7 passed (7)` - green |
| after | `AssertionError: expected 'Tool is not yet implemented. The
cros…' to contain 'Tool \'demo-tool\' is not yet impleme…'` |

The sibling "works without a hint" assertions are load-bearing and
untouched.
</details>

<details>
<summary>#942 &mdash; native-profiler: the wording assertion matched
inside missing_cpu.xml</summary>

`/missing|not found|unreadable/i` matched the fixture filename, so any
product copy
passed. The wording now has to land before the backticked path (a
negated character class
cannot cross into it), which no path can satisfy.

Mutation - reword the ENOENT copy:

```diff
-    if (code === "ENOENT") return `not found at \`${filePath}\``;
+    if (code === "ENOENT") return `absent at \`${filePath}\``;
```

| | result |
|---|---|
| before | `Tests 1 passed (1)` - green (also green with the state word
deleted outright) |
| after | `AssertionError: expected '# iOS Instruments Analysis…' to
match /-\s*\*\*cpu\*\*:[^…]*(?:missing\|not found\|unreadable)/i` |
</details>

<details>
<summary>#841 &mdash; vega-cli: the output-cap reaps had nothing to
observe</summary>

`expect(await waitForClear(SENTINEL_OVERFLOW)).toBe(0)` reads 0 just as
readily for a
launcher that never spawned a worker. The cap trips on the first write,
so there was no
window to see one in: the fake now spawns its sentinel worker **before**
the flood and
holds the flood - and so the reap - until the test writes a gate file,
having seen the
worker. Both the stdout and the stderr twin.

Gated rather than timed, so the observation is decided, not raced. Cost
is ~+110ms and
~+50ms on the two tests (165/177ms to 278/226ms).

Mutation - the flood branches' workers stop carrying the sentinel, reap
path untouched:

```diff
-  const worker = spawn("sleep", [secs], { stdio: "ignore" });
+  const worker = spawn("sleep", ["600.0"], { stdio: "ignore" });
   ...
-  spawnSync("sleep", [secs]);
+  spawnSync("sleep", ["600.0"]);
```

| | result |
|---|---|
| before | `Tests 12 passed (12)` - green, and `pgrep -f '^sleep
600\.0$'` empty (the reap works; nothing observed it) |
| after | both overflow tests `AssertionError: expected +0 to be 1` |

Cross-check that the pair pins the production reap: with `reapOnce()`
made a no-op in
`vega-cli.ts`, both fail `expected 2 to be +0`.

**Flake runs: 30 consecutive passes** (2 x 15, before and after the
comment pass),
`12 passed (12)` every time.
</details>

<details>
<summary>#861 &mdash; four it.each rows named only argv[0] (plus a fifth
site)</summary>

A single `%j` against a multi-element row renders only the first
element. Each row is now
one argv, nested, so `%j` names all of it.

`packages/argent-cli/test/tools-help.test.ts`

```
- prints usage for "--help" without contacting the tool-server   <- duplicate
- prints usage for "--json" without contacting the tool-server   <- false: a bare --json
                                                                    prints no usage and
                                                                    does hit the server
+ prints usage for ["--help","--json"] without contacting the tool-server
+ prints usage for ["--json","--help"] without contacting the tool-server
```

`packages/argent/test/installer-help.test.ts`

```
- treats "--foo" as a help request      <- rendered twice; a bare --foo is the opposite
+ treats ["--foo","--help"] as a help request
+ treats ["--foo","help"] as a help request
- does not treat "--metro-port" as a help request
+ does not treat ["--metro-port","8082"] as a help request
```

Swept the class across every `it.each` array-literal table in the repo
(154 sites). One
more reproduces it -
`packages/argent-cli/test/telemetry-command.test.ts`, fixed here:

```
- prints usage for status and writes nothing    <- rendered twice
- prints usage for disable and writes nothing   <- false: a bare `disable` writes config
+ prints usage for ["status","--help"] and writes nothing
+ prints usage for ["status","-h"] and writes nothing
+ prints usage for ["disable","-h"] and writes nothing
```

Left alone: ~64 sites with a discriminating label column (`_label`
first, house style) and
16 whose first column is a real discriminator - names unique and
truthful in both groups.
`providers-command.test.ts:440` drops a third "shape" column from its
name but stays
unique and true, so it is a legibility nit, not this defect.
</details>

<details>
<summary>Checks run</summary>

- `npm run format`, `npm run lint` (the latter needs `npm ci` inside
`packages/docs` locally)
- `npx tsc --noEmit -p
packages/{tool-server,argent-cli,argent}/tsconfig.test.json`
- `packages/tool-server`: `npx vitest run --shard=1/6` .. `6/6` - 6163
passed, 1 skipped
- `npx vitest run --root packages/argent-cli` (619), `--root
packages/argent` (80)
- `vega-cli-timeout.test.ts` x30 consecutive, zero flakes
</details>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Tests**
* Improved test parameterization so command-line help cases display
complete argument lists accurately.
* Strengthened error-message and profiler warning assertions to verify
exact wording and placement.
* Improved subprocess timeout and output-limit tests by synchronizing
worker startup and cleanup checks.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-09 12:38:37 +02:00
Ignacy Łątka c3bb2990ea test(flows): stop pinning idle and type-focus waits to exact wall-clock boundaries (#1066) 
Real-timer assertions in the flows suite were pinned to exact wall-clock
boundaries, so a batch run reddened them for reasons unrelated to
whatever was
being tested. Each now pins the same property with slack, or with no
clock at
all — every one was mutation-tested to confirm it still fails loudly
when the
behaviour it guards is removed.

Test-only; no docs change.

Fixes #1000
Fixes #795

<details>
<summary>Per-assertion: what it pins, and what changed</summary>

| Site | Pinned property | Before | After |
| --- | --- | --- | --- |
| `flow-type-focus` "skips the focus poll…" | the fixed 500ms settle
still runs on a source that cannot report focus | `gap >= 500` | `gap >=
450` |
| `flow-type-focus` "waits for the tapped field…" | settle **plus** at
least one 300ms focus poll | `gap >= 800` | `gap >= 720` |
| `flow-idle-run` "holds a still screen…" | `stableFor` is served, not
ignored | run timed, `[750, 2400)` | span between first and last tree
read, `>= 750` |
| `flow-idle-run` "settles inside the smallest timeout…" | the hold is
counted across the polls, so the parser's floor is servable | run timed,
`[750, 1000)` | read span `>= 750`; "inside the wait" carried by
`warning === undefined` |
| `flow-idle-run` "does not call a slow but answering source a wedged
one" | an abandoned read is not a wedge when the source's own slowest
answer explains it | 2300ms reads / 4600ms wait, 5s default test timeout
| 2800ms / 5400ms, own 20s timeout |
| `flow-idle-run` "does not report a tree-only settle…" | the tree-only
settle owes the same hold | churn window clocked from before the run |
clocked from the first read |
| `flow-idle-run` H1/H2 "settled early and then moved / went blank" |
the tree-only verdict is not a write-once latch | 4 quiet reads in a
2500ms wait | 3 quiet reads in a 6000ms wait, reads asserted |

The two focus bounds had zero slack in the one direction that matters: a
500ms
`setTimeout` measured on `Date.now()` spans 499ms roughly once in 200
runs
(400 samples, idle machine: `499ms x2, 500ms x48, 501ms x289, 502ms x55,
503ms x2, 505ms x1`). 10% of slack keeps them failing by hundreds of
milliseconds if either wait is dropped.

The two idle `elapsed` bounds timed the whole run, so the flow's
start-up —
parse, device resolve — counted against the step's own budget. Timing
between
the reads the hold is actually measured across removes that, and the
upper
bounds went with it: every way out of the wait except the settle carries
a
warning, so `warning === undefined` already says it landed inside.

The remaining three are fixtures, not assertions: each assumed a number
of poll
rounds, or a wall-clock window opened before the run, that a batch run
does not
serve. They now count reads, or anchor the window on the first read.

</details>

<details>
<summary>Repro — before</summary>

`npx vitest run packages/tool-server/test/flows` on `origin/main`:

```
 FAIL  flow-idle-run.test.ts > settles inside the smallest timeout the parser allows for the hold
AssertionError: expected 2439 to be less than 1000

 FAIL  flow-idle-run.test.ts > does not call a slow but answering source a wedged one
Error: Test timed out in 5000ms.

 FAIL  flow-idle-run.test.ts > does not report a tree-only settle when the hold was never served
AssertionError: expected 'settled on the UI tree alone — this s…' not to contain 'UI tree alone'

 FAIL  flow-idle-run.test.ts > does not pass on a screen that settled early and then started moving again
AssertionError: expected 'settled on the UI tree alone — this s…' to contain 'never held still'
```

#1000's own intermittency did not surface in 10 rounds of
`npx vitest run --shard=3/3` (4 idle, 6 under a 10-way CPU load), so it
was
forced instead: the `setTimeout`-vs-`Date.now()` measurement above is
the whole
mechanism, and the assertion had no margin over it.

</details>

<details>
<summary>Repro — after</summary>

Six batch rounds of `npx vitest run packages/tool-server/test/flows`, on
a
machine carrying other work (load average 22-30 throughout). Rounds 2-6
were
additionally narrowed to `--maxWorkers=3`, which is harsher than CI: the
13
`flow-composition` cases of #1067 pin one of the three workers for ~13
minutes
and starve the rest.

| round | workers | result for the assertions this PR touches |
| --- | --- | --- |
| 1 | default | all clean |
| 2 | 3 | H1 starved to 3 reads |
| 3 | 3 | all clean |
| 4 | 3 | all clean |
| 5 | 3 | all clean |
| 6 | 3 | H2 starved to 3 reads |

Every other failure in those rounds was the same 13 `flow-composition`
timeouts (#1067), plus load artifacts in `flow-selector-scopes`,
`flow-tool-step-tree-target-e2e`, `flow-launch-pins-tree-target` and
`flow-gesture-settle` — none of whose assertions this branch touches.

The two starved rounds were a real gap, not noise: H1 and H2 each need a
fourth read to reach a verdict at all, and
a 3500ms wait serves eighteen on an idle machine but three on a starved
one.
The case then saw a screen that never moved and passed the wait cleanly,
so the
assertion ran against `undefined`:

```
AssertionError: the given combination of arguments (undefined and string) is
invalid for this assertion.
```

Each case now budgets for the starved run and asserts the reads it was
served,
so an environment that cannot reach the verdict says so:

```
AssertionError: reads served — under four, nothing moved to be judged:
expected 3 to be greater than or equal to 4
```

Confirmed by stretching each read to 1800ms, which starves the wait to
three
reads without touching the wait itself.

</details>

<details>
<summary>Mutation tests (source broken, assertion must fail)</summary>

Each mutation was applied to `flow-actions.ts`, the test run, then
reverted.

| Mutation | Assertion under test | Result |
| --- | --- | --- |
| delete `sleepOrAbort(TYPE_FOCUS_SETTLE_MS, …)` | both focus gaps |
FAIL — `expected 0 to be greater than or equal to 450`; `expected 303 to
be greater than or equal to 720` |
| focus poll sleeps `0` instead of `POLL_INTERVAL_MS` | `>= 720` | FAIL
— `expected 504 to be greater than or equal to 720` |
| drop `&& heldForMs >= stableFor` from the settle | both idle read
spans | FAIL — `expected 403 to be greater than or equal to 750` (both)
|
| drop `slowestAnsweredReadMs +` from the hung check | "slow but
answering" | FAIL — step errors, `expected false to be true` on `r.ok` |
| drop `&& now - treeSince >= stableFor` from `treeSettledAtLastRead` |
"tree-only settle" | FAIL — `expected 'settled on the UI tree alone…'
not to contain 'UI tree alone'` |
| `if (lastRead === "timeout")` without `&& treeReadHung` | "still
warns, rather than erroring" | FAIL — `expected 'error' to be 'pass'` |
| make `treeSettledAtLastRead` a write-once latch | H1 "settled early
then moved again" | FAIL — reports the tree-only settle |
| drop `treeSettledAtLastRead = false` from the blank-read branch | H2
"settled early then went blank" | FAIL — reports the tree-only settle |

The `treeReadHung` one did **not** fail at `timeout: 3000`: with the
round
period dividing the wait exactly, drift decided whether the closing read
was
abandoned at all. `3200` leaves the seventh read 200ms — less than the
source
needs — so the case ends on an abandoned read by arithmetic.

</details>

<details>
<summary>Why the latch waits stop at 6000ms, and H3 is
untouched</summary>

The first attempt at these two used a 12000ms wait, which passed CI and
broke
the budget-tail case below them in the same file — that one has to fit
three
reads into a 2000ms window, and with 24s of polling ahead of it it saw
two:

```
expected 'the screen came back with content on 2 reads in 2000ms…'
  to contain 'never held still'
```

Deterministic, not load: 4/4 runs on the branch, 0/4 on `origin/main`,
and it
passes in isolation. Bisected across the file:

| latch wait | whole file |
| --- | --- |
| 2500 (main) | 46 passed |
| 3500 | 46 passed |
| 6000 | 46 passed |
| 12000 | budget-tail fails every run |

So the latch cases take 6000ms and no more. H3 itself is left exactly as
it is
on `main`: widening its wait scores its abandoned closing read as a
wedged
source (`HUNG_TREE_READ_MS`) rather than a step that ran out of time,
and
slowing its source starves it below the three reads a verdict takes —
both
were tried against CI and both moved the failure rather than removing
it.

</details>

<details>
<summary>Deliberately not changed</summary>

- `flow-idle-run`'s `elapsed < 3000` bounds at "honours its timeout",
"rides
  over a dropped capture" and "stops on abort": 1.8–2.2s of slack over a
sub-second expectation already clears a scheduling delay of hundreds of
  milliseconds.
- `flow-composition.test.ts` times out 13 cases at 60s each in the same
batch
run, on `origin/main` as well as here, while passing in 27s on its own.
It is
  a batch artifact of a different kind and belongs in its own issue, not
  papered over from here.

</details>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Improved idle-flow timing validation by measuring intervals between
reads for more reliable settle detection.
* Updated timing scenarios and thresholds to account for delayed reads,
startup overhead, slow tree changes, and early-settle behavior.
* Added checks to ensure sufficient reads occur before slow-flow
warnings and to detect later movement or blanking.
* Relaxed Android and iOS focus-wait timing thresholds to accommodate
minor timer measurement jitter.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-09 12:36:01 +02:00
Arkadiusz b6576ef60c optimization: move some instructions out of the argent-device-interact skill into reference files (#968) 
# Why

Skills are getting bloated affecting benchmark results.

# What

- added `<SKILL>/references/chromium.md` — no point of loading that
information to the context when argent is used with iOS or Android
- added `<SKILL>/references/secrets.md` — typing secrets is rather
unusual.
- reduced duplicated information for await-ui-element by redirecting to
the tool's description

# Note

Not tested. Relying on CI.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Documentation**
- Clarified supported device-interaction targets, including that TV
targets are not supported.
- Narrowed Chromium support to Electron apps and Chromium-based browsers
with a CDP endpoint.
  - Added guidance for Chromium tabs, cookies, and web storage.
- Documented secret placeholder resolution, safeguards, and submission
requirements.
- Updated UI-wait guidance for hidden elements, root containers, roles,
and timeout notes.
  - Clarified platform-specific interaction behavior and tool usage.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-08 14:00:32 +02:00
filip131311 aa90873b59 feat(android): read the web DOM inside a WebView in describe (#1052) 
Android `describe` rendered an `android.webkit.WebView` as a single
opaque leaf labelled `[web-view]` and threw the subtree away. Chromium
publishes the web DOM to the accessibility tree on both capture paths
(the android-devtools helper and the `uiautomator dump` fallback), so
the sentinel hid content the agent can act on. This keeps the WebView as
a landmark and lets the normal trim run over the DOM beneath it.

Supersedes #955, which reached the same diagnosis. The parser rules for
the doubled in-app node, the web list clip and the text-run role are
carried over from it; the rendered-output cap, the `truncated` flag
plumbing and the flow-tree parity rework are not, so this stays confined
to the Android parser and adapter (about 120 source lines) with no
renderer, contract, skill or tool-description changes.

### What changes inside a WebView, and only there

- **Doubled node merged.** An in-app WebView dumps as a nested pair: the
app's view (resource-id, gesture flags), then Chromium's root web area
under the same class carrying the page `<title>` as text. The inner half
folds into the outer node, so the tree reads `WebView "<title>"`. A
Chrome tab arrives as one node and is untouched.
- **Web list clip.** A `<ul>` arrives as `android.widget.ListView` with
`scrollable="false"`. Inside a WebView only the framework flag decides
what clips, so a list never drops the items laid out around it, and the
role is `List` rather than `ScrollView`.
- **Text runs.** A labelled leaf `android.view.View` with no gesture
flag is reported as `StaticText`. Outside a WebView a bare `View` keeps
its role (Compose semantics nodes).
- **Headings survive.** The WebView branch returns before the label
dedupe, so an `h1` that repeats the page title is kept.

### The cold-read race is fixed, not documented

Chromium builds a WebView's accessibility tree on the first request for
it. Measured on API 35 / WebView 124: a read taken 6 s after page load
still shows the WebView with no children; the page appears only after a
read has asked for it — 10–400 ms later for a small in-app page, ~600 ms
later for a Chrome tab over a long article. `describeAndroid` re-reads
in 250 ms steps within a 1.5 s budget while the tree still has the
unread shape, on both paths. A screen without a WebView never pays for
it (one call, pinned by test). A WebView that stays empty after the
budget is labelled `(no web content exposed)`.

The unread shape is an `android.webkit.WebView` with no children, or —
for a browser tab, which has no WebView view of its own before the page
is published — Chrome's childless content view labelled "Web View"
(English UI only; other locales see the tab the way they do today).

### Measured on a Pixel 9 / API 35 emulator with the branch tool-server

| Screen | Before | After |
|---|---|---|
| In-app checkout form, cold launch | `WebView "[web-view] (no label)"`
| full form in one call, 1.2 KB, 0.6 s |
| 120-paragraph page | sentinel | the ten visible paragraphs, 1.8 KB
(existing scroll clip bounds it) |
| Wikipedia article in Chrome, helper attached after the page loaded |
`FrameLayout "Web View"` | 4.7 KB, heading, links, lists, 1.4 s |
| Settings home (native) | — | byte-identical to the previous parser |

### Testing

- New `android-webview-describe.test.ts` on three live captures checked
in as fixtures (in-app, Chrome, cold): merged landmark, `List` role,
heading kept, re-read fires on the cold in-app shape and on Chrome's
content-view placeholder, never on a native screen, and gives up within
the budget.
- `uiautomator-parser-v2-trim.test.ts`: the opaque-leaf case is replaced
by the merge, the empty-WebView leaf, and a native `View` that must not
be remapped.
- Full tool-server suite, `tsc`, eslint, prettier, docs build.

### Docs

No docs, skill or tool-description change. The page already names the
accessibility tree as the source on Android; WebView content is now part
of that tree and the agent needs no new instruction, since the cold read
no longer produces an empty WebView.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01GZbkzeVC9faSxTFi4DbMuT


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Android WebView content is now captured and presented with meaningful
controls, text, lists, and links.
* Web content is consolidated into a clearer, unified accessibility
view.
* **Bug Fixes**
* Improved Android screen descriptions when WebView content is initially
unavailable by retrying the accessibility hierarchy.
* Added safeguards for WebViews that never expose content, including a
clear fallback message.
* **Tests**
* Expanded coverage for in-app WebViews, Chrome tabs, cold captures, and
native Android UI filtering.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Hubert Gancarczyk <hubert.gancarczyk@swmansion.com>
 2026-09-07 16:35:42 +02:00
Hubert Gancarczyk 236db0077f fix(tool-server): support ios-remote in the await tools (#1051) 
## Problem

The `await-ui-element` and `await-screen-idle` tools refused a remote
simulator. The tool server sent this error:

```
Tool 'await-ui-element' is not supported on ios-remote simulator (no ios-remote support declared).
```

Both tools poll the same accessibility tree as `describe`. The
`describe` tool reads that tree correctly on a remote simulator.
Therefore the tree source was never the problem.

## Cause

Each tool declares its platform support in a capability record. The
`describe` tool declares `appleRemote`. The two wait tools did not
declare it. The HTTP layer rejects the request before the tool starts.

Three more branches test `platform === "ios"`. That test is false for a
remote device. These branches select the tree source, the dependency
preflight, and the tvOS probe.

Nobody excluded the platform deliberately. Git history has no commit
that adds `appleRemote` to either file. The `appleRemote` concept
arrived on 2026-07-01. The `await-screen-idle` tool arrived two days
later.

## Effect on an agent session

The MCP layer calls `await-screen-idle` before each automatic
screenshot. On a remote simulator that call failed. The fallback then
slept for the full budget:

- 1500 ms for each tap or gesture
- 2000 ms for `open-url`
- 3000 ms for `launch-app` and `restart-app`
- 15000 ms for `run-sequence`

The user saw no error. The fallback exists for an old tool server, not
for a current server that refuses a device.

## Change

This change adds three lines to each tool:

1. The capability record declares `appleRemote: { simulator: true }`.
2. The tree fetch sends a remote device to `describeIos`. Before, a
remote device fell through to the Chromium branch.
3. The dependency preflight requires `sim-remote`, and not `xcrun`.

The `describe` tool uses the same three settings. The ax-service
blueprint sends the accessibility read through the sim-remote tunnel.

## Test result

I ran the branch tool server against a leased Argent Cloud runner. The
device was a remote iPhone 17 Pro on iOS 26.5. The app was Bluesky. The
app read its bundle from a local Metro server through the sim-remote
tunnel.

| ID | Case | Result |
| --- | --- | --- |
| T1 | `visible` by text | `success: true` |
| T2 | `exists` by identifier | `success: true` |
| T3 | `exists` by a second identifier | `success: true` |
| T4 | `text` condition matches the label | `success: true` |
| T5 | compound role and text selector | `success: true` |
| T6 | absent text gives `unmet` | `success: false` |
| T7 | wrong expected text gives `unmet` | note quotes the actual text |
| T8 | 5000 ms budget honoured | `elapsed: 5003` |
| T9 | `hidden` on an absent element | `success: true` |
| T10 | screen settles | `settled: true, polls: 2` |
| T11 | screen honours `minStableMs: 2000` | `waitedMs: 2265, polls: 8`
|
| T12 | wait across an app restart | `success: true, elapsed: 27301` |
| T13 | `hidden` after the restart | `success: true` |
| T14 | flow record boundary | UNKNOWN verdict |

All 14 cases passed. Two results give the strongest evidence:

- Case T12 blocked for 27.3 seconds across an app restart. The app
rebuilt its bundle, the element appeared, and the tool returned success.
- Case T6 returned `cause: "unmet"`. A failed tree read returns `cause:
"unreadable"`. Therefore the tool read the remote tree and judged the
condition.

### Unit tests

Five new unit tests drive both tools with a remote udid. They use a mock
accessibility service. Each test fails against `main` and passes with
this change.

Local checks:

- `npm run build`: clean
- `npm test -w @argent/tool-server`: 431 files, 6153 tests, 0 failures
- `npm run lint`: clean
- `npx prettier --check`: clean

## Note on flow records

The flow tools declare no capability. Therefore they accept a remote
device today. This change lets the re-probe reach `fetchTree`, which has
no support for `ios-remote`. The recorder catches that error. The
recorder then keeps the step and marks the verdict UNKNOWN, not
known-bad.

An earlier test asserted the refusal. This change inverts that test and
records the new behavior in its comment.

## Documentation

This change needs no documentation update. The tool reference lists
these two tools with a description only. That page has no platform
column, and no page describes remote simulators.

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
 2026-09-07 14:42:23 +02:00
Hubert Gancarczyk 0f90d182b2 feat(flows): record a script step by running it (#940) 
#864 (the executor) and #865 (the `script:` step) are merged. This
branch now
sits directly on `main` as a single commit, so the diff and the commit
list are
this change alone.

A `script:` step could only enter a flow by being typed into the YAML
after the
recording ended. The authoring skill said so in as many words, and
listed it
among the legal unrecorded insertions. That made it the one directive
exempt
from every rule the recorder exists to enforce, and it cost two things:

- **The script never ran while the flow was authored.** It first
executed during
the closing replay, where a failure reads as a flow that was never right
rather than a script that needs one more attempt. Worse, the device
steps
recorded after it were walked against backend state the author had
established
by hand — so the recording proved nothing about whether the script
establishes
  that same state.
- **The author never saw what the script returned.** A later release
reads
`{{output:user.id}}`; the only way to know that path exists is to have
seen the
  document.

This adds one MCP tool, `flow-add-script`, that runs a local `.mjs` and
records
the step it ran. **No new flow syntax** — the `script:` step is
unchanged, and
this is a second way for one to enter a flow file.

```text
flow-add-script { name: "checkout", project_root: "/Users/dev/shop",
                  path: "../../scripts/seed-order.mjs", timeout: 60000 }
```

## What it adds

- **One shared execution path.** Path resolution, the on-disk casing
guard, the
executor call and the verdict were private to `flow-run.ts`. They are
now
`runFlowScriptStep` in `flow-script-step.ts`, with the file-ref helpers
in
`flow-file-refs.ts`, and `runScriptStep` is that call plus its
`ExecState`
plumbing. This is the load-bearing change: "the recording runs the
script
exactly as replay will" is one function, not two code paths that agree
today.
- **A failure records nothing.** The opposite of `flow-add-step`,
deliberately.
A failed script did not establish the state the rest of the walkthrough
would
then be recorded against, so an appended red step costs every step after
it,
not just itself. The `message` also says whether a child actually *ran*
— a
path that resolved to no file forked nothing, while a script that threw
halfway did not roll back — because the author's next move is a retry or
a
  cleanup and they cannot pick without knowing.
- **The returned document, as JSON text.** Text rather than an object
because
this is the first tool result carrying bytes the server did not author:
every
  result is deep-walked for `__argentClientFile` and `__argentArtifact`
directives, which match on shape alone and write or fetch files on the
agent's
  machine. As a string it inherits none of that. Bounded at 64 KiB with
  `outputTruncated`, for the reason `log` is bounded.
- **`{{output:` refused by `parseFlow`.** Surfacing the document invites
an
author to write a reference that has no meaning yet — it would be
printed
literally by a step that passes, green and wrong. The scan runs over
exactly
the field list the output release declares supported, so it is a seam
that
release converts from reject to resolve rather than scaffolding it
deletes.
Patterns are excluded: a `{{` in a regular expression is a quantifier
the
  author meant.
- **Two boundaries.** A `client`-mode recording is refused — its `.mjs`
never
left the agent's machine, the same anchor reason `run:`, `snapshot` and
  uploaded flows are refused. And the tool is `longRunning`, so the MCP
adapter's 30s fetch abort cannot retry a call whose whole purpose is a
side
effect; the comment records what that flag does *not* close, and where
the
  rest belongs.
- **`flow-add-step` says when the call already ran.** The `{{output:`
refusal is
the one refusal this release adds on a path where the tool call has
already
fired, and its message asks for an edit-and-retry — which repeats the
device
  action. Verified: a `keyboard` step recorded twice left
`{{output:code}}{{output:code}}` in the field. That one refusal now
carries
  what already happened.

## Documentation

Rule 5 of `argent-create-flow` no longer lists `script:` as a legal
unrecorded
insertion — leaving it there would tell agents to keep doing the thing
this PR
removes. `live-authoring.md` gains the recorder contract, the return
fields that
are not self-evident, and the ordering rule: a setup script is recorded
*before*
the `restart-app` it prepares state for, because that is where it runs
at
replay. A leading script does not make the flow a fragment. `qa-flows`
and
`reliability-and-recovery` follow the count and the wording.

## Verification

`npm run build`, `npx eslint . --max-warnings 0`, `npx prettier --check`
over
tracked files, and `npm run typecheck:tests -w @argent/tool-server` are
green.
`npm test -w @argent/tool-server`: 358 files, 4657 passed, 1 skipped.

Exercised end to end against a live tool server built from this branch,
not
only in tests — recorded a flow, then replayed the saved YAML through
`argent flow run` with no hand editing:

```
flow-add-script → status "pass", durationMs 124
  log: "seeding order for the flow\nwarn: using the fake backend\n"
  outputJson: {"order":{"id":"ord_4417","total":39.5},
               "user":{"id":"usr_9","email":"buyer@example.com"}}
  recorded: 1. script: ../../scripts/seed-order.mjs (timeout 30000ms)
```

```yaml
steps:
  - script:
      path: ../../scripts/seed-order.mjs
      timeout: 30000
  - echo: order seeded; expect Home
```

```
Flow "checkout"
  ✓  1 script ../../scripts/seed-order.mjs
       │ seeding order for the flow
       │ warn: using the fake backend
  › order seeded; expect Home

PASS — 1 passed, 0 failed, 0 errored, 0 skipped
```

The three refusals, live on the same recording:

- A script that threw after creating records: `status "fail"`,
`stepCount`
unchanged at 1, and the message naming the side effects that were not
rolled
  back.
- A path resolving to no file: refused before any fork, no `durationMs`,
and
  "Nothing ran, so there is nothing to clean up".
- `flow-add-echo` holding `{{output:user.id}}`: refused with the
author-facing
  message naming the step, the field and the later release.

No device is involved at any point — a recording may open with a seeding
script
before it boots anything.



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added the `flow-add-script` tool to run local `.mjs` scripts and
record successful executions in flows.
* Added script status, output, duration, logs, and failure details to
recording results.
* Added clearer handling for script cancellation, timeouts, missing
files, and path casing.

* **Documentation**
* Updated flow guidance to clarify end-to-end classification and script
setup or cleanup usage.
* Documented script path requirements, failure conditions, and recording
workflows.

* **Bug Fixes**
* Flows now reject unsupported output-reference templates with more
precise diagnostics.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-07 12:59:29 +02:00
Hubert Gancarczyk 9c4bc9ef5b feat(flows): run an external script as a flow step (#865) 
Stacked on #864, which adds the executor. This branch adds the `script:`
step
itself: the YAML directive, the runner integration, the CLI and MCP
rendering,
and the authoring reference.

```yaml
- script: { path: ../../scripts/seed-order.mjs }
- script: { path: ../../scripts/seed-order.mjs, timeout: 60000 }
```

A `script:` step runs an external `.mjs` file from the project, in a
fresh Node
process. What the script does is its own business — seeding an order,
creating
a test account, cleaning up what a run left behind are the reasons flows
have
wanted one. It drives no device, so a flow of nothing but scripts runs
with
nothing booted.

## What it adds

- **The directive.** The value is always a map; `path` is required and
obeys
the same name rules a `run:` target does, spelled for `.mjs`. It
resolves
against the directory of the flow file containing the step, so a
fragment
reaches the same script whichever flow composed it. A mis-cased path is
refused rather than run, because macOS and Windows would open it and
Linux CI
  would not.
- **The report.** The script's stdout and stderr come back on the step
and
print under the step line, on a pass as well as a failure — it is the
only
record of what the step did to the backend. Both renderers (CLI, MCP)
carry
  it, live and buffered.
- **The fail/error split.** A `fail` is the script's own answer; an
`error` is
the host's. That is what lets CI tell a regression from the machine it
ran
  on.
- **Two boundaries.** An uploaded flow carrying a `script:` step is
refused
before anything runs — its `.mjs` never left the client. A flow whose
script
  step sits beside a `run:` step still resolves a device.

## Review follow-ups

The last three commits answer a review of the three above them.

- A launch behind a leading `script:` was not read as the flow's leading
launch, so a Chromium flow that seeded a backend first hoisted no boot,
bound
  to whatever browser happened to be up, and passed a launch it never
  performed. The same lead-in walked a fragment past the
`executionPrerequisite` refusal its `echo:`-led twin is given. Both
readers
  now share one exhaustively-switched predicate.
- Coverage: the whole failure-kind table, the mis-cased arm CI never
ran, the
two unreached `scriptFileProblem` arms, symlink resolution for a script
path,
  and the CLI live-progress closure that no test had executed.
- Docs: the reference claimed the log was secret-redacted (it is not),
never
mentioned the environment allowlist, the working directory, the
time-limit
ceiling or the log caps, stated a fail/error rule that was not the
code's,
and was missing from its own table of contents. The skills stated a
closed
list of legal unrecorded insertions that excluded the only way this step
can
  ever be written.

## Verification

`npm run build`, `npx eslint . --max-warnings 0`, `npx prettier --check
.`,
`npm run knip`, `npm run typecheck:scripts` and `npm run test:scripts`
are
green, as are the three `tsconfig.test.json` typechecks. Suites:
tool-server
357 files / 4552 passed, `@argent/cli` 469, `@argent/mcp` 83,
`@argent/registry` 90.

Reproduced end to end against a real tool server and a real Electron
app, not
only in tests: a flow of `script:` then `launch:` now boots its own
Chromium
instance and reports it, where before it attached to a stray browser and
reported a green launch for an app path that did not exist.



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Added support for local `.mjs` scripts as flow steps, including
configurable timeouts, validation, cancellation, and device-free
execution.
- Script output is displayed in CLI runs and included in MCP flow
results, with indentation and truncation notices.
  - Script failures now provide clearer diagnostics and verdicts.
  - Flow authoring tools recognize and guide creation of script steps.

- **Documentation**
- Added guidance for script syntax, execution limits, output handling,
classification, and remote-run restrictions.
  - Updated flow creation and QA guidance to include script steps.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
 2026-09-07 12:08:13 +02:00
filip131311 c573e9c524 feat(telemetry): report device_kind next to platform on tool events (#1049) 
## Why

Telemetry cannot tell a physical iPhone from a simulator: `platform` is
`ios` for both, and the tool-server's `kind` never reaches the events.
Physical-iOS support shipped in 0.24.0 with no way to count its adoption
except through `ios_device_*` failure stages on `tool:fail`. Physical
Android phones have the same gap.

## What

`tool:invoke` / `tool:complete` / `tool:fail` gain `device_kind:
simulator | emulator | vvd | device | app`, derived from the same device
id as `platform` (single classifier in `telemetry-platform.ts`, which
`debugger:tool_outcome` and `lens:*` also go through for their
platform). `platform: ios` + `device_kind: device` is a physical iPhone.

The kind is emitted only for positively recognised id shapes, so the
metric is not polluted by ids that are not devices:

| id shape | platform | device_kind |
|---|---|---|
| simulator UUID / `remote:` | ios / ios-remote (tvos when the cache is
warm) | simulator |
| physical iPhone UDID | ios | device |
| `emulator-NNNN` | android (android-tv when warm) | emulator |
| USB serial, 6–20 alnum with a digit and a letter | android | device |
| non-loopback `ip:port` | android | device (best effort) |
| loopback `127.0.0.1:port` / `localhost:port` / `::1:port` | android |
emulator |
| anything else non-iOS-shaped (Metro 40-hex handle, `booted`,
`Pixel_7`, IPv6, mDNS) | android | omitted |
| `chromium-cdp-<port>` / `amazon-…` | chromium / vega | app / vvd |
| `avdName`-only boot | android | omitted |

A child sub-tool that names its own device replaces the parent's
platform, kind and provider label wholesale, so an `avdName`-only child
never reports the parent's `simulator` next to its own `android`. A
logical-keyed Metro session strips the kind.

`unknown` (in the registry `DeviceKind`) is rejected by the sanitizer
like `platform: "unknown"`; no registry change.

## Querying

- Physical iPhone adoption: `tool:invoke` where `platform = 'ios' and
device_kind = 'device'`, distinct users over it. `isIosPhysicalUdid` is
a positive match, so invoke is clean.
- Physical Android: count on `tool:complete` (the device answered).
`tool:invoke` / `tool:fail` still carry residual mistakes that happen to
look like a serial (`Pixel7`), and a non-loopback `ip:port` may be a
networked virtual device.

## Docs

- `Telemetry.md` → 1.04, effective date 7 September 2026 (adjust to the
release date on merge). Discloses the new category and, while touching
that section, the external-provider label that #735 added without a
notice line — happy to drop that second bullet if it should ship
separately.
- No docs-site change: `docs/reference/telemetry.mdx` links to
`Telemetry.md` and has no per-property list.

## Notes

- Pre-existing, unchanged: HTTP-edge failures carry no
`tool_invocation_id`; `stop-all-simulator-servers` declares no
capability, so its invoke row has neither field while its fail row has
both. `device_kind` follows `platform` in each site.
- Legacy 40-hex iPhone UDIDs already classify as `android` today; they
now get no kind rather than a wrong one.
- Design went through three adversarial review rounds before
implementation and one on the diff; the fallback-bucket and
child-attribution rules above came out of those.

## Verification

- `packages/telemetry`: 319 tests, `packages/tool-server`: full suite
5670 passed / 1 skipped; `typecheck:tests` both packages; `eslint
--max-warnings 0`; `knip`; prettier.
- New: `test/telemetry-device-kind.test.ts` (the shape table above),
plus cases in `http-tools-meta`, `http-platform-alias`, `sanitize`,
`registry-listener`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01BSGpeAwvoCv2JAhbLsuto2


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Telemetry now records a device category—such as simulator, emulator,
physical device, virtual device, or desktop app—alongside the platform.
* Device categories are inferred from device information and may include
a short label for external device providers.
* Device category information is included in tool invocation,
completion, and failure telemetry when available.
* Device identifiers themselves continue to be excluded from telemetry.
* **Documentation**
* Updated the Argent Privacy Notice (Telemetry) to version 1.04,
effective 7 September 2026.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-07 11:55:57 +02:00
Hubert Gancarczyk 1a7e97fb14 feat(flows): run a local script in a bounded child process (#864) 
Adds the executor a flow `script:` step runs on: a fresh Node child per
step,
a protocol over its IPC channel, and the two watchdogs that make a hung
or
runaway script the runner's problem rather than the host's.

The step itself is not here. This branch stops at the executor and its
unit
tests; `feat/flow-script-step` stacks the YAML directive, the runner
integration, and the reference docs on top.

## What it does

- **One child per step.** Spawned with an old-space heap limit, an
explicit
working directory, and an environment built from an allowlist rather
than
  copied from the tool server.
- **A deadline and a lifeline.** A separate watchdog holds the deadline,
so a
script that wedges the event loop still dies on time; a second one reaps
the
whole process tree when the parent goes away, so a grandchild cannot
outlive
  the run.
- **A concurrency queue.** Slots are bounded per server. A step that
never gets
one is refused with a message that says so, and an aborted run frees its
slot
  immediately.
- **Log budgets.** 64 KiB per step and 256 KiB per run, counted on the
bytes
the report keeps: redaction and V8 frame collapsing both run before
anything
is counted, so what the limits bound is the size of the report rather
than
  the size of the script's writes.
- **Secret hold-back.** The scrub walks a chunk and stops where a value
could
still begin, so neither half of a value split across two chunks is
released
  on its own, and a shorter value is never taken where the longer one
  containing it has not arrived yet.
- **A failure taxonomy.** Twelve kinds, split into what the script did
(it
threw, it did not load, it exited non-zero, it wrote an unusable
`output`)
and what the host did to it (a limit, a signal, a spawn that failed, a
queue
  slot it never got).

## Docs

`packages/docs/docs/reference/configuration.mdx` lists the two
configuration
keys this branch adds, `scripts.maxTimeoutMs` and `scripts.heapLimitMb`.
The
`script:` step itself is documented with the step, on
`feat/flow-script-step`.

## Verification

`npm run build`, `npx eslint . --max-warnings 0`, `npx prettier --check
.`,
`npm run knip`, `npm run typecheck:scripts`, the test typecheck across
every
workspace, the tool-server suite (4719 passed, 1 skipped) and `npm run
test:scripts` (92 tests) are green, as is `npx docusaurus build`. The
executor's
own behaviour is covered by the eight `test/flows/script/` files added
here,
against real child processes, and the compiled `dist/` executor was
driven
against its copied runner assets over every path the review reached.

Each review fix carries the run that reproduced it before the change and
the
mutation that proves the new test fails without it: the prefix-secret
leak and
all three npm `node-options` routes were reproduced end to end through
the
executor against real child processes, and the five coverage findings
were
confirmed by re-applying the exact mutation each thread named.




<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added reliable execution of trusted flow scripts with concurrency
control, cancellation, timeouts, memory limits, logging, and detailed
failure reporting.
* Added safeguards for process cleanup, watchdog termination, output
validation, and secret redaction.
* Added global configuration for maximum script runtime and memory
usage.
* **Bug Fixes**
* Improved handling of script failures, stalled processes, malformed
output, and child-process termination.
* **Documentation**
  * Documented script resource limits and global configuration behavior.
* **Chores**
* Updated builds and packaging to include required flow-script runtime
assets.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
 2026-09-07 11:34:30 +02:00
Mateusz Aliyev 6e9e45c9f0 feat: attach to devices offered by external providers (#735) 
This pull request lets Argent drive a simulator or emulator another
process is already running, attaching to its simulator-server rather
than spawning a second one.

A provider writes a JSON descriptor to `~/.argent/providers/` listing
the devices it offers and the mechanisms Argent may use on each. They
appear in `list-devices` with an `ext:` id and work with the existing
tools. The file is re-read on every call, so a withdrawal or a narrowed
grant applies immediately.

`ios.additionalDeviceSets` (#600) already makes such a simulator
reachable by UDID. What changes is ownership. `boot-device` and
`stop-simulator-server` refuse these devices, anything the provider did
not grant is refused with a message naming it and Argent uses only the
endpoints its own simulator-server build serves. A grant binds to the
device rather than to one of its names, so the real udid or serial is
gated exactly like the `ext:` id. Android emulators are covered too and
a device visible to both a provider and `adb`/`simctl` is listed once.

The contract ships as `schemas/device-provider-v1.json`, validated by
`argent providers check`, and is documented in
`docs/reference/device-providers.mdx`. Includes unit tests and an e2e
phase where the harness acts as its own provider.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added support for discovering and connecting to externally managed
devices.
* Added `argent providers` commands for listing, validating, publishing,
withdrawing, and pruning providers.
* Added provider-aware simulator, debugger, profiler, native tools, and
device listings.
* Added capability controls, endpoint validation, revocation handling,
and provider-specific diagnostics.
  * Added automatic CLI discovery for provider integrations.
* **Bug Fixes**
  * Improved service recovery and paused-runtime error reporting.
  * Improved simulator keyboard and paste command reliability.
* **Documentation**
* Documented provider descriptors, capabilities, lifecycle rules, and
CLI usage.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-04 22:02:57 +02:00
Ignacy Łątka 4c645f1619 fix(chromium): correct the hidden-window guard's docs and pin its exemptions (#611) 
Follow-ups from a post-merge review of #605. Each one is a comment
thread on that PR.

No behaviour changes to what the guard does — the three emitted
`failure_stage` strings and the `CHROMIUM_WINDOW_HIDDEN` code are
byte-for-byte identical, so telemetry stays joinable.

## Changes

**Derive `failure_stage` from `action`.** It was a second free-form
`string` parameter sitting next to `action`, and all three call sites
passed a strictly redundant pair — `("tap",
"chromium_tap_window_hidden")` and so on. Two adjacent same-typed params
meant transposing them or mistyping the stage compiled cleanly and read
fine in review. That carries extra weight here because #605 deliberately
froze `chromium_scroll_window_hidden` to keep pre- and post-migration
rows joinable. `action` is now `"tap" | "drag" | "scroll"`, so a typo is
a compile error, and the docblock spells out all three literals so they
stay greppable in `src`.

**Pin the keyboard exemption.** Key events skip hit-testing and stay
fast on a hidden window, so `keyboard` is deliberately unguarded. That
was correct but lived only in a comment: adding the guard to `keyboard`
would have made it refuse input that demonstrably works, and the suite
would have stayed green. Now covered by
`keyboard-chromium-unguarded.test.ts`.

**Pin the `button` exemption too.** The same file now asserts that
`assertSupported` rejects a Chromium device for `button`. The claim that
`button` can never reach the guard rested entirely on a capability
literal no test read — adding `chromium: { app: true }` to it reddened
nothing.

**Correct the same comment's claim about `button` — three times.** The
first pass replaced the wrong rationale (that `button` skips the guard
because key events bypass hit-testing) with the real one: its capability
omits chromium. The replacement was itself wrong about where Chromium's
button surface lives. `sendButton` is reached only from
`handleWsCommand`, i.e. the chromium-server **WebSocket** channel — the
HTTP router exposes screenshot / clipboard / fps / navigate / reload /
history / viewport / stream and no button at all. It also over-claimed:
`sendButton` emulates `Back` alone as an Alt+Left chord and throws for
every other button, so "chromium hardware buttons exist" contradicted
`button/index.ts` ("Chromium apps have no hardware buttons"). The
wording now agrees with both neighbours. A third pass dropped the last
false note in it: the lockout test said the gate stops `button` "before
it can reach the guard", but `button` has no chromium branch and never
imports the guard — there is no guard on its path to reach.

**Give the keyboard measurement its missing condition.**
`Input.dispatchKeyEvent` at 1–14 ms against 5002–5005 ms per mouse move,
and "focus emulation keeps input fast even for a genuinely minimized
window", describe the same scenario with opposite outcomes — those
numbers can only come from a window where focus emulation was not in
effect. The docblock says so, and names focus emulation alone:
`primePageSession` applies it to every session at connect, so the probe
reads "hidden" only where the runtime refused it.

The anti-throttling launch flags are not part of that condition and do
not narrow the guard's reach. A window carrying all three, minimized
without emulation, still reads "hidden" and still costs ~5 s per
`Input.dispatchMouseEvent` — Electron 42.5.0 5001–5005 ms and Chrome 152
5002–5010 ms, 3 trials each — so an argent-spawned app is no less
exposed than an externally launched one. `boot-electron.ts`'s
`ANTI_THROTTLING_ARGS` docblock, where that claim originated, is
corrected to match, along with its test.

**Two mechanisms that were stated unconditionally.** `Page.bringToFront`
un-minimizes on Chrome 152 (`windowState` minimized → normal,
`visibilityState` hidden → visible, mouse dispatch 5002–5006 ms → 16–25
ms, 3/3) and not on Electron 42.5.0 (returns ok, stays `"hidden"`, stays
5001–5005 ms, 3/3), so `flow-run`'s comment calls the outcome
runtime-dependent rather than denying it. And the capability gate that
keeps `button` off a Chromium device is not universal: `assertSupported`
runs at the HTTP edge and in run-sequence's pre-flight, but
`Registry.invokeTool` skips it, so a flow step dies resolving
`services()` with `SimulatorServer.factory does not support platform
"chromium"` rather than an `UnsupportedOperationError`.

**Replace the try-boundary rationale.** It said the property access had
to stay inside the `try` because fakes "may lack `cdp`" — but for such
an api `chromium.cdp` yields `undefined` without throwing; the TypeError
comes from reading `.send`, which is inside the `try` wherever
`chromium.cdp` is read. Verified: hoisting `const client =
chromium.cdp;` above the `try` leaves every relevant test green. The
placement would only matter if `chromium` itself were nullish, which no
registry path produces.

**Cover the no-`cdp` probe path directly.** The existing test claimed to
cover "api objects with no `cdp` at all" but used a rejecting `cdp.send`
— a different failure mode. The real shape was exercised only
incidentally, by bare fakes in `chromium-drag.test.ts` and
`tools/gesture-tap.test.ts`.

**Cover the other half of "a failed or empty read proves nothing".**
Only the throw paths were pinned. Flipping the comparison to `value !==
"visible"` left the entire suite green, so a probe that resolves without
a usable value — `Runtime.evaluate` against a torn-down document — would
have started refusing tap/drag/scroll on a fully visible window.

**Pin the action word in the user-facing message.** Hardcoding `Cannot
tap:` for all three tools also stayed green, so a scroll refusal reading
"Cannot tap" could have shipped unnoticed.

**Update `flow-run`'s comment.** It still named gesture-scroll as the
only tool that fails fast on a hidden window; tap and drag have since
#605. It also over-promised "fail fast": on the flow path
`frontChromiumPage` has just primed focus emulation, so the three tools
proceed. They carry the guard; whether it fires depends on the
mitigations.

## Verification

- Full gate on the branch: `tsc --build`, `typecheck:tests`, `eslint
--max-warnings 0`, `prettier --check`, and the whole tool-server suite
sharded — **360 files / 4504 passed, 1 skipped, 0 failed**.
- Every empirical claim in the comments was re-measured on a live CDP
testbed rather than carried over: an Electron 42.5.0 app and Chrome 152,
each launched with and without the three `ANTI_THROTTLING_ARGS` plus
`--remote-debugging-port`, minimized, probed for
`document.visibilityState` and per-event `Input.dispatchMouseEvent` /
`Input.dispatchKeyEvent` latency before any focus emulation, then after
`Page.bringToFront`, then after `Emulation.setFocusEmulationEnabled`.
Runs were sequential, one browser at a time, 3 trials per condition.
- The `button`-on-chromium flow path was run, not just read:
`registry.invokeTool("button", { udid: "chromium-cdp-9222", button:
"home" })` with the simulator-server blueprint registered throws
`ToolExecutionError … SimulatorServer.factory does not support platform
"chromium"`.
- The measurements behind the keyboard exemption were taken on a real
minimized Electron window (v42.5.0): `Input.dispatchKeyEvent` 1–14 ms,
against 5002–5005 ms for each of ten consecutive
`Input.dispatchMouseEvent` moves, with `Runtime.evaluate` staying at 1–3
ms.
- Mutation-tested every pin rather than assuming it bites. Each of these
fails the suite: adding the guard to `keyboard/chromium`; adding
`chromium` to `button`'s capability; making the probe rethrow instead of
swallowing; drifting the derived stage template; collapsing the stage to
one action for all three tools; flipping the comparison to `value !==
"visible"`; and hardcoding the action word in the message.


- No docs update needed: comments only, no user-facing behaviour or
tool-surface change.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Documentation**
- Clarified how window minimization, focus emulation, and
anti-throttling behavior affect Chromium interactions.

- **Bug Fixes**
- Standardized hidden-window handling and error reporting for tap, drag,
and scroll gestures.
  - Preserved keyboard input on hidden Chromium windows.

- **Tests**
- Expanded coverage for hidden-window gestures, keyboard input, missing
browser capabilities, and varied visibility responses.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-04 14:43:54 +02:00
Ignacy Łątka e895dd0a5d fix(flow): give every flow in a directory run a verdict on stdout (#739) 
## Every flow run says how it ended, on stdout

`argent flow run` wrote some outcomes only to stderr. A CI job that
archives stdout — the
common case — got a log with a hole in it.

**A directory run**, three flows, one with a `run:` the parser rejects:

```
$ argent flow run <dir> > out.txt 2> err.txt ; echo $?
1

── out.txt ──                      ← the log that gets archived
[1/3] a-ok.yaml
  PASS — 0 passed, 0 failed, 0 errored, 0 skipped (no test steps)
[2/3] b-broken.yaml
[3/3] c-ok.yaml
  PASS — 0 passed, 0 failed, 0 errored, 0 skipped (no test steps)

FAIL — 3 flows: 2 passed, 1 failed, 0 skipped
```

`[2/3]` has a header and no outcome. The tally says one flow failed and
stdout names none;
the reader's inference that `b-broken` was the one is available only
because it is the sole
entry missing a line.

**A single-flow run** had the same gap, and worse — with live step lines
the last thing the
log holds is a `✓`, so a run that died reads as one that passed and got
truncated:

```
$ argent flow run checkout.yaml > out.txt 2> err.txt ; echo $?
1

── out.txt ──
Flow "checkout"
  ✓  1 tap
  ✓  2 assert                      ← ends here
```

## The fix

An indented verdict on stdout for every branch that had none, on both
runners:

```
  ✗ not run (invalid flow)          ← rejected by validation; a batch continues
  ✗ not run (no device resolved)    ← nothing matched the run, or several did and none was singled out
  ✗ not run (rejected)              ← rejected for a reason neither of the above names
  ✗ did not finish (run error)      ← in a batch, this flow is the one that stopped it
  ✗ did not finish (no run report)
```

stderr is untouched, so anything reading that stream sees exactly what
it did before, and the
detail is not duplicated onto the merged stream. Both runners print the
verdict directly under
the header and the stderr detail after it, so a `2>&1` log of a batch
reads like a `2>&1` log of
a single run. No verdict reaches `--json` — every one is behind
`!args.json` — so neither runner
ever mixes prose into that stream. What each emits is unchanged: a batch
still prints its one
aggregate document on every path, and a single run still prints the
report alone, which means
nothing at all when the run produced no report.

### What a batch's `--json` carries

The aggregate is the mode a CI job consumes for a directory, so the
classification the verdict is
chosen from travels with it: a failed entry carries `error_code` and
`error_kind` beside `error`,
under the names `--json-stream` already publishes. Without them an
invalid flow file and an
unresolvable device are the same `{"status": "fail", "error":
"<prose>"}` entry with the same key
set, told apart only by a message the server assembles from device lists
and remedies.

### Picking the wording from the failure code, not the kind

`error_kind: "validation"` marks any rejection the server scopes to one
call; it says nothing
about the flow file. Device resolution rejects that way too
(`FLOW_DEVICE_RESOLUTION`), so
keying the wording off the kind made a batch run with nothing booted
report every one of a
directory of good YAML files as `not run (invalid flow)` — sending the
reader to the wrong
file. `rejectionVerdict` switches on `error_code` instead: only a code
whose subject is the
file (`FLOW_FILE_INVALID`, `FLOW_ENTRY_UNRECOGNIZED`,
`FLOW_E2E_HAS_PREREQUISITE`) licenses
"invalid flow"; anything unrecognized stays neutral and leaves the
reason to the stderr line
beside it.

`FLOW_NAME_INVALID` is deliberately absent. `argent flow run` only ever
sends `flow_path`, and
both of its paths screen the stem against `SAFE_FLOW_NAME` first — which
*is*
`FLOW_NAME_PATTERN`, the pattern `assertSafeFlowName` applies on the
server — so the code
cannot come back from a flow run. `FLOW_PROJECT_ROOT_INVALID` is out for
the same reason:
`project_root` is `process.cwd()`, always absolute and never
`..`-bearing.

Which rejections let the batch continue is unchanged — still `errorKind
=== "validation"`.

### Also on the single-flow path

`!report || !("steps" in report)` took a primitive wire value straight
into `in`, which throws
on one — `TypeError: Cannot use 'in' operator to search for 'steps' in
a-string`, surfacing
through the CLI's top-level catch as a raw stack trace. The batch path
guards this deliberately
("must classify as no report, not as an infra throw"); this one now does
too.

### Docs

The help text said a directory run prints "only failing steps plus a
final summary" and named
an invalid file as the only rejection it survives. It now describes what
it prints and what it
continues past.

`packages/docs/docs/reference/flow-yaml.mdx` still described the batch
as printing "only the
failing steps and a summary" and named an invalid file as the only
rejection it survives. It now
describes what a directory run prints, tables the six outcome lines with
their meanings, states
what the `--json` aggregate holds down to `error_code`, notes that a
single-flow `--json` failure
writes nothing at all to stdout, and adds `--json-stream` to the option
table — the mode that
carries the failure as a record.

Two claims that survived into the first pass of that page were wrong and
are corrected here: a
flow that fails its *steps* also leaves the batch running (only a
transport failure, a rejection
of another kind, or a reply that is not a report stops it), and the
aggregate entry for such a
flow carries a report and no `error`. The `flow run --help` blurb
carried the first of those two
as well.

`argent-create-flow` carried the same stale sentence when this branch
opened, and an earlier
commit here fixed it too. #730 has since split that skill into
`references/` and dropped the
standalone-runner paragraph from the tree, so there is nothing left to
correct there and the
merge resolution takes the hunk out.

## Checks

- E2E through the built CLI (`packages/argent/dist/cli.js`) for every
transcript above, plus
the `--json` shapes piped through `python3 -m json.tool` to prove one
document.
- `packages/argent-cli`: 512 passed across 24 files (`flow.test.ts`
148).
- Mutation matrix over the new behaviour, 24 mutations applied and
reverted individually:
deleting each of the five verdicts, dropping either `!args.json` guard,
weakening
`isFlowReport` in six ways (each conjunct dropped in turn,
`every`→`some`, and reverts to
both earlier readings), deleting each `rejectionVerdict` arm, swapping
two wordings,
dropping either `liveSteps === 0` header guard, dropping `stopped =
true`, restricting the
batch verdict to the first flow, and emitting a verdict on the success
path. Every one
  reddens a test; no gaps.
- Reverting only the source hunks fails exactly the tests added here; no
pre-existing test
  changes state.
- E2E against a real tool-server for the review follow-ups: a directory
of one unparseable
YAML plus one flow needing a device, run with the verdict and the stderr
detail merged
(`2>&1`) to show the order, and again under `--json` —
`FLOW_FILE_INVALID` / `validation`
arrive in the aggregate entry and stdout parses as one document. A
`--platform vega` run with
  no VVD gives the real `not run (no device resolved)` line.
- Mutations for the follow-ups, each applied and reverted: dropping the
failure signal from the
aggregate, relaxing the batch guard to `!args.json ||
rejectedThisFlowOnly`, dropping
`&& !args.jsonStream` from the single-flow guards, restoring the old
detail-before-verdict
  order, and deleting the batch verdict. Every one reddens a test.
- prettier, eslint (`--max-warnings 0`), `tsc --build`, `tsc -p
tsconfig.test.json`, knip and
  `npx docusaurus build` in `packages/docs` all clean.

Two findings from reviewing this branch are out of scope and filed
instead: #1025 (`--json`
prints the usage text on stdout for a flag error, where `--json-stream`
keeps stdout
machine-readable) and #1026 (a directory run's artifact export sits
outside the loop's `try`, so
an export throw loses the batch summary, the skip entries and the
aggregate).

Directory runs arrived in #576, so the batch half is a defect in a new
feature rather than a
regression; the single-flow half predates it.





<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added clear outcome lines for flow runs, including validation
rejections, execution errors, missing reports, and skipped flows.
* Batch runs continue after flow-specific validation rejections and stop
when other errors occur.
* Added machine-readable failure classifications to JSON output,
including batch aggregates.
* Added `--json-stream` for one JSON record per line during single-flow
runs.
* JSON modes now emit only machine-readable output, with aggregate
details for directory runs.

* **Documentation**
* Expanded flow command documentation with batch behavior, outcome
statuses, and JSON output details.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-04 14:28:52 +02:00
Ignacy Łątka f05d17c1d3 refactor(flow): let each step kind own how it reads (#744) 
Stacked on #574 (merged; retargeted at `main`).

## Summary

`summarizeSteps` (flow-finish-recording.ts) was a 50-line `switch` over
`FlowStep["kind"]`. `stepTarget` (flow-run.ts) was a second one, over
the same union, with its own selector and condition spellings. Neither
could be found from the other, and `summarizeSteps`' `case "tool":
default:` arm meant a step kind added without wording rendered as a
`tool:` step instead of failing.

Both surfaces now read from one definition per kind — the same shape as
a tool owning its own log wording in `ToolDefinition.interaction`
(#582):

```ts
const FLOW_STEP_DEFINITIONS: {
  [K in FlowStep["kind"]]: FlowStepDefinition<Extract<FlowStep, { kind: K }>>;
} = {
  "scroll-to": {
    summary: (step) => `${yamlSelectorLabel(step.target)} (${step.direction})`,
    target: (step) => { ... },
  },
  ...
};
```

Keying the record by `FlowStep["kind"]` is what replaces the default
arm: a new kind is a compile error until it says how it reads on
**both** surfaces. That also retires the `renderToolArgs` interpolation
guard, which existed only because the default arm could hand it a step
with no `args` field.

`flow-run.ts` sheds a net 113 lines and five imports;
`flow-finish-recording.ts` sheds a net 180 and is now just the tool. The
two shared label templates (`conditionLabel` parameterized by selector
spelling, and a new `whenLabel` for the platform-vs-UI guard arm) mean
the recorder's `when:` tail and the runner's no longer duplicate the
platform arm.

## Test

The extraction is rendering-neutral: every summary line and report
target the switches produced moved over byte-for-byte (with four later
corrections, below).

- Proved with a throwaway equivalence test that inlined both
pre-refactor switches verbatim and compared them against the new
definitions over 1264 generated steps: all 15 kinds, 10 selector shapes
(loose, regex, scoped, quote-bearing), every wait condition × text-match
mode × expectation, every scroll direction, and the cyclic-`args` path.
Identical on all of them. Mutating two renderings (`equals` comparator,
default scroll direction) failed it, so it discriminates.
- Replaced by
`packages/tool-server/test/flows/flow-step-definitions.test.ts`, kept:
assertions pinning the exact summary line and report target of every
kind, in a table keyed by `FlowStep["kind"]` so a new kind fails
`typecheck:tests` here too (verified by deleting the `wait` entry:
`TS1360`). Deleting a kind from the definitions record fails the build
the same way (`TS2741`).
- `npm run build`, `eslint --max-warnings 0`, `prettier --check .`, `npm
run typecheck:tests --workspaces`: clean.
- `packages/tool-server`: 329 files / 3906 tests passed.
`packages/argent-cli`: 459 tests passed.

## Review follow-up (9cf51322)

Four corners where the carried-over rendering misdescribed what replays,
fixed with pinning tests:

- `delayLabel`: a hand-edited `delayMs` above `setTimeout`'s 2³¹−1
ceiling sleeps a clamped 1ms tick but read `(after 3000000000ms)` — the
summary now describes no delay there.
- `scroll-to`: the `within` container is parsed, serialized, and changes
what replays, yet neither surface rendered it; both do now.
- `snapshot`: the summary now carries `cropOn` and `maxMismatch`
(baseline key and tolerance). Only `cropOn` was on the report target;
`maxMismatch` was rendered on neither surface (the run report surfaces
the tolerance through the step's `reason`, not its target).
- `type`: the summary JSON-quotes the text, the module's own convention
for literals, so embedded quotes/newlines stay one unambiguous line.


## Review follow-up (7c804907)

A second pass (7 lenses + a 32-mutation pass that also re-derived the
neutrality claim against main over a 136-step corpus, and confirmed the
mapped-type compile gate by adding a kind) found no HIGH or MEDIUM. One
rendering defect this PR introduced, plus the boundaries the case table
left open:

- `scroll-to`'s report target spelled the scroll container ` within (…)`
— exactly how `selectorLabel` spells a selector's own relational scope.
`scroll-to: { target: Footer, within: { id: list } }` (scroll that
container) and `scroll-to: { target: { text: Footer, within: { id: list
} } }` (match only inside it) both rendered `"Footer" within (id=list)`;
carrying both put the direction between two identical-looking scopes. It
now reads ` in scroll container (…)`, the noun flow-actions already uses
in its own "scroll container … is not visible" failure. `main` did not
collide here only because it dropped `within` entirely.
- New pinning cases, each verified to fail without the code it pins:
`setTimeout`'s ceiling from both sides (`2**31-1` keeps its label,
`2**31` loses it) and its 1ms floor; `maxMismatch: 0`, the one value
separating `!== undefined` from a truthiness test; `cropOn` +
`maxMismatch` together, which fixes their suffix order; and a
multi-field selector, which fixes the report target's field order.
- `Number.isFinite(ms) &&` in `delayLabel` became redundant the moment
correction #1 added the ceiling — NaN and ±Infinity both fail the range
test. Removed (proved a no-op by mutation).
- Prose corrected where it had stopped matching the code: the
point-gesture comment argued a criterion the file does not follow
(`snapshot` and `scroll-to` are equally not recorder-built and this PR
renders their fields anyway) and listed two of the four still-unrendered
replay-affecting fields; `flow-tools.test.ts` still claimed to be the
only coverage of the long-press rendering; and the new case table
justified a no-op `as number` cast with a reachability claim that is
wrong in both halves (`flow_add_step`'s `delayMs` is
`z.number().int().min(0)` with no ceiling, so the recorder reaches these
values too).

Follow-ups filed rather than folded in, both pre-existing and unchanged
here: #808 (an `echo` message or `tool` name containing a newline breaks
the one-line-per-step summary contract, the same hazard correction #4
fixed for `type`) and #809 (`type.submit`, `await.timeout`,
`idle.timeout`, `idle.stableFor` change what replays and render on
neither surface).


## Review follow-up (4560f412)

Round from @hubgan's review (11 inline) plus CodeRabbit's one. Nine
acted on, two verified and rejected — the rejection reason is now in the
code, so it stays answered rather than being re-raised.

**Rendering**

- `type`'s report target carried the typed text with **no bound**.
Unlike the summary, that target is emitted on every replay — stdout,
`--json`, the MCP blocks handed to the agent, and the appended
`~/.argent/mcp-calls.log`, which has no cap or rotation — so a literal
written into a flow file shipped its whole length to each. Now capped at
200 chars with the same `…(+N chars)` elision `parseFlow` uses on the
entry it echoes (`MAX_ENTRY_RENDER_CHARS`). The **summary keeps the text
whole**: it is also `stepAnchor`, so eliding there would collide two
steps differing only past the cap and drop the recording's verdicts on
both. Elides before quoting, so a cut cannot halve an escape.
- `selectorLabel` and `describeSelector` interpolated `textMatches`,
`identifier` and `role` raw — a newline in one splits a step across two
report lines, an escape byte drives the terminal. That is the hazard
`text`'s quoting already names, so it now covers its neighbours: new
`escapeInline` (flow-utils) spells out control characters **only**. Not
JSON escaping: doubling a backslash would print a regex source nobody
could copy back into the .yaml. `describeSelector`'s literal arm moved
from a raw `"${v}"` to `JSON.stringify` — byte-identical for ordinary
values, unambiguous for a quote in the content.

**Types**

- `FlowStepDefinition`'s two renderers were method shorthand, and a
method's parameters are checked **bivariantly even under `strict`**: a
definition annotated narrower than its slot (one requiring `times` on
`tap`) was accepted and then read a field the step need not carry, with
a green typecheck. Property types now — the same assignment is `TS2322`.

**Tests** — each verified against the mutation it exists to catch

- Every case in the table goes through `serializeFlow` → `parseFlow` and
is compared on its anchor. That round-trip stability is what
`scroll-to.within`, `snapshot.cropOn` and `snapshot.maxMismatch` newly
depend on, and nothing asserted it; deleting `yamlSelectorLabel`'s key
sort now reddens the suite. The `when` case with `steps: []` became a
two-step block — `parseFlow` refuses an empty guarded block, so no count
below 1 is renderable, and `when #1` already pins the singular.
- The case table's value type is a non-empty tuple of cases **narrowed
to the key's own kind**. `"wait": []` and a `wait` slot holding an
`echo` case both typechecked clean before; both are `TS2322` now.
- The `when:` pluralization coupling only ran a two-step block, so
hardcoding either surface's tail to `(N steps)` left everything green.
Both counts run now, and against the whole skip reason rather than its
suffix.
- New pins for the escaping above, on both surfaces, each confirmed to
fail without the line it pins.

**Prose corrected against the code**

- `stepAnchor`: there are **two** anchor comparisons, not three, and
only the per-verdict one is unconditionally raw-vs-parsed — host mode
re-parses both sides of the whole-file check, which is raw-vs-parsed
only in client persist mode.
- `delayMs` bounds: `.int()` admits nothing strictly between 0 and 1, so
the 1 ms floor is hand-edit-only; the ceiling is recorder-reachable, as
stated.
- `0` is not a parse-only `delayMs`: `min(0)` accepts it and
`serializeFlow` writes it back out.

**Rejected, verified**

- Escaping `snapshot.name` (CodeRabbit) would be dead code: `parseFlow`
gates it on `FLOW_NAME_PATTERN` — letters, digits, `_`, `-` — because it
becomes a baseline filename, and that parse is the **only** construction
site of a snapshot step. A comment there now says so.
- "`textMatches`' `/…/` delimiters are unpinned" no longer holds at this
head: `flow-progress.test.ts`'s swipe-`from` target asserts `to (0.1,
0.5) from /^Card \d+$/`, and dropping the delimiters fails it. Measured
by mutation.

**Verification**: 371 files / 4950 tests in `packages/tool-server`;
`argent-cli` 527, `argent-mcp` 87, `registry` 120. `npm run build`,
`typecheck:tests --workspaces`, `eslint --max-warnings 0`, `prettier
--check .`, `npm run knip`: clean. 13 mutations aimed at the new code,
all killed.

**Docs**: no update needed. `packages/docs/` documents flow YAML, the
CLI and the tools; it does not specify how a step summary or a
`StepReport.target` is spelled, so nothing there goes stale. That covers
the two earlier rendering changes from `3e2244b3` as well —
report-target text literals becoming JSON-quoted, and the `type` target
carrying the typed text (now bounded, above).



## Review follow-up (9f8a1e29)

The two threads left open after approval.

- `typedTextLabel` compared and cut on `text.length`, which counts
UTF-16 units. 199 ASCII chars plus one astral char is 200 characters but
201 units, so the target quoted a lone high surrogate and announced `(+1
chars)` for text that fits under the cap whole. It counts and slices
code points; the boundary is pinned on both sides of the cap and each
assertion is red without the fix. Two sibling truncations are
deliberately left, both pre-existing and outside the step-summary code:
`badEntry` slices an already JSON-stringified entry, and
`cappedReason`'s cap bounds the EMITTED length, where counting code
points would let the emitted string overshoot its own bound.
- The `textMatches` delimiter thread, re-measured at this head: dropping
the `/…/` fails `flow-progress.test.ts:231` and
`flow-step-definitions.test.ts:451` - 2 failed | 1571 passed of 1573.
Restoring them returns the suite to green.

**Verification**: `packages/tool-server` 371 files / 4950 tests; `eslint
--max-warnings 0`, `prettier --check .`, `tsc --noEmit` on both
`tsconfig.json` and `tsconfig.test.json`: clean.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Improvements**
* Flow step summaries and run reports now use consistent formatting
across supported step types.
* Type steps display their target identifier and entered text more
clearly.
* Selector, gesture, condition, delay, and snapshot details are rendered
consistently, including safer handling of special characters.
  * Tool steps with cyclic arguments are handled safely in summaries.

* **Tests**
* Added comprehensive coverage for step rendering, flow numbering,
conditions, edge cases, and report output.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Hubert Gancarczyk <hubert.gancarczyk@swmansion.com>
Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
 2026-09-04 12:54:13 +02:00
Sebastian Flajszer 1068430b91 feat(ios-device): small fixes related with ios-device support (#1031) 
Follow-up to #944: the fixes made after the review that did not make the
squash.

## What changes

- **Signing note**: delivered only to a call whose device argument
resolves to a physical iPhone, including `flow-execute`'s `device`, and
held (not dropped) across other calls. The label is now `Apple
Development (KEYID)`; the certificate CN's Apple ID no longer reaches
agent context or logs.
- **Flow device resolution**: the no-device-matched refusal names the
cabled phone and the exact `device <udid>` / `--device` spelling. A
physical iPhone is never picked automatically.
- **`gesture-swipe`**: the description states the hardware semantics
(edge gesture needs `fromX` 0, `durationMs` sets drag speed,
`momentum:false` does not damp).
- **Skills**: `argent-ios-simulator-setup` skips `kind: "device"`
entries; `argent-ios-device-setup` and `-interact` state the runner
build timing, the full list of tools that do not exist on hardware, the
backgrounded-target behaviour of each read tool, `actionButton` only on
models that have one, and the swipe and `open-url` rules; the flow,
Metro and profiler skills each carry one line on physical-iPhone limits.
- **Docs**: `physical-ios-devices`, `flows`, `flow-yaml` and `tools`
corrected to match the behaviour above; README platform row; runner
`PROTOCOL.md`.

## Verification

- tool-server suite 400 files / 5173 tests, `tsc`, lint, prettier and
the docs build clean.
- Live re-check against 0.23.0 on three simulators and a physical iPhone
15: no regression; the note lands once on the first phone call with no
e-mail address, and flow auto-detection is byte-identical to 0.23.0 with
0, 1 and 3 simulators booted.
 2026-09-03 14:53:25 +02:00
Ignacy Łątka f106e486e0 fix(tools-client): make the server identity guard survive a clipped ps and a sanitized PATH (#713) 
## Problem

`processCommandMatches` reads a tool-server's command line to confirm a
pid is one of ours before signalling it — the guard that stops
kill-before-respawn from killing an unrelated process that inherited a
recycled pid.

It read that command line with:

```ts
execFileSync("ps", ["-p", String(pid), "-o", "command="], …)
```

Two things are wrong with that call, and they fail the same way.

**1. procps-ng truncates `ps -o command=` to `$COLUMNS`.** When the
recorded bundle path does not fit that width, the command line comes
back clipped, never matches its own marker, and the function returns
`false`.

**2. A bare `"ps"` resolves through `PATH`.** An MCP server launched
from a GUI / launchd context inherits a sanitized `PATH` that omits
`/bin`, where the spawn `ENOENT`s — and the `catch` turns that into the
same `false`, on *every* call rather than only a long path.

`false` is the fail-safe answer — *don't kill something you can't
identify* — which is right when `ps` is genuinely missing or
unsupported. In both cases above it is wrong, and it fails in the
expensive direction: the kill is skipped, a second server spawns, and
**the live one is orphaned**.

Measured on Linux (procps-ng 4.0.4, stdout piped, same pid):

```
COLUMNS=<unset>  plain_len=157  ww_len=157
COLUMNS=40       plain_len=40   ww_len=157
COLUMNS=80       plain_len=80   ww_len=157
```

and, replicating the guard's own `execFileSync` with
`PATH="/usr/local/bin"`:

```
bare "ps"  → throws ENOENT
"/bin/ps"  → OK
```

## The twin already had both

`packages/tool-server/src/utils/vega-process.ts` pins the width flag in
both its arg sets:

```ts
export const PS_ARGS = ["-A", "-ww", "-o", "command="] as const;
export const PS_ARGS_WITH_PID = ["-A", "-ww", "-o", "pid=,command="] as const;
```

…and resolves the binary absolutely:

```ts
export const PS_BIN = ["/bin/ps", "/usr/bin/ps"].find((p) => existsSync(p)) ?? "ps";
```

That `PS_BIN` pin landed in #508, which fixed the identical
sanitized-`PATH` failure across all four `ps` spawn sites in
`tool-server` (a VVD silently listed twice). `launcher.ts` lives in a
different package and was the one site left behind, so this PR takes
both halves. `grep '"ps"'` over the tracked sources now returns no
bare-`ps` production call site anywhere in the repo.

## Why no existing test caught it

Six tests across `launcher-sweep`, `launcher-duplicate-spawn` and
`launcher-version-gate` do fail without `-ww` — but only when `COLUMNS`
happens to be narrow in the environment running them. Nothing sets it,
so on a normal-width terminal or in CI the whole suite is green either
way. Nothing at all covered the `PATH` half.

## The tests

Four, all in `launcher-sweep.test.ts`. Each spawns a real `node <bundle>
start` keepalive, writes a state record, and runs
`sweepDeadStateFiles()`.

**`terminates a live server whose bundle path is longer than the
terminal is wide`** narrows `COLUMNS` to 40 for the duration (the
`execFileSync` child inherits it) and restores the ambient value
afterwards, distinguishing "was unset" from "was set to something".

It **skips where `ps` ignores `$COLUMNS`**. BSD `ps` clips only to a
*terminal* width, and the guard hands it `stdio: ["ignore", "pipe",
"ignore"]` — never a terminal — so on macOS this assertion holds against
the un-hardened guard too. `psClipsToColumns()` probes that with the
guard's own binary and options against the live pid, rather than reading
`process.platform`, so the test says "this platform cannot observe the
bug" instead of passing vacuously. On CI (`unit-tests.yml` →
`ubuntu-latest`) it runs: the job log shows `test/launcher-sweep.test.ts
(10 tests)`, none skipped.

Its fixture is also sized past **132**, not just past `COLUMNS`.
procps's single `-w` widens to `max($COLUMNS, 132)` rather than lifting
the limit, so a shorter command line cannot tell `-ww` from a
one-character regression to `-w` — straight back to the orphaning bug,
green. The test asserts `` `node ${bundle} start`.length >
PS_SINGLE_W_FLOOR ``, so a future fixture rename that shortened the path
fails rather than quietly making it vacuous.

**`terminates a live server when PATH omits every directory holding
\`ps\``** points `PATH` at a directory that does not exist for the
duration and restores it the same way. It discriminates on every
platform, macOS included.

**Two look-alike tests** cover the guard's structural match, which is
what `-ww` makes load-bearing: command lines the width limit would have
cut off before they could collide now reach the regex whole, and neither
half of it was pinned — both could be deleted with the suite green.

- `does not signal a process that merely mentions the bundle path` — a
live process carrying the recorded path in its argv without running it.
Red when the trailing `` ` start` `` requirement is dropped, which would
SIGTERM a user's `tail -f <bundlePath>`.
- `does not signal a different install whose bundle path ends with ours`
— a second install nested under a longer prefix. Red when the leading
`(?:^|\s)` boundary is dropped, which would SIGTERM a live server
another session is using.

Both record a bundle path that is **not** on disk and assert it, because
`sweepDeadStateFiles` short-circuits on one that is and would never
reach the guard. That assertion is the pin: with a shared fixture path
and only declaration order keeping them apart, swapping the two blocks
made the first pass against a guard that had lost the exact behaviour it
names.

## Verification

Six mutations, each redding **exactly one** test and leaving the other
nine green — which is why each test had to be added. Linux runs are
`node:24-bookworm`, procps-ng 4.0.2 (baseline there: `10 passed (10)`).

| mutation | red test | where |
|---|---|---|
| drop `-ww` | `…longer than the terminal is wide` | Linux |
| `-ww` → `-w` | `…longer than the terminal is wide` | Linux |
| `PS_BIN` → bare `"ps"` | `…PATH omits every directory holding \`ps\``
| Linux + macOS |
| drop the regex's `(?:^\|\s)` anchor | `…install whose bundle path ends
with ours` | macOS, in **both** test orders |
| drop the regex's `` ` start` `` tail | `…merely mentions the bundle
path` | macOS, in **both** test orders |
| ps `timeout: 2_000` → `1` | 3 tests (macOS) / 4 (Linux) | both |

- Unmutated: Linux `10 passed (10)` — nothing skipped, so CI gates all
of it; macOS `9 passed | 1 skipped (10)`.
- The probe was checked against the environment it exists for: with
`PATH` holding only node's directory (no `ps`, `/bin/ps` present), the
suite is `9 passed | 1 skipped`; resolving the probe through `PATH`
instead of `PS_BIN` there gives `Error: spawnSync ps ENOENT`, `1
failed`.
- Package suite: `12 files, 194 passed | 1 skipped (195)` on macOS.
- `eslint --max-warnings 0` 0, `prettier --check` 0, `tsc --build` 0,
`tsc -p tsconfig.test.json --noEmit` 0, `npm run knip` 0 — exit codes
read directly, after `npm run build` (the type-aware lint rules report
spurious errors against an unbuilt workspace).

## Note on the tests' timeout

`waitForExit`'s window exceeds vitest's 5 s default, so the tests carry
an explicit `20_000`. That is what makes a mutation fail as `expected
false to be true` — an unterminated server — rather than as an
uninformative bare timeout.

## Found alongside, filed separately

Out of this PR's scope, all reproduced: #832 (on Windows,
`ensureToolsServer`'s kill-before-respawn is the one of three call sites
without the `win32` guard, so the wedged server is never killed before
the respawn), #833 (`LOCK_WAIT_TIMEOUT_MS`'s budget comment counts one
`ps` where the critical section runs three, so its "worst case ≈ 26s" is
not a bound), and #834 (`stop-metro` swallows a missing `lsof` into
`stopped: false`, reporting "port is free" when it could not look — the
same PATH-resolved-silent-failure class this PR fixes for `ps`).


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
- Improved detection of the correct background server process, including
long command lines and shell-wrapped processes.
- Prevented unrelated or similarly named processes from being stopped
during cleanup.
- Improved recovery when an automatically started server becomes
unresponsive or cannot be verified.
- Added more reliable handling for restricted environments, unusual
installation paths, and processes with similar names.
- Improved diagnostics when process verification encounters command-line
inspection errors.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-03 14:37:03 +02:00
Sebastian Flajszer 1e69c90981 feat(ios-device): add physical iOS support (#944) 
# Physical iOS device support

Try it now (cabled iPhone, unlocked, Developer Mode on):

```bash
argent run list-devices                                            # copy the 25-char UDID
argent run launch-app --udid <UDID> --bundleId com.apple.Preferences
```

First launch builds and signs the on-device runner: 3 to 5 minutes, keep
the phone unlocked. Every later start: seconds, from cache. No signing
setup needed if an Apple ID is signed into Xcode; the team is detected
from the keychain.

## What this adds

- Physical iPhone support, on by default: a cabled iPhone simply appears
in `list-devices`. iPhone-only: physical iPads are skipped at discovery.
- 15 tools ported to hardware. Flows replay on device.
- Zero-config signing from the keychain; `ARGENT_IOS_TEAM_ID` is an
override, nothing is persisted.
- Self-contained npm install: the runner's Swift sources ship in the
package. New package `@argent/ios-device-runner` (an XCUITest bundle
hosting an HTTP command server on the device), new tool-server backend
under `src/utils/ios-device/`.
- The one always-on cost: `list-devices` on macOS now runs physical
discovery in parallel with the other platform probes, under the existing
8s devicectl timeout; off-macOS it early-returns.

## Suggested review order

Rough estimate: half a day for the full pass.

1. `packages/ios-device-runner/PROTOCOL.md`, then `README.md` (wire
contract and reliability model, ~15 min)
2. `packages/tool-server/src/utils/ios-device/` (devicectl, usbmux,
team-detect, runner-build, runner-commands: the core)
3. `packages/tool-server/src/blueprints/ios-device-runner.ts` (service
lifecycle, crash recovery)
4. Per-tool device arms and the capability gates
(`ios-physical-device-classification.test.ts` is the ratchet)
5. Docs page, the two new skills, the Swift runner sources

## Requirements

1. macOS host with full Xcode (iOS platform installed); uses `xcrun
devicectl` and `xcodebuild`.
2. An Apple ID signed into Xcode; automatic signing mints the runner's
provisioning profile.
3. USB cable only. Wi-Fi is deliberately unsupported: the transport is
usbmux, and a Wi-Fi-only device lists as `paired`, not `connected`.
4. iPhone paired with and trusting the Mac, Developer Mode on, A12/2018
or newer (modern 8+16-hex UDID, the CoreDevice floor). Legacy 40-hex
UDIDs are not classified as iOS.
5. Signing resolves as: `ARGENT_IOS_TEAM_ID` in the tool-server's
environment (explicit override, CI) -> keychain detection (newest
identity first, memoized per server run, never persisted). One
certificate: nothing to do. Several teams: newest wins; the first tool
result names the pick, the runner-ups, and the override. No certificate:
the error walks through Xcode > Settings > Accounts and Manage
Certificates > + > Apple Development, then retry; it never names the env
var, since a team id alone cannot sign. Bundle ids derive per team
(`com.argent.runner.t<teamid>`). `ARGENT_IOS_RUNNER_PROJECT` overrides
the project path for unusual layouts.

Build artifacts cache under `~/.argent/ios-device-runner`, keyed by
runner sources, Xcode/SDK version, and signing. A team with no
registered devices self-heals: the failed generic build retries against
the connected phone, which registers it.

## How it works

1. `xcrun devicectl` (CoreDevice) is the control plane: discovery,
readiness, app lifecycle. Readiness gates on the wired transport, so an
unplugged phone is refused before any build starts.
2. A signed XCUITest runner is built lazily with the resolved team,
started via `xcodebuild test-without-building`, and parks as an HTTP
command server for 24h. On an npm install the Xcode project is the copy
shipped inside the package.
3. The listener binds to the device's loopback interface; the host
reaches it over usbmuxd with a dependency-free `node:net` client.
4. Mutating commands are journaled by `commandId`; a lost reply is
resolved by asking `status` for the journaled fate, never by replay
(at-most-once delivery).
5. Routing hangs off `resolveDevice` UDID-shape classification plus a
per-tool capability allowlist. CLI and MCP layers are untouched; outside
tool-server, docs, and the runner package the diff is failure-code
constants (registry), two agent skills plus rule updates (skills), the
pack step (packages/argent), and the e2e smoke script.

## Supported tools

| Tool | Status | Notes |
| ----------------- | --------- |
------------------------------------------------------------------------------------------------------------------------------------------------------------
|
| launch-app | Supported | Plain `devicectl` launch. System UI
(SpringBoard/Spotlight) registers a session only; no launch attempt.
Carries a note when signing is not resolvable yet. |
| restart-app | Supported | System UI bundle ids rejected before any
device contact. |
| reinstall-app | Supported | Same system UI pre-flight. |
| screenshot | Supported | Served through the runner; device-wide, needs
no registered app. (No host-side devicectl screenshot exists on any
toolchain; that dead route was removed.) |
| describe | Supported | AX tree snapshot via the runner. Snapshots the
session app's process, so another process's dialogs are not in the tree.
A backgrounded session app fails with an actionable error instead of
being silently re-fronted. Focus polling is off for this source; typing
uses a fixed settle. |
| keyboard | Supported | Types into the focused element; typing with
nothing focused is refused with "tap the text field first". `enter` and
`backspace` are the named keys (both exist as software-keyboard
characters; there is no per-keycode HID surface on hardware).
Whitespace-only keys rejected, not silently "typed". |
| gesture-tap | Supported | `clickCount > 1` dispatches as one native
multi-tap gesture, not separate taps. |
| gesture-swipe | Supported | Single-finger drag between two points. |
| gesture-custom | Supported | Two shapes only: press-hold and straight
single-finger drag. Second finger or `Move` waypoints rejected with
authoring guidance, before any runner starts. |
| button | Supported | `home`, `volumeUp`, `volumeDown`, `actionButton`
(volume buttons are simulator-unavailable in the SDK, not
device-unavailable). `power`/`appSwitch` have no public XCUIDevice API
and reject before device contact; a button the model lacks gets a typed
refusal via `hasHardwareButton`. |
| await-ui-element | Supported | Polls the same runner snapshot describe
returns; waits and taps see identical frames. Degrades to `cause:
"unreadable"` with a note on a backgrounded app, never a raw throw. |
| await-screen-idle | Supported | Same describe path; resolves
not-settled instead of throwing, so a follow-up screenshot still runs. |
| run-sequence | Supported | Outer gate only; each step pre-flights its
own capability. |
| open-url | Supported | devicectl launches the receiving app with the
URL as its launch payload. http(s) defaults to Safari; any other scheme
needs the new optional `bundleId` param (hardware cannot resolve a
scheme's handler system-wide). |
| screenshot-diff | Supported | Live captures through the runner
(full-resolution, device-wide, no registered app needed); the diff
itself runs on the host. Baselines are per device model/resolution;
`rotation` is ignored on hardware. |

Flows run through these tools; the device flow tree gets the same
`subtreeText` hoisting as every other platform, so container-scoped
`text` asserts work.

## Not supported (400 at the capability gate)

1. gesture-pinch, gesture-rotate: no two-finger coordinate API on
hardware. Pinch/rotate flow directives also fail upfront with a clear
error.
2. screen-recording-start/stop (video): simulator-only capture pipeline.
3. rotate, paste, shake, settings-permissions: simulator-only backends,
not ported (paste also hits the iOS 16+ cross-app paste permission
prompt).
4. native-devtools, debugger, profiler families: simulator-side
instrumentation (DYLD injection does not exist on hardware).
5. Physical iPads: filtered at discovery for now.

A test ratchet (`ios-physical-device-classification.test.ts`) keeps
`apple.device` declarations in sync with the ported list.

## Core contracts

1. Observation never mutates the screen: a backgrounded session app
makes `describe`/`await-*` fail with an error naming the three ways out
(screenshot, launch-app, or launch-app com.apple.springboard). Mutating
commands do re-front, and their result carries `reactivated: true`.
2. The runner listener is unauthenticated; mitigation is the
loopback-only bind.
3. `list-devices` states: `connected` (cabled now, usable) vs `paired`
(unreachable, never auto-bound). Wired transport alone decides; a
Wi-Fi-migrated CoreDevice tunnel does not count.
4. The multi-team signing pick rides the first affected tool result as a
one-shot note (the pre-existing notes channel). The `list-devices`
result shape is unchanged from main: devices and avds, nothing else.
5. Rejections are typed 400 validation errors end to end, so a batch
flow run skips one failing flow instead of aborting.

Also: the runner is a first-class device-owned service
(`stop-simulator-server` and `stop-all-simulator-servers` reap it,
detached xcodebuild child included; a crashed runner respawns on the
next call). A passcode-locked phone refuses launches; the devicectl
10002 hint also covers a pending system prompt on an unlocked phone.
Build failures surface xcodebuild's own `error:` lines with signing
hints (no registered devices, free-team app-id cap, missing
provisioning, the `errSecInternalComponent` partition-list fix). An app
XCTest reports as `.unknown` (a swipe-killed foreign app) is refused
with launch-app guidance. Status-bar pinning is skipped on hardware;
pixel-settle masks the top band.

## Testing

- Unit coverage, hardware-independent: UDID classification and
capability routing; usbmux framing and transport; devicectl list/hints
parsing incl. the silent iPhone-only filter; keychain detection against
real certificate fixtures; runner build/cache self-heals (poisoned
cache, concurrent checkout, fresh-team retry); crash/journal recovery;
the backgrounded-observation and reactivated contracts; keyboard guards
incl. the not-focused refusal; system-UI pre-flight; per-tool device
arms.
- Manual smoke on a real device: `scripts/e2e-ios-physical-device.mjs`.

## Docs and skills

1. `features/physical-ios-devices.mdx`: the user-facing walkthrough
(zero-config signing with the sign-in fallback, prerequisites,
app-scoped model, troubleshooting).
2. `fundamentals/supported-platforms.mdx`: physical iPhones in the
platform matrix (iPads not supported yet).
3. `reference/tools.mdx` and `reference/configuration.mdx`: list-devices
states and the `ARGENT_IOS_TEAM_ID` override.
4. Two skills ship with the package via `argent init`:
`argent-ios-device-setup` (setup ladder plus a troubleshooting table
keyed to real error texts) and `argent-ios-device-interact` (the
app-scoped hardware contract, supported subset with alternatives,
gesture recipes, pitfalls). The agent rule and sibling skills route to
them.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
  * Added experimental USB-connected physical iPhone support.
* Physical iPhones now support discovery, app lifecycle actions, URL
opening, accessibility snapshots, screenshots, taps, swipes, custom
gestures, keyboard input, waits, flows, and screenshot comparisons.
* Added automatic signing-team detection with optional
`ARGENT_IOS_TEAM_ID` configuration.
* Added clear handling for paired or unavailable devices and unsupported
hardware actions.

* **Documentation**
* Added setup, troubleshooting, capability, and physical-device usage
guidance.
* Clarified that physical iPads and several simulator-only tools and
gestures remain unsupported.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-03 14:33:11 +02:00
mjq2020 23181baaa8 fix(network): report UTF-8 response byte size (#783) 
## Summary

- measure React Native response bodies from a cloned Blob instead of a
decoded JavaScript string
- report one consistent metric: decoded response entity bytes
- avoid Content-Length because HEAD/304 and compressed responses give it
different semantics
- keep response text capture independent, and leave the size unknown if
Blob conversion is unavailable

## Root cause

body.length counts UTF-16 code units. Re-encoding the decoded text as
UTF-8 fixes ordinary UTF-8 text, but it overcounts replacement
characters from non-UTF-8 and binary responses. Content-Length is also
not a safe fallback: it can describe an unsent GET body for HEAD/304 or
the compressed wire representation while the response body is
decompressed.

Response.blob().size measures the bytes in the decoded entity supplied
by the React Native response. The same measurement therefore applies to
text, binary, empty, cached, and compressed responses.

## Tests

- network interceptor regression: 23/23 passed
- covers Unicode JSON, HEAD, 304, non-UTF-8 replacement characters,
compression, binary bodies, and unavailable Blob conversion
- npm run build
- npm run typecheck:tests -w @argent/tool-server
- targeted ESLint and Prettier checks on both changed files

Fixes #662

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Bug Fixes**
- Improved reported network response sizes to reflect the actual decoded
response data.
- Corrected size calculations for binary content, compressed responses,
and text containing non-ASCII characters.
- Added reliable handling for `HEAD` and `304` responses, which now
report zero size.
- Prevented failed size calculations from incorrectly reporting a
zero-byte response.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-09-02 10:32:58 +02:00
Ignacy Łątka 1409195138 test(run-sequence): pin the one observation a sequence returns (#934) 
## What

`run-sequence` tells agents:

> One screenshot is captured automatically after the whole sequence (not
per step)

The MCP-side half of that promise is already pinned — argent-mcp's
auto-screenshot tests name `run-sequence` explicitly, twice. The
tool-server half is pinned by nothing: this adds the gate that pins it,
plus the renderer-side pin in argent-mcp. **No behavior change.**

## Why it is unpinned

The steps stay silent only because no tool `ALLOWED_TOOLS` admits
returns an image or an artifact handle. Nothing checks:

- which tools the allow-list admits,
- that every admitted name still resolves to a registered tool (a stale
name still reads as allowed and stays advertised in the description, but
every step naming it dies on `ToolNotFoundError` and halts the
sequence),
- that none of them declares `outputHint: "image"`.

Admitting a frame-producing tool would go unnoticed: a sequence result
reaches `toMcpContent` whole, and its `materializeArtifacts` walk
collects handles from anywhere inside it, `steps[].result` included,
appending one image block per handle. That walk recurses into arrays
with `Promise.all`, so those frames arrive in **completion order, not
step order** — the agent could not even tell which one is the screen the
sequence ended on.

Both halves are verified against the code, not assumed:

- Putting an artifact handle in one step's result makes `toMcpContent`
emit an image block for it, confirming the walk reaches
`steps[].result`.
- Materializing three step handles whose fetches complete fastest-last
yields `[s2, s1, s0]` — exactly reversed from step order.

## What is gated

**tool-server** — `test/run-sequence-observation-gate.test.ts`, against
the real registry (`createRegistry()`, no device). `ALLOWED_TOOLS` is
exported for it:

- the allow-list admits exactly the reviewed set of 14 step tools
- every name still resolves to a registered tool
- none of them declares `outputHint: "image"`

**argent-mcp** — `test/content.test.ts`:

- a multi-step sequence result renders with no frame of its own, against
a `fetchImpl` that resolves and is asserted never to be called (without
a resolving fetch the assertion would hold equally for a sequence whose
step frames merely failed to download, since `materializeArtifacts`
swallows a failed download and renders the handle as nothing)
- the positive control that pin rests on: a handle on `steps[].result`
does surface as an image block, so the walk ceasing to reach steps
cannot turn the pin vacuous

The membership of `run-sequence` in `AUTO_SCREENSHOT_TOOLS` was already
pinned by existing assertions and needed no new test.

## Verification

Every test was mutation-checked — each fails on the broken code, not
just passes on the good:

| Mutation | Fails |
| --- | --- |
| add `screenshot` to `ALLOWED_TOOLS` | reviewed-set + `outputHint`
checks |
| rename an allow-list entry the registry lost | reviewed-set +
registration checks |
| give a step result an image artifact handle | the frameless-sequence
check, on both its fetch-call and its image-block assertion |
| stop `materializeArtifacts` recursing into arrays | the
step-carried-handle check, and nothing else in the file |

Locally green: `prettier --check`, `tsc --build`, `typecheck:tests` for
both touched packages, the full argent-mcp suite (82 tests), and the
gate test (3 tests).

## Scope note

The allow-list is spelled out rather than counted, so adding a step tool
means answering the question the file exists to ask — does it put a
frame of its own in front of the agent? The gate deliberately sits on
the allow-list rather than on the renderer: a tool that would break the
guarantee is caught before it can be dispatched as a step, rather than
having its frames filtered afterwards.

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
 2026-08-31 20:51:21 +02:00
filip131311 927bfc630f chore: bump version to 0.23.0 (#987) 
Bumps all workspace packages from 0.22.1 to 0.23.0.

- All 16 `packages/*/package.json` bumped via `npm version 0.23.0
--workspaces`
- `package-lock.json` regenerated on Node 24 / npm 11 (`npm install
--package-lock-only --ignore-scripts`); diff is version-only
- `server.json` updated in both places (top-level `version` and
`packages[0].version`)
- `npm run check:versions` passes

No docs update needed — version-only change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01VGjTuE8XKfrmmhJEjchBFq

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Chores**
- Updated the release version across all packages and the server
manifest from 0.22.1 to 0.23.0.
- Ensures package metadata and server configuration consistently reflect
the new release.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-31 19:42:32 +02:00
filip131311 397dce87f6 feat(installer): allowlist.enabled config — opt out of allowlist refresh on update (#986) 
A minimal opt-out for the allowlist refresh in `argent update`: a new
config key **`allowlist.enabled`** (project + global scopes, restrictive
merge — `false` in either scope wins, so a committed project opt-out
holds for the team).

**Nothing changes by default.** The key has no default value; while it
is unset, `argent init` and `argent update` behave exactly as today.
Only an explicit

```
argent config set allowlist.enabled false
```

changes anything: `argent update` then skips re-applying editor
auto-approve allowlist rules entirely (rules already written stay) and
prints a dim one-liner saying why. `argent init` is untouched — it still
asks its auto-approve question and applies the answer as before.

### Docs

`reference/configuration.mdx` gains the key's row and a short behavior
note. `docusaurus build` and `npm run format` are clean.

### Tests

- `allowlist.enabled` merge semantics (restrictive, no default) in
configuration-core.
- Suites: installer 555, configuration-core 125; tsc, eslint
--max-warnings 0, prettier, both knip gates clean.
- Live-tested from a packed tgz in an isolated HOME: `init -y` and
`update` with the key unset are byte-identical to main's behavior; with
`allowlist.enabled false`, `update` re-imposes nothing and prints the
skip note.

Supersedes #787.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_014b5AyEqWd16k1s8xvve3dS

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Added an `allowlist.enabled` setting to control editor auto-approve
allowlist updates.
- Allowlist updates now run when enabled or unset, while an explicit
disabled setting prevents changes.

- **Documentation**
- Added configuration reference details, including scope precedence and
unset behavior.

- **Bug Fixes**
- Preserved non-blocking handling when individual editor allowlist
updates fail.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
 2026-08-31 19:29:37 +02:00
Ignacy Łątka a2ed83e041 fix(tool-server): await the simulator-server command ack so a lost tap fails (#985) 
## The defect

`sendCommand` returns `void`. It writes the JSON command onto the
WebSocket — or queues it behind `once("open")` — and returns without
ever reading the reply:

```ts
export function sendCommand(api, cmd): void {
  const ws = getOrCreateWs(api);
  const payload = JSON.stringify({ id: String(++cmdId), ...cmd });
  if (ws.readyState === WebSocket.OPEN) ws.send(payload);
  else ws.once("open", () => ws.send(payload));   // queued, never awaited
}
```

simulator-server *does* acknowledge every command; argent just never
looked. Probing the shipped binary's `/ws` endpoint:

| sent | reply |
|---|---|
| `{"id":"1","cmd":"touch","type":"Down",…}` |
`{"id":"1","status":"ok"}` |
| `{"id":"7","cmd":"button","button":"notARealButton"}` |
`{"status":"error","message":"parse error: unknown variant
`notARealButton`…"}` |

Because the return type was `void` there was no ack to check and no
error to surface, so `gesture-tap` ended in an unconditional `return {
tapped: true, timestampMs }`.

**Repro:** shut a simulator down underneath an open session. The server
sends no reply at all and `gesture-tap` still reports `{ tapped: true
}`. Every transport failure — that one, a dead socket, a wedged device —
looks identical to success from the agent's side.

This is argent's own bug, and it is what turns an Apple regression into
a silent one: awaiting the reply would have made #932 an actionable
failure on day one instead of a silent no-op.

## The fix

`sendCommand` now returns a promise that resolves on `{"status":"ok"}`
and rejects when:

| condition | `error_code` |
|---|---|
| server answers `status: "error"` | `SIMULATOR_COMMAND_REJECTED` |
| socket fails or closes with the command in flight |
`SIMULATOR_COMMAND_TRANSPORT_FAILED` |
| reachable server never answers (5s) | `SIMULATOR_COMMAND_ACK_TIMEOUT`
|

Every WebSocket call site awaits it: `gesture-tap`, `gesture-swipe`,
`gesture-custom`, `gesture-pinch`, `gesture-rotate`, `button`, `rotate`.

Two details of the wire protocol shaped the implementation:

- **A success reply echoes the command id; an error reply carries no id
at all.** So a rejection can only be matched positionally, against the
oldest outstanding command. Since every send is awaited, that is
normally the only one in flight.
- **The abort paths in `gesture-swipe` / `gesture-rotate` lift the
finger before unwinding, and that lift is now fallible.** It is
therefore best-effort: a refused lift rides along as the `AbortError`'s
`cause` rather than replacing it — matching the existing pattern in
`gesture-drag`, because `format-error.ts` and `cdp-session.ts` key on
`err.name === "AbortError"`.

## Verified on a real simulator

Before, after shutting the device down mid-session: `{"tapped": true}`.
After:

```
result: {"tapped":true,"timestampMs":…}          ← healthy tap, unchanged

--- simulator shut down underneath the open session ---
tap FAILED as it should:
   code: SIMULATOR_COMMAND_TRANSPORT_FAILED
   msg : simulator-server did not accept the 'touch' command: connect ECONNREFUSED
         127.0.0.1:62818. The command was NOT delivered to the device. …
```

The failure signal is fully populated for telemetry (`error_kind:
"network"`, `network_failure: "connection_reset"`, `failure_command:
"simulator_server"`).

## Cost

An ack is a localhost round-trip behind an already-open socket: **0.06ms
p50 / 0.17ms max**, measured over 200 sends against a booted iOS sim.
Awaiting one per event adds a flat **~30ms to a swipe of any duration**
— against the 36–142ms of `setTimeout` skew the 16ms frame loop already
carries:

| `durationMs` | wall before | wall after | added |
|---|---|---|---|
| 200 | 236ms | 266ms | +30ms |
| 500 | 559ms | 591ms | +31ms |
| 1000 | 1142ms | 1178ms | +35ms |

Flagging it explicitly since the swipe cadence is deliberately tuned. If
reviewers would rather keep the cadence byte-identical, the alternative
is to pipeline within a gesture and await all acks at the end; that
costs more code and defers failure to the end of the gesture, so I kept
the simpler per-send await.

## Deliberately out of scope

- **The MoQ (`ios-remote`) transport** is unchanged — it has no
equivalent ack.
- **The stdin `pressKey` channel**, used by `keyboard` and paste's ⌘V,
is untouched: simulator-server never replies on stdin, so it has the
same blind spot with no local remedy. The WebSocket protocol *does*
expose an acked `key` command
(`{"cmd":"key","direction":"Down","code":4}` → `{"status":"ok"}`), so
moving those onto it would close the gap — a separate, riskier change to
the text-entry path that deserves its own PR.

## Tests

`packages/tool-server/test/simulator-command-ack.test.ts` — 8 tests
against a fake `/ws` server that mirrors the real wire format, **all 8
fail on `main`**: ack resolves, `status: error` rejects, socket death
rejects in flight, silent server times out, no phantom success,
`gesture-tap` throws instead of claiming a rejected tap, and the aborted
swipe still reports `AbortError`.

Full suite: 369 files / 4840 tests pass, `npm run lint` clean, `tsc
--build` clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_011bEvNDV7o3CpKdFmzjVBYF

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Simulator commands now confirm successful delivery before completing.
* Added clear errors for rejected commands, connection failures, and
acknowledgement timeouts.
* **Bug Fixes**
* Button, rotation, tap, swipe, pinch, and custom gestures now report
failures instead of silently succeeding.
* In-progress gestures handle connection interruptions and cleanup
failures more reliably.
* **Tests**
* Added coverage for command acknowledgements, rejections, timeouts,
transport failures, and gesture error handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: latekvo <f.kaminski2000@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-08-31 18:31:53 +02:00
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 -->
 2026-08-31 14:49:11 +02:00
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 |
 2026-08-31 12:51:26 +02:00
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 -->
 2026-08-31 11:52:20 +02:00
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 -->
 2026-08-28 17:20:51 +02:00
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
 2026-08-28 17:07:15 +02:00
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.
 2026-08-28 12:49:10 +02:00
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.
 2026-08-28 09:55:47 +02:00
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.
 2026-08-28 09:55:47 +02:00
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.
 2026-08-25 17:27:12 +02:00
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>
 2026-08-25 17:05:57 +02:00
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
 2026-08-25 14:46:36 +02:00
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
 2026-08-25 13:32:36 +02:00
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.
 2026-08-25 13:26:46 +02:00
filip131311 c2324f052d feat(telemetry): project-scope opt-out (restrictive merge) + init --local --no-telemetry (#952) 
## Why

Teams that must not send usage data to a third party (a
strict-compliance customer asked) had no way to turn telemetry off for a
repository. `telemetry.enabled` was global-only
(`~/.argent/config.json`), so a committed local install (`init --local`)
still defaulted every teammate to **on** — `--no-telemetry` only opted
out the machine that ran `init`.

## What

- **`telemetry.enabled` accepts the project scope.** `consent.ts` now
reads both `~/.argent/config.json` and
`<project-root>/.argent/config.json` (root resolved from `cwd`, per-file
cache keyed by path) and merges **restrictively**: `false` in either
document wins. A committed project opt-out holds on every clone, and a
project file can never re-enable a global opt-out. Env overrides
(`DO_NOT_TRACK`, `ARGENT_TELEMETRY`) still outrank both.
- **`argent telemetry enable|disable --scope global|project`** (default
`global`). `status` names the document that decided (`config.json
(project)`, `(global)`, `(project and global)`). `enable` at one scope
while the other still opts out reports that telemetry stays disabled
instead of claiming success. Unknown scope / argument → exit 2, nothing
written.
- **`argent init --local --no-telemetry`** also writes
`telemetry.enabled: false` to `.argent/config.json`, and the "commit
these files" checklist lists it. Global mode is unchanged.
- `argent config set/unset telemetry.enabled` stays delegated to `argent
telemetry` (`manageCommand`), so the lifecycle work (draining the live
client) is still done by the dedicated command.

## Tests

Unit: `telemetry` (+8 project-scope cases: single-scope opt-out, both
restrictive directions, both-off, env precedence, live re-read of the
project file, malformed project file), `configuration-core` (schema
scopes), new `argent-cli/test/telemetry-command.test.ts` (real-fs, 6
cases), `argent-installer` — all green; eslint, prettier, knip,
`docusaurus build` green.

Live, from a real `npm pack` tarball of this branch (sandboxed `HOME`):

1. Fresh project → `init --local --yes --no-telemetry --from <tgz>`:
writes `.argent/config.json` `{telemetry:{enabled:false}}` + global
opt-out; summary lists `.argent/config.json` to commit.
2. `telemetry status` → `disabled`, source `config.json (project and
global)`. `telemetry enable` (global) → "stays disabled: config.json
(project) still opts out"; from `~` (no project) → `enabled`. `enable
--scope project` → enabled; `disable --scope project` → disabled.
`--scope team` → exit 2.
3. **Teammate simulation**: copied only the committed files to a fresh
clone with a fresh `HOME` (no global opt-out), bundle built with a dummy
OTLP token, `ARGENT_TELEMETRY_DEBUG=1`, drove `argent mcp` (initialize →
tools/list → `list-devices` call). Control clone without
`.argent/config.json`: tool-server emitted `toolserver:start`,
`tool:invoke`, `tool:complete`, `toolserver:stop`. Clone with the
committed opt-out: **0 events**, and the first-run telemetry notice is
not printed.

## Caveat (documented)

The project file is resolved from the working directory of the process.
Editors start the MCP server in the workspace root, so it and the
tool-server it launches read it. The tool-server is shared by every
editor on the machine and reads the project file of the workspace that
started it; the global opt-out or `DO_NOT_TRACK` still covers the
machine-wide case.

## Docs

Updated in this PR: `reference/configuration.mdx` (scopes + merge
wording), `reference/telemetry.mdx` (new "Opt out for a whole
repository" section), `reference/cli.mdx` (`init`, `telemetry` rows),
`fundamentals/installation.mdx` (`--no-telemetry` in local mode).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01BkkAihA8Kp26hHS4uhkNGu
 2026-08-25 11:24:02 +02:00
Hubert Gancarczyk 50c9580e30 fix(tools): make a schema failure name the parameter that was sent (#718) 
> Stacked on #717.

## The problem

An invalid tool call returned Zod's raw issue array. Someone who wrote
`flow_name` instead of `name` got back, depending on the tool:

```
Invalid params for tool "flow-start-recording": [{"expected":"string","code":"invalid_type","path":["name"]}]
Invalid params for tool "flow-execute":         [{"code":"custom","path":["flow_path"],"message":"Pass exactly one flow source: name or flow_path."}]
```

The first names the parameter the tool **wanted** and never the one the
caller **sent**. The second — flow-execute's, whose `name` is optional
so `flow_path` can stand in — names neither: it is anchored on the one
source field the caller had no reason to send. Either way the mistake is
invisible, and working it out costs a whole turn.

## What changes

**1. `describeParamIssues` — one sentence per bad parameter.**

Missing fields read as `` `name` is required (string) and was not
provided`` rather than as a type error about `undefined`. Unknown keys
are named and path-qualified, so a key nested in `selector` is not
reported as a bare name that contradicts the top-level list printed
beside it. The message closes with the caller's own keys — `You sent:
\`flow_name\`, \`project_root\`.` — which is what makes the mistake
self-evident.

Key names only, never values: this string reaches logs, telemetry and
the agent transcript, and a params object can carry a secret.

Wired into both entry points: the registry's `invokeTool` and the HTTP
boundary.

**2. `flow-execute` accepts `flow_name` as an alias for `name`.**

The tool is called `flow-execute`, so "the flow's name" spells itself
that way; this is a mistake worth absorbing rather than rejecting. The
name is resolved in code rather than by the schema — a Zod `required`
failure would once again name only the field it wanted — and the error
text names the spellings that are silently discarded (`flowName`,
`flow`, `flow_file`), because by then Zod has already stripped them and
the caller has no other way to learn their key was dropped.

**3. A flow DIRECTIVE passed as `command` points at the tool that
records it.**

`command: "echo"` used to come back as a bare "Tool not found". It now
names `flow-add-echo` — and says to call it **directly**, since routing
it through the recorder would write the echo *and* a `tool:
flow-add-echo` step that fails on every replay.

The hint distinguishes directives the recorder **rewrites** from those
it stores **raw** for the polish pass, because promising a rewrite that
does not happen sends the author looking for a directive that is not in
the file. `wait` and `long-press` get their own answers: neither has a
recording tool at all.

Only a genuine not-found is rewritten — a tool that ran and failed still
reports its own error.

**4. A recorder tool as `command` is refused.**

`flow-add-echo`, `flow-add-step`, `flow-start-recording`,
`flow-finish-recording` each mutate the recording itself, and nesting
one reported success either way. Each is refused for its own reason:
`flow-add-echo` would write the echo **and** a replay-breaking `tool:`
step, `flow-start-recording` would **erase** this flow at replay,
`flow-finish-recording` would end the take it is a step in, and
`flow-add-step` cannot record itself. Now nothing is executed and
nothing is written.

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
 2026-08-25 10:48:49 +02:00
filip131311 efae6a8fb6 feat(tools): opt-in emulator sound on boot-device + boot-sound default flag (#813) 
Field report ([Slack thread](https://swmansion.slack.com)): when a
session needs to test audio, it is very non-obvious that argent boots
the Android emulator muted — the agent (and the user) burned significant
time discovering it. The mute default is intentional (several
agent-driven emulators all playing sound on the host would be worse), so
this PR keeps it and instead makes it **discoverable and overridable**:

## 1. `sound` argument on `boot-device` (default `false`)
- `sound: true` boots the emulator with audio output (drops the mute
launch flag).
- Applied consistently to the snapshot-loadability probe, hot boot, and
cold boot — the qemu-device-topology parity invariant is preserved. A
`default_boot` snapshot saved in the other audio mode fails the probe
and falls through to a cold boot that re-saves it, so toggling costs one
slower boot and nothing else.
- iOS/Vega/Electron are unaffected (argent never mutes them); the
argument documents itself as Android-only.

## 2. Agent-facing information: argument description only
Per the thread, the info lives in the tool argument's description (no
skill changes): it states the muted default, *why* (multiple
agent-driven devices sharing the host's speakers), the `force: true`
hint for an already-running muted emulator, the cold-boot cost of
toggling, and the `boot-sound` flag. The underlying emulator flag is
described behaviorally ("muted"), not by name, per our agent-docs
convention.

**Validated that the description alone is sufficient** by driving three
models through the Cursor `agent` CLI with the schema excerpt and the
exact scenario from the field report ("testing notification sound, hear
nothing, emulator already booted with defaults"). All three —
composer-2.5, gemini-3.7-flash, gpt-5.2 — correctly diagnosed the muted
default and produced the exact fix `boot-device({avdName, sound: true,
force: true})`, including the weakest model. Transcript excerpts in the
PR discussion below if wanted.

## 3. `boot-sound` feature flag
`argent enable boot-sound` defaults the `sound` **argument** to `true`.
It only moves the argument's default, read live per call: an explicit
`sound: false` on a call still boots muted, and without the flag the
default remains muted. Registered in `FLAG_REGISTRY` so `argent
enable`/`argent flags` pick it up with no further wiring.

## Tests
- New `boot-device-sound.test.ts`: mute-by-default, `sound: true`
dropping the mute flag from **both** probe and hot-boot spawn (parity),
cold-boot spawn, flag-driven default, and
explicit-`sound:false`-beats-flag.
- The three existing Android boot test files now pin `isFlagEnabled` to
`false` so a developer's real `flags.json` can't flip the args they
assert on.
- `tool-server` boot suites 62/62 green, `argent-cli` flags 33/33,
`configuration-core` 122/122; lint, `typecheck:tests`, and knip all
pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
 2026-08-24 14:51:16 +02:00
filip131311 8dda6aa1f9 docs: cover the paste tool on the interacting features page (#941) 
The `paste` tool was the only capability in the **Interacting** tool
group with no mention on the `features/interacting-with-apps` page —
typing, gestures, shake, buttons and permissions are all covered there.
It already has its row in the tools reference.

This adds the minimal parity:

- one sentence in **Gestures and text**: for long text, the agent puts
the text on the clipboard and pastes it in one step;
- the **Limits** bullet for the sim/emu-only restriction, merged into
the existing shake bullet, which has the same restriction.

No reference changes needed — `reference/tools.mdx` already lists
`paste`.

Checks: `npx docusaurus build` in `packages/docs` passes; `npm run
format` from the repo root leaves the file unchanged.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01YPSUSz8gDxGm7tb9tA3b4D
 2026-08-24 12:47:44 +02:00
filip131311 b232114422 docs: audit every comment in src against the code it describes (#931) 
Audits every comment in `src/` and `scripts/` against the code it
describes.

**462 files, 57 commits, net −8,194 lines.** Comment text only — the
whole branch is code-identical to `main`.

## Method

One subagent per file, strictly sequential. Scope was `src/` +
`scripts/` (459 files); three more files were added at the end because
they carried dead references of the same kind — two test headers citing
design docs that do not exist, and `publish-npm.yml` citing a retired
workflow. Each agent verified every comment — line, block, JSDoc, file
header, trailing — against the surrounding code and the rest of the
repo, following identifiers, paths, tool ids, config keys, env vars and
issue links to see whether they still exist and still behave as
described. Rules:

- **A false or misleading comment is deleted, not reworded.** If a claim
could not be confirmed by reading the source, it went. That is why the
deletion count is so much larger than the rewrite count.
- Survivors are cut to the shortest form carrying something the code
does not already say. Restatement, preamble, hedging, changelog prose
and ASCII banners are gone; the non-obvious *why* stays.
- Preserved byte-identical: license headers, pragmas and directives
(`@ts-*`, `eslint-disable`, shebangs, `/// <reference>`), JSDoc tag
tokens, everything inside a string or template literal, and the sole
comment inside an otherwise empty block (ESLint `no-empty` counts a
comment-bearing block as non-empty).

## Verification

Every file passed two independent gates before being recorded as done:

1. `comments-only` — the required check.
2. A second comment-stripping comparator with a proper mode stack,
written for this pass because `comments-only`'s flat scanner desyncs on
nested template literals and quote-bearing regex literals and then
reports comment lines as code changes. Two files hit that false FAIL
(`utils/android-profiler/pipeline/index.ts`,
`scripts/extract-tools.mjs`); in both the "changed code" it printed was
literally `//` lines, and the second checker confirmed the code was
byte-identical.

After the last file, all 462 changed files were re-checked against
`main` with the same comparator, rather than trusting any agent's
self-report. **459 code-identical; 2 are non-code (`.svg`, `.md`); 1
intentional.**

The intentional one is `packages/argent/scripts/bundle-tools.cjs`: the
changed template literal *is* the comment header of the file it
generates, `packages/native-devtools-android/src/bundled-meta.ts`.
Fixing only the generated file would have been reverted by the next
build, so the generator changed too — and it has been verified to
reproduce the committed generated file byte-for-byte.

## Representative false claims removed

Not wording nits — statements a reader would have acted on:

- **Reversed directions.** `proxyStart`'s JSDoc had the tunnel backwards
(it is a reverse tunnel: the host binds first and the simulator dials
in). A `paste()` doc had the pasteboard copy direction reversed.
- **Contradicted by the code below it.** A timeout budget multiplied by
three where the probes run concurrently — the same comment said so six
lines later. A "warn once" that warns on every call. A "binary search"
that is a linear scan.
- **Named things that do not exist.** A `vega-fast-cli` binary, a
`finish-recording.ts`, a `publish-next.yml` workflow, two
`profiler-react19-*.md` design docs, a `DebuggerTarget.ts`, a commit
hash git does not know, two tool ids, an `ensureEnv` cycle.
- **Wrong by construction.** "Welford accumulators" across four files
where the code keeps naive `n`/`sum`/`sumSq`; `sum`/`sumSq` documented
over `actualDuration` when reduce sums `selfDuration`; a strict-mode
halving written `n/2` where the code ceils; field docs listing enum
values the producers never emit.
- **Guarantees the code does not make.** A validation matrix claiming to
cover "EVERY tool" that skips flagless ones; a Pareto cutoff that
`slice(0, 20)` makes inert; an idempotence claim where the real rule is
at-or-ahead; a capability note describing a clean 400 the shape-based
device resolver can never produce.
- **Unverifiable assertions** about prebuilt binaries, external CLIs and
the cloud SDK — deleted rather than kept as folklore, since nothing in
the repo can confirm them.
- **Stale numbers**: invented Android tool versions, hard-coded tool
counts and description lengths that had drifted.

## Review

A Fable agent reviewed both halves adversarially for over-deletion,
misread code, `no-empty` hazards and byte-identity violations.
Second-half verdict: **SHIP**, with two one-line restores, both applied
in the final commit — the `npm view ""` rationale behind a blank-token
guard, and the note that `argent-mcp` keeps a copy of
`SECRET_PLACEHOLDER_MARKER` it cannot import.

## Code issues surfaced but deliberately not fixed

This pass changes comments only. Eight genuine findings are logged for a
follow-up:

1. `telemetry/src/consent.ts` — a non-ENOENT read error returns null and
falls through to the default-on path, so file errors *can* silently flip
telemetry on.
2. `chromium-server/navigation.ts` — `navigate()` is reachable from
`POST /api/navigate` with only a `typeof === "string"` check; open-url's
schema is a bare `z.string()`, so the "already validated by zod" premise
never held.
3. `http.ts` — `constantTimeEqual` returns early on a length mismatch,
so the auth token's length is observable.
4. `describe/index.ts:~114` — the ios-remote branch passes `{ isTvOs:
false }` unconditionally, so a remote tvOS simulator takes the iOS
ax-service path, though `isRemoteTvOsSimulator` exists and shake/paste
do use it.
5. `devices/boot-device.ts` — `-crash-report-mode never` is passed
unconditionally *and* appended again by the feature-detecting path, so
every emulator spawn passes it twice.
6. `react-profiler/pipeline/04-rank.ts` — `PARETO_THRESHOLD_PCT` is
dead: `slice(0, 20)` always wins.
7. `reaped-sessions.ts` — a user-facing hint string tells the agent that
`react-profiler-start { force: true }` disposes the debugger and
profiler session; it does not. Left byte-identical because it is a
string literal, not a comment.
8. `utils/simctl-backend.ts` — `localSimctl` is exported with no
importers anywhere.

## Docs

No documentation change is needed: this pass touches only source
comments, and no user-facing capability, tool, CLI flag, config key or
flow-file behaviour changed.

---------

Co-authored-by: filip131311 <f.kaminski2000@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-08-24 12:16:20 +02:00
filip131311 5d76ed4d66 feat: tag artifacts with a semantic kind at registration (#564) 
Fresh, from-current-main take on the artifact-kind idea from #398 /
#466. Knowing what an artifact *represents* (not just its MIME type) is
what a UI needs to group and name artifacts (e.g. in EAS); this PR
delivers exactly that — the tagged handle — without the
declared-output-map machinery.

## Summary

- Adds a closed `ArtifactKind` union to the artifact wire contract;
`kind` is carried on `ArtifactHandle`, `ArtifactEntry`, and the
`/artifacts` inventory items.
- `ArtifactStore.register` now takes a single `{ hostPath, kind, ... }`
options object, so the compiler forces every producer — current and
future — to declare what it is emitting.
- Tags **all** producers on main today, including the ones #398/#466
predate: screenshot (all four capture paths), screenshot-diff, **flow
snapshot baselines/diffs** (`screenshot` / `screenshot-diff-context`),
**screen recordings** (new `screen-recording` kind), and the
native/React profiler exports and reports.
- Types the native-profiler export keys at their source (`IosExportKey`
from the `EXPORTS` table, `AndroidExportKey`) and maps them through a
kind table that is **total over both unions** — a newly added export key
fails to compile until classified, replacing the previous stringly-keyed
loop (and the `?? "native-profile-trace"` silent-default this design
would otherwise need).
- Client-side (`@argent/tools-client`), the mirrored handle type keeps
`kind` **optional and widened** (`ArtifactKind | (string & {})`): an
older, already-running tool-server sends no kind, and a newer server may
send kinds this client build predates. Consumers treat it as advisory
metadata; unknown kinds are opaque, never an error.

## Differences from #398 / #466 (deliberate)

- No `@argent/artifacts` package, no third `ToolDefinition` generic, no
per-tool output-name registrar, no runtime throw for “undeclared output”
— the kind is plain data at the registration call site, checked at
compile time where it can be, and a wrong label can never fail a tool at
runtime. If a pre-invocation `artifacts` declaration in tool-listing
metadata is ever needed (an actual EAS consumer), it can be layered on
later without changing this wire contract.
- The two `ArtifactKind` definitions (registry / tools-client) stay
deliberately separate — same as the existing `ARTIFACT_MARKER`
must-match convention — because the client-side type is intentionally
*looser* (optional + open) than the server-side one for version skew.
- Extending the union stays backward-compatible on the wire.

## 2026-08-24 update

Merged current main (0.22.0) into the branch. The flow-visual rewrite
that landed since (cropOn crops, per-app snapshot-key dedupe, diff
scratch-dir hygiene) is preserved verbatim; its five registration sites
now carry kinds (`screenshot` for baseline/current,
`screenshot-diff-context` for the annotated diff — the cropped `current`
included). No producer added since the PR was opened needed a new kind:
`paste`, `shake`, and the other new tools emit no file artifacts, and
every register call site in the tree now goes through the typed options
object.

No docs update is needed: the docs site (added since this PR) does not
document the artifact handle wire contract; `kind` is advisory wire
metadata with no user-facing surface.

## Validation

At the merge with main (0.22.0), 2026-08-24:

- `npm run build` (full `tsc --build`) — clean
- `npm run typecheck:tests` in registry, tools-client, tool-server, mcp,
cli — clean
- Full suites green: registry 90, tools-client 191, tool-server 4495 (+1
skipped), mcp 80, cli 472
- `prettier --check .`, `eslint . --max-warnings 0`, and the knip gate —
all pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)
 2026-08-24 12:09:51 +02:00
Kacper Kapuściak e0fcf6c995 chore(docs): add Google Search Console verification meta tag (#939) 
Adds the `google-site-verification` meta tag to the docs site via
Docusaurus `headTags`, so docs.swmansion.com/argent can be verified as a
property in Google Search Console.

## Test plan

- `npx docusaurus build` in `packages/docs/`, confirm the meta tag
renders in the built HTML `<head>`.
- After deploy, hit Verify in Google Search Console.

No further docs update needed: this is a site config change with no user
facing capability change.
 2026-08-24 10:36:46 +02:00
Hubert Gancarczyk f21811d069 fix(flow): re-probe a recorded wait against the tree the runner reads (#717) 
> Stacked on #715.

## The problem

`await-ui-element` evaluates against the tree it reads — the
**accessibility hierarchy** on iOS and Android, the **CDP DOM** on
Chromium, the **toolkit page source** on Vega. The `await:` / `assert:`
directive that the polish pass converts a recorded wait into is
evaluated against the **runner's** tree instead.

They overlap, but outside Vega neither contains the other — an id
present in one can be absent from the other, and on iOS even the role
vocabularies are disjoint. Both directions have been hit in practice: an
id visible to the runner but not the recorder, and text-field values
visible to the recorder but not the runner.

The consequence is that a check can pass live and fail once converted.
That makes the core promise — *each step is executed live so you verify
it works before it's recorded* — untrue at exactly the point it is
relied on.

## What changes

After a recorded `await-ui-element` succeeds, the recorder re-runs the
same condition against the tree the runner resolves directives against,
and reports the result on the step it just wrote.

**It warns; it does not refuse.** What gets written is a raw `tool:
await-ui-element` step, and at replay that tool reads the same tree it
just passed against — so "this would fail every run" was never true of
the form actually recorded. What the probe genuinely reports is whether
the **polish-time conversion** is safe, which is why the warning says to
act on it *then* rather than re-record now (re-recording would duplicate
the step).

Because polish begins after `flow-finish-recording`, each verdict is
also carried there: it is repeated on its own step's `summary` line, and
`message` counts how many steps carry one. A warning raised at step 7 of
a 40-step recording is otherwise long out of reach by the time it is
actionable.

## Three outcomes, kept distinct

| Runner tree says | Reported as |
| --- | --- |
| condition holds | nothing — step recorded clean |
| condition does not hold | recorded + warning naming the causes to rule
out before converting |
| tree unreadable, or too slow to answer | recorded + warning that the
conversion is **UNKNOWN, not known-bad** |

The middle row is deliberately not "the conversion will fail". The
identical verdict comes back when the screen simply moved on between the
live wait and the re-probe, and on `text` checks when the selector
matches several elements and the two sides elect different ones — so the
consequence stays conditional on the cause, and the message names the
causes in the order worth ruling out.

The third row matters: nothing was compared, so calling it a divergence
would send the author off to rewrite a selector that may be perfectly
good. It is also the injection-free case the flow skills explicitly
sanction, so refusing there would block a supported way of working. On
iOS the quoted reason ends "provide bundleId explicitly" — advice aimed
at the native tools. The warning corrects it, because no directive
carries a bundleId and forwarding one would make the probe answer a
question the runner never asks.

## Platform-specific explanation

The *why* differs per platform, and stating the iOS story everywhere
makes the message wrong where someone is trying to act on it. iOS gets
AX-vs-native-hierarchy; Chromium gets "the flow tree keeps only
addressable nodes"; Android gets the importance trim; Vega gets "the
trees agree, so the screen changed". The named tool for reading the
runner's side follows the same rule — pointing an Android author at
`native-find-views` points at a tool they cannot call.

The remedy is per **condition** as well as per platform: on `hidden` a
determinate verdict means the runner's tree still *has* the element, so
"retarget at an id the full hierarchy carries" is the opposite of what
helps.

## A failed live wait is reported by cause

`await-ui-element` returns `{ success: false }` for three different
things: a condition read and found false, a tree source that never
answered, and a cancelled wait. Only the first judges the condition, so
only the first is narrated as "the wait never held" and prescribes
re-recording or deleting the step. The other two observed nothing and
are reported as UNKNOWN — the same unknown-vs-known-bad distinction the
probe draws for the runner's tree.

A wait nested inside a recorded `run-sequence` gets neither warning:
that tool reports its own shape, which carries no `success` key. The
skill says so, and says to read `toolResult` there.

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
 2026-08-23 22:18:26 +02:00
filip131311 366f17c619 chore: bump version to 0.22.0 (#891) 
Bumps every workspace package, `package-lock.json`, and both
`server.json` version fields from 0.21.0 to 0.22.0 (18 files). Lockfile
regenerated on Node 24 / npm 11 so the diff is version-only; `npm run
check:versions` passes.

Covers everything on main since `v0.21.0`: `paste` tool (#879),
rotation-aware Android capture (#693), 0.25 default screenshot scale
(#878), no-reboot describe on externally booted sims (#583/#877), NDJSON
flow progress (#855), OTLP telemetry (#570), native-devtools
restart_required derivation (#560), Android profiler empty-CPU
explanation (#669), plus test/CI fixes.

Full-surface live QA of this main (tgz vs npm 0.21.0) on iOS, Android,
tvOS, Electron, CLI and Linux/remote found zero regressions. Two release
notes: the published artifact must be built with
`ARGENT_OTEL_INGEST_TOKEN` set (a local tgz without it compiles
`readIngestToken()` to an empty string, making telemetry a silent
no-op), and remember to push the `v0.22.0` git tag so online `init`
skills installs resolve.

No docs update needed: version-only change.
 2026-08-21 19:33:57 +02:00
filip131311 172e4d3a64 fix(android): capture the screen the way it is actually oriented (#693) 
Part of #609.

## What was wrong

On a rotated Android device `screenshot` returned a portrait-framed
image with the content lying sideways. `describe` frames and gesture
coordinates were **already upright** — uiautomator measures against the
rotated display — so the capture was the one surface out of step. An
agent reading that image saw geometry that disagreed with every
coordinate it was given.

Measured on a landscape Pixel_9 (API 36, `mCurrentOrientation=1`):

| | space |
|---|---|
| `describe` | upright — `"Search Settings"` centre `(0.530, 0.234)` |
| taps | upright — tapping describe's `"Display & touch"` centre opens
it |
| `screenshot` (default) | **portrait-native, sideways** |

## The fix

simulator-server already applies `rotation` correctly; nothing was
telling it which one. The rotation is now queried from the device per
capture, so `screenshot`, `screenshot-diff`'s live captures and the
auto-attached "Screen after action" screenshot all land in
`describe`/tap space.

**Nothing is tracked.** A cached orientation could only be staler than
the ~40 ms probe, and serving a stale one is exactly the failure being
fixed. When the rotation cannot be read the request is left
byte-identical to before rather than guessing — a wrong orientation is a
confidently wrong image, which is worse than a sideways one.

**iOS is deliberately untouched.** There the whole surface is
consistently portrait-native, so rotating only the capture would *break*
an agreement rather than restore one. The iOS half of #609 needs a
different change and is not in this PR.

### Also fixed: the legacy uiautomator describe path

Same root cause, worse symptom. `wm size` reports the **unrotated** size
— on the landscape Pixel_9 it still answered `Physical size: 1080x2424`.
Dividing rotated bounds by that does not merely squash frames:
`isVisibleRect` prunes nodes whose left edge is past the screen width,
so **the right-hand half of a landscape screen vanished from the tree**.
The dump carries `<hierarchy rotation="N">`, so the divisor is oriented
from bytes already in hand — no extra round-trip on the path that runs
precisely when the helper could not be reached.

## On the rotation-name table

Pinned by measurement, never by what the names mean — the reporter of
#609 found the same names are 180° inverted on iOS. `adb exec-out
screencap` is rotation-aware, so it is ground truth; each candidate was
scored as mean absolute difference against it both as-is and rotated
180°, so an inverted mapping could not pass:

```
rotation 1  LandscapeLeft       2.19  (vs 6.54 for its 180° twin)
rotation 3  LandscapeRight      2.19  (vs 6.34)
rotation 2  PortraitUpsideDown  2.67  (vs 13.13)
rotation 0  Portrait            identity
```

This table is the **inverse** of simulator-server's own `90° →
LandscapeRight` convention, because it compensates for a rotation that
layer applies while decoding the scrcpy stream. That makes it a property
of a particular simulator-server build rather than of Android — which is
why a table test alone is not enough, and the delivered PNG's aspect is
checked against the rotation requested, falling back to an unrotated
capture if they disagree.

## Verified live

Branch tool-server against a real emulator:

- landscape, default capture → **2424×1080 upright**, MAD 2.19 vs ground
truth (6.54 vs its 180° twin)
- cropping `describe`'s frame for the toolbar out of that default
capture shows exactly the "Display & touch" toolbar — the two spaces
agree
- rotation 0 → 1080×2424, MAD 2.82, unchanged
- rotated **iPad** → still 2064×2752 sideways, i.e. iOS provably
untouched

Plus 27 new unit tests, including one that asserts the legacy path
*drops* the right-hand node before the fix and keeps it after. Full
suite 3125 passing; lint, prettier, `typecheck:tests` and the
tool-description gate all clean.

## Not in scope

- The iOS half of #609.
- `rotate` on Android silently no-ops when auto-rotate is enabled (it
returned `{orientation:"LandscapeLeft"}` while `dumpsys` still read
`cur=1080x2424`). Filing separately.
- `screen-recording` still records sideways — same root cause, different
pipeline. Worth noting a session's screenshots are now upright while its
video is not.
- Existing visual baselines captured on a rotated Android device will
stop matching, since the baseline key embeds capture dimensions and
those now swap. This fails loudly as a missing baseline rather than
silently comparing the wrong thing — and those baselines were sideways
captures of an upright UI.

---

> **#673 and #658 are both stacked on this branch** — each conflicts
with this one (`screenshot/index.ts` and `screenshot-diff/index.ts`
respectively) but not with each other, so both were rebased onto this
branch as siblings rather than chained. Merge this first; GitHub then
retargets both to `main`.

---------

Co-authored-by: Filip131311 <f.kaminski2000@gmail.com>
 2026-08-21 17:34:59 +02:00