Commit Graph

59 Commits

Author SHA1 Message Date
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
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
Kacper Kapuściak 36aa14a5fc docs: add Argent documentation (#814)
Adds the Argent documentation site, published to
[docs.swmansion.com/argent](https://docs.swmansion.com/argent): a
branded Docusaurus project under `packages/docs/`, the full first set of
pages, and the CI that checks and deploys it.

<img width="1512" height="945" alt="Screenshot 2026-08-21 at 13 05 04"
src="https://github.com/user-attachments/assets/4607e143-4359-49a8-93a2-aaa3af4c9534"
/>

## Site

A standalone Docusaurus 3.9 project, built on the shared Software
Mansion docs theme
([`@swmansion/t-rex-ui`](https://www.npmjs.com/package/@swmansion/t-rex-ui))
and restyled for Argent:

- **Palette** from [argent.swmansion.com](https://argent.swmansion.com):
dark `#0D0F26`, blue `#99DAFF`, lavender `#E4E1FF`, light `#FEFEFE`,
mist `#F1F1F1`. The `--swm-*` token names stay, since the shared theme
resolves against them.
- **Typography**: DM Sans and DM Mono, matching the landing page.
- **Sidebar** with lucide icons on every page entry
(`sidebar_custom_props.icon` in the front matter, registered in
`src/theme/SidebarIcon`), the logo kept on the page background and the
panel inset from the edge.
- **Video component** for embedded screen recordings, with
`scripts/encode-video.sh` producing a web-sized MP4 and a poster frame
for each clip in `static/video/`.
- **Copy page button** and a few theme fixes: inline code badges in
tables, paginator hover.
- **No landing page.** That stays at argent.swmansion.com. The root
route redirects to Getting started.

## Content

Three sections in `docs/`:

- **Fundamentals**: getting started, installation, supported platforms.
- **Features**: interacting with apps, flows, network, screen recording,
lens, visual regression, profiling, debugging. Conceptual overviews with
recordings, each linking to the tools reference.
- **Reference**: tools, CLI, configuration, flow YAML, editors,
telemetry.

Prose follows Simplified Technical English. The conventions (style,
front matter, icons, checks) are written down in
`packages/docs/CLAUDE.md`, and a root `CLAUDE.md` adds a checklist so
that code changes to tools, CLI, configuration or flows update the
matching docs page in the same pull request.

## CI and deploy

- `Docs build` runs `format:check`, `lint`, `typecheck` and `build` on
every pull request that touches `packages/docs/`. The build has
`onBrokenLinks: "throw"`, so it catches links left dangling by a moved
page.
- `Docs publish` deploys `packages/docs/build` through the GitHub
Actions Pages source on every push to `main` that touches the docs. The
repository Pages source needs to be set to **GitHub Actions** for the
first run.

## Notes for review

- The site is excluded from the root `packages/*` workspaces
(`!packages/docs`) and keeps its own `package.json` and
`package-lock.json`, so its dependency tree stays out of the toolkit's
lockfile. Root Prettier, ESLint and knip ignore it; it formats and lints
itself with the repo `.prettierrc` and its own `eslint.config.mjs`.
`check-workspace-versions.mjs` skips it so its `0.0.0` version is not
read as drift.
- `webpack` is pinned to `5.105.4` and `@docusaurus/plugin-content-docs`
/ `theme-common` to `3.9.2` via `overrides`. Newer webpack fails
Docusaurus 3.9's ProgressPlugin option validation, and a hoisted
`plugin-content-docs@3.10.2` produced a duplicate React context that
crashed SSR.
- **Search is not wired up yet.** The shared theme always mounts a
DocSearch bar, so an `algolia` block has to be present. It reads
`ALGOLIA_APP_ID` / `ALGOLIA_API_KEY` / `ALGOLIA_INDEX_NAME` from the
environment and the bar stays hidden until Argent has its own DocSearch
application. Follow-up PR.

## Testing

- `npm run format:check`, `npm run lint`, `npm run typecheck` and `npm
run build` pass in `packages/docs/`.
- Root `prettier --check`, `eslint` and `npm run knip` pass.
- Walked the served production build in the browser in light and dark
themes while iterating: root redirect, docs pages, sidebar icons,
videos, TOC, footer.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-21 16:03:35 +02:00
Ignacy Łątka 8c8cdb885e fix(e2e): stop the harness reporting checks it never made (#595)
Found while running the full E2E harness against the packed 0.17.0
bundle for the release check. All of these are harness bugs, not product
bugs.

## The core problem

The harness reported results it had not actually measured.

`assert_reject` looked for zod issues on **stdout**, but the CLI prints
them on stderr and leaves stdout empty on a rejection. Both arms of the
check called `pass()`, so the miss was invisible: the zod-path argument
every case passes in was decorative, and a CLI that simply exited 1 for
every input would have scored the same 88/88. **All 88 validation cases
asserted nothing beyond "exit code was non-zero."**

Once that check could fail, it exposed the rest:

- **awk has no `\b` word boundary.** `/\bnumber\b/` matches nothing, so
every flag kind resolved to `unknown`, `model_number_flags` was always
empty, and **the bad-type matrix never emitted a single case** - while
`_build_args` handed the string `"x"` to every required
numeric/boolean/object field.
- The describe parser matched `--[a-zA-Z0-9-]+`, **truncating every
snake_case flag at the underscore** (`--device_id` → `device`), and kept
the `-json` display suffix (`--selector-json`) that is not part of the
schema key. Both produced payloads carrying keys no schema knows.
- `missing-required` asserted `invalid_type` for enum flags, which zod
reports as `invalid_value`.

Parsed kinds went from **270 unknown / 21 enum / 1 array** to 131
string, 68 number, 28 boolean, 25 integer, 21 enum, 16 array, 3 object,
2 unknown (the last two are `any`, which is correct).

## Teardown and the exit code

`run-e2e.sh` had no `trap`. An unbound variable inside a phase is fatal
to the whole shell - `run_phase || warn` does not catch it - so an abort
skipped the cleanup phase, the report, and `rm -rf $E2E_WORK`, leaking a
~500MB sandbox plus a tool-server, Electron, Metro and a fixture http
server. Report generation, teardown and the exit code now live in an
EXIT trap, with a completion flag: without it an aborted run exits **0**
on "0 failures", which is verified below by mutation.

Teardown runs *before* the sandbox is removed - `$E2E_HOME` holds the
detached tool-server's pid file and that server is started with no idle
timeout, so deleting the sandbox first strands a process `argent server
stop` can no longer find.

`90-cleanup.sh` gated `stop-all-simulator-servers` and `stop-metro` on
`ARGENT_TOOLS_URL`, which the harness never sets (`ensure_server`
deliberately does not pin it), so **neither ever ran** and every run
left a simulator-server and Metro alive - while `teardown-complete`
passed regardless.

## Other findings fixed

- **`--android-avd` could never work**: `boot-device` was asked for 840s
inside `run_tool`'s 120s timeout, and the booted serial stayed in a
`local`, so the RN tier blanket-skipped all 24 debugger/profiler tools
for want of a device that was right there. Nothing shut the emulator
down either - the tool-server only reaps devices Lens booted, not ones
`boot-device` started.
- **The validation tier was byte-identical green against a dead
server.** `argent run` does no client-side validation, so every call
failed with a transport error that no one distinguished from a
rejection. It refuses to run instead, and no longer counts
`timeout(1)`'s 124-127 as a rejection.
- **The post-uninstall restore** hardcoded `--omit=optional` and the
sandbox prefix - dropping electron on a full run (the chromium tier then
skipped itself as "electron not installed"), and under `--system`
restoring the machine's real global `argent` into the sandbox instead of
putting it back.
- **The Android teardown claimed to leave an injected device alone** and
then issued the same `stop-simulator-server` call as the self-booted
branch, discarded the result, and recorded a skip saying it had not -
tearing down the allocator's device service immediately before the RN
tier drove that same serial.
- **The chromium port-owner fallback** matched `:$port` as a substring
and took the first pid anywhere in `ss -ltnp` output, so cleaning up
`:1234` would kill a process listening on `:12345`.
- **The RN tier's `stop-metro`** sat after every early return (a failed
`debugger-connect` left Metro running) and ran unconditionally, killing
a developer's own Metro on :8081 that the harness never started.
- **The chromium display gate** accepted an installed `xvfb-run` as a
substitute for `DISPLAY`, but nothing wraps the Electron spawn in it -
so a headless box with it installed cleared the gate and failed
`boot-device`, turning the gate red for exactly the environment it
exists to skip.
- A phase file with a syntax error silently re-ran the **previous**
phase under the new phase's name; an unknown `--phase` selected nothing
and exited 0; `-h` printed 7 lines of source code; a value-taking flag
with no value died on an unbound `$2`; two runs in the same second
shared one results file.
- `flow-add-step` declares its own `args` property, which makes `--args`
that field's value rather than the whole-payload escape hatch, so
`run_tool` cannot address its fields. Skipped with that reason rather
than failing on a payload the tool never received.

## Verification

Against `swmansion-argent-0.17.0.tgz`, `introspection,validation`:

| | before | after |
|---|---|---|
| totals | 177 pass / 0 fail / 5 skip | 221 pass / 0 fail / 6 skip |
| rejection cases naming the field they test | **0 of 88** | **130 of
131** |
| bad-type cases emitted | **0** | **43** |

- `assert_reject` checked against right-field, wrong-field, wrong-code,
no-path and unreachable-binary inputs. Every one of those cases passes
on the un-fixed code.
- Abort path: injected an unbound variable into a phase - cleanup phase
runs, report written, sandbox removed, exit 1, `PARTIAL` reported. With
the completion flag mutated to always-set, the same abort exits **0**,
which is what pins it.
- awk `\b`, the `-json` suffix, the underscore truncation, the `ss` port
collision and the `timeout` 124-127 contract were each reproduced
standalone before being fixed.

## Found by running against a live Android emulator

The android tier executed end-to-end for the first time: **23 pass / 1
fail / 10 skip**, including the `screenshot-diff` pair that the original
unbound-variable bug aborted on. Three further defects only surfaced by
running:

- **Nine `pass`/`fail`/`skip` calls passed their message as the third
argument**, which `_record()` takes as the *case name* — so the detail
column was empty and the case string changed every run. The chromium
boot failure was the visible cost: it recorded `case=" "` with **no
diagnostic whatsoever**.
- **A call killed by `timeout(1)` writes nothing**, so `RT_OUT` is empty
exactly when the operator most needs to know what happened. A hung
`boot-device` was recorded as a failure with a blank detail. All nine
ad-hoc detail formatters now share `rt_detail()`, which names the
timeout (124) and a command `timeout` could not run (125-127).
- **The chromium display gate tested that `$DISPLAY` was non-empty, not
that it was usable.** A display-manager session switch on this box left
`$DISPLAY` pointing at a server the process holds no cookie for;
Electron then bound its CDP port and never answered on it (verified: the
TCP connection is accepted, the HTTP response never arrives), so the
tier hard-failed instead of skipping.

Moving `profiler-stack-query` and `profiler-combined-report` into the
native block also closed a hole their removal had opened: both were
listed in `RN_TOOLS` and in the failure-arm skips but had no case on the
success path, so a successful `react-profiler-start` left them with no
pass, fail or skip at all.

## Not covered here

- The **postinstall assertion** is left alone - #556 already fixes it,
and this branch does not touch it. Until that merges the install phase
still records that one failure.
- The **RN tier still has not executed.** The emulator itself is fine —
it cold-boots in ~24-30s and ran the whole android tier — but it dies
from CPU starvation (`detected a hanging thread 'QEMU2 main loop'`)
whenever this machine's load average climbs, which it does to 90-120 on
16 cores from concurrent work. Four attempts: it survived the android
tier at load ~10, and was starved each time by a competing `npm install`
or by Metro's own bundler startup. The four RN profiler fixes are
verified against the tools' source contracts (the throw sites and their
required params), not against a live run.
- The **chromium tier still has not executed** past `boot-device`, for
the display reason above.
- Known remaining, deliberately left: `assert_ok` cannot fail for calls
whose result is a constant `true`; the cookie/storage set→get pair never
verifies the round-trip value; a tier-level skip writes one row so
`report.py`'s coverage matrix silently drops the tools that tier would
have recorded; `report.py` renders an empty or partial log as a clean
green report.
2026-08-20 16:32:09 +02:00
Ignacy Łątka 3761fb331b fix(e2e): make the debugger assertions fail when the debugger is not connected (#704)
Found while reviewing #610 and set aside as out of scope for that
review: the weaker of the two assertions is a live defect on `main`
today, the other becomes one when #610 lands.

## The defect

`assert_ok` (`lib/common.sh:122-130`) records PASS on `RT_RC == 0` and
never reads `RT_JSON`. Both tools on lines 84 and 86 answer **HTTP 200
while reporting that the debugger is not connected**, so the two
assertions that exist to check the debugger chain pass over a dead
session.

**Line 84 is broken on main today.** Half-close the CDP socket
server-side (the socket lands in CLOSING with no `close` event, so no
terminated cascade) and the cached service still resolves:

```
ground truth: {"connected":false}

=== shipped assertions (lines 84/86) ===
  ✓ debugger-status status
  ✓ debugger-log-registry logs
=== patched assertions ===
  ✗ debugger-status [status] expected .connected to be true, got 'false'
  ✓ debugger-log-registry logs
```

**Line 86 breaks once #610 lands**, which turns the unreachable-runtime
500 into a 200 `not_connected` payload. Same two lines against a build
of that branch with nothing listening on the Metro port:

```
ground truth: {"status":"not_connected","connected":false,"reason":"metro_not_running"}

=== shipped ===   ✓ debugger-status status   ✓ debugger-log-registry logs
=== patched ===   ✗ debugger-status [status] expected .connected to be true, got 'false'
                  ✗ debugger-log-registry [logs] expected (.status // "connected")=connected got 'not_connected'
```

## Why these two jq paths

Measured against live tool-servers built from both `main` and #610, in
both the connected and the not-connected state:

| | main connected | main dead socket | #610 connected | #610
not-connected |
|---|---|---|---|---|
| `debugger-status` | `.connected=true` | `.connected=false` |
`.connected=true` | `.connected=false` |
| `debugger-log-registry` | **no `.connected`** | - | **no
`.connected`**, `.status="connected"` | `.status="not_connected"` |

`.connected` is absent on log-registry's success path in *both* builds,
so `assert_true '.connected'` would fail a healthy run - that is why the
second line uses `(.status // "connected")`, which is `"connected"` on
main (field absent) and on #610's success, and `"not_connected"` on its
failure. Both assertions therefore work on main and keep working after
#610.

No new helper: `assert_true` / `assert_field` already exist and the
harness already gates `launch-app '.launched'`, `restart-app
'.restarted'`, `gesture-tap '.tapped'` the same way. `debugger-status`'s
`.connected` was the one flag of that shape left bare.

## Verification

Repro is a real tool-server per build plus the mock Metro/CDP target,
driving the assertion lines lifted verbatim out of the patched phase
file. The unpatched `assert_ok` passes in **both** states - that is the
mutation control. Timing note for anyone re-running it: the half-closed
socket self-heals after ~30s when the `ws` close timeout fires, so steps
5 and 6 have to run back to back.

## Deliberately not in this PR

The same sweep found ~20 sibling `assert_ok` sites whose tool reports
failure inside a 200 payload - `await-ui-element` `{success:false}`,
`await-screen-idle` `{settled:false}`, `run-sequence` `{completed:0}`
with the error in `steps[]`, `stop-metro` (whose entire body is inside a
`try/catch` returning `{stopped:false}`, so `assert_ok` on it can never
fail), `chromium-cookies set` `{set:false}`, `react-profiler-status`
`{session_status:"no_react_runtime"}`, and the string-returning tools
that answer `"No network traffic captured..."` at 200. Those verdicts
are read off each tool's result contract, not driven live - confirming
them needs the Android and Chromium tiers - so they are left for #595,
which is already rewriting this harness with the same intent.

One adjacent bug worth recording while it is fresh:
`50-rn-bluesky.sh:96` extracts `RID` with `jq '(.requests // .entries //
[])[0].requestId'` from `view-network-logs`, which returns a formatted
**string**, not an object. `RID` is therefore always empty, line 98 is
dead and line 100's skip is always taken.

Merges cleanly with #595 (test-merged: `Automatic merge went well`; that
PR rewrites the `local D=` line above but touches neither assertion).
2026-08-20 16:31:26 +02:00
Ignacy Łątka 7d51d880af fix(scripts): stop the version guard passing a workspace it failed to read (#608)
Follow-up to #598, which merged while these were still being verified.
Three defects in the version guard it added, plus the test gaps that let
them through.

### An unreadable `packages/*` manifest was silently dropped

The scan's `catch { continue; }` swallowed every `readFileSync` +
`JSON.parse` failure, not just a missing file, so a manifest that is
*present but unparseable* left the lockstep comparison entirely — and
the run then affirmed a version it never checked:

```console
$ node scripts/check-workspace-versions.mjs      # BOM on a drifted packages/registry manifest
All workspace packages and server.json are at 0.18.0.
exit=0
```

Control: identical tree with the BOM removed → `exit=1`, `0.17.0:
@argent/registry`.

A BOM is the trigger that matters. A trailing comma is caught upstream
(`npm ci` fails `EJSONPARSE`, and `prettier --check .` flags the file),
and `chmod 000` is a workstation-only state since `actions/checkout`
writes 644. A BOM is invisible to both — `prettier --check` reports the
file clean and `npm install` parses it — and `repo-hygiene.yml` runs the
guard on a bare checkout with no `npm ci` in that job, so nothing in the
pipeline sees it.

Now only `ENOENT` is skipped, which is what the comment (`// directory
without a package.json`) ever meant to cover; anything else fails naming
the file. This half is pre-existing — the same `catch` sits on the merge
base and fails open there identically — but #598 added `readTrackedJson`
precisely so a missing/malformed tracked JSON file fails on-message, and
applied it to two files while the other sixteen manifests kept the
silent skip.

The same fail-open had a second, subtler window: a manifest whose *parse
succeeds* but returns a non-object (`null`, a bare scalar, or an array).
`JSON.parse` accepts all three, so none of them ever reached the
`catch`, and the `manifest?.version` read then skipped the package as if
it carried no version — a present-but-unusable manifest still leaving
the comparison and affirming a version it never read. That shape is
exactly what `readTrackedJson`'s shape check rejects for `server.json`
and `packages/argent`, so the scan now rejects it the same way
(on-message, naming the file) with tests for the `null` and array forms.

### The failure report told you to edit the wrong file

Three mismatch lines report a problem in `packages/argent/package.json`
(no `version`, no `name`, no `mcpName`), but the trailer read
"server.json is not under `packages/*` … edit it by hand to match
`packages/argent/package.json`" — the opposite of the fix:

```console
$ node scripts/check-workspace-versions.mjs
server.json is out of sync with packages/argent/package.json:
  packages/argent/package.json has no mcpName — the registry reads it off the published tarball to prove the namespace

server.json is not under packages/*, so a workspace bump leaves it behind — edit it
by hand to match packages/argent/package.json.
```

Every mismatch line already begins with the file it concerns, so the
trailer now says that instead of naming one file for all of them.

### An array `packages[]` entry bypassed the shape guard

`typeof [] === "object"`, so `["@swmansion/argent", "0.18.0"]` fell
through to the field comparisons and came out as two `undefined`
mismatches rather than the wrong shape it is.

### Test gaps

Each of the following was mutated and survived the whole suite plus
`eslint` and `tsc` before this change, and is killed by exactly the test
added for it now:

| mutation | was |
| --- | --- |
| `catch` swallows every manifest read error | 27/27 green |
| `readTrackedJson(argentManifestPath)` → raw `JSON.parse` | 27/27 green
|
| drop `names.sort()` in the drift report | 27/27 green |
| `manifest.name ?? entry.name` → always the manifest name (drops the
`entry.name` fallback) | 27/27 green |
| version-less manifest joins the version map | 27/27 green |
| either failure report's guidance trailer deleted | 27/27 green |
| drop the array arm of the entry shape guard | 27/27 green |

`readTrackedJson`'s malformed-JSON arm is unreachable from the
`packages/argent` side — the scan reads the same file first and stops
there — so the two tests added for that call site cover the arms that
are reachable (missing, and not-an-object).

The `names.sort()` gap was a fixture artifact: the only two-package
fixture used directories `alpha`/`argent` holding
`@argent/alpha`/`@swmansion/argent`, already in sorted order, so the
sort was a no-op. The fixture now puts `@zzz/scanned-first` in directory
`aaa`, so scan order and sorted order disagree.

Same story on the name-fallback row: the `manifest.name ?? entry.name`
*fallback* arm (a versioned manifest carrying no `name`) had no fixture
at all, so writing it as `manifest.name` (dropping the fallback → prints
`0.16.0: undefined`) survived the whole suite. The reverse mutation —
`entry.name` only — was never the gap: the merge base's fixture in
directory `registry` named `@argent/registry` already asserts the
manifest name, so `@argent/registry`-expecting lines fail under it. The
new fixture (a versioned manifest with no name) is what pins the
fallback arm.

### Verification

- `test:scripts` 91/91, `eslint --max-warnings 0`, `tsc -p
tsconfig.scripts.json`, `prettier --check` — all clean on top of `main`
at eea6c27c.
- Each repro above re-run against the fix; the BOM case additionally
re-run against the merge base to confirm the pre-existing fail-open.
2026-08-18 14:12:31 +02:00
Ignacy Łątka 5ac6805040 fix(native-devtools): derive restart_required from the running process instead of asserting it (#560)
this is urgent 

---

## Problem

Every `native-*` tool can get stuck telling the agent to do something
that will never work.

I hit this during an end-to-end pass over 0.17.0. After a mid-session
simulator reboot, `native-devtools-status` reported:

```json
{ "envSetup": true, "appRunning": true, "connected": false,
  "requiresRestart": true, "nextLaunchWillBeInjected": true, "injectable": true }
```

and every `native-*` tool returned `restart_required` with "Call
restart-app then retry." Following that advice never helped, and the
message offered nothing else. `flow-execute` was blocked too, since its
`launch` step needs the connection, and flow recording silently degraded
a portable `tap: { selector }` to raw coordinates when it couldn't reach
native devtools.

The app was injected the whole time. All three dylibs were mapped into
the process and the simulator log showed a successful dial on every
launch, while the tool-server held the listening end of that exact
socket and still reported `connected: false`. `argent server stop &&
argent server start` cleared it instantly.

## The real defect is the state, not the wording

`requiresAppRestart` returned `true` for every app without a live
connection:

```ts
async requiresAppRestart(bundleId) {
  if (connections.has(bundleId)) return false;
  await reverifyEnv();
  return true;   // <- asserted, never measured
}
```

That is not a measurement. `restart_required` was a constant wearing a
state's clothes, and the advice built on it stays literally true
forever, so an agent obeying it restarts the app indefinitely.

Softening the message would have left that intact. This PR measures the
state instead.

## Fix — derive it from the running process

`restart-app` relaunches into the simulator's **current** launchd
environment, so it can only ever help a process launched under different
terms than a fresh launch would get. That is answerable: read the
process. `launchctl list` already gives its pid; `ps eww -p <pid> -o
etime=,command=` gives the environment it was exec'd with and its age.
Simulator apps are ordinary host processes owned by the same user, so
both are readable.

`appConnectionState` (replacing `requiresAppRestart`) returns:

| state | meaning | remedy |
| --- | --- | --- |
| `connected` | devtools are live | — |
| `not_running` | no process to read | `launch-app` |
| `stale_process` | no bootstrap dylib, a previous run's endpoint, or
older than this service's listener | `restart-app` |
| `unregistered` | injected against **this** endpoint, started after the
listener came up, still silent | restart the **tool-server** |
| `indeterminate` | process could not be read (ios-remote, unreadable
`ps`) | `restart-app`, with the loop warning |

Only `not_running` / `stale_process` / `indeterminate` report
`restart_required`. `unregistered` gets a new `service_stale` precheck
status that names the tool-server restart and explicitly says not to
relaunch the app — the launch a restart would perform has demonstrably
already happened.

The listener-age comparison is load-bearing: after a tool-server restart
the unix path is identical (it is derived from the udid) but the inode
is new, so an injected process older than the listener still needs
relaunching. Without that check this would send people off to restart a
tool-server they had just restarted — a new loop in place of the old
one. Both grace comparisons lean away from `unregistered` - the first
towards `stale_process`, the second towards `indeterminate`, and both of
those ask for a relaunch - so an uncertain read costs at most one wasted
`restart-app`.

Call sites follow the same rule rather than each inventing guidance:

- `describe` withholds `should_restart` for `unregistered` (setting it
would rebuild the restart-app → describe loop through a live service
instead of a system app) and carries the diagnosis as a `hint`, which
still marks the empty read untrustworthy for `await-ui-element`.
- `flow-run` raises the measured diagnosis instead of a blanket
"relaunch it" — both when a flow
resolves a selector against the full hierarchy, and at the `launch`
step's readiness gate, which
previously polled for the connection and then guessed at "a stale or
duplicate argent server".
- `native-devtools-status` reports `state`, and `requiresRestart` is now
false for `unregistered`.
- The six `native-*` tools type their block as the shared
`NativeDevtoolsPrecheckBlock` instead of restating it, so a new precheck
outcome cannot reach an agent through a tool that never mentions it.

## Scope

Why the service can stop registering a connection it is listening for is
still #561 and is not fixed here. What changes is that the state is now
measured, so that defect reports itself accurately instead of
masquerading as an app that needs restarting.

## Verification

**E2E, real iOS 18.6 simulator, Bluesky (RN 0.81.5).** Two tool-servers
were raced onto the same per-udid socket — the #561 shape, where the app
is genuinely injected and connected, just to the newer instance. Both
were then queried on the same app at the same instant:

| | `connected` | `requiresRestart` | `state` | `native-describe-screen`
|
| --- | --- | --- | --- | --- |
| main, stale instance | false | **true** | — | `restart_required` —
"Call restart-app then retry." |
| this branch, stale instance | false | **false** | `unregistered` |
`service_stale` — "Restarting the app cannot change that … restart the
tool-server" |
| healthy instance | true | false | `connected` | `ok`, 44 elements |

Restarting the app in main's row lands on the newer instance again,
leaving the queried one stale forever. That is the loop, reproduced on
demand.

The other states were walked on the same simulator: a pre-existing
process against a fresh listener → `stale_process` (`requiresRestart:
true`); `restart-app` → `connected`; terminate → `not_running` ("Call
launch-app (or restart-app)").

**Unit.** `native-devtools-app-state.test.ts` drives the real blueprint
factory with a stubbed process table and covers all five states plus the
`etime` and env-token parsers. Mutation-checked — dropping the
listener-age comparison, the endpoint match, or the connect grace each
fails exactly one test and nothing else.

**Suite.** `vitest run` in `packages/tool-server`: 296 files, 3113
passed, 1 skipped. `tsc --build` across the workspace, `tsc --noEmit -p
tsconfig.test.json`, `eslint` and `prettier --check` on every changed
file: clean. `origin/main` merged in first.



## Review round

A review pass over the derivation itself, with each finding reproduced
before it was acted on.

**`describe` computed the diagnosis and threw it away.**
`buildAppStateMessage` ran for every
unconnected state, but only `unregistered` carried it out; the rest
returned a bare
`should_restart: true`. That boolean reaches an agent as an undocumented
JSON field — `describe`'s
own description never mentions it — and the single place it is rendered
as prose is
`await-ui-element`'s timeout note, which spells it *"call restart-app
and retry"*. So the two states
whose message carries the escape hatch lost it exactly where it was
needed. `indeterminate` is the
sharp end: on `ios-remote`, whose app processes are out of reach of the
local process table, it is
the **only** reachable unconnected state, so that path had no exit at
all. The diagnosis now rides
out as a `hint` for every state; `should_restart` still marks only the
states a relaunch fixes.

**`native-devtools-status` measured the same device twice.**
`isAppRunning` ran its own
`launchctl list`, and `appConnectionState` ran another one after
re-verifying the launchd env — so
the two snapshots sat seconds apart and could contradict each other
(`appRunning: true` beside
`state: "not_running"`), with `requiresRestart` gated on the staler of
the pair. Both are derived
from one measurement now; only `indeterminate`, which genuinely leaves
running-ness unanswered,
pays for a second probe.

**Coverage.** Mutation testing found the 3 s grace term entirely
unpinned — the fixtures sat 200x
away from the boundary on either side, so both dropping the term and
weakening `>=` to `>` kept the
suite green while flipping a relaunchable process to `unregistered`, the
one verdict the file exists
to withhold. Also unpinned: the `reverifyEnv` call (swapping it for the
latching `ensureEnvReady`
the comment explicitly forbids passed the whole suite) and an
unparseable `ps` age staying "no
evidence" rather than becoming a measurement. All three are now covered,
and each new test was
confirmed to fail on the un-fixed code.

**Verified against the real thing.** The `launchctl` and `ps` contracts
were re-checked on an
iOS 18.6 simulator rather than reasoned about: row layout and pid
column, one-line `ps eww` output
with the argent tokens present in a live app's environment, `etime`
matched against `lstart` to the
second, and the four `etime` format strings read out of the `ps` binary.
Two corrections came out of
it — the environment can reach `kern.argmax`, which is exactly Node's
default `maxBuffer`, so the
probe now sets the same 16 MiB its siblings use; and `ps` suppresses the
environment of a
SIP-protected platform binary, which the docstring's "never as 'not
injected'" promise did not
account for (unreachable here, since `com.apple.*` is rejected as
non-injectable upstream, but the
promise now says so).

**Also corrected:** the `argent-device-interact` skill offered
`restart-app` as the only remedy for
a missing injection; the dylib-platform check in
`download-native-binaries.sh` and a
`requiresAppRestart` reference in `boot-device.ts` described states that
no longer exist; and three
tool descriptions still said *"Fails if native devtools are not
connected"* directly beside the new
sentence explaining that this case returns a status rather than failing.

**Known and deliberately left** (both pre-existing, neither introduced
here):
`describe` with no `bundleId` auto-targets from the connected list, so
an unregistered app throws
`NATIVE_TARGET_NO_CONNECTED_APPS` before any state is measured and the
message is swallowed by the
fallback's catch — surfacing it would newly mark those empty reads blind
for `await-ui-element`'s
`hidden` condition, which wants its own verification pass. And a dylib
that is inserted but silently
skipped by dyld (a platform mismatch) reads as injected-but-silent, so
`service_stale` → tool-server
restart → `stale_process` → `restart-app` is a two-step cycle;
distinguishing it needs a load
confirmation the process table cannot give.



## Review round 2

A second pass over the derivation, run as four independent sweeps
(claims-vs-code, nearest-twin
and reachability, non-happy paths and inputs, and absence). Three of the
four independently landed
on the same defect.

**The flow launch gate was the one caller that never asked whether the
app could be injected at
all.** Every other consumer of `appConnectionState` gates `com.apple.*`
first - the precheck
throws, `describe` returns its terminal hint, `native-devtools-status`
short-circuits,
`flow-ios-tree` only ever auto-resolves a connected app.
`waitForNativeDevtools` measured the raw
launch id. That matters because the launchd environment carrying the
bootstrap dylib is
simulator-wide, so a system app's process inherits the very tokens the
measurement reads.
Confirmed on an iOS 18.6 simulator: `com.apple.Preferences`, launched
under argent, reports

```
71940	0	UIKitApplication:com.apple.Preferences[9a63][rb-legacy]
etime=00:03
DYLD_INSERT_LIBRARIES=.../libArgentInjectionBootstrap.dylib
NATIVE_DEVTOOLS_IOS_CDP_SOCKET=/tmp/argent-nd-39B9C547.sock
```

Run through the shipped algorithm that is `processCarriesInjection:
true` against an older
listener, i.e. **`unregistered`** - so a flow with an `ios:` launch
entry of `com.apple.Preferences`
failed its launch step with *"restart the tool-server"*, for an app no
tool-server restart can ever
help. Library validation means it can never connect, so the gate now
says that terminally.

**`describe` dropped the entire diagnosis when the probe rejected.**
`appConnectionState` re-applies
the launchd env before it can answer anything, so it throws outright on
a sim that goes away
mid-call; that landed in the native fallback's outer `catch` and
returned an empty tree with no
hint. An agent saw "nothing on screen" rather than "could not be read",
and `await-ui-element` had
nothing to append to its timeout note. The two other consumers of the
same call already degraded to
`indeterminate` instead - this one now matches them.

**`native-devtools-status` re-ran the probe that had just failed.** One
route into `indeterminate`
is `inspectRunningApp` rejecting, i.e. `simctl spawn <udid> launchctl
list` failing. The
running-ness fallback then issues that same command unguarded, so a
shut-down simulator surfaced a
raw subprocess error rather than the structured `init_failed` guidance -
which the branch 40 lines
above it handles correctly, for exactly this reason.

**`native-devtools-status` was the only surface withholding the loop
escape.** Every other consumer
carries `buildAppStateMessage`, whose `indeterminate` text ends *"do not
keep restarting the app;
restart the tool-server"*. This tool returned booleans only, and
`requiresRestart: true` is the
whole of what an `indeterminate` app tells you. On ios-remote, where no
app process can ever be
inspected, that is the only state a running app reaches - so the tool
whose stated job is verifying
readiness answered with the restart loop and nothing else. It now
returns `message`.

**Also fixed:** both probes log their own failure (a broken probe
degrades every app to
`indeterminate`, indistinguishable at the tool surface from a genuinely
uninspectable one - the
sibling `ps` probe in `vega-process.ts` logs for the same reason); the
gate skips its
several-second measurement once the run is cancelled; and the gate no
longer repeats the bundle id
its caller's prefix already names.

**Prose that had drifted from the code:** `not_running` is reachable on
ios-remote too, so
`indeterminate` is only the sole unconnected state a *running* app
reaches there; the second grace
comparison leans to `indeterminate`, not `stale_process`; four of the
five states settle
running-ness, not four besides `not_running`; a null pid means a row
that did not parse, not a job
launchd reported as having no process (measured: a `UIKitApplication`
row is removed outright when
the app exits, and `-` appears only on daemon labels);
`waitForNativeDevtools`'s documented return
contract described the branch the change had rewritten; and a rationale
comment was attached to the
test above the one it described.

**Coverage.** The gate's measured reason had none at all - replacing the
whole diagnosis with a
literal left all 542 flow tests green. Eleven mutations were run against
the fixes and every one is
now killed: the injectability guard, the measured reason, the doubled
bundle id, `describe`'s reject
guard, `describe`'s `unregistered` hint merge, the `message` field, the
precheck fallback, the
`appRunning` guard on `requiresRestart`, a state renamed in the
description only, the connect grace
widened from above (the old fixtures all sat 10x clear of it, so it
could have grown to 25 s
unnoticed), and the `indeterminate` guidance line.


---

## Review round 3 — external review findings

Six findings from @hubgan's E2E review, each reproduced before it was
acted on. Fixed in b9d17437
and 3eb59ef7. **This round supersedes several claims made above — they
are listed at the end.**

**The launch gate refused every `launch:` of an Apple system app.** A
coordinate-only flow gave
`["launch:error","tool:skip"]` with `resolveService` called 0 times —
the guard short-circuited
before any measurement — and the reason it emitted said *"Drive it by
coordinate instead"*, from
the gate that had just killed a purely coordinate-driven flow.
`argent-create-flow`'s worked
example (SKILL.md:171, :199-209) is exactly that shape.

The premise underneath it did not survive checking. #453, which
motivated `isInjectableBundleId`,
recorded `connected: false` for `com.apple.Preferences` on an iOS 26.5
sim; the review recorded
`connected: true`, with both dylibs mapped, on 18.5. So injection into a
simulator system app
appears to be runtime-dependent rather than categorically impossible,
and neither reading is
checkable from CI. The gate therefore no longer decides: it polls
exactly as before — where the app
does connect, nothing changes — and only on timeout does a
non-injectable id return "no failure"
instead of a reason. The launch step's job is to start the app; it
succeeded. **Whether
`isInjectableBundleId`'s blanket prefix rule is correct at all is now an
open question, and a
separate one from this PR.**

**The flow tree source could not report it either, because its
measurement was unreachable.**
`resolveNativeTargetApp(api, undefined)` draws candidates from
`listConnectedBundleIds()` — the same
map `appConnectionState` reads to answer `connected` — so a
non-connected app threw the stock
auto-target error first, whose "Launch or restart the app first" is the
loop this PR exists to
break. Driving the real factory: `appConnectionState === "unregistered"`
and
`listConnectedBundleIds() === []` at the same instant. The race between
the two is closed, not
narrow: no macrotask boundary separates them, while the socket `close`
handler is one. The branch's
own test passed only because its stub let the two accessors disagree.
The flow now threads the id
its `launch` step named — which does not come from that map — through
`ActionEnv` → `fetchFlowTree`
→ `queryFullHierarchyTree`; the unreachable branch is gone.

**The gate emitted `buildAppStateMessage` verbatim for every state but
`unregistered`.** Those are
written for the native-* tool surface, whose reader has not launched
anything, so a crash-on-launch
app was answered with "Call launch-app (or restart-app) then retry" —
the action the step had taken
9.5 s earlier. `flowLaunchGateReason` now rewrites each state for a
caller that has just launched,
exhaustively, so a state added later cannot inherit the tool-surface
remedy.

**`describe` without a `bundleId` — the default form — lost the whole
diagnosis.** Auto-targeting
throws before anything is measured and the outer catch returned the tree
bare, so
`await-ui-element`'s blind-read guard (which keys off exactly
`hint`/`should_restart`) let a
`hidden` wait release in 0 ms against an element still on screen, where
the same call *with* a
`bundleId` correctly refused. An empty read the hierarchy could not
corroborate now says so. A
device with **no** native-devtools service at all is deliberately left
unqualified — that is an
absence of a second opinion, not a failed attempt at one, and marking it
blind broke a real
`await-ui-element` case.

**Three of the six `native-*` descriptions named the terminal system-app
rejection and three did
not**, though all six throw the identical error (verified: one
byte-identical message across all
six). The clause is inlined verbatim in all six —
`scripts/extract-tools.mjs` accepts only a single
plain literal and silently drops anything else from the SpiderShield
scan.

**`not_running` cannot tell "stopped" from "not installed"** — both
simply lack a
`UIKitApplication:<id>` row — and for the second reading its only remedy
is refused outright by
`simctl launch`. It now names both readings instead of asserting the one
it cannot see. A real
probe was considered and not taken: `get_app_container` needs a booted
sim and ios-remote has no
such verb, so it would answer `undefined` on exactly the host where
`indeterminate` is already the
only reachable state.

### Claims above that this round supersedes

- *"Library validation means it can never connect, so the gate now says
that terminally."* — the
gate now lets system-app launches through; the premise is contested (see
above).
- *"The flow launch gate was the one caller that never asked whether the
app could be injected at
all"* and *"`flow-ios-tree` only ever auto-resolves a connected app"* —
both inverted here.
- *"Known and deliberately left"* — the `describe` no-`bundleId` path
was closed, with tests, so
  that paragraph now over-counts by one.
- The suite figures in **Verification** and **Review round 2** are
pre-round-3 counts.


---

## Review round 4 — reviewing round 3

Three lenses over 856efd8e's two parent commits (claims-vs-code,
non-happy
paths + reachability, absence + mutation). Round 3 did not survive
intact, and
what follows is what was wrong with it.

**Three statements were false in prose an agent acts on.** `connecting`
requires
a process inside the 3 s connect grace, while this gate takes its
verdict 9.5 s
after launching the app — so its remedy claimed the step had launched
the very
process it was measuring, next to a base message reading "it launched
moments
ago". The figure that arm and `not_running` quoted was the connect wait
alone,
omitting the 1.5 s post-launch settle that precedes it
(`LAUNCH_TO_VERDICT_MS`
now names the whole spend). And `stale_process` blamed the launchd
environment,
which is false for the producer reached only when
`processCarriesInjection` is
**true** — that process carries this endpoint and is merely older than
the
listener. The measured text it wraps names both producers, so the two
contradicted each other in one message.

**A `hidden` wait still released in 1 ms.** Round 3 carved out a failed
native-devtools resolution as "no service on this device at all". The
blueprint
is registered unconditionally, so on iOS that rejection only ever means
the
service failed to come up — a socket bind losing to a concurrent
same-udid
server, which this repo documents as the dominant crash source in 0.16.0
telemetry. Reproduced: `describe` → no hint, then `await-ui-element
hidden` →
`{"success":true,"elapsed":1}`. It is now as unexplained as its
siblings.

**The failure-code discrimination was defeatable.** The flow tree source
classified auto-targeting's throw by reading a code off a module-local
`Symbol`,
so a duplicate `@argent/registry` instance would read it as absent and
fall back
to the exact message the change replaces. It tests the empty connected
list
directly now — the same condition, structurally, with no symbol
involved. The
`connected` arm added in round 3 was unreachable for the same reason the
branch
it replaced was, and is gone.

**Coverage the mutation lens found missing.** Three of the four sites
threading
the launched bundle id were unpinned — including the `settleTree` read
every
`tap`/`type`/`scroll-to` uses, so an action step could have silently
lost the
terminal reason with the suite green. Also unpinned: `unexplainedHint`
dropping
a pre-existing degraded hint (where its exact twin fifty lines up **is**
pinned), the tree source's "Flows resolve selectors" half, and both of
the
gate's withholdings.

**Test cost.** The four per-state gate tests drove the whole runner in
pumped
fake time, ~9 s each, which is what pushed this file past vitest's
default under
parallel load. The mapping is a pure function and is tested as one; one
end-to-end test still proves the gate reaches it. 46 s → 16 s, with more
tests
than before. Widening the fake-timer step was tried and is worse (52 s)
— the
pumps are what let the run's disk I/O proceed.

**Also corrected:** comments the round-3 commits had falsified in the
very files
they edited; the `ActionEnv` claim that a fragment never carries a
launch step
(a flow is classified by its FIRST non-echo step, so a later launch is
legal and
does thread its id); `isBlindRead`'s enumeration of what sets its flags;
and
change narration that had leaked into permanent prose.
`argent-create-flow` now
states what a `com.apple.*` target costs a flow — its worked example is
selector-driven, and round 3's justification comment claimed the
opposite.

**Verification.** 17 mutations across both rounds, every one killed by a
specifically-named test. Two consecutive full runs of
`packages/tool-server`:
3181 passed, 1 skipped, and 6 failures in `boot-device-hotboot.test.ts`
that are
pre-existing on this host (it asserts `-gpu darwin`/`swiftshader` where
a Linux
box resolves `-gpu host`; the file is untouched by this PR). `tsc
--build`,
`tsc --noEmit -p tsconfig.test.json`, eslint and prettier clean on every
changed
file; `scripts/extract-tools.mjs` emits all 75 tools with zero warnings.


---

## Review round 5 — reviewing round 4

Round 4 left two of the defects it set out to fix and introduced one. It
also
broke CI.

**ESLint failed on the push.** Two `` \` `` escapes inside a
double-quoted string
(`no-useless-escape`). The pre-push check missed it because it was piped
through
`tail`, so the shell read the pipe's exit status rather than eslint's
and
reported success over a failure. Gates are run unpiped now, and
repo-wide —
`eslint .` is what CI runs, and a changed-files invocation covers a
subset of it.

**`unregistered` still quoted the connect wait alone** — the exact
understatement `LAUNCH_TO_VERDICT_MS` was introduced for, applied in
round 4 to
`not_running` only. And the end-to-end test asserted `8000 ms` while its
sibling
fifty lines down asserted the whole spend, so the file enforced two
contradictory rules and the *correct* figure would have failed CI. The
post-launch settle counts: the connection is read off the live map and
the poll
checks it once before its first sleep, so a dial landing inside those
1.5 s is
caught. All three arms that quote a figure now quote the same one,
pinned
together.

**`stale_process` named the launchd environment as fact** four lines
below a new
comment saying that is false for one of the state's two producers. It is
in fact
sound — but only where the message already scopes it, to a *second*
landing,
whose relaunch makes the process younger than any established listener
and so
rules that producer out (it needs `processAge + grace >= listenerAge`).
The
comment now says which producer and why. The test named
"does not blame the launchd environment" asserted only the phrase it
kept, so it
was green with the blame present; it now pins the ordering it is named
for.

**The `connected` arm is reachable — round 4's "unreachable" claim was
wrong.**
`appConnectionState` re-reads the live connections map *after* its env
re-apply
and process probe (`native-devtools.ts`), several simctl round-trips
past the
empty list that routed the call there, precisely so a dial landing in
that window
is not reported as unregistered. Folding the arm away left that case
throwing a
bare "Flows resolve selectors…" with no app named and no remedy. It is a
branch
again, and says the connection arrived mid-read.

**`unexplainedHint` dropped `screenshot` along with the `bundleId`
suggestion**,
so a caller that passed a bundle id received a hint with no next action
at all —
while `argent-device-interact` tells the agent to follow that hint.
`screenshot`
is unconditional now. Its JSDoc also claimed two of three call sites are
reached
only after an explicit id resolved and measured connected; all three are
reachable with an explicit id, as the file's own comment twenty lines
above says.

**Also:** `argent-create-flow` recommended `tool: gesture-tap` where the
same
file documents the first-class, equally tree-free `tap: { x, y }`; and
change
narration removed from four more comments.

### Standing caveats

- **The `com.apple.*` premise is still unsettled and is not settled by
this PR.**
  #453 recorded `connected: false` on iOS 26.5; the review recorded
`connected: true` with both dylibs mapped on 18.5. Every change here is
correct
under either reading, but `isInjectableBundleId` remains a blanket
prefix
  predicate and may be wrong on some runtimes. Worth its own issue.
- `boot-electron-spawn-error.test.ts` and `boot-device-hotboot.test.ts`
fail on
the Linux host used for this work; both were confirmed failing at the PR
base
  `b2e5c54d` and neither file is touched here.


---

## Review round 6 — reviewing round 5

Round 5's own headline fix was both broken and unpinned.

**The hint stopped parsing.** Moving `screenshot` out of the conditional
clause
without rejoining the sentence produced, on the default no-`bundleId`
form:

> … not evidence that nothing is on screen. Pass `bundleId` to have the
connection state measured, or Take a `screenshot` to see what is there.

That is the exact string `argent-device-interact` instructs an agent to
act on.
Both forms are one sentence again, and both are now pinned by a test
anchored to
the end of the string rather than matching a substring.

**And deleting the action entirely left the suite green.** The defect
round 5
describes — "a caller that passed a bundle id got a hint with no next
action at
all" — could be reintroduced with 635 tests passing. Nothing asserted
the hint
named a next step. It does now.

**Two claims round 5 corrected in the source survived verbatim in
tests.** The
"two of the three call sites are reached only after an explicit id
resolved and
measured connected" partition (all three are reachable with an explicit
id), and
"waited only NATIVE_READY_TIMEOUT_MS" — sitting ten lines above the
assertion
that now forbids that figure.

**A guard test did not test its own name.** `does not blame the launchd
environment for a stale process` pinned only *where* the blame sits.
Restating
it as fact on a first landing — `It lands here twice, so the …` —
passed. It
asserts the conditional now, which is the whole distinction: only a
repeat rules
out the producer that carries this endpoint.

**The `connected` branch still had no coverage** — which is how round 4
came to
delete it as dead in the first place. It is exercised now, including
that its
message still names the app and an action.

**Also:** the terminal system-app reason recommended `tool: gesture-tap`
where
the same round-5 fix in `argent-create-flow` had moved to the
first-class,
equally tree-free `tap: { x, y }`. The twin was missed.

**Verification.** Four mutations against these fixes, all killed:
dropping the
`screenshot` action; reintroducing the broken join; stating the launchd
blame as
fact; folding the `connected` branch away. Gates run unpiped and read by
exit
code — `eslint .` 0, `tsc --build` 0, `tsc --noEmit -p
tsconfig.test.json` 0,
`prettier --check .` 0.


---

## Review round 7 — reviewing round 6

**The gate asserted the contested fact as settled.** Round 3's
justification for
keeping the 8 s wait read "a system app's process inherits the injection
tokens
**and does connect on some runtimes**". Eight other places in this repo
say the
opposite — `blueprints/native-devtools.ts` ("our dylib can never load",
and the
agent-facing precheck throw), `native-devtools-status`, `describe`'s iOS
hint,
and `test/describe-tool.test.ts` ("a com.apple.* app never connects"), a
file
these commits edit. `flow-ios-tree.ts` contradicted *itself*, its
docblock
saying "can never load" above a message saying "cannot be relied on to
inject".

The fact is not settleable here, so nothing asserts it now. The comment
cites
both observations — #453's `connected: false` on iOS 26.5, the review's
`connected: true` with both dylibs mapped on 18.5 — and justifies the
wait
without picking: polling costs this step its timeout under the
pessimistic
reading and is the only thing that makes selectors work under the
optimistic
one, so waiting spends time on an open question where skipping would
bake an
answer into behaviour. The tree source's docblock is hedged to match the
message
it already had.

**This PR does not resolve that inconsistency**, and
`isInjectableBundleId`
remains a blanket `com.apple.` prefix predicate. It deserves its own
issue.

**A guard let the thing it guards be deleted.** Round 6 made the
repeat-landing
assertion conditional (`if (blame !== -1)`), so removing the whole
clause —
leaving a reader who lands there twice with "re-run the flow" and no
escape —
passed 36/36. Unconditional again, wording pinned.

**A rationale that was false in both directions.** "Keeps the
`not.toContain`
guards non-degenerate: they would pass vacuously if the constants were
made
equal" — equal constants make them *fail*, and 500 vs 1500 makes them
fail for
the wrong reason (`"1500 ms".includes("500 ms")`). It pins the substring
property those guards actually need.

**Two more presence-only guards.** "keeps the ax-degraded boot guidance
**ahead
of** the unreadable-hierarchy note" asserted only that both strings
appear, and
passed with them swapped. And round 6's own `tap: { x, y }` twin fix had
no test
at all — the same absence that got the `connected` branch deleted as
dead two
rounds earlier. Both pinned, and the `tap: { x, y }` claim was verified
end-to-end: that step form parses, resolves through `resolveTargetPoint`
with no
tree read, and passes in a flow whose selector step fails with this very
message.

**Two test names promised more than they checked** — a remedy "that is
not the
action the step just took", asserted for states whose remedy is another
relaunch; and a crash-on-launch guard forbidding only the literal `Call
launch-app` while the arm legitimately suggests launch-app for watching
the exit.

**Verification.** Three mutations, all killed: deleting the
repeat-landing
clause, reverting the coordinate form to `tool: gesture-tap`, and
putting the
read-note ahead of the boot guidance. Gates read by exit code, unpiped:
`eslint .` 0, `tsc --build` 0, `tsc --noEmit -p tsconfig.test.json` 0,
`prettier --check .` 0. Suite: 3187 passed, 1 skipped, and the 6
pre-existing
`boot-device-hotboot` failures on this Linux host.


---

## Review round 8 — reviewing round 7

**The gate's own justification was false, and the disproof was already
in this
PR's review.** Round 7's comment claimed polling "is the only thing that
makes
selectors work at all" under the optimistic reading. It is not:
`settleTree`
retries a failed tree read for `SETTLE_TIMEOUT_MS` on top of
`POST_LAUNCH_SETTLE_MS`, so roughly the first 4.5 s of connect latency
is
absorbed with no gate at all — which is exactly why the review's
fragment (no
`launch` step, therefore no gate) resolved selectors 4/4 against
`com.apple.Preferences` on 18.5. Confirmed with an in-repo
counterfactual:
at 2000 ms connect latency the flow passes with or without the gate; at
6000 ms
only with it. What the poll adds is the band from ~4.5 s out to
`LAUNCH_TO_VERDICT_MS`, and the comment now says that.

**Hedging one surface left its twins absolute — the same failure mode, a
fourth
time.** The precheck throw, 230 lines from the message this PR hedged
and in the
same file, still told agents Argent "can never be injected into it"; so
did the
predicate's docblock, `native-devtools-status`'s description ("the dylib
can
never load into them", "TERMINAL state — the app can never be
injected"), and
`describe`'s terminal hint. An agent reading `argent-create-flow` was
told
"depends on the simulator runtime" and the status tool told it "can
never".

Every agent-facing surface now says injection cannot be RELIED on, and a
test
pins that none of them claims impossibility — the six native-*
descriptions, the
status description, the shared recovery text, the device-interact skill,
and
(separately, since it is module-private and reachable only through the
tool's
output) `describe`'s terminal hint. Each surface is asserted non-empty
first, so
a missing one cannot satisfy the negative match vacuously.

**The decision is untouched.** `isInjectableBundleId` still answers "not
injectable", the precheck still throws, and the description still says
`Do NOT restart/retry` — pinned by the same test, because softening the
claim
must not soften the behaviour or #453's restart loop returns. An app
that MIGHT
connect cannot be the basis of a retry loop either, which is now the
predicate's
recorded reason.

**Also:** the round-7 reword left "The flow / flow path" duplicated;
withholding
the verdict before the measurement (promoted to its own paragraph last
round)
was asserted nowhere, so moving it after `appConnectionState` left the
suite
green — a test now proves the measurement is not taken for an app whose
verdict
is withheld either way; and two more test names were narrowed to what
they
actually check.

**Verification.** Mutations run against every decision these commits
added.
Killed: re-asserting "can never load" in the status description;
dropping its
terminal instruction; re-asserting impossibility in `describe`'s hint;
moving
the withholding after the measurement; and the earlier rounds' decisions
re-checked and still pinned. One survivor is unpinnable by construction
— a
claim inside a code comment, which no test can assert. Gates read by
exit code,
unpiped: `eslint .` 0, `tsc --build` 0, `tsc --noEmit -p
tsconfig.test.json` 0
(this one rejected a tuple vitest had accepted), `prettier --check .` 0.



---

## Review round 9 — scope: the skill docs

This is a state-derivation fix. It should not be rewriting skill docs,
and the
earlier rounds had it doing exactly that.

**`argent-create-flow` is back to its base.** Nothing in it was
falsified by this
change. The `com.apple.*` caveat rounds 4-6 grew there documents the
question
this PR explicitly declines to settle — whether `isInjectableBundleId`'s
blanket
prefix rule is right at all — and that belongs to its own issue, not to
a
paragraph in a skill an agent reads before recording a flow.

**`argent-device-interact` only got smaller.** It said *"call
`restart-app` first
if needed"* — the unconditional-restart advice the measured states exist
to
replace — and that clause is deleted. The four-status routing table the
first
round put in its place is gone with it: the remedy already travels on
the tool's
own output and on the six `native-*` descriptions, both pinned by tests,
and a
second copy in a doc no code reads is one more surface to drift.

Tests follow the prose. The two `SKILL.md` reads in
`native-devtools-status.test.ts`
go with the text they pinned, and two comments that cited the reverted
skill text
are corrected.

### Claims above that this round supersedes

- The first round's *"the `argent-device-interact` skill offered
`restart-app` as
the only remedy for a missing injection"* — the clause is deleted, not
replaced.
- Round 4's *"`argent-create-flow` now states what a `com.apple.*`
target costs a
flow"*, round 5's *"`argent-create-flow` recommended `tool:
gesture-tap`"*, and
round 6's twin fix in the skill — all in the reverted paragraph. The
matching
source-side fix in `flow-ios-tree.ts` stands; only its stated twin is
gone.
- Round 8's cross-surface certainty test no longer reads the skill. It
covers the
status description, the six `native-*` descriptions and the shared
recovery text
  — the surfaces that make the claim.

**`origin/main` merged in, and #519 needed one test to follow.** It
landed after
this branch last merged main and makes a `getFullHierarchy` reply with
no windows
an error — an untrustworthy read, not a blank screen. `does not measure
when
auto-targeting resolved an app` had been using an empty `windows` as a
don't-care
return, so the call now ended before it could observe anything. The stub
serves a
real window, which is the state the test names. Mutation-checked:
dropping the
empty-connected-list term from the guard makes the measurement fire and
fails
that test alone.

**Verification.** `packages/tool-server`: 304 files, 3242 passed, 1
skipped, on a
macOS host (the `boot-device-hotboot` failures noted in the standing
caveats are
Linux-only and do not reproduce here). Gates read by exit code, unpiped:
`eslint .` 0, `tsc --build` 0, `tsc --noEmit -p tsconfig.test.json` 0,
`prettier --check .` 0.
2026-08-17 21:06:26 +02:00
Hubert Gancarczyk 20b5e8b4f0 fix(tool-server): reject a keyboard call carrying both text and key (#579)
## Problem

`keyboard` accepted `text` and `key` in one call, and that shape has no
meaning a caller can rely on. The same request reads two ways:

- `{ text: "hello", key: "enter" }` — "type, then submit"
- `{ text: "abc", key: "backspace" }` — "delete, then type" reads just
as naturally

Whichever order a backend picks, the other reading silently corrupts the
field. The backends did not even agree: `typeAndroidPhone` pressed
`params.key` **before** `params.text`, while simulator-server, chromium
and vega all did the reverse and the tool's own description promised
"the text is typed first and the key is pressed after it".

## Change

**`keyboard` takes `text` or `key`, never both.** A combined call is
rejected in `execute`, above the platform dispatch, so it resolves no
`ARGENT_SECRET_*` value and reaches no device — no backend has to defend
against the shape.

**To mix text and a key, use `run-sequence`.** That is what the error
message, both parameter descriptions, the tool description and the skill
docs now say:

```json
{ "udid": "<UDID>", "steps": [
  { "tool": "keyboard", "args": { "text": "hello world" } },
  { "tool": "keyboard", "args": { "key": "enter" } }
]}
```

Two separate calls work too, but cost an extra round-trip — and are
**not** equivalent when the text carries a secret (see below).

### Why the rule is prose, not JSON Schema

An earlier revision hand-wrote `inputSchema` with `not: { required:
["text", "key"] }`. That is no longer legal: #782 banned every top-level
combinator repo-wide, `not` included, because the Messages API rejects a
request whose tool schemas declare one — and that 400 fails **every tool
in the request**, not just this one.
`tool-input-schema-contract.test.ts` enforces it across the catalog.

So the constraint is restated in **both** fields' `.describe()` (the
`boot-device` house rule) and in the tool description.
`keyboard-text-key-exclusive.test.ts` pins both texts, and pins that the
advertised schema stays combinator-free.

### What the error message carries

Three things a caller needs and cannot get elsewhere:

1. **The literal retry**, not an ellipsis the Android backend cannot
type.
2. **The TV caveat.** A TV target rejects `key` outright
(`platforms/tv.ts`), so the prescribed `{ key: "enter" }` step cannot
succeed there. The guard runs above the dispatch on purpose, so it would
otherwise swallow that diagnosis; the caveat is carried statically
rather than by an async TV probe.
3. **The secret steer**, only when `text` holds a `{{secret:…}}`
placeholder. The auto-screenshot skip is decided per tool call from the
whole request, so:

   | call shape | auto-screenshot |
   | --- | --- |
   | one `run-sequence` carrying both steps | skipped |
   | `{ text: "{{secret:…}}" }` alone | skipped |
| a separate `{ key: "enter" }` | **not** skipped — captured after the
key lands, and can hand the still-visible secret back as pixels |

The check is the syntactic `.includes` `flow-utils.ts` already uses, so
the guard still resolves nothing. A plain combined call does not carry
the sentence, with a positive control pinning that.

### Simplifications the rule makes possible

With at most one of the two ever reaching a backend, the
resolve-then-inject splits that existed only to order a combined call
fold back into their injectors (`android-input.ts`, `vega-input.ts`),
and each backend collapses to two exclusive branches.

### Fixed alongside

`run-sequence`'s description claimed "No screenshot is captured
automatically". `shouldAutoScreenshot("run-sequence")` is `true` with a
15000 ms settle cap, so that line was already false — and this change
makes it load-bearing, since the secret guidance rests on it.

## Compatibility

A stored flow or scripted `run-sequence` whose step is `{ tool:
keyboard, args: { text: "…", key: "enter" } }` now errors on replay, and
the steps after it are skipped. The recorder never emitted the combined
shape (`flow-add-step` executes the call before it appends the step), so
the blast radius is hand-authored flows and scripted `run-sequence`
calls — which is exactly what the old worked example taught. The fix is
to split the step in two.

## Test coverage

- **`keyboard-text-key-exclusive.test.ts`** (new, replaces
`keyboard-key-order.test.ts`) — the rejection on all four backends with
nothing injected and no service resolved, plus a positive control per
backend that text alone and a key alone still work; shape-not-truthiness
(`{ text: "", key: "enter" }` rejects); at-most-one rather than
exactly-one (`{}` stays a no-op); rejection before secret resolution;
the run-sequence retry, TV and secret sentences, with a positive control
against an unconditional secret steer; and the two channels the
constraint actually reaches a client through.
- **`keyboard-backend-fidelity.test.ts`** — absorbs what key-order
uniquely pinned. The unknown-key 400 now names the offending key on iOS
and chromium here, on android in `keyboard-android.test.ts`, and on vega
in `vega-injection.test.ts` (tightened from the bare `/Unknown Vega
key/` prefix), so stripping the name is red on every backend.
- **`keyboard-android.test.ts`** — the combined-call tests are gone.
What they uniquely covered survives as single-parameter tests: the
multi-segment `%` codepoint count, the pressed key's identity against a
second literal keycode, and un-typeable text rejected with no on-device
side effect. The exhaustive 24-keycode loop now runs through
`injectAndroidNamedKey` and asserts the emitted `input keyevent <code>`,
so it pins the wire and not just the lookup.
- **`interaction-messages.test.ts`** — the two formatters see different
shapes now. `startedMsg` renders before the rejection, so it still words
four (including the combined one); `completedMsg` runs only after
success, so it words three. Both empty-request branches stay pinned.
- **`keyboard-tv.test.ts`** — the TV `key` rejection is independent of
this guard, which only ever sees requests carrying both. Its header says
so, since the exclusivity message's TV clause depends on it.
- **`scripts/e2e-full/phases/30-android.sh`** — the combined call
becomes an `assert_reject`, and the prescribed `run-sequence` remedy is
driven end to end.

## Live verification

Isolated tool-server built from this branch (own port, shared MCP server
untouched), against the Android settings search field on an API 36 AVD,
read back out of the view hierarchy rather than off a screenshot:

| Call | HTTP | Field / result |
| --- | --- | --- |
| seed `{ text: "XYZ" }` | 200 | `XYZ` |
| `{ text: "abc", key: "backspace" }` | **400** | field still `XYZ` —
byte-identical, nothing typed |
| `run-sequence` [`{text:"abc"}`, `{key:"backspace"}`] | 200,
`completed: 2` | `XYZab` — exactly what the combined call used to
produce |
| `{ text: "hello" }` | 200 | `{ typed: "hello", keys: 5 }` |
| `{ key: "enter" }` | 200 | `{ typed: "enter", keys: 1 }` |
| `{}` | 200 | `{ typed: "", keys: 0 }` — still a no-op |
| `{ text: "", key: "enter" }` | **400** | shape, not truthiness |
| `{ text: "{{secret:APP_PASSWORD}}", key: "enter" }` | **400** |
message carries the ONE-`run-sequence` steer and the
still-visible-secret reason |
| `{ text: "hi", key: "enter" }` | **400** | message does **not** carry
the secret sentence |

`GET /tools` reports `["properties", "required", "type"]` for `keyboard`
— no top-level combinator.

Suite: `packages/tool-server` 336 files / 4041 passed,
`packages/registry` 90 passed, `packages/argent-mcp` 79 passed. `tsc
--build`, `tsc --noEmit -p tsconfig.test.json`, eslint (0 warnings),
prettier and knip all clean.

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
2026-08-17 15:01:23 +02:00
Ignacy Łątka c821a463b0 fix(e2e): pin the absence of an npm postinstall hook (#556)
Found while running the full E2E harness against the packed 0.17.0
bundle.

## Problem

The `install` phase asserted that `scripts/postinstall.cjs` ships in the
package:

```bash
if [ -f "$pkg/scripts/postinstall.cjs" ]; then
  ...
else
  fail "$P" postinstall init-hint "postinstall.cjs not shipped"
fi
```

But the postinstall was deliberately dropped in #510 ("drop the npm
postinstall - its jobs move to runtime"), and neither the file nor a
`postinstall` entry in `package.json` exists any more. So this check
failed on every run, and since the harness exits non-zero on any
failure, the release gate was red for a non-issue - which also buries
any real failure in the noise.

## Change

Pin the intended state instead of the old one: assert the package ships
neither `scripts/postinstall.cjs` nor a `postinstall` entry in the
shipped `package.json`, so the hook can't creep back in. Installs that
pass `--ignore-scripts` (pnpm, hardened CI) skip postinstall silently,
which is why #510 removed it in the first place.

## Verification

- The assertion discriminates across every input it has to separate: no
`postinstall` key and no `scripts` key at all pass; an injected
`.scripts.postinstall`, a present `scripts/postinstall.cjs`, a missing
`package.json` and a malformed one each fail, with the manifest cases
reporting that rather than "no hook". `jq -e` exits non-zero for an
unreadable file just as it does for an absent key, so the manifest is
parsed first — otherwise this absence check would pass whenever it could
not look.
- Full `install` phase re-run against `swmansion-argent-0.17.0.tgz`: no
failures, harness exit code 0 (it was one failing check before).


## Not covered here

The deleted check was the only assertion touching #510's postinstall.
Its init-hint banner went away with the hook, so nothing is left
uncovered there; its other job — killing a stale tool-server so a
freshly installed binary takes effect — moved to runtime in #510 and
this phase does not exercise that. Worth its own check, but it is a new
test rather than a repair of this one.
2026-08-17 13:23:38 +02:00
Ignacy Łątka c876fe8dbc ci: add knip anti-dead-code gate (#572)
## What

Adds a CI gate for dead code, using [Knip](https://knip.dev).

We didn't have one. ESLint's `no-unused-vars` only sees unused locals
inside a module, so whole files nobody imports, exports nobody uses, and
dependencies declared but never required all slipped through. Knip
covers that gap in one tool (roughly `ts-prune` + `depcheck` +
`unimported` combined).

The new `.github/workflows/knip.yml` workflow ("Dead Code", job `Knip`)
runs `npm run knip` on push and PR to `main`. `CONTRIBUTING.md`'s
pull-request checklist gains it as a step, with the reason it has to run
against a tree with no build output. Knip reads source directly, mapping
each package's `dist` entry back to `src`, so the job needs no workspace
build. It installs with `--ignore-scripts`: an analysis that only reads
`package.json` and source gains nothing from the seven packages here
that carry a native install script, six of which reach this runner
(`fsevents` is darwin-only). Five try a prebuilt binary before
compiling, and `dtrace-provider` always runs node-gyp but ends in `||
node suppress-error.js` — and off mac/solaris/freebsd its `binding.gyp`
picks a stub target with no sources, so on ubuntu-latest node-gyp builds
nothing for it and `node-pty` is the only package that reaches a
compiler at all — an `optionalDependency`, whose failure npm ignores.
The exposure is a bump that leaves a package with no prebuild for the
runner, which is exactly what `dependabot.yml` pins `tree-sitter`
against (0.25.0 ships no Node 24 prebuild and its compile breaks `npm
ci`).

While adding it I checked whether any other installed checker was
missing a gate. None were: ESLint (`lint.yml`), Prettier (`format.yml`),
`tsc` build + `typecheck:tests` + `typecheck:scripts` + vitest
(`unit-tests.yml`), the workspace-version / focused-test / merge-marker
checks (`repo-hygiene.yml`) and lockfile sync (`lockfile.yml`) are all
already enforced. Dead code was the only gap.

## How it's enforced

`npm run knip` is two invocations:

```
knip --exclude exports,types && knip --include exports,types,classMembers,nsExports,nsTypes --max-issues 206
```

Every issue type knip reports is an error — that is its default
severity, so `knip.jsonc` sets no `rules` block at all. The pre-existing
backlog is bounded by the `--max-issues` ceiling on the second pass
instead of being hidden behind a non-failing severity: it cannot grow
unnoticed, and the number comes down as follow-up passes pay it off.
When a change legitimately adds an export nothing imports yet, the
number moves up in the same commit — that bump is what review should
see, rather than the contributor deleting someone else's backlog to make
room. Two PRs making that identical one-line edit merge without
conflicting, so `main` lands one short of the truth and its
push-triggered run is what catches it. Splitting the passes also keeps a
real failure out from under the backlog — the enforced classes report on
their own, and the second pass only runs once the first is clean.

The second pass names `classMembers`, `nsExports` and `nsTypes`
explicitly because they sit in knip's own `defaultExcludedIssueTypes`:
`--exclude exports,types` never covered them and `--include
exports,types` did not ask for them, so a new unused class member cost
nothing and moving an export behind opaque namespace use took it out of
the ceiling's scope entirely. `classMembers` has 12 findings on the
unbuilt tree the ceiling counts (9 on a built one); the other two have
none today. None of the three is gated at zero — all five parked classes
share the one ceiling, so a new `nsExports` finding passes if something
else in the backlog is retired in the same PR. Naming `classMembers`
also turns knip's `isSkipLibs` off (`create-options.js:122`), which
makes it search declaration files for external references and
**suppresses** 44 export/type findings the same run reports without it —
57/181 becomes 44/150. That, not the 12 findings it adds, is why the
ceiling is lower than the two counts alone would suggest;
`--include-libs` with no `classMembers` reproduces 44/150 exactly.

The ceiling counts what CI counts, against a tree with no build output.
Whether a workspace's `main`/`types` path resolves to an existing file
changes the result, and not by way of anything in that file — a
zero-byte `packages/registry/dist/index.js` moves the count just as a
real build does, while a zero-byte `dist/zzz.js` does nothing. A built
tree reports fewer issues — 43/147/9 = 199 built, against 44/150/12 =
206 unbuilt — so `npm run knip` passing after a local `tsc --build` is
not evidence the job will pass, which is how the Knip job first went red
on 199. The ceiling is 206 and it is exact: 206 passes, 205 fails.
Remove every `dist/` before trusting a number from it.

The ceiling is one total across those classes, so it bounds the backlog
rather than each class: paying off one parked export licences one new
one elsewhere in the same PR. Knip has no per-class ceiling.

`treatConfigHintsAsErrors` is on, so an exemption that stops suppressing
anything fails the job rather than printing a hint below the backlog.

## Config

`knip.jsonc` documents the false positives so the gate is both green and
meaningful.

- **`@swmansion/argent`** is a thin dispatcher whose real payload is an
esbuild bundle of the other workspaces
(`packages/argent/scripts/bundle-tools.cjs`). Knip can't follow imports
across that boundary, so packages consumed only by the bundled-in code
read as unused; 11 are exempted by name here, though not all for that
reason — the http3-quiche transport is pulled in by
`@fails-components/webtransport` itself at runtime, and no first-party
file imports it. They're exempted **by name**, not behind a catch-all:
`ignoreDependencies` also suppresses `unlisted`, and a pattern that
matches everything would take this workspace's own `src/` and `scripts/`
out of that check — where an import with no matching declaration is a
genuine bug rather than a bundler artifact. (It reaches `unresolved`
only nominally: knip matches the list against a package name parsed out
of the specifier, and an unresolvable relative path yields none, so even
a catch-all leaves those reported. `ignoreUnresolved` is the option that
does silence them.)
- **`tool-server`** reaches a few packages in ways static analysis can't
see. The exemptions cover two distinct classes, kept apart in the config
so each stated reason matches the finding it silences:
- *declared but loaded too dynamically to detect* (would be reported as
**unused dependencies**): `tree-sitter` / `tree-sitter-typescript` via
dynamic `_require`, and the http3-quiche transport as a runtime
companion of `@fails-components/webtransport`;
- *imported but deliberately absent from that `package.json`* (would be
reported as **unlisted dependencies**): `sharp` (kept out so the ~30 MB
native binary isn't a cost every consumer pays) and `electron` (an
`optionalDependency` of the published package at one call site, the
Electron main-process API at the other).
- Root `scripts/`: knip resolves those from `package.json` scripts
**and** from `run:` steps in `.github/workflows` via its github-actions
plugin — multi-line `run: |` blocks included. What the plugin doesn't
parse is a PowerShell body, so `scripts/ci/windows-chromium-e2e.mjs` —
invoked at `windows-e2e.yml:176`, in the `windows-latest` job where
PowerShell is the default shell — is the one entry declared by hand.

### Known limits

- `ignoreDependencies` exempts a package **name across its whole
workspace**, in both directions; knip has nothing narrower. While
`sharp` is listed for `tool-server`, an undeclared `import "sharp"`
anywhere in that workspace passes too. The list is held to packages that
genuinely cannot be declared, for exactly that reason.
- Knip analyzes JavaScript and TypeScript only (`DEFAULT_EXTENSIONS`),
so **only 11 of the 30 files under `scripts/` are in the gate at all**.
The other 19 — 15 `.sh` files, `e2e-full/lib/report.py`,
`ci/e2e-chromium-page.html`, and e2e-full's `README.md` and
`results/.gitignore` — are invisible to it however they are reached —
including the whole `scripts/e2e-full/` harness (13 files), invoked from
nothing outside its own directory, its only mention elsewhere being a
`.prettierignore` entry.
- Reachability's widest hole is the workspace entry. Whatever
`src/index.ts` re-exports is reached, so a single `export { x } from
"./dead-module";` takes that module out of `files` altogether — verified
against a module reported as an unused file one run earlier. The same
line applied to parked exports retires them from the count without
deleting anything: re-exporting `equalsCI`, `includesCI` and `Settled`
from `tool-server`'s entry takes 206 to 203 and frees three ceiling
slots. The ceiling bounds what knip can see, not what is dead.
- Test files are entry points, so a production module kept alive only by
its own test still passes `files`. Being reached by a test says nothing
about being reachable in production, which is what the check
approximates, and two modules sit in exactly that gap: `tools/paste` is
imported by two test files and registered by nothing, so it is
unreachable over MCP and HTTP, and `utils/ios-profiler/lifecycle.ts` is
a re-export barrel whose header keeps it "for source compatibility with
existing imports", of which its own test is now the only one — its four
production consumers all import `profiler-shared/lifecycle` directly.
That single test import is exactly what keeps `files` from reporting it:
repoint it and knip returns `Unused files (1)`. `knip --production` is
the lens that reports the class; the repo isn't clean under it — 13
unused files (those two, seven root scripts,
`packages/argent/scripts/{bundle-tools,observe,sync-readme}.cjs` and
`packages/registry/tests/helpers.ts`) with the export/type backlog at
227/206 — measured with `classMembers` off, so comparable to the 57/181
above rather than to the ceiling's 44/150 — so a production-mode pass is
a follow-up of its own.
- The same entry treatment hides the test trees themselves. Root
`package.json`'s `test:scripts` runs `node --test`, which is what makes
knip's node plugin contribute its test entries, and that plugin
registers four entry patterns in **every** workspace: `**/test/**`, plus
`*{.,-,_}test.<ext>`, `test-*.<ext>` and `test.<ext>` matched anywhere.
So none of the **380** JS/TS files under `packages/*/test/` can be
reported unused, and under `packages/*/tests/` only a name none of those
three match can — of today's 15 that is `registry/tests/helpers.ts`
alone, which is why it is the lone `tests/` entry in the `--production`
list above. A `tests/test-utils.ts` or `tests/test.ts` would be just as
invisible. A dead `zz.test.ts` planted in `packages/registry/tests/`
passes; a dead `zz-helper.ts` in the same directory fails.
- `knip@5.88.1` is the last 5.x release ever published — 6.0.0 landed
the next day and `latest` is now 6.x. The pin is deliberate: knip 6
removed the `classMembers` issue type outright, and the shipped script
hard-errors on it (`ERROR: Invalid issue type: classMembers`, exit 2).
Upgrading is therefore not a version bump — it means re-deriving the
ceiling, rewriting the paragraph that explains it, and resolving the
pass-1 findings knip 6 adds (six unlisted binaries: `tail`, `plutil`,
`xctrace`, `netstat`, `pkill`, `pgrep`; two unresolved imports:
`/usr/bin/osascript`, `/usr/libexec/PlistBuddy`). Dependabot's
`npm-minor-patch` group cannot move it, since no 5.x exists above
5.88.1; the `npm-major` group will keep offering the 6.x jump.
- Knip folds `optionalDependencies` into the declared set, so importing
`electron` or `node-pty` is not `unlisted` — they pass because it read
them. What it never reports is an optionalDependency nothing uses, so
only that direction is outside the gate, along with `argent-cli`'s
runtime `req("node-pty")`, resolved through a
`createRequire(import.meta.url)` in `lens-pty.ts`, which knip can't see
either.

## Dead code this found

- `@types/semver` and `@argent/telemetry` from `packages/argent` — both
were being suppressed by exemptions whose stated bundler-boundary reason
could not apply. esbuild strips types and never resolves `@types/*`, and
`packages/argent` names `semver` nowhere; `@argent/telemetry` is aliased
to an absolute source path by `bundle-tools.cjs`, so the declaration
bought nothing. Removing them is proved safe by building: four of the
five bundles still emit and telemetry is inlined in each with no runtime
`require` of it. The fifth, preview-window's main, sits behind a
downloaded native asset absent from a script-free install, so it is
covered by mechanism rather than observation — `bundle-tools.cjs` passes
the same `alias` map to every build, and esbuild's `alias` bypasses node
resolution entirely: deleting `node_modules/@argent/telemetry` outright
leaves all four emitted bundles byte-identical.
Removed alongside, but **not** gate-reported: the `benchmark` npm script
in `packages/argent`, whose `scripts/benchmark.cjs` does not exist. A
dangling script target is a blind spot of this gate — restoring the
script leaves knip at exit 0.

- Four files with no importers: the `render-hook` injected script,
react-profiler's `source-map` + `snippet-extractor`, and
`scripts/setup-in-workspace.cjs` (referenced nowhere, and its own usage
string names a `scripts/setup-project.cjs` that doesn't exist).
- `superagent` in `tool-server`'s `test/http-streaming.test.ts` —
imported for a type but declared in no manifest in the repo, resolving
only through npm hoisting of supertest's own dependency. Reverting that
import to `superagent` reports `Unlisted dependencies (1) … :3:32` and
exits 1; `@types/supertest/index.d.ts:10` re-exports the identical type
(`type Response = superagent.Response`).
- `@clack/prompts` and `@moq/lite` from `tool-server`'s `package.json` —
declared, imported nowhere. (`@moq/net` is the one actually used;
`@moq/lite` isn't a peer of it.)

Also deleted here, but **not** something this gate reported:
`src/tools/simulator/simulator-server.ts`. It sat in the `files` blind
spot above — `interaction-messages.test.ts` imported it, which is enough
to count as reached. `setup-registry.ts` never registers it, so it was
unreachable over MCP and HTTP, and that catalog roll-call was its only
reference; the import and the catalog count (77 → 76) went with it.

## Lockfile

Adding knip moves two transitive packages: `@napi-rs/wasm-runtime` 1.1.5
→ 1.1.6 and `@tybys/wasm-util` 0.10.2 → 0.10.3. Knip pulls in
`@oxc-resolver/binding-wasm32-wasi`, which floors
`@napi-rs/wasm-runtime` at `^1.1.6`, lifting the hoisted entry that had
resolved to 1.1.5 for `@rolldown/binding-wasm32-wasi`'s `^1.1.4`, which
then cascades to `@tybys/wasm-util`. No runtime impact — both are
support packages for the wasm32-wasi fallback bindings, which don't get
installed where a native binding exists; `npm ls` reports both
extraneous on darwin-arm64. Those two are the only resolved versions
that change in the diff; the rest is knip's own tree (49 packages added)
and `@moq/lite` removed.

## Verification

- `npm run knip` exits 0.
- **Coverage of the enforced set.** With no `rules` block, knip's
included set is 15 issue types: ten enforced at zero tolerance by the
first pass, plus the five the second pass names under the ceiling.
Eleven mutations cover all ten, each flipping the gate to exit 1 and
back to 0 on revert:

  | class | mutation |
  | --- | --- |
| `files` | a throwaway `.mjs` in `scripts/`; a throwaway `.ts` in
`packages/argent/src` |
| `dependencies` | a bogus declared dep in
`packages/tool-server/package.json` |
  | `devDependencies` | a bogus declared devDep in the same manifest |
| `optionalPeerDependencies` | `bytes` moved to an optional
`peerDependency` there |
  | `unlisted` | an undeclared import in `packages/argent/src/cli.ts` |
  | `unresolved` | an unresolvable relative specifier in the same file |
| `binaries` | a root `package.json` script invoking a nonexistent
binary |
| `duplicates` | `export default sleep` alongside the existing named
export in `utils/timing.ts` |
| `enumMembers` | an unused member on an enum a second module imports
and uses |
| `catalog` | a `catalog` block in the root `package.json` naming a
package nothing uses |

One nuance worth recording: `duplicates` is narrower than it looks. It
reports a symbol exported both by name and as `default`
(`sleep|default`); a second *named* alias of the same symbol is not a
duplicate, and a freshly added symbol nothing references is not reported
at all — so several naive probes read as a miss.

`enumMembers` needed a shape the repo does not currently contain, not an
unreachable one. The only exported enum here, `ServiceState`, is
re-exported through `@argent/registry`'s entry, where knip counts
exports as used, so an unused member added to it is **not** reported —
but an enum in a non-entry module that another module imports and uses
does report one (`Blue ZzColor …/zz-enum.ts:3:3`, exit 1; deleting the
unused member returns exit 0). That is the ninth of the ten covered.

`catalog` is reachable here despite the repo using npm workspaces:
knip's catalog loader falls back to `manifest.catalog` when there is no
pnpm or yarn workspace file, so a `catalog` block in the root
`package.json` reports `Unused catalog entries (1)` and exits 1.
- The two new mechanisms discriminate as well: one extra unused export
takes the backlog to 207 and fails the ceiling, and a stale
`ignoreDependencies` entry produces `Configuration hints (1) … Remove
from ignoreDependencies` and exit 1.
- `npm ci --ignore-scripts` in a clean worktree, followed by `npx knip`,
is green — the job doesn't need any of the postinstalls it now skips.
- `tsc --build`, `typecheck:tests` (all workspaces),
`typecheck:scripts`, `test:scripts`, `eslint . --max-warnings 0`,
`prettier --check .` and the full workspace vitest suite (4711 passed, 3
skipped across 13 workspaces; 3070 of those are `@argent/tool-server`
alone) are green; `package-lock.json` is untouched by the review pass.
2026-08-10 17:09:16 +02:00
Hubert Gancarczyk afd722924f feat(flow): support concurrent flow recordings (#574)
## Problem

Only one flow could be recorded at a time, anywhere on the machine.
Recording state was three module globals in `flow-utils.ts` — harmless
in a per-client process, but the tool-server is a **host-wide singleton
per install bundle**. Every MCP client, every subagent sharing a
parent's connection, every `argent` CLI call and every project on the
machine attaches to the same process, the same `Registry`, and those
same three globals. HTTP requests are served concurrently with no
serialization.

Three failures followed, all reproduced against the pre-change code:

1. **A second recording clobbered the first.** `startRecordingSession`
just overwrote, so the first agent's next `flow-add-step` silently
appended into the *other* agent's flow file — in a different project.
2. **Any agent could finish any agent's recording.**
`flow-finish-recording`'s schema was literally `z.object({})`.
3. **Replay stomped recording.** `resolveFlowFilePath` called
`setActiveProjectRoot`, so a `flow-execute` in project B rebound the
global root mid-recording of project A. Already worked around, locally
only, in `flow-add-step`.

## Approach

Recording state is now a `Map` keyed by the resolved flow file path,
`<project_root>/.argent/flows/<name>.yaml` — the identity of the
artifact being built, so "same key" means "same output file" and two
sessions on one key are a genuine collision rather than an accident of
scoping. `flow-add-step`, `flow-add-echo` and `flow-finish-recording`
each take `name` + `project_root`, so every call is self-contained;
required rather than optional-with-fallback, since a fallback would
succeed while an agent is alone and fail only once a second recording
exists.

`resolveFlowFilePath` is pure, which fixes defect 3 and retires
`FLOW_PROJECT_ROOT_REQUIRED` (the state it guarded can no longer exist).

Rejected alternatives — keying by device udid (flows are deliberately
device-portable, and `flow-add-echo`/`flow-finish-recording` have no
device at all), an opaque `recordingId` (an agent that loses the token
cannot recover), and per-caller identity from the transport
(`ToolContext` carries no caller identity, and subagents share their
parent's MCP connection, so it would fail the exact scenario being
fixed).

### Locking, and why readers needed more than a lock

Every mutation of one flow file runs under a lock keyed by that path —
appends, `flow-start-recording`'s truncate-and-register, and
`flow-finish-recording`'s read-summarize-clear. Keyed by file rather
than owned by the session, because a restart *replaces* the session: a
session-owned lock cannot exclude the operation that supersedes it. Per
file, not global, so two recordings never queue behind each other. An
append whose session was finished, restarted or evicted meanwhile fails
with `FLOW_NO_ACTIVE_RECORDING` rather than writing into a different
take.

The lock serializes writers only, and no reader of a flow YAML can join
it — `flow-execute`'s own load, its `run:` fragment load,
`flow-read-prerequisite`, `flow-add-step`'s sibling-fragment check, and
the `argent` CLI reading from another process entirely, where an
in-process lock cannot reach. Both writers used a plain `fs.writeFile`,
which opens `O_TRUNC`, so a reader could land between the truncate and
the write. That case is silent: `parseFlow("")` returns `{ steps: [] }`
with no error and `summarize` derives `ok` from "no failures", so a
`flow-execute` racing an append reports a **top-level PASS having
replayed zero steps**. Both writes now go through a sibling temp file
and a rename, so a reader sees either the whole old file or the whole
new one — and unlike the lock, that also holds cross-process.

### `stop-all-simulator-servers` gets a `devices` scope

Every agent is told to call this at session end, and unscoped it walked
the whole registry — so agent A wrapping up tore down agent B's devtools
mid-recording, degrading B's flow to brittle coordinate taps. It now
takes an optional `devices` scope and reports `unmatched` ids, so a
mistyped id doesn't read as a clean machine.

Which namespaces count as "owned by a device" turned out to be the
load-bearing part, because nothing cascades to most of them and
`unmatched` turns any omission into an actively wrong diagnosis — a
correct device id reported as a typo. The set now covers `AXService`
(the in-sim ax daemon, spawned `--timeout 3600`; an iOS session that
only ran boot/launch/describe owns this and nothing else),
`ScreenRecordingSession` (an ffmpeg child plus the touch-visualizer
overlay it enabled on the device), `NativeProfilerSession` (an xctrace
child, or an on-device perfetto process and its trace file) and
`JsRuntimeDebugger` (a bound loopback server, the CDP socket to Metro, a
log handle). The debugger URNs interpose the Metro port —
`<ns>:<port>:<deviceId>` — so the matcher understands both shapes,
consuming only the first colon so a wireless adb serial after the port
still compares whole.

`stop-simulator-server` shared none of this and had drifted: it looked
URNs up with an exact, case-sensitive `services.get()`, so a lower-cased
UDID silently no-op'd there while the scoped stop-all reaped it. Both
tools now share one matcher. Their namespace *sets* stay deliberately
different, documented where they are defined — `stop-simulator-server`
is also the documented recovery for a wedged transport, and widening it
to devtools/AX would make a routine retry drop the native-devtools
connection another agent's recording depends on, which is the hazard the
`devices` scope exists to prevent.

## Verification

**End-to-end, three devices, one shared tool-server** (iPhone 16 Pro,
iPhone 17 Pro, Pixel 3a): three interleaved recordings, plus an
unrelated `flow-execute` in a fourth project while all three were live.
Both iOS devices held their own live native-devtools concurrently (two
per-UDID sockets, `connected: true`). Each YAML contained exactly its
own steps in order, taps captured as selectors rather than coordinates,
and neither picked up another's or the third root's path. All three
replayed green — also through the **released 0.17.0 CLI** against the
modified server, which checks wire compatibility. A device-scoped
teardown of sim 1 left sim 2 and the emulator running.

Re-run against the final build:

- Two interleaved recordings across two project roots on one device,
with a **separate process** polling one of the flow files as fast as it
could for the duration. Zero empty or truncated observations; each YAML
held exactly its own steps; both replayed green (6/6 and 5/5). Against
the pre-change in-place write the same reader observes zero-step reads
interleaved among the real ones — the silent-PASS input.
- A scoped teardown of the iPhone reaped `AXService` and
`NativeDevtools` for that UDID only, reported no `unmatched`, and left
the second sim and the emulator running. Lower-casing the UDID matched;
a genuinely bogus id was still reported.
- With a screen recording live on the Pixel 3a, a scoped teardown reaped
`ScreenRecordingSession` and the **ffmpeg child actually exited** (1 →
0). Before the fix it survived to the 180 s cap while the tool called
the correct serial a typo.

The pre-change repros were re-run afterwards to confirm observed
behavior changed, not just the tests.

`npx vitest run` in `packages/tool-server` (295 files, 3154 tests) and
`packages/argent-cli` (280), `tsc --build`, `tsc --noEmit` on both test
projects, eslint and prettier — all green.

## Review notes

Several rounds of review found real defects in this change; each was
reproduced before fixing and re-verified after. Worth knowing when
reading the diff, since the tests exist to pin them:

- `flow-start-recording` truncated the `.yaml` outside any lock, so a
step from the take being discarded landed in the freshly reset file and
was reported as success (26-29 of 200 runs).
- `flow-finish-recording` never took the lock; its `await fs.readFile`
was a yield during which a concurrent append committed, leaving the
reported summary disagreeing with disk (11-16 of 200).
- A failed finish destroyed the recording. Hand-editing the `.yaml`
mid-recording is a documented workflow, so `parseFlow` can legitimately
throw — and the error told the agent to call `flow-start-recording`,
which truncates the very take it would recover.
- `unmatched` reported a device this session had just stopped, because
`disposeService` returns a node to IDLE and keeps it.
- A bare IP claimed every wireless-adb device at that address, since an
adb serial is itself `ip:port`.
- The eviction backstop stamped with `Date.now()`; ties at millisecond
resolution let it drop the session that was just used.
- `requireRecordingSession` ended with "Call flow-start-recording first"
— reached for a key that was finished, superseded or evicted as well as
one never started, and on those branches the file on disk is fully
populated while that call truncates it and reports no `restarted`.
- The atomic write regressed twice on its own terms: its scratch name
was derived from the flow file's basename, which has no length cap, so a
long flow name that appended fine before failed `ENAMETOOLONG`; and the
cleanup guard began after the temp file was created, leaking a scratch
file into a committed directory on any write that failed after opening.
- Two tests passed against implementations they claimed to reject: the
`stop-simulator-server` narrowness guard used a udid that classifies as
*android*, where the iOS-only services it guards can never appear, so
widening the iOS branch kept it green; and the client-mode finish
asserted only the *shape* of `savedTo`, never its content, though that
directive is the only thing that lands the file in client mode.

## Notes for reviewers

- Client/remote persist mode is the least-exercised path, and now has
its own concurrency tests (overlapping appends, and a restart
superseding an in-flight one) — it previously had none. The
`project_root` key is byte-stable across the file boundary (`kind:
"probe"` passes it through unchanged), which is what keeps a remote
recording addressable from `flow-add-step`, which declares no file
input.
- Two spellings of one flow file still mint two sessions — and two
independent locks — over one file, which bypasses every guarantee here.
Two ways in: the root spelled two ways (a symlink, or a case variant on
APFS), and the likelier one, the *name* cased two ways on a
case-insensitive volume. Neither is normalized away because the correct
normalization is the filesystem's: case-folding the key would wrongly
merge two distinct flows on ext4, and resolving symlinks is impossible
in client mode, where the root does not exist on this host. Documented
at `getFlowPath`.
- No recording tool can detect that another agent took its key.
`flow-add-step`, `flow-add-echo` and `flow-finish-recording` all
re-resolve the key on every call, so after a takeover they act on
whichever take is now live and report success: an append lands in the
other agent's file, and a finish finishes and clears it. The only case
that fails loudly is the narrow one the liveness check covers - an
append already IN FLIGHT when the restart lands. There is no caller
identity to key ownership on (see the rejected alternatives above), so
this is spelled out for the agent under "Pick a name unique to your
task" in `argent-create-flow` rather than mechanized.
- Skew hazard in one direction only: a *new* client against an *older*
tool-server. Zod strips the undeclared `name`/`project_root`, and the
old server falls back to its module globals — so concurrent recordings
silently collapse onto one session. The other direction (released 0.17.0
CLI against this server) was verified end-to-end.
- `packages/argent-private/docs/reference.md` still describes
`flow-add-step` as appending to "the current recording", and names two
tool ids that do not exist (`flow-insert-echo`/`flow-run` are file
names; the ids are `flow-add-echo`/`flow-execute`). It is a submodule,
so the fix cannot ride on this branch.

- A restart reported `discardedSteps` from the superseded session's
in-memory flow while it truncates the *file*. Hand-editing the `.yaml`
mid-recording is documented, so the two diverge: four steps wiped, one
reported. It now counts the file, and reports no number at all when that
file cannot be read or parsed - 0 is the answer a genuinely empty take
gives. Client mode still counts from memory (this host cannot see the
client's file), and the description now says which mode it is promising.
- `ChromiumJsRuntimeDebugger` cascades from `ChromiumCdp` and was
therefore already being torn down - silently, while `NetworkInspector`
and `ReactProfilerSession` cascade the same way and were named.
`stopped` is documented as the services that were live and got shut
down, so it is listed now too.
- The client-mode case named for a superseded in-flight append restarted
*between* calls, so the next append re-resolved the key and succeeded -
the guard was never reached, and neutering it left the whole file green.
It now parks an append in its live sub-tool call across the restart.
- `stop-tools.test.ts`' mock handed the tool the live service map where
the real `getSnapshot` copies, so a cascade rewrote the state the sweep
was still reading and the answer depended on map insertion order. The
mock copies and recurses like `_teardown` now, and the cascade case runs
under both orders.
- Prose the code contradicted, each verified against source before
rewriting: Vega owns `JsRuntimeDebugger`/`NetworkInspector` once the
debugger has run (and can match anything at all through an ERROR node);
"nothing here cascades" was denied twice in its own file; an iOS
boot/launch/describe session owns `NativeDevtools` too; `:tcp` is
mintable but unreached in production; the tool-server is a singleton
*per install bundle*, not per machine; `argent flow list` does enumerate
`.argent/flows`; the stop-tools comments narrated a "before" that only
existed on this branch.

Found while verifying, out of scope, worth their own issues: a `tool:
flow-execute` self-reference recurses past both cycle guards (`run:` is
guarded, a nested `flow-execute` reseeds the stack); react-profiler
artifacts collide host-wide on a second-resolution filename;
`stop-metro` is port-scoped and kills another agent's bundler;
`debugger-reload-metro`'s HTTP fallback broadcasts to every attached
client. A tool's `capability` is enforced only by the HTTP layer, not by
`registry.invokeTool`, so a call that reaches the registry another way -
`flow-add-step` takes `command` as a bare string - can resolve a service
the tool declares it does not support, leaving an ERROR node behind
(e.g. `SimulatorServer:<vega serial>`).

---------

Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>
2026-08-10 10:54:58 +02:00
Ignacy Łątka 7e4ece697d chore: add server.json for the official MCP registry (#598)
Adds the two files the official MCP registry
(`registry.modelcontextprotocol.io`) needs in order to list argent.
Argent is not in that registry today
(`?search=io.github.software-mansion` returns 0 servers).

The registry is metadata-only — it points at the npm package rather than
hosting anything.

### `server.json`

Describes the server and how a client starts it: `npx @swmansion/argent
mcp`. The `mcp` positional is the dispatcher case in
`packages/argent/src/cli.ts:137`, and it is the same subcommand `argent
init` writes into every client config —
`packages/argent-installer/src/mcp-configs.ts:130-141` spells it four
ways (`node <path> mcp`, `yarn argent mcp`, `npx --no-install argent
mcp`, and the global `{ command: MCP_BINARY_NAME, args: ["mcp"] }`,
where `MCP_BINARY_NAME` is `"argent"`). None of them is literally the
npx-by-package-name form in `server.json`, which is the form the
registry expects a client to construct from `identifier` + `runtimeHint`
— so it is verified below by running it, not by matching a string.

`repository.id` is the GitHub repo id (`1164885639`); the schema
recommends it so a registry can detect a delete-and-recreate of the
repo.

### `mcpName` in `packages/argent/package.json`

The ownership proof. The registry fetches the npm package and requires
`mcpName` to equal the server name in `server.json` before it will
accept a publish under `io.github.software-mansion`. It is the field the
registry's own quickstart names for npm packages. Nothing in the publish
path drops it: `scripts/pack-mcp.cjs` shells out to `npm pack` without
rewriting the manifest, and the canary path's
`scripts/next-canary-version.mjs --write` round-trips the JSON and only
sets `version`.

### Namespace

`io.github.software-mansion/argent` — GitHub OAuth auth. The registry
grants an org namespace only to org **Owners**; plain membership is no
longer enough. If no Owner is available to run the publish, the
alternative is DNS auth via an apex TXT record on `swmansion.com`, which
changes the name to `com.swmansion/argent` (and the `mcpName` value with
it).

### Version sync

`server.json` sits at the repo root, outside `packages/*`, so a release
bump would have left its two `0.18.0`s untouched — and the registry
rejects a publish whose `packages[].version` names a version npm has
never seen. `scripts/check-workspace-versions.mjs` (already run by the
Static checks job) also holds `server.json` to the version in
`packages/argent/package.json` — the package it names — and pins the two
cross-file links a publish depends on: `name` == `mcpName`, `identifier`
== the published package name. Both halves are reported in one run, so a
half-finished bump doesn't cost a CI round-trip per file.

### Not in this PR

- **Publishing.** Nothing has been published; this is just the manifest.
Publishing is `mcp-publisher login github && mcp-publisher publish`, and
it can only succeed **after** a release whose npm tarball carries
`mcpName` — i.e. not against `0.18.0`, which is `latest` (published
2026-07-30) and predates `mcpName` — `curl
registry.npmjs.org/@swmansion%2Fargent/0.18.0 | jq .mcpName` → `null`.
The first publishable release is the next bump.
- **`github.com/mcp`.** A separate, manual step. Onboarding a new server
there is hand-curated: per
[github/github-mcp-server#1257](https://github.com/github/github-mcp-server/discussions/1257)
(trent-j, a collaborator on that repo, 19 May 2026) "onboarding a new
server is still a manual curation process today. Once a server has been
onboarded, newly published versions from the OSS registry should sync
from there." The nomination channel is `partnerships@github.com`, from
GitHub's [registry how-to
post](https://github.blog/ai-and-ml/generative-ai/how-to-find-install-and-manage-mcp-servers-with-the-github-mcp-registry/)
(24 Oct 2025) — that address does not appear anywhere in the discussion
thread.

### Verification

- `mcp-publisher validate` (v1.8.0, which posts the file to the live
registry) → `server.json is valid`. Mutation-checked what that actually
covers: removing `transport` fails it and removing `$schema` is a 422,
but a foreign namespace (`io.github.anthropics/argent`), a nonexistent
npm identifier and a `99.0.0` package version all still pass — so it
checks shape only, and the ownership/existence checks happen at publish
time. Also validated offline against the pinned `2025-12-11` schema.
- Ran the entrypoint the file declares. `npx -y @swmansion/argent@latest
mcp` fed an `initialize` over stdio answered
`{"protocolVersion":"2024-11-05","capabilities":{"tools":{}},"serverInfo":{"name":"argent","version":"0.18.0"},"instructions":"…"}`,
with stderr empty.
- Mutation-checked the new version guard: drifting `server.json`
`version`, drifting `packages[0].version`, deleting `mcpName`, and
typo-ing `identifier` each fail `check-workspace-versions.mjs` with the
specific line; the pre-existing packages/* drift check still fires on
its own.
- Asserted across the two files: `mcpName` == `server.json` `name`,
`identifier` == package `name`, all three versions equal, description 89
chars (registry limit is 100).
- `prettier --check` and `eslint` clean on both changed files.
- Mutation-checked the guard as a whole — 22 mutants, 1 survivor.
Killed: reverting the self-invocation guard to a raw `argv[1]` compare
and making it never match; `exit(1)` → `exit(0)` on the failure path;
dropping either `failed = true`; exiting early on `packages/*` drift
instead of collecting; `packages.slice(0, 1)`; hardcoding `packages[0]`
in either message; dropping the no-packages-array guard, the
non-object-entry guard, the non-object-JSON guard, and the presence
checks for `version`, `name` and `mcpName`; `continue` → `break` in the
`packages/*` scan (both arms) and in the entry loop; dropping the
on-message read, the `git checkout` hint, and the per-version name
accumulator. The survivor is the `packages/*` readdir sort: it fixes the
scan order rather than any output, so no single run can observe it — it
is there so a scan that stops early drops the same packages on every
machine instead of a filesystem-dependent set.
- Reproduced the symlink no-op on the real repo: with
`packages/registry` at `9.9.9` and `server.json` at `0.17.0`, `node
/tmp/argent-alias/scripts/check-workspace-versions.mjs` now exits 1 and
prints both problems, where before it exited 0 with no output at all.
- Closed the vacuous-pass class in `serverJsonMismatches`: all four
comparisons it makes were bare `!==`, so a field absent from *both*
sides compared equal and passed. A `packages/argent/package.json` with
no `version` printed `All workspace packages and server.json are at
undefined.` and exited 0; with no `name`, a `server.json` naming nothing
installable was reported in sync. `version` and `name` now require
presence, as `mcpName` already did.
- `readTrackedJson` guarantees an object, not just parseable bytes — a
`server.json` holding `null` used to reach the field reads and throw,
and `"0.18.0"` or `[]` produced mismatch lines that never said the file
was the wrong shape.
- Full CI parity locally: `test:scripts` 85/85, `tsc -p
tsconfig.scripts.json`, `eslint --max-warnings 0`, `prettier --check`.
2026-07-31 12:36:44 +02:00
Ignacy Łątka 19995985d3 docs(scripts): stop citing flow-add-step's since-reworded description in a fixture comment (#527)
Follow-up to #436. The escape-cooking fixture's comment cited
flow-add-step.ts as a currently-shipping example of a `\n` description;
main's flow rewrites reworded that description, so the citation is
stale. The comment now leans on the fixture alone instead of any live
tool's wording. No code change - `npm run test:scripts` still 59/59.
2026-07-23 19:40:07 +02:00
Ignacy Łątka 7400179b5b Windows support: Android + Chromium host control plane (#421)
## What

Makes Argent run on Windows. iOS Simulator is macOS-only (Apple), so on
Windows that means the **Android + Chromium** host control plane. The
blocker was twofold: (1) no Windows `simulator-server` binary, and (2)
POSIX-only assumptions in the tool-server.

Companion radon PR builds the binary and runs the Android emulator E2E
on Windows: software-mansion/radon#134.

## Changes

**Windows-aware tool-server / resolver / bundling**
- New shared `commandOnPath()` helper — `where` on Windows, `command -v`
via `/bin/sh` on POSIX. `android-binary`, `check-deps`, and `vega-cli`
previously hardcoded `/bin/sh`, which never matches on Windows.
- `android-binary`: append `.exe` to `adb`/`emulator` in the
`$ANDROID_HOME` fallback; probe `%LOCALAPPDATA%\Android\Sdk` (Android
Studio's Windows default).
- simulator-server resolver + dispatcher + `bundle-tools` +
`download-simulator-server.sh`: ship and resolve `simulator-server.exe`
on `win32` (new `simulatorServerBinaryName()`), add `win32` to the
bundled host keys, add the `simulator-server-argent-windows.exe`
download target with PE arch validation.
- `stop-metro`: resolve listening PIDs via `netstat -ano` on Windows
(`lsof` is POSIX-only and threw ENOENT).
- `adb.ts`: AVD root resolution used `$HOME` + `startsWith("/")`
(rejects `C:\…`); now `os.homedir()` + `path.isAbsolute()`.
- **`boot-device`**: the emulator was spawned `detached` with
`stdio:"ignore"`; on Windows a detached emulator with NUL stdout/stderr
never reaches `sys.boot_completed` (verified on `windows-latest`: the
identical flag set boots fine with real handles, hangs with NUL).
Redirect the detached emulator's output to a temp log file on win32.
POSIX keeps `"ignore"`.

**Tests** — cross-platform unit coverage for `commandOnPath` (both
branches), win32 `.exe` Android resolution, win32 `.exe`
simulator-server resolution, and a win32 AVD drive-path case; dep-gate
tests now mock at the `commandOnPath` boundary so they're
platform-agnostic.

**CI** — `windows-e2e.yml` (`windows-chromium` on `windows-latest`):
runs the resolver/dep unit tests natively on Windows, confirms `adb.exe`
resolution via the SDK root, then drives a **real headless Chrome over
CDP** through the tool-server (discover → screenshot → describe → tap →
observe the DOM mutation). Flow in `scripts/ci/windows-chromium-e2e.mjs`
(also runnable locally / in Docker). The `simulator-server.exe` build +
the full **Android-emulator E2E on Windows** live in radon CI (the
binary's source is the private radon repo).

## Verification (all green)
- **Chromium control plane on real Windows** (`windows-latest`): `✓
discovered chromium-cdp-9222 → ✓ screenshot → ✓ describe → ✓ gesture-tap
→ ✓ DOM mutation observed`. Resolver/dep unit tests run natively;
`adb.exe` resolves via the SDK root (it's *not* on PATH on the runner —
the gap the resolver closes).
- **Android emulator on real Windows** (radon#134, `windows-latest` /
WHPX): the real Argent path — `boot-device` → `{ booted: true }` →
screenshot via `simulator-server.exe` → gesture-tap — all green. (The
boot-device fix above is what makes the detached emulator reach
`boot_completed`.)
- **Local clean-room E2E**: the Chromium flow runs green in a headless
Linux Docker container (fresh `npm ci`, no host pollution).
- **No regressions**: full tool-server suite (1624) +
native-devtools-ios (16); ESLint/Prettier/typecheck clean; **Wayland
E2E** (Linux AVD boot) and **Vega VVD E2E** still pass (they exercise
the changed `adb.ts` / `boot-device.ts` / `vega-cli.ts` paths).

## Notes
Android-emulator E2E needs WHPX nested virtualization — confirmed
available on the current hosted `windows-latest` image (Server 2025);
the emulator boots hardware-accelerated in ~3 min.

---------

Co-authored-by: Ignacy Łątka <claude-hubert.gancarczyk@swmansion.com>
Co-authored-by: Ignacy Łątka <latekvo@users.noreply.github.com>
2026-07-22 13:53:32 +02:00
Juliusz Wajgelt 013d5b60c5 fix: get ax-service working on ios-remote (sim-remote --json spawn + TCP artifact diagnostics) (#536)
## Summary

Three fixes that together get `describe` (and the other ax-service /
native-devtools tools) working against **ios-remote** (`sim-remote`)
devices — including from a **Linux** host. Reproduced end-to-end against
a live remote iPhone 17 simulator over the QUIC relay.

Before: `argent run describe --udid remote:<UDID>` returned an empty
`ROOT AXGroup` with a misleading "reboot the simulator" hint.

### 1. `sim-remote spawn` output was never valid JSON
(`fix(tool-server)`)

`simctlSpawn` parses the CLI's stdout as one `{exit_code,stdout,stderr}`
(or `{pid}`) JSON object, but `sim-remote spawn` only emits that when
passed `--json`. Without it a **non-detached** spawn streams the child's
raw output live, so `JSON.parse` throws `Unexpected end of JSON input`.

This broke ios-remote describe at the first step:
`remoteIosHost.bootstrapAx` runs `defaults write com.apple.Accessibility
…` through a non-detached `simctlSpawn`, which failed before the
ax-service daemon ever started — surfacing only as an empty AX tree.
Fix: pass `--json` unconditionally (no-op for the already-JSON
`--detach` path).

### 2. The tcp ax-service was resolved/packaged under a host-platform
dir (`fix(native-devtools-ios)`)

The tcp `ax-service` is a **darwin/iOS-simulator** binary that is
uploaded to and `simctl spawn`d on the **remote** macOS orchestrator, so
it's needed on any *host* platform. But it was resolved (and packaged)
under `bin/<hostPlatform>/tcp` via `platformBinDir()`, so a **Linux host
looked at `bin/linux/tcp` and never found the darwin-built binary** —
the ios-remote path was effectively macOS-host-only. (The tcp dylibs
were already neutral at `dylibs/tcp`.)

Moved it to a platform-neutral `bin/tcp`, mirroring `dylibs/tcp`, and
rewired every producer/consumer:
- resolver: `platformTcpBinDir()` → `tcpBinDir()` = `BIN_DIR/tcp`
- `native-devtools-ios/scripts/build.sh`: `bin/darwin/tcp` → `bin/tcp`
- `scripts/download-native-binaries.sh`: `TCP_BIN_DIR` → `bin/tcp`
- `packages/argent/scripts/bundle-tools.cjs`: copy from/to `bin/tcp`
- tests assert the neutral location on both linux and darwin

### 3. Missing tcp artifacts were undiagnosable
(`fix(native-devtools-ios)`)

When a tcp artifact is absent (local/dev build, older package), the bare
`... not found` error was thrown deep in the ax-service factory, where
`describeIos` swallowed it and returned an empty tree plus
`DEGRADED_HINT` — telling the agent to `boot-device --force`, a dead
end. The only escape was discovering `ARGENT_SIMULATOR_SERVER_TCP_DIR`
by reading source. Fix: `requireTcpArtifact` throws an actionable error
naming the exact path, the override env var, and how to build it;
`describeIos` surfaces that instead of the reboot hint.

## Testing

- New `sim-remote-spawn-json.test.ts` — asserts `--json` on both plain
and `--bin/--detach` argv, before the `--` separator.
- `native-devtools-ios/test/index.test.ts` — the tcp binary resolves
from the neutral `bin/tcp` on **both** linux and darwin; the not-found
errors name their override env var.
- `native-devtools-ios` (16) + ios/describe tool-server suites pass;
typecheck, eslint, prettier clean.
- Manual end-to-end against `remote:<UDID>` (darwin host): binary at
`bin/tcp` → full tree from `source: "ax-service"`, no hint; binary
removed → actionable `ARGENT_SIMULATOR_SERVER_TCP_DIR` hint.
2026-07-22 08:47:52 +02:00
Ignacy Łątka cd4190b80e Full tgz-only E2E harness (scripts/e2e-full) (#495)
Now verified end-to-end on the real Mac:
install/introspection/validation, the android tier (against a booted
emulator), and the chromium tier all pass from a locally packed tgz; the
rn tier auto-skips without `~/dev/bluesky` as designed.

## What this is

A release-gating end-to-end harness under `scripts/e2e-full/` that
starts from **only a `swmansion-argent-*.tgz` bundle** and exercises the
whole product: the install flow, every CLI command, all 70 tools'
argument validation, and a happy-path run of every tool against real
devices. Intended to be run on the real Linux box and the real Mac
before a release.

Everything runs under a throwaway `HOME` + npm prefix, so it never
touches the machine's real `~/.argent`, editor MCP configs, or global
packages (safe on a shared box). It emits a JSONL log + a markdown
coverage report and exits non-zero if any hard assertion fails.

## Phases

| phase | needs | covers |
|---|---|---|
| `install` | npm + network | `npm i -g <tgz>`, bundled binaries, `init`
(global + `--local`), `update`, `uninstall`, telemetry, MCP-config
generation |
| `introspection` | — | `--version/--help`, `tools describe` for **all
70** tools, feature flags, `server start/status/logs/stop`,
`link/unlink` |
| `validation` | — | per tool: missing-required / bad-enum / bad-type
rejection (schema-driven, no hardware) |
| `android` | Android emulator | happy-path of every
touch/gesture/screenshot/app-lifecycle tool |
| `chromium` | Electron (bundled optional dep) + a display | boots a
generated Electron app; drives CDP tools
(scroll/drag/tabs/cookies/storage) |
| `rn` | `~/dev/bluesky` + Android device | debugger + react/native
profiler + network chain against the real Bluesky app |

Tiers auto-skip with a recorded reason when prerequisites are missing.
iOS / tvOS / Vega are intentionally out of scope for now.

## What I actually verified (real runs, Linux)

- **install** 18✓/0✗, **introspection** 85✓/0✗ (all 70 `tools
describe`), **validation** 81✓/0✗, **chromium** 24✓/0✗ (real Electron
app + CDP).
- Holistic run: **190✓ / 0✗ / 34∼, exit 0.** Failure-gate confirmed
(non-zero on fail), `bash -n` clean on all files, all 72 android/rn JSON
arg literals linted.

## What I did NOT verify live (please scrutinize)

- **`android` and `rn` tiers were never run against a live device.** The
Android emulator would not stay up on my box (memory-constrained shared
machine; QEMU threads hung seconds after boot across ~7 attempts). The
code is complete and uses the same machinery the `chromium` tier
exercised live, but it has **not** been proven end-to-end. This is the
main reason for the "not certain how well it works" caveat.
- The RN tier's exact debugger/profiler arg shapes are derived from
`tools describe`, not confirmed against a running Metro.

## Run it

```bash
bash scripts/e2e-full/run-e2e.sh                                   # everything applicable to this OS
bash scripts/e2e-full/run-e2e.sh --skip-install --phase introspection,validation   # fast offline core
bash scripts/e2e-full/run-e2e.sh --phase android --android-serial emulator-5554     # inject a device
```
2026-07-20 16:43:34 +02:00
Ignacy Łątka 78d3184ba8 fix(scripts): stop extract-tools dropping tools with long descriptions (#436)
## Defect

`scripts/extract-tools.mjs` extracts MCP tool definitions by regex to
feed the downstream `spidershield scan --tools-json` security scan. For
each `id:` match it scanned only the next 2000 chars (`const descWindow
= afterId.slice(0, 2000)`) for the `description:` value, and the
description regexes require the string's closing delimiter to fall
inside that window.

Any tool whose description's closing delimiter sits past ~2000 chars is
therefore matched as `null` and **silently dropped** - never emitted,
never security-scanned. Two shipping tools hit this:

- `run-sequence`
(`packages/tool-server/src/tools/run-sequence/index.ts`) - a ~4285-char
multi-line template-literal description whose closing backtick sits
~4324 chars past its `id:`.
- `describe` (`packages/tool-server/src/tools/describe/index.ts`) - a
~2195-char description whose closing backtick sits ~2252 chars past its
`id:`.

**Security impact:** `describe` is the accessibility / DOM tree tool,
exactly the kind of tool a `spidershield` scan should cover, and it was
being silently excluded alongside `run-sequence`. A dropped tool is not
a scan finding, so the gap was invisible.

### Repro (before)

```
$ node scripts/extract-tools.mjs | grep -c '"run-sequence"'
0
$ node scripts/extract-tools.mjs | grep -c '"describe"'
0
$ node scripts/extract-tools.mjs | grep -c '"gesture-tap"'
1
# total tools emitted: 70  (run-sequence AND describe both missing)
```

### After

```
$ node scripts/extract-tools.mjs | grep -c '"run-sequence"'
1
$ node scripts/extract-tools.mjs | grep -c '"describe"'
1
# total tools emitted: 72  (both captured, run-sequence's full 4285-char description intact)
```

## Fix

- Drop the fixed 2000-char lookahead. Each tool's `description:` is
resolved at the **same object-literal brace level** as its `id:` by a
forward scan that tracks brace depth over real code only: comments,
string literals, template literals (with `${...}` interpolations skipped
opaquely, recursively), and regex literals (prev-token heuristic incl.
regex-permitting keywords like `return`) are skipped whole, so nothing
inside them can open a fake string, shift the depth, or match as a
`description:`/`id:` token. The value is then parsed to its actual
closing delimiter, so a description of any length is captured in full.
- Because matching is by brace scope rather than raw position, a stray
`id:`/`description:` token can't corrupt extraction: a description-less
tool can't borrow the next tool's description, an `id:` appearing inside
description text or a comment can't truncate or overwrite anything, and
an `id:` key nested inside a value object (e.g. `defaultPayload: { id:
"example" }`) is neither emitted as a spurious tool nor allowed to drop
the real one.
- Captured text follows full JS cooked-string semantics: named escapes,
`\xNN`, `\uNNNN`, `\u{...}`, identity escapes, line continuations, and
template `\r\n`/`\r` -> `\n` normalization - the emitted (and
security-scanned) text is the string the runtime renders.
- A description value that is not a single plain literal - a
concatenation, a `${...}` interpolation, a method/operator suffix
(`"...".slice(0, 3)`), `as const` - is never emitted as a
truncated/wrong literal: it warn-skips on **stderr** (stdout stays valid
JSON for the scanner). Non-interpolated template-literal ids (`` id: `x`
``) are extracted; interpolated ids stay out of scope like
const-reference ids.
- Duplicate ids keep the first occurrence and warn (exported as a pure,
unit-tested function).
- The extractor stays dependency-free (node: builtins only): the
tool-description-quality workflow runs it on a bare checkout without
`npm ci`.
- Output JSON shape is unchanged.

## CI guard

Adds `scripts/extract-tools.test.mjs` (node's built-in test runner,
wired into `npm run test:scripts`; 36 tests). Ground truth: the
real-tree tests parse every tool source file with the actual
**TypeScript parser** (a root devDependency, test-only) and require the
extractor's output to equal the AST **exactly - names and descriptions
byte-for-byte**. Any drop, corruption, or fabrication on a shape that
enters the tree fails CI loudly, with no reliance on stderr warning
formats and no false red from `id:`-like text in comments or strings.

Two policy gates keep un-extractable shapes out of the tree instead of
letting them silently leave the scan: a `description` that cannot be
read statically (non-literal value, shorthand/getter/method/computed
member, or a spread sibling that may carry/override it) and a
plain-literal description written **before** its `id:` (the scan is
forward-only) each fail with an actionable, file-naming message.

Fixture tests pin the parsing rules independent of any real tool's
wording - long descriptions (>4000-char value, >2000 chars of siblings
between `id:` and `description:`), embedded `id:`/`description:` tokens,
comments, keyword-position and sibling regex literals, division, nested
templates and interpolation content, template/`as const` ids, the full
escape suite, CRLF cooking, single-quoted forms, trim, warn-skip
classes, and the duplicate-id warning. Each new fixture was verified to
fail under a targeted mutation of the extractor.

Note: tools registered with a const-reference / dynamic id (e.g. `id:
AWAIT_UI_ELEMENT_TOOL_ID`) rather than a static literal stay out of
scope for this extractor - a separate, pre-existing limitation unrelated
to the description window, and one this static/lexical extractor cannot
close soundly (it has no tool-scope knowledge, so a blanket warning on
every `id: <identifier>` would also fire on legitimate nested non-tool
keys like `{ id: someVar }` in a payload/example object). The full set
currently taken out of the scan this way is `app-map-crawl`,
`app-map-export`, `app-map-navigate`, `app-map-query`,
`await-screen-idle`, and `await-ui-element`. A proper fix is a follow-up
covering const-ref id resolution plus a real-tree emit-count and
empty-stderr assertion so this silent-omission class trips CI if it ever
regresses.

## Verification

- `node scripts/extract-tools.mjs` -> 72 tools, `run-sequence` and
`describe` both present (total 70 -> 72), byte-identical to the
TypeScript-AST parse of the tree, no stderr warnings
- `npm run test:scripts` -> all pass (`extract-tools.test.mjs`: 36
tests)
- mutation checks: window-bounding, depth-anchor removal, trim removal,
dedup neutralization, single-quote-branch neutralization each fail the
suite
- `npm run typecheck:scripts` -> clean
- `prettier` + `eslint` on changed files -> 0 problems
- lockfile: `npm install --package-lock-only --ignore-scripts` -> no
diff (idempotent)
2026-07-20 12:56:44 +02:00
Juliusz Wajgelt b90970ffa8 ci: ship TCP-transport iOS binaries in the published package (#487)
## Why

argent's `ios-remote` (sim-remote) support drives **ax-service** and
**native-devtools** over a TCP socket, resolving them from
`bin/darwin/tcp/ax-service` and `dylibs/tcp/*.dylib`. `bundle-tools.cjs`
already stages those into the published package **when present**, but
`download-native-binaries.sh` never fetched them — so
`@swmansion/argent` 0.15.0 shipped with no TCP variant at all, and every
`ios-remote` `describe` / `native-*` call failed with "not found".

## What

- **`download-native-binaries.sh`** — fetch the TCP dylibs tarball into
`dylibs/tcp/` and `tcp-ax-service` into `bin/darwin/tcp/ax-service`
(best-effort: skipped with a warning on pre-sim-remote release tags),
with the same Mach-O platform + codesign verification applied to the
iOS/tvOS assets.
- **`publish.yml` / `publish-next.yml`** — hard `test -x`/`test -f`
gates for the four TCP files so a missing build fails the publish loudly
instead of silently regressing (mirrors the existing tvOS gates).
- **`package.json`** — add `build:ios-binaries:tcp` and wire it into
`pack:mcp:local` so local packs match CI.

`bundle-tools.cjs` needs no change — its `ax-service (tcp)` asset entry
and recursive `dylibs/` copy already handle the `tcp/` slot.

## Verified locally

- `npm pack --dry-run -w @swmansion/argent` now lists
`bin/darwin/tcp/ax-service` +
`dylibs/tcp/{libArgentInjectionBootstrap,libKeyboardPatch,libNativeDevtoolsIos}.dylib`.
- `package-manifest.test.ts` (download↔bundle-tools path agreement) and
`native-devtools-ios/index.test.ts` (TCP path resolution) pass.
- End-to-end against a real sim-remote device: with the TCP binaries in
place, `describe` returns the full ax tree and `native-describe-screen`
/ `native-find-views` / `native-full-hierarchy` all return `status: ok`.

## Paired PR

The producer side — the workflow that builds, signs, and uploads these
TCP release assets — is **software-mansion/argent-private#26**. Both
must land for a release to ship the TCP binaries.

## Known follow-up (out of scope here)

`axServiceBinaryPathTcp()` resolves
`bin/<hostPlatformKey()>/tcp/ax-service`, and `bundle-tools.cjs` stages
the TCP ax-service **darwin-only**. So `describe` via ios-remote works
from a macOS argent host (verified), but a **Linux** host would hit "not
found" at `bin/linux/tcp/`. The TCP dylibs are fine cross-host (flat
`dylibs/tcp/`, un-gated). Full Linux-host support means staging the
darwin TCP ax-service into each host's `tcp/` dir, modeled on the
per-platform `simulator-server` loop — left as a follow-up.
2026-07-10 12:42:47 +02:00
filip131311 d010ead67e fix(scripts): verify downloaded dylib Mach-O platforms (#450)
## What

Adds a platform-verification gate to
`scripts/download-native-binaries.sh`: after downloading the native
binaries, assert every arch slice of each injection dylib has the
expected Mach-O platform (iOS = `IOSSIMULATOR`, tvOS = `TVOSSIMULATOR`),
and fail loudly otherwise.

## Why

The script pulled the flat iOS `libArgentInjectionBootstrap.dylib`
release asset on trust. When a mis-built release ships a
**tvOS-platform** binary in that slot, `dyld` silently refuses the
platform-mismatched `DYLD_INSERT_LIBRARIES` insert, so native-devtools
never connects — every `native-*` tool returns `restart_required`, with
**no error** at download, sign, or pack time. That is exactly what
happened (the `argent-main` / `argent-v0.14.0` flat bootstrap asset was
`TVOSSIMULATOR`).

This guard runs on both paths that ingest the binaries — local
`pack:mcp` (`pack-mcp.cjs`) and the publish CI (`publish.yml` /
`publish-next.yml`) — so a bad upstream release can no longer be bundled
into an npm publish silently. `vtool` is macOS-only, so the check is
skipped on hosts without it (matching the existing `codesign` guard).

Complements the producer-side fix in `argent-private`
(`build-native-binaries`: build the tvOS dylibs outside `build/dylibs/`
so the flatten can't collide, plus a platform gate).

Verified: flags a `TVOSSIMULATOR` bootstrap, passes an `IOSSIMULATOR`
one.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
2026-07-01 19:31:15 +02:00
Ignacy Łątka c4a9f0bd8a ci: publish a "next" canary on every push to main (#423)
**Update after team discussion:** the open question from the original
version of this PR ("should we fire `next` on every push to main?") got
a yes, so this now does exactly that — continuous canary publishing from
`main`. Original tag-sync wiring is still here too. Branch is merged up
to current `main`.

### what it does

every push to main publishes a canary prerelease under the `next`
dist-tag, so `next` always tracks the tip of main.

- **"Publish prerelease to npm (next)"** now also triggers on `push:
branches: [main]`. on that path it derives a version, stamps it into
`packages/argent/package.json`, and publishes `--tag next`. the manual
`workflow_dispatch` path is untouched (gated behind `if:
github.event_name == 'workflow_dispatch'`).
- **version scheme** = `<patch-bump of the highest published
version>-next.<counter>`. a prerelease of the next patch still outranks
the current `latest` (so `next` lands above it) but stays under the
eventual stable. the **counter resets per base**: next index after the
highest `<base>-next.<k>` already on npm, or 0 if the base has no
canaries yet — so each patch line starts at `.0`:

  ```
  0.13.1-next.0, 0.13.1-next.1, ...   ← latest is 0.13.0
       ... 0.13.1 ships stable ...
  0.13.2-next.0, 0.13.2-next.1, ...   ← counter restarts
  ```

lives in `scripts/next-canary-version.mjs` (+ tests) as a pure function
of the published-versions list, reusing the semver helpers from the sync
script.
- **`argent -v` reports the right thing.** the CLI reads its version
from `package.json` at runtime, so stamping is enough — and there's an
in-pipeline step that runs the built `argent --version` and fails the
publish if it doesn't match the version about to ship. verified
end-to-end against the real registry: computed `0.13.1-next.0`, stamped,
built, `argent --version` → `0.13.1-next.0`.
- **collisions self-heal.** the counter is registry-derived, so canary
publishes are serialized (a `concurrency` group) to kill the concurrent
race; the publish step additionally retries a version-exists collision
(npm read-after-write lag on back-to-back runs) by recomputing. any
other publish failure still fails fast.
- canary builds pull the floating `argent-main` / `radon-main` signed
binaries. sync workflow still fires after every publish (self-heals
`next`, and still does the real work after a *stable* release).

### stuff to sanity-check before merge

- this publishes to npm on **every** merge to main — a lot of versions +
a full build/publish run each time. fine if that's the intent (it's the
whole point), just calling it out.
- **native binaries on the canary path are the floating `argent-main`
tag** — if radon's binary build lags a native-source change on main, a
canary can ship fresh JS against slightly-older signed
dylibs/ax-service/APK with no signal (the *missing* case fails loud +
safe; the *stale* case is silent). there's no per-commit signed-binary
build to pin against, so this is inherent to canary-from-main rather
than something this PR can fix — flagging it as the one real "preview
channel can be skewed" caveat.
- during a rapid burst of merges, GitHub's concurrency supersedes
intermediate queued runs — published `-next.N` stay contiguous from 0,
but not literally every commit in a burst gets its own canary.
- like any push trigger, it only goes live once this is on main.
2026-07-01 18:01:53 +02:00
Mateusz Ścianek b8afbed183 feat(tv): Apple TV (tvOS) and Android TV support (#374)
Adds end-to-end support for driving **Apple TV (tvOS)** and **Android TV
(leanback)** targets through argent, using a focus-driven interaction
model (TVs have no touchscreen — interaction is moving a focus highlight
with the remote).

This PR was reworked after review to **reuse the existing cross-platform
tool surface and the `tv-remote` tool from the Vega/Fire TV work
(#366)** rather than ship a separate family of `tv-*` tools.

## What this delivers

TV targets are driven by the **same cross-platform tools as every other
platform**, routed through the existing `dispatchByPlatform` fork
mechanism — there are no dedicated
`tv-describe`/`tv-navigate`/`tv-type`/`tv-set-focus` tools:

- **`describe`** — on a `runtimeKind: "tv"` target, returns the
focus-driven view (focused + focusable elements) instead of a tap tree.
On Android TV it auto-falls-back to the full `uiautomator` tree when the
RN focus engine exposes nothing.
- **`tv-remote`** — the cross-platform remote/D-pad tool from #366, now
extended with `ios` (Apple TV HID daemon) and `android` (Android TV adb
keyevents) branches alongside the existing `vega` branch. Single button,
a `repeat`, or a whole path (`["up","right","select"]`) in one call.
- **`keyboard`** — types into the focused field on a TV target (named
keys are rejected there — they're navigation, which belongs to
`tv-remote`).
- **`button`** stays hardware-only (phones/tablets); it is not a TV
tool.

**Apple TV** runs two native daemons (in-sim AX service + host-side HID
daemon), shipped via the `argent-private` submodule. **Android TV**
reuses the same tool surface, adb-backed (`input keyevent`, `uiautomator
dump`, `input text`).

## Remote vocabulary parity

`tv-remote` exposes the full 16-button vocabulary
(`up`/`down`/`left`/`right`/`select`/`back`/`home`/`menu`/`playPause`/`rewind`/`fastForward`/`next`/`previous`/`volumeUp`/`volumeDown`/`mute`):

| | Apple TV | Android TV | Vega |
|---|---|---|---|
| D-pad / select / back / menu / home / playPause |  |  |  |
| media-transport + volume/mute |  rejected |  |  |

Media-transport/volume keys **genuinely work on Android TV** (real
keycodes — verified live: `volumeUp` moved `STREAM_MUSIC`, `mute`
toggled per `dumpsys audio`). On the **Apple TV simulator** they are
**rejected with a clear error**: on-device testing confirmed the tvOS
sim's HID stack silently drops Consumer-Control events, so returning
success would be a lie.

## Key design points
- **tvOS UDIDs are UUID-shaped and indistinguishable from iOS by
shape.** Handled with `runtimeKind` detection (tvOS runtime string for
Apple TV; `pm list features` leanback/television for Android TV — *not*
`ro.build.characteristics`, which lies on TV emulators).
- Converted launch-app / restart-app / screenshot / screenshot-diff /
run-sequence from **eager** to **lazy** service resolution, so a tvOS
target never spins up (and hangs on) the iOS-only simulator-server /
native-devtools blueprints.
- Native injection selects the platform-matched (TVOSSIMULATOR) dylib
slice; daemons recycle across sim reboots and app relaunches so
injection and focus survive.
- SKILL.md footprint minimized per review: the two TV skills were
collapsed into a single lean **`argent-tv-interact`** (~40 lines).

## Verification
- `npm run build`, ESLint, Prettier, and the full tool-server suite
(**1565 tests**) all pass; CI green (incl. the Apple TV / Android TV /
Vega e2e jobs).
- Core flows verified live against real apps on both an Apple TV 4K
simulator and a Google ATV emulator.

## Dependency / merge order
Pins the `argent-private` submodule to the tip of its `feat/tv-support`
branch (software-mansion/argent-private#20). **Merge that PR to `main`
first, then bump the submodule pointer here to the merged SHA before
merging this PR.**

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-01 14:59:37 +02:00
Ignacy Łątka 05e619e55d ci: device E2E smoke tests for every hosted-CI platform (#371)
## What

`wayland-e2e.yml` boots an Android AVD on Linux under headless Weston
and asserts the full pipeline works without a display. It guards exactly
one cell of the support matrix. This adds the same
boot→screenshot→gesture-tap smoke test for every other (target × host)
cell that can run on GitHub-hosted runners.

| target | host | job | status |
|---|---|---|---|
| Android emulator | Linux (KVM) | `wayland-e2e.yml` (existing) |  |
| iOS simulator | macOS | `ios-sim-macos` |  |
| Chromium / Electron | Linux (Xvfb) | `chromium-linux` |  |
| Chromium / Electron | macOS | `chromium-macos` |  |

## How

- **`scripts/e2e/drive-device.sh`** — shared driver. Boots one device
through the tool-server, then asserts `booted:true` → screenshot has
real (non-blank) pixels → `gesture-tap` round-trips. Every tool takes
the device id as `udid` and screenshots come back as the same
`data.image.hostPath` envelope on all platforms, so the body is uniform;
each job just supplies the cell-specific boot JSON + device id.
- **`packages/tool-server/test/fixtures/electron-smoke-app/`** — minimal
self-contained Electron app the Chromium jobs point `electronAppPath`
at. It vendors its own electron via its own lockfile (kept out of the
repo root lockfile); CI `npm ci`s it so `boot-electron` resolves
`./node_modules/.bin/electron`.
- **`.github/workflows/e2e-device-smoke.yml`** — three jobs,
`workflow_dispatch` + path-filtered `pull_request`. iOS downloads the
darwin native binaries (injection must succeed for `bootIos` to report
`booted:true`); Chromium-Linux runs under Xvfb with `--no-sandbox`. The
tool-server is started with `TS_NODE_TRANSPILE_ONLY=1` so cold ts-node
startup doesn't flakily exceed the readiness poll on loaded macOS
runners (types are still enforced by the typecheck job).

## Known gaps (supported on real machines, not runnable on hosted
runners)

- **Android emulator on macOS** — the arm64 emulator needs HVF
(`-enable-hvf`), but hosted macOS runners are themselves VMs with no
nested virtualization. Confirmed by running it: qemu dies with `HVF
error: HV_UNSUPPORTED` ~18s into boot, regardless of GPU mode or RAM.
The Android boot/screenshot/tap path is already regression-guarded on
Linux via KVM (`wayland-e2e.yml`), so macOS adds no runnable coverage.
- **Physical devices** — real iOS (CoreDevice HID) and real Android
(adb-USB) have no attached hardware on hosted runners.

Both would need self-hosted runners (a Mac exposing HVF / wired-up
devices). Documented in the workflow header.

## Verification

- All checks green: the 3 device jobs + ESLint + Prettier + Unit tests.
- Locally (before CI): ran `drive-device.sh` end-to-end against a real
tool-server → boot `booted:true`, screenshot **142 KB** (floor 20 KB),
tap `tapped:true`, exit 0; and confirmed via the Argent stack that a tap
at (0.5, 0.5) lands on the fixture's centered button and increments its
counter.
2026-06-26 17:44:17 +02:00
Ignacy Łątka 10ecc9e53d fix(release): auto-sync the npm 'next' dist-tag to the highest published version (#357)
Auto-sync the npm `next` dist-tag to the highest published version (prerelease or stable) so it stops drifting behind `latest`.

- New dependency-free `scripts/sync-next-dist-tag.mjs` (SemVer comparator verified against the `semver` lib in `scripts/sync-next-dist-tag.test.mjs`).
- Runs as a post-publish step in `publish.yml` and `publish-next.yml` (with `continue-on-error` so a transient registry blip never fails an already-successful release), plus a manual `Sync 'next' dist-tag` workflow with a dry-run toggle.
- Only ever advances `next` (never regresses it) to survive a stale post-publish packument read.
- Package-name arg parsing rejects flag/blank tokens so the scoped `@swmansion/argent` default reliably wins.
2026-06-24 12:29:45 +02:00
Piotr Krawiec 0bed3624a0 feat(vega): Amazon Vega (Fire TV) platform support (#366)
## Summary

Adds **Vega (Amazon Fire TV)** as a third Argent platform alongside iOS
and Android. Vega apps are React Native 0.72 + Hermes running on a QEMU
**Virtual Device (VVD)**, driven by a **D-pad tv-remote rather than
touch**. This PR extends Argent's platform abstraction so the existing
tool surface gates correctly on `vega`, and adds the Vega-specific tools
needed to actually drive a Fire TV app end to end.

### Platform abstraction

- `Platform` gains `vega`; `DeviceKind` gains `vvd` (the QEMU VVD);
`ToolCapability` gains a `vega { vvd, device }` block; `ToolDependency`
gains `vega`.
- `assertSupported()` selects the `vega` capability matrix for Vega
devices; `dispatchByPlatform()` gains an optional `vega` branch, and a
Vega dispatch with no branch throws `NotImplementedOnPlatformError`
(501).
- New `vega-cli` util (modeled on `adb.ts`) resolves `vega`/`kepler`
from `PATH` or `~/vega/bin` with the same hardened exec/quoting;
`check-deps` resolves and hints the `vega` dependency.
- The `simulator-server` blueprint is never wired for `vega` (Vega uses
`adb` + the `vega`/`kepler` CLI, not the proprietary binary).

### How Vega is wired

- **Input / screenshot / describe** go over `adb` (input via
`inputd-cli`, screenshots host-side via the Android emulator console,
element tree via the on-device automation toolkit).
- **App lifecycle** goes over the `vega`/`kepler` CLI.
- v1 is **Virtual-Device-only**: every Vega capability is declared
`vvd`. Physical Fire TV is out of scope and untested — the tool-server
does not connect to or detect hardware, so `resolveDevice` classifies
every `amazon-` serial as a VVD by shape. A physical device is therefore
not classified as `device` and not rejected at the capability gate;
supporting and gating real hardware is deferred to a version where it
can be tested.
- Vega runs **React Native 0.72**, which predates the new React Native
DevTools. The CDP/Hermes debugger surface (`debugger-*`, React/native
profilers) depends on integration points the new DevTools provides, so
**debugging and profiling are out of scope for this PR** — Vega
profiling/crash analysis is handled by the
`amazon-devices-buildertools-mcp` server instead.

### Also includes

- `argent-vega` skill documenting the full Vega workflow (target
discovery, the describe → compute-path → `tv-remote` navigation loop,
text injection).
- Vega VVD end-to-end CI workflow.

## Feature support by platform

Legend:  supported ·  not supported · ⚠️ degraded / conditional · —
not applicable

| Feature (tool) | iOS | Android | Chromium | **Vega (VVD)** | Vega
notes |
|---|:---:|:---:|:---:|:---:|---|
| **Device discovery** (`list-devices`) |  |  |  | **** | Tagged
`platform:"vega"`, `kind:"vvd"`; pass `serial` as `udid` |
| **Boot / start device** |  |  | — |  |  |
| **Launch app** (`launch-app`) |  |  |  | **** | `bundleId` =
interactive-component app id from `manifest.toml` |
| **Restart app** (`restart-app`) |  |  |  | **** | Terminate +
relaunch |
| **Reinstall app** (`reinstall-app`) |  |  |  | **** | Uninstall +
install a `.vpkg` |
| **Screenshot** (`screenshot`) |  |  |  | **** | Host-side via
emulator console (needs `adb`); rotation ignored (fixed landscape) |
| **Describe / element tree** (`describe`) |  |  |  | **** |
On-device automation toolkit; surfaces `[focused]`/`[selected]` +
normalized frames |
| **Touch gestures** (`gesture-tap`/`swipe`/`scroll`/`drag`/`custom`) |
 |  |  | **** | Vega is tv-remote-driven; gestures error — use
`tv-remote` |
| **Pinch / rotate gesture** (`gesture-pinch`/`gesture-rotate`) |  | 
| ⚠️ | **** | N/A on a TV |
| **Remote / D-pad** (`tv-remote`) | — | — | — | **** | Vega-only tool;
single key, repeat, or a whole path in one call |
| **Hardware buttons** (`button`) |  |  | — | **** | Remote keys live
in `remote` instead |
| **Text input** (`keyboard`) |  |  |  | **** | Whole-string
injection via `inputd-cli send_text` |
| **Rotate orientation** (`rotate`) |  |  | — | **** | TV framebuffer
is fixed landscape |
| **Open URL / deep link** (`open-url`) |  |  |  | **** | Not wired
on Vega |
| **JS debugger** (`debugger-*`) |  |  |  | **** | RN 0.72 predates
the new RN DevTools — out of scope for this PR |
| **Native devtools / AX** (`native-describe-screen`, etc.) |  |  | —
| **** | iOS-only |
| **Native profiler** (`native-profiler-*`) |  |  | — | **** | Use
`amazon-devices-buildertools-mcp` (Perfetto) instead |
| **React profiler** (`react-profiler-*`) |  |  | — | **** | Out of
scope; RN 0.72 / new RN DevTools not supported |
| **Screenshot diff** (`screenshot-diff`) |  |  |  | **** |
Host-side image comparison; platform-agnostic |
| **Flows record/replay** (`flow-*`) |  |  |  | **** | Records any
tool-call sequence |

## Test plan

- [x] `list-devices` surfaces connected Vega devices (`platform:"vega"`,
`kind:"vvd"`)
- [x] `launch-app`, `restart-app`, `reinstall-app`,
`list-installed-apps` work on Vega
- [x] `tv-remote` drives focus (single key, `repeat`, and multi-key
path) and `keyboard` injects whole strings
- [x] `screenshot` returns a frame via the host-side VVD path
- [x] `describe` returns the on-screen element tree with `[focused]`
state and normalized coordinates
- [x] Vega VVD e2e CI workflow passes
- [ ] Test multiple platforms being driven at the same time
    - [x] Android + Vega
    - [ ] iOS + Vega
    - [ ] chrome + Vega

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-22 20:31:45 +02:00
Ignacy Łątka dc0930d50c ci: prevent package-lock drift + add repo-hygiene checks (#362)
## Why

We hit a mismatch between a locally-generated `package-lock.json` and
the one on `main`. `npm ci` validates the lockfile loosely and accepts a
stale lock that a fresh `npm install` would rewrite, so the drift
slipped through. This adds CI to catch that, plus a few related
common-mistake guards.

## What

**`Lockfile` workflow** (Node 20) — runs `npm install
--package-lock-only --ignore-scripts` and fails if `package-lock.json`
changes. This is the gap `npm ci` leaves: it asserts the lock is exactly
what npm produces, not merely "consistent enough".

**`Repo Hygiene` workflow** — three fast static guards:
- **Merge-conflict markers** — fail on any `<<<<<<<` / `=======` /
`>>>>>>>` left in a tracked file.
- **Focused tests** — fail on `describe/it/test/bench.only` in
`*.test.ts(x)` / `*.spec.ts(x)`. Uses a POSIX-ERE word-boundary pattern
so it actually fires under `git grep -E` (a `\b` pattern silently
matches nothing there).
- **Workspace version sync** — fail when `packages/*` versions drift,
via `scripts/check-workspace-versions.mjs` (also `npm run
check:versions`).

To make versions consistent, `@argent/ui` is bumped `0.0.1 → 0.12.0` (it
is `private`, so no publish impact).

## Verification (Node 20 / npm 10.8.2, matching CI)

- Lockfile check: no false positive on `main`; idempotent; catches a
tampered lock.
- Merge-marker & `.only` greps: proven to catch real violations and
reject lookalikes (`await.only`, `audit.only`, `foo.it.only`); clean
across the repo.
- `check-workspace-versions.mjs`: reports all packages at `0.12.0`;
passes `eslint` and `typecheck:scripts`.
- All new/changed files pass `prettier --check`; both workflow YAMLs
parse.

## Note: npm version coupling

The check runs on Node 20 (npm 10.8.2), matching the committed lock's
format. npm 11 (Node 24) regenerates the same lock minus cosmetic
`"peer": true` markers — no resolution change, but a contributor who
regenerates on npm 11 would trip the strict diff. The repo pins no
toolchain today (CI mixes Node 20 and 24). Pinning npm/Node repo-wide
would fully close this; left out of scope here — happy to add it as a
follow-up.
2026-06-19 15:19:19 +02:00
Ignacy Łątka acfe80ab6b chore: add ESLint with typescript-eslint (#349)
## What

Adds type-aware linting to the workspace. Until now the only static gate
on JS/TS was Prettier (formatting) and `tsc` — there was no linter, so
the whole class of bugs `tsc` doesn't catch (floating promises, misused
promises, throwing non-Errors, unsafe stringification, dead code) went
unguarded across ~450 TS files.

- **`eslint.config.mjs`** — flat config. `recommendedTypeChecked` runs
on the TS sources via an explicit `project` list (each package keeps
tests in a separate `tsconfig.test.json` that `projectService` can't
auto-discover, so it's listed explicitly). Type-checked rules are
disabled for the loosely-typed plain-JS build/dev scripts. The
`argent-private` submodule and build artifacts are ignored (mirrors
`.prettierignore`).
- **`lint` / `lint:fix`** npm scripts.
- **`.github/workflows/lint.yml`** — builds the workspace (type-aware
rules need the referenced projects' declarations) then runs `eslint .`.
Node 24 (ESLint 10 requires `^20.19 || ^22.13 || >=24`).

## Rule calibration

This is an adoption PR, so it's deliberately calibrated rather than
maximal:

- **Kept on (bug-catchers):** `no-floating-promises`,
`no-misused-promises` (with `checksVoidReturn: false` to drop the noisy
callback variant), `only-throw-error`, `no-base-to-string`,
`prefer-promise-reject-errors`, `no-unused-vars` (`_`-prefix escape
hatch), plus the ESLint recommended set.
- **Parked off as documented debt:** the high-volume style/`any`-family
rules — `no-unsafe-*`, `require-await`, `no-unnecessary-type-assertion`,
`restrict-template-expressions`, `unbound-method`, `no-explicit-any`,
etc. These flag broad classes of pre-existing code and are out of scope
here; each is commented as a ratchet target for follow-up passes.

All findings from the kept-on rules are fixed in this PR (dead
code/imports, error-cause chaining, **3 genuine floating promises**,
non-Error throws, unsafe stringification). Stale unused `eslint-disable`
directives left over in the tree were removed by `--fix`.

## Verification

Locally, all green: `eslint .` (0 errors), `tsc --build`,
`typecheck:tests`/`typecheck:scripts`, the full `vitest` suite (no
behavior change from the fixes), and `prettier --check`.

## Not included (intentionally)

Other static-analysis gates discussed (CodeQL, dependency-audit,
ShellCheck, actionlint) are out of scope for this PR.
2026-06-17 12:20:55 +02:00
filip131311 8718d82905 feat(android): drive connected physical Android devices (#342)
## What

Adds support for driving a **physically connected Android phone** (USB
or wireless adb), not just emulators. Once a phone with USB debugging is
attached, it shows up in `list-devices` and every existing interaction
tool works against it — no new tools, no per-tool special-casing.

## How it works

A physical phone is driven by a different simulator-server controller
than an emulator: it deploys the Android Studio **screen-sharing agent**
to the device and streams **H264 over adb** (`android_device`
subcommand), whereas an emulator streams decoded RGB over the emulator
gRPC bridge (`android` subcommand). Both controllers expose the **same**
HTTP/WebSocket/stdin API, so the only thing that has to change on the
argent side is *which subcommand the device's service blueprint spawns*.
Everything downstream — `screenshot`, the `gesture-*` family, `button`,
`keyboard` (simulator-server) and `describe`, `launch-app`,
`restart-app`, `reinstall-app`, `open-url` (adb) — is unchanged.

- **Classification** (`utils/device-info.ts`): a non-`emulator-*`
Android serial (a USB hardware serial, or an `ip:port` from wireless
debugging) resolves to kind `device` instead of `emulator`, purely from
serial shape — the same signal radon uses to detect connected phones,
and with no extra adb round-trip on the hot path. New
`isAndroidEmulatorSerial` helper.
- **Routing** (`blueprints/simulator-server.ts`): spawn `android_device
--id <serial>` for kind `device`, `android` for an emulator, `ios` for a
simulator. The capability matrix already declared `android.device: true`
across the tools, so nothing else gates physical devices out.
- **Surfacing** (`tools/devices/list-devices.ts`): Android entries now
carry a `kind` (`"emulator"` | `"device"`); the tool description
documents physical-device detection. Physical phones don't need
`boot-device` — just connect them.
- **describe/android**: resolves the real device kind instead of
hard-coding `emulator` (the adb path is identical either way).
- **Packaging**: `download-simulator-server.sh` fetches and
`bundle-tools.cjs` ships the screen-sharing agent (`.jar` + per-ABI
`.so`) next to each platform's simulator-server binary, where the
`android_device` controller resolves it at runtime.

## Dependencies (draft)

This is a **draft** because it assumes the matching simulator-server
changes land and publish:

- **software-mansion/radon#128** — Linux/cross-platform OpenH264 H264
*decoder* for the `android_device` controller (macOS keeps
VideoToolbox). Gives the argent-built binary a decoder on every host.
- **software-mansion/radon#127** — enables `android-device` in `argent`
builds and publishes the `screen-sharing-agent.tar.gz` release asset
that the download script consumes.

Until those release artifacts are available, the download step degrades
gracefully (warns, skips) and physical-device support is simply
unavailable.

## Testing

Verified end-to-end against a physical **Galaxy S24** (`arm64-v8a`,
Android API 36) by running the tool-server against a locally built
argent simulator-server (radon#128) with the agent resources staged:

| tool | path exercised | result |
|------|----------------|--------|
| `list-devices` | adb | phone listed as `kind: "device"`, `isEmulator:
false` |
| `screenshot` | screen-sharing agent → H264 → VideoToolbox decode | 
correct, correctly-oriented PNG |
| `button` (home) | simulator-server stdin `pressKey` |  |
| `gesture-swipe` | simulator-server WebSocket touch |  |
| `describe` | adb (android-devtools) |  |

Full tool-server unit suite: **1157 passed**. Added coverage for the new
classification (`device-info.test.ts`) and the `android_device`
subcommand routing (`simulator-server-blueprint.test.ts`).

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

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 10:23:01 +02:00
filip131311 73cf47bae6 feat: support arm64 Linux simulator-server binary (#328)
## Summary

The only Linux simulator-server binary argent ships today is x86_64
(`simulator-server-argent-linux` — verified: `ELF 64-bit … x86-64`). On
arm64 Linux hosts (Lima/UTM VMs on Apple Silicon, arm cloud instances,
Raspberry Pi-class devices) the package has no usable binary — the
x86_64 ELF would only fail with `ENOEXEC` at spawn time.

This PR teaches the whole binary pipeline about a **host platform key**:
`process.platform` everywhere, except arm64 Linux which maps to
`linux-arm64`. macOS keeps a single `darwin/` dir because its binary is
universal (lipo); Linux binaries are single-arch ELFs, so each arch gets
its own platform-keyed directory. x86_64 Linux keeps the existing
`linux` key for backward compatibility.

## Changes

- **`@argent/native-devtools-ios` resolver** — new exported
`hostPlatformKey()`; `simulatorServerBinaryPath()` resolves
`bin/linux-arm64/` on arm64 Linux and names the searched key in its
error message.
- **`argent-simulator-server.cjs` dispatcher** (npm `bin` entry) —
mirrors the same key (duplicated deliberately; the dispatcher ships as a
standalone file).
- **`bundle-tools.cjs`** — `linux-arm64` added to the supported host
platform dirs; copied into the package when staged, warn-and-skip
otherwise (same as x86_64 Linux).
- **`scripts/download-simulator-server.sh`** — downloads
`simulator-server-argent-linux-arm64` into `bin/linux-arm64/`; tolerates
the asset being absent (warn + skip) until the upstream release ships
it, so publishing keeps working in the meantime.
- **`scripts/dev.cjs`** — copies the current host's binary by key.
- **publish / publish-next / build-package-artifact workflows** —
`linux-arm64` added to the binary inventory; gated by the existing
`ARGENT_REQUIRE_LINUX_BINARY` flag (now covering both Linux keys via a
`linux*` glob).

## Tests

- Resolver: new arch-aware suite — `linux`+`arm64` → `bin/linux-arm64`,
`linux`+`x64` → `bin/linux`, `darwin`+`arm64` → `bin/darwin`, and the
missing-binary error names `linux-arm64`.
- Dispatcher: end-to-end test faking `process.platform`/`process.arch`
in a child process to exercise the real resolution path on any host.
- Full suite: 146 files / 1651 tests pass. `tsc --build` clean.
- `download-simulator-server.sh` exercised against the live `radon-main`
release: downloads darwin + linux, warn-and-skips the not-yet-published
arm64 asset.

## Dependencies

Companion PR software-mansion/radon#126 adds the `ubuntu-24.04-arm`
build leg that publishes `simulator-server-argent-linux-arm64` to
`simulator-server-releases`. This PR is safe to merge first (the
download script and verify steps tolerate the missing asset); arm64
hosts start working once a release containing the new asset is cut.
2026-06-12 16:20:23 +02:00
Paweł Fornagiel 3ccd89f333 feat: add native android profiling (#275)
# Android native profiling via in-process Perfetto WASM

Connected with:
https://github.com/software-mansion/argent-private/pull/16

## TL;DR

Adds **Android** to the native profiler (previously iOS-only) and does
it with a
brand-new engine: instead of shipping Perfetto's `trace_processor_shell`
native
binary and forking it per query, we now run **Perfetto's trace-processor
compiled
to WebAssembly, in-process** inside the tool-server.

- **One ~13 MB `trace_processor.wasm`** runs on every OS/arch — no
per-platform
  binary, no subprocess, fully offline.
- The native profiler is refactored from an iOS-only implementation into
a
**platform-dispatch facade** (`platforms/ios.ts` +
`platforms/android.ts`) over
a new **`profiler-shared/`** layer, so iOS and Android share the
aggregator,
  types, lifecycle, and time-alignment code.
- New **release/pack pipeline** fetches + sha256-verifies the WASM
bundle from
`argent-private-releases`, and bundles it (plus SQL queries and the
Perfetto
  TraceConfig) into the published `@swmansion/argent` package.

End-to-end on a 76 MB trace with 1013 jank rows: **6.3 s** total analyze
(the older per-hang-subprocess approach was projected at ~47 minutes and
never
finished within the tool-call deadline).

---

## TESTING MANUALLY

To test manually run the following packing command checked-out on this
branch:
```
npm run pack:mcp -- argent-@pFornagiel-native-android-devtools-perfetto
```

This will bundle the package with the new binaries. After this PR is
fixed and the binaries are resolved from the main release, the
`argent-@pFornagiel-native-android-devtools-perfetto` release can be
deleted.

## Why

Android needed native CPU/hang/memory profiling to reach parity with
iOS. The
natural engine is Perfetto's trace-processor, but the obvious ways to
ship it are
both bad:

- **`trace_processor_shell` binary** — per-platform (a mac-arm64 pack
shipped a
mac-arm64-only tarball), pinned an OS/arch matrix, and cost a fresh ~1.3
s
subprocess fork **per query**. With one subprocess forked per detected
hang, a
  large trace projected to ~47 minutes.
- **`get.perfetto.dev/trace_processor` Python launcher** — adds a
runtime Python 3
dependency, resolves "latest" at first run (defeats version pinning),
and makes
  the first analyze slow via a network fetch.

The WASM engine is one cross-platform artifact, in-process and
warm-cached: the
trace is parsed **once** and reused across the whole analyze run plus
every
drill-down query.

---

## The three big pieces

### 1. In-process Perfetto WASM engine

New package surface in `@argent/native-devtools-android`:

| File | Role |
| --- | --- |
| `src/wasm-trace-processor.ts` (+476) | The whole engine: boot, glue
patching, warm-cache, fault containment, chunked parse, cell decoding |
| `src/perfetto-engine.d.ts` (+49) | Hand-written types for the
version-agnostic RPC decoder (imported by path, so tsc can't see its own
`.d.ts`) |
| `src/errors.ts` (+68) | `TraceProcessorUnavailableError`
(`wasm_load_failed` / `wasm_path_invalid`) |
| `src/bundled-meta.ts` (+12) | `PERFETTO_VERSION` stamp, regenerated at
pack time |
| `src/index.ts` | Re-exports + package-root accessors
(`traceProcessorQueriesDir`, `traceConfigPath`) |

Public surface: `resolveTraceProcessorAssets()`,
`queryWarm<Row>(tracePath, sql)`,
`ensureTraceProcessorReady(tracePath)`, `disposeWarmEngine(tracePath)`.

Highlights of how it works:

- **Boots Google's prebuilt web/worker wasm under Node.** Installs a
minimal
worker scope (`self`, `WorkerGlobalScope`, `location`), reads the wasm
bytes,
patches the Emscripten glue (`useMemory64=false`, hand the bytes in
directly,
expose the bridge), and drives it over a `MessageChannel` + `EngineBase`
subclass. Glue patches are applied **by unique string anchors that throw
if they
  don't match** — the canary for glue-format drift on a Perfetto bump.
- **Warm-engine cache, one per trace path.** LRU with `MAX_WARM_ENGINES
= 3`
(bounds resident memory at ~26–76 MB trace + wasm heap each), 5-min idle
dispose
  (timer `unref()`'d), no failed-promise caching.
- **Fault containment (`fatal` promise).** `EngineBase.fail()` throws
synchronously
inside the `MessagePort.onmessage` handler (e.g. an RPC framing error);
left
unhandled it becomes a Node `uncaughtException` and would **kill the
entire
tool-server**, wiping every device + profiler session. Each engine
exposes a
`fatal: Promise<never>`; the handler try/catches, rejects `fatal`,
evicts the
engine, and every load/query is `Promise.race`'d against it — so a
contained
  fault is a clean rejection and the server stays up.
- **Chunked trace parse.** Perfetto's RPC ring buffer rejects any frame
> 64 MiB,
  so the trace is fed in 32 MiB chunks before `notifyEof()`.

### 2. Android native profiler + the shared/dispatch refactor

**Capture** (`utils/android-profiler/capture.ts`, `detect-app.ts`):
spawns
`adb shell perfetto --txt -c - --background-wait -o
/data/.../argent-<ts>.pftrace`,
piping the TraceConfig on **stdin** (SELinux denies `shell:s0` writes
under
`/data/misc/perfetto-traces/`, so the config can't be staged as a file),
waits for
the PID on stdout, then on stop sends `kill -TERM`, polls `/proc/<pid>`,
and
`adb pull`s the trace.

**Pipeline** (`utils/android-profiler/pipeline/`): PerfettoSQL does the
parsing and
aggregation, so the Android pipeline is intentionally **two files, not
iOS's four**.
The per-hang annotation pass is **batched** — `runBatchedHangFolds`
inlines all
hang windows as a `VALUES` table into one SQL script (two derived views
unioned in
a single terminal SELECT, because the engine returns only the final
statement's
rows) and runs it once against the warm engine.

**The platform-dispatch refactor** —
`native-profiler-start/stop/analyze` shrank
from monolithic iOS files (~160–330 lines each) into **thin routers**
(~60–110
lines) that branch on `api.platform`:

```
native-profiler-start ─┬─ ios?    → platforms/ios.ts      (xctrace)
                        └─ android → platforms/android.ts  (adb perfetto)
```

A new **`utils/profiler-shared/`** layer holds everything both backends
use:

| File | What was hoisted |
| --- | --- |
| `types.ts` | Unified `Bottleneck = CpuHotspot \| UiHang \| MemoryLeak
\| MemoryRssGrowth`, each now carrying a `platform` field; shared
`NativeProfilerAnalyzeResult`; `RECORDING_CAP_MS` |
| `aggregate.ts` | Generic `aggregateCpuHotspots(AggregatorInputRow[])`
— burst windowing (`BURST_GAP_MS=500`), severity banding (RED >15%,
YELLOW 3–15%, <3% dropped). iOS feeds it via a pre-pass; Android feeds
it precomputed SQL rows |
| `lifecycle.ts` | `shutdownChild` SIGINT→SIGTERM→SIGKILL ladder +
`waitForChildExit` (no PID-reuse hazard) |
| `thread.ts` | `normalizeThreadName` for both xctrace and Perfetto
thread names |
| `time-align.ts` | New `buildPerfettoAnchor()` + `windowsOverlap()`
alongside the existing iOS/React anchors |
| `format.ts` | `formatBytes` (moved out of the combined report) |

iOS was refactored to fit this shape: `ios-profiler/lifecycle.ts` and
`types.ts`
become thin re-exports; `pipeline/02-aggregate.ts` shrank from ~200 to
~60 lines
(keeps its iOS-only dominant-function pre-pass, delegates
grouping/bursting/
severity to the shared aggregator). **No iOS behavior change** — same
outputs,
shared internals.

**Session blueprint** (`native-profiler-session.ts`): generic
`capturePid` /
`captureProcess` (was `xctracePid` / `xctraceProcess`), new `platform`
and
`androidOnDeviceTracePath` fields, and **platform-branched dispose**
(iOS SIGKILLs
xctrace; Android `kill -KILL`s perfetto, `rm`s the device trace, and
disposes the
warm WASM engine).

**Drill-down tools**: `profiler-load.ts` learns to restore Android
`.pftrace`
sessions (with an optional metadata sidecar / `app_process` override);
`profiler-stack-query.ts` splits into iOS and Android branches. Unlike
iOS (which
caches `parsedData` in memory), Android **re-queries the `.pftrace`**
per
drill-down — the warm engine is the cache, so each query costs only SQL
execution
time.

### 3. Packaging & release pipeline

The WASM bundle's three third-party artifacts (`trace_processor.wasm`,
`engine_bundle.node.js`, `engine.mjs`) + `LICENSE` are **never
committed**
(`.gitignore` lists the generated files but not the dir). They're built
and
sha256-checksummed in `argent-private` CI, published to
`argent-private-releases`, and on our side:

- **`scripts/download-trace-processor.sh`** (+88, new) — `gh release
download` →
verify tarball sha256 (**FATAL on mismatch** — the blobs are unsigned) →
extract
  → re-verify via `SHA256SUMS` → assert all four present.
- **`scripts/pack-mcp.cjs`** (+60, new) — single pack entry point:
downloads
simulator-server, native binaries, **and** the trace-processor bundle,
then
builds and `npm pack`s `@swmansion/argent`. `package.json`'s `pack` /
`pack:mcp`
  now call this script.
- **`packages/argent/scripts/bundle-tools.cjs`** (heavily reworked,
+574/-...) —
copies `assets/trace-processor/`, `assets/queries/`, and
`argent.tracecfg.pbtxt`
into `packages/argent/assets/`, and regenerates `bundled-meta.ts` to
stamp the
  Perfetto version.
- **`publish.yml` / `publish-next.yml`** — run the download step and
then **verify**:
  all four WASM files present, the wasm magic-byte check
(`head -c4 … | grep $'\x00asm'`), plus the queries dir, TraceConfig,
manifest,
  and Android helper APK.

Asset reorg: `native-devtools-android` now ships `dist/`, `bin/`, and
`assets/`
(SQL queries, TraceConfig, manifest moved under `assets/`); `.gitignore`
swaps the
old per-file `packages/argent/manifest.json` entry for the whole
`packages/argent/assets/` dir.

---

## Testing

15 new test files under `packages/tool-server/test/android-perfetto/`
cover the
engine and pipeline, including:

- `dispatch.test.ts` — start/stop/analyze route to the right platform
backend.
- `aggregate-shared.test.ts` — the shared aggregator on both iOS and
Android-shaped input.
- `hang-folds-batched.test.ts` / `hang-fold.test.ts` — the batched
per-hang SQL fold.
- `hang-severity.test.ts` — jank-reason → RED/YELLOW classification.
- `dispose.test.ts` / `dispose-warm-engine.test.ts` — session dispose
kills the daemon, rms the device trace, and tears down the warm engine.
- `stop-recovery.test.ts` — `profilingActive` is forced false even if
`adb pull` fails (so a retry-start doesn't wedge).
- `run-tp.test.ts` — `renderSqlTemplate` token rendering + the "unused
substitution" guard.
- `manifest-hint.test.ts` — zero-callstack detection emits the manifest
hint.
- `profiler-load.test.ts` — `.pftrace` session restore with/without the
metadata sidecar.

Plus `native-profiler-analyze-failure.test.ts` for the
engine-unavailable banner,
and updated iOS tests for the shared-lifecycle refactor.
`@argent/native-devtools-android` gains its own vitest setup
(`vitest.config.ts`, `tsconfig.test.json`, `test/errors.test.ts`).

---

## Notes & follow-ups

- **Manifest requirement.** Perfetto's `linux.perf` data source needs
`/proc/<pid>/mem` access, granted only to a **debuggable** or
`<profileable
android:shell="true"/>` app. A release build without either silently
produces
zero callstacks; the pipeline detects this and emits a **manifest hint**
instead
  of an "All clear" report.
- **Version pinning lives in the `argent-private` submodule**
(`PERFETTO_VERSION`
/ `PERFETTO_WASM_TAG` / `LYNX_TRACE_PROCESSOR_VERSION`). Bumping is
documented in
`argent-private/docs/ANDROID_PERFETTO.md`. JS↔engine version skew is
structurally
  impossible (version-agnostic decoder + vendored wasm).
- **`disposeWarmEngine` is exported but not wired into session-stop** —
warm
engines are reclaimed only by the idle timer + LRU cap. Acceptable today
(bounded
memory); wire it into session dispose if explicit teardown is ever
needed.
- **Submodule bump.** This PR moves `packages/argent-private` to
`22e55f90` — see the companion `argent-private` PR description,
including its
merge-hygiene note (that branch should pick up latest `argent-private`
main
  before the pointer is finalized).

### Companion docs added in this branch
- `utils/android-profiler/ANDROID_PROFILER_REFERENCE.md` — stack,
capture, queries, manifest requirement.
- `utils/android-profiler/PIPELINE_DESIGN.md` — the "why" (two-file
pipeline, shared-aggregator hoist, re-query-don't-cache, batched fold).

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-09 16:06:24 +02:00
filip131311 18974cab93 fix(release): ship ax-service in bin/darwin/ so describe works in published packages (#303)
## Summary

`describe` is broken in every published package since **0.9.0**: on a
normally-booted simulator it returns only a bare `ROOT AXGroup` (empty
tree) or silently falls back to native-devtools, always accompanied by
the misleading hint *"this simulator was not booted through argent …
call boot-device with force=true"* — even right after a forced argent
boot, so there is no recovery.

**Root cause:** the published npm package is **missing the `ax-service`
binary** entirely.

## Bisect

| Release | `ax-service` in tarball | `describe` |
|---|---|---|
| 0.7.1 | `bin/ax-service` (flat)  | works |
| 0.8.1 | `bin/ax-service` (flat)  | works |
| **0.9.0** | **absent**  | **broken** |
| 0.10.0 | **absent**  | **broken** |

- **Last good release:** 0.8.1
- **First broken release:** 0.9.0
- **Breaking commit:** `80661fad` — *feat: Linux support (#249)*

## Why it broke

#249 migrated native binaries to a per-platform `bin/<platform>/` layout
and updated **two of the three** places that touch the ax-service path:

- `packages/native-devtools-ios/src/index.ts` — `axServiceBinaryPath()`
now resolves `bin/darwin/ax-service` (was flat `bin/ax-service`).
- `packages/argent/scripts/bundle-tools.cjs` — copies the published
binary **from** `bin/darwin/ax-service`.

…but it **did not** update `scripts/download-native-binaries.sh` — the
script the standard `pack` / CI release path runs — which kept
downloading ax-service to the **flat `bin/` root**.

Producer (download script) and consumer (bundler/resolver) disagreed.
During `pack`, the bundler looked under `bin/darwin/`, found nothing,
**warned and skipped the copy**, and the tarball shipped with no
ax-service. At runtime the ax-service factory throws *(binary not
found)* → `describe` catches it → empty tree + degraded hint →
native-devtools fallback (which itself fails on plain screens like
SpringBoard).

`pack:mcp:local` (which runs `build:ios-binaries` → `build.sh`, already
writing to `bin/darwin/`) was unaffected — only released/downloaded
builds were broken, which is why it wasn't caught locally.

## Fix

`download-native-binaries.sh` now downloads ax-service into
`bin/darwin/`, matching the resolver and the bundler. Plus a regression
test (`package-manifest.test.ts`) that pins the producer→consumer path
agreement so the two scripts can't silently drift again (verified it
fails on the pre-fix path and passes after).

## Verification

Rebuilt via **`npm run pack:mcp`** → tarball now contains
`package/bin/darwin/ax-service` (byte-identical, signed Mach-O). Drove
the **packaged** binary's `describe` RPC directly and via the full MCP
`describe` tool:

- iPhone 17 Pro — iOS 26.4 → `source: ax-service`, full tree (32
elements), no degraded hint 
- iPhone Air — iOS 26.4 → `source: ax-service`, full tree 
- iPhone 16 Pro — iOS 18.5 → `source: ax-service`, full tree (33
elements) 

Before the fix all three returned a bare `ROOT AXGroup` (or
native-devtools fallback) + the degraded hint.

Tests: `packages/tool-server` ax-service/describe suites (40) green;
`packages/argent` suite (12, incl. new guard) green.

## Notes
- Script-only change to release tooling + one test; no runtime TS
changes. The bundled `bin/` artifacts are gitignored (produced by
`pack`), so the diff is just the script and the test.
- Independent of `fix/boot-device-refresh-stale-services` (stale-service
eviction after reboot) — that fix can't help while the binary is absent
from the package; both are needed for fully healthy `describe`.
2026-06-03 22:28:22 +02:00
Ignacy Łątka 74c2065ca0 fix: multiple minor bugs (#194)
## Human Summary

This PR aims to fix bugs that are benign for now, could become serious
in the future:

- multiple CSRF via CORS misconfiguration on `tool-server`
- multiple SSRF that were possible due to the CSRF
- path traversal via flow creator
- multiple shell injections
- lack of auth on `MCP/CLI <-> tool-server` path (bug details discussed
in person). After the other fixes this is somewhat redundant, but
doesn't hurt and protects against theoretical access attempts from
within other processes running on the user's device.

## AI Summary

<details>
Bundles six independent fixes that together close a CSRF cluster on the
local tool-server. The vulnerabilities chain because the HTTP API has no
auth and serves wildcard CORS, so any web origin (Firefox
unconditionally; Chrome via PNA-disabled or loopback-served) can drive
the tool-server from a cross-origin tab. Each individual fix lives on
its own branch (linked below) and stands alone — this PR exists so the
cluster can be reviewed and shipped as one piece.

### Fixes

1. **`fix(http): require auth token + drop wildcard CORS`** —
per-process 32-byte hex Bearer token, generated by the launcher,
persisted in `~/.argent/tool-server.json` with mode `0600`, required on
every route except `/preview`. Constant-time compare. Wildcard CORS
dropped. Threaded through MCP server + CLI + tools-client.
Backwards-compat: when `ARGENT_AUTH_TOKEN` is unset (e.g. `npm run
dev`), the server logs a one-shot warning and skips the gate.
2. **`fix(http): reject non-loopback Host headers`** — middleware
(before the auth gate) returns 403 for any request whose Host is not
`127.0.0.1`, `localhost`, or `::1`. Closes DNS-rebinding bypass.
3. **`fix(ios-profiler-start): prevent shell injection via device_id`**
— `detectRunningApp(udid)` no longer interpolates `params.device_id`
into `execSync` template strings. Both `xcrun simctl spawn ${udid} …`
and the piped `xcrun simctl listapps ${udid} | plutil …` are rewritten
to `execFileSync` array form, with the pipe to `plutil` happening in JS
(input option) rather than the shell.
4. **`fix(flows): validate name to close path-traversal in
getFlowPath`** — `getFlowPath()` requires `^[A-Za-z0-9_\\-]+$` and
verifies `path.relative(getFlowsDir(), filePath)` does not start with
`..`. Centralizing the check in `getFlowPath` covers every caller
(start-recording, add-step, insert-echo, finish-recording, run,
read-prerequisite) without per-tool churn.
5. **`fix(source-resolver): reject paths outside projectRoot +
non-source extensions`** — `readSourceFragment()` previously accepted
any `file` field on the SourceLocation it was given (the field traces
back to a React fiber's `_debugSource.fileName`, which is
attacker-controllable). Now: containment check via
`path.relative(projectRoot, abs)` plus an extension allowlist
(`.js/.jsx/.ts/.tsx/.mjs/.cjs/.json`).
6. **`fix(source-maps): reject non-loopback URLs (close SSRF)`** —
`doRegister()` validates `sourceMapURL` before fetching: only
`http://`/`https://` URLs whose hostname is loopback. Closes a
blind-SSRF probe of the host network (cloud metadata at
`169.254.169.254`, intranet pages, etc.).

### Individual branches

- `fix/auth-token-drop-cors`
- `fix/host-header-validation`
- `fix/profiler-command-injection`
- `fix/flow-name-validation`
- `fix/source-resolver-traversal`
- `fix/source-map-ssrf`

If you want to land them as separate PRs, the integration branch can be
dropped after each lands.

## Test plan

Unit tests (already in CI):

- [x] `npm run build` passes (workspace tsc clean)
- [x] `npm run test -w @argent/tool-server` — **510 passed (was 473 on
`main`)**, +37 new tests across:
- `test/http-auth.test.ts` — 10 tests (auth, CORS removal, dev-mode
escape hatch)
- `test/http-host-validation.test.ts` — 7 tests (loopback allow,
DNS-rebinding reject)
- `test/flows/flow-utils.test.ts` — +5 tests (path-traversal name
rejection)
- `test/metro/source-resolver.test.ts` — +5 tests (containment +
extension allowlist)
- `test/metro/source-maps-ssrf.test.ts` — 10 tests (loopback-only allow)

Live end-to-end (run locally against this branch's bundled artifacts):

- [x] Launcher spawns tool-server; `~/.argent/tool-server.json` contains
a 64-char hex token and is mode `0600`
- [x] `GET /tools` without `Authorization` → **401**
- [x] `GET /tools` with wrong token → **401**
- [x] `GET /tools` with correct token → **200** (61 tools)
- [x] Request with `Host: attacker.example` → **403**
- [x] No `Access-Control-Allow-Origin` header on responses
- [x] Cross-origin command-injection probe via `device_id=\"x;touch
/tmp/marker;#\"` does NOT create the marker file (Vuln 4 closed)
- [x] `flow-start-recording` with `name=\"../../../tmp/escape\"`
rejected with `Invalid flow name`
- [x] `flow-start-recording` with a valid name still works
- [x] `tools-client.fetchTools()` and
`tools-client.callTool('list-simulators')` succeed via the auth-token
threading (the path the MCP server uses internally)
- [x] MCP-stdio entry point (`startMcpServer`) end-to-end: `initialize`
→ `tools/list` (61 tools) → `tools/call('list-simulators')` returns
content. Confirms the launcher → token → fetch → tool-server flow is
intact for the production `argent mcp` path.

## Notes for reviewers

- `/preview` is intentionally exempt from the auth gate (it was not
gated before; tightening it deserves a separate PR).
- The `ARGENT_AUTH_TOKEN`-unset escape hatch keeps `npm run dev`
workable; production `argent mcp` always passes a token via env, so the
warning never fires there.
- The auth bundle changes the public return shape of `ensureToolsServer`
from `Promise<string>` to `Promise<{ url, token }>`. The two in-tree
callers (`argent-mcp/mcp-server.ts`,
`argent-tools-client/tools-client.ts`) were updated; external consumers
using `ARGENT_TOOLS_URL` should also set `ARGENT_AUTH_TOKEN` to match
the server.

</details>
2026-06-03 15:57:42 +02:00
Ignacy Łątka 80661fad8f feat: Linux support (#249)
This PR makes Linux work.

# AI Summary 

<details>
Brings Argent up on Linux as a host: per-platform npm bundle, renderer
selection that actually works on hybrid-GPU / Wayland / containerized
Linux, hot-boot guarding that survives the SwiftShader composite gap, a
real Linux preflight, and a Wayland-headless E2E job in CI. macOS stays
on
the path it was on before (no `-gpu` flag was passed; AVD config
decided);
nothing about the iOS Simulator behavior changes.

The diff is broader than "ship the Linux artifact" — calling it out
below
because several pieces affect macOS too and reviewers should be primed
for
that rather than skim past expecting "just packaging."

### What's actually in here

**npm packaging (the original framing — still load-bearing)**

| File | Change |
|---|---|
| `packages/argent/scripts/argent-simulator-server.cjs` (new) | Node
dispatcher exposed as the `argent-simulator-server` bin entry. Joins
`process.platform` into the bin path and execs. Forwards
SIGTERM/SIGINT/SIGHUP parent→child so non-TTY supervisors can't orphan
the binary. |
| `packages/argent/scripts/bundle-tools.cjs` | Copies per-platform
binaries from `packages/native-devtools-ios/bin/<platform>/` into
`packages/argent/bin/<platform>/`. darwin required, linux best-effort. |
| `packages/argent/package.json` | `bin.argent-simulator-server` points
at the dispatcher. |
| `packages/native-devtools-ios/src/index.ts` | Resolver joins
`process.platform` into the simulator-server bin path. iOS-only dylibs
and `ax-service` throw `"requires a macOS host"` on Linux callers so
they get the root cause, not "file not found". |
| `scripts/download-simulator-server.sh` | Downloads
`simulator-server-argent-{macos,linux}` into platform-keyed subdirs.
**Purges each platform dir before its download attempt** so a failed
download can't leave stale binaries in place from a prior run. |
| `scripts/dev.cjs` | Local dev copies the current host's binary into
`bin/<process.platform>/` to match the resolver layout. |
| `.github/workflows/publish{,-next}.yml` | Replaces single `test -x
darwin/...` with a per-platform inventory loop. darwin always required;
Linux gated by `ARGENT_REQUIRE_LINUX_BINARY=1` (flip once radon#118
lands). |

**Linux runtime — emulator boot path**

| File | Change |
|---|---|
| `packages/tool-server/src/tools/devices/boot-device.ts` |
`selectGpuMode()`: Linux defaults to `-gpu swiftshader` (`auto` lands on
lavapipe with ~10× cold-boot slowdown; `-gpu host` produces silent
black-window failures on Optimus / NVIDIA+libglvnd / Wayland-on-hybrid /
containers). macOS keeps `auto` (ANGLE→Metal).
`ARGENT_EMULATOR_GPU_MODE` overrides, **validated against `emulator
-help-gpu`'s whitelist** so a typoed value fails fast instead of burning
the whole boot budget. `ARGENT_EMULATOR_NO_WINDOW=1` appends
`-no-window` for headless contexts (CI / containers / Wayland sessions
with no Qt wayland plugin). |
| `packages/tool-server/src/tools/devices/boot-device.ts` (cont.) |
**Cold-boot:** `awaitFirstRealFrame` polls SwiftShader's lockscreen
composite (PNG byte-count via `screencap -p`); blocks `booted:true`
until a real frame shows up. Without this, callers chaining `boot-device
→ screenshot` get an all-black PNG because `sys.boot_completed=1` fires
5–60 s before the first composite. |
| `packages/tool-server/src/tools/devices/boot-device.ts` (cont.) |
**Hot-boot:** `assertScreencapAlive` polls within an 8 s budget instead
of single-probing. The earlier single-probe caught the sticky-blank
state but also killed the emulator on every transient snapshot-restore
blank, forcing "every boot is cold" cycles. Sticky vs transient is
discriminated by whether the budget exhausts. |
| `packages/tool-server/src/utils/linux-preflight.ts` (new) |
`linuxBootDiagnostics()` checks `/dev/kvm` access. `diagnoseAvdSizing()`
warns when a target AVD's `hw.ramSize`/`vm.heapSize` is below the floor
an aggressive RN/Expo app needs (2G/228M avdmanager defaults wedge under
load). |
| `packages/tool-server/src/utils/android-binary.ts` |
`defaultAndroidRoots()` probes typical SDK install locations when
`ANDROID_HOME`/`ANDROID_SDK_ROOT` are unset — **cross-platform**,
benefits macOS launches from GUI-launched MCP hosts (Claude Code Desktop
etc.) where shell rc env doesn't propagate. |

**Cross-platform fixes that landed alongside**

| File | Change |
|---|---|
| `packages/argent-installer/src/utils.ts` | `getLatestVersion()` now
redirects `npm view`'s stderr to `ignore` so `EBADDEVENGINES` warnings
from a consumer project's `package.json` (e.g. Bluesky's pnpm pin) don't
leak to the user's terminal. **Cross-platform fix**, not Linux-specific.
|

**CI**

| File | Change |
|---|---|
| `.github/workflows/wayland-e2e.yml` (new) | Boots an AVD through the
tool-server under headless Weston; asserts `hw.gpu.mode = swiftshader`
and that screenshots + gesture-tap work without a display. Currently
runs on `pull_request` for the affected paths + `workflow_dispatch`.
Pinned to `radon-daily` for the simulator-server until radon#118 ships a
Linux asset to `radon-main`. |

### Tests added in this PR

- `packages/argent/test/dispatcher.test.ts` — e2e against tmpdir-staged
dispatcher + fake `<platform>/simulator-server`: missing-binary error
path, exit-code passthrough, argv passthrough, SIGTERM forwarding.
- `packages/tool-server/test/native-devtools-ios-resolver.test.ts` —
`requireDarwin` gates for dylibs and `ax-service` on Linux, per-platform
`simulator-server` path resolution, missing-binary error,
`ARGENT_NATIVE_DEVTOOLS_DIR` override.
- `packages/tool-server/test/linux-preflight.test.ts` — KVM diagnostic
shape + `diagnoseAvdSizing` (ram low, heap low, both, neither, missing
keys).
- `packages/tool-server/test/boot-device-hotboot.test.ts` — extended
with `it.each([linux→swiftshader, darwin→auto])` so a single CI runner
exercises both GPU branches (was runner-platform-dependent, regressions
could pass on both runners individually). New cases for the wedged-frame
fallback (under fake timers because the polling budget is short but
real-time enough to exceed vitest's 5 s default), the GPU-mode whitelist
rejection, and the cold-boot composite-wait timeout.
- `packages/tool-server/test/android-binary.test.ts` — extended for
`defaultAndroidRoots` probe ordering and HOME pinning.

### Verification

- `tsc --build`: clean apart from pre-existing
`@argent/native-devtools-android` import errors unrelated to this
branch.
- `vitest run` in `packages/tool-server` and `packages/argent`: 689 + 11
= 700 tests passing; the 4 pre-existing test-file load failures
(native-devtools-android missing) reproduce on `main` and are out of
scope.
- `npm pack -w @swmansion/argent`: produces `swmansion-argent-0.8.0.tgz`
with the per-platform layout.
- Wayland-e2e: triggered on this branch — link will be in a PR comment
once the run completes.
- Manually verified end-to-end on a Wayland-on-hybrid-GPU developer
laptop: AVD boots through the tool-server with `-gpu swiftshader`,
screencap returns real frames, gesture tools work.

### Dependencies / what still gates this

- **radon#118** must land before the Linux artifact ships through the
default channel. The download script tolerates the asset being absent
today (best-effort), so this PR is technically safe to merge before —
Linux installs just won't get a working `simulator-server` until the
upstream artifact is published. Once it lands:
  - Drop the `radon-daily` arg in `.github/workflows/wayland-e2e.yml`.
  - Flip `ARGENT_REQUIRE_LINUX_BINARY` to `1` in both publish workflows.
- **wayland-e2e is not a required status check** today (no branch
protection on `main` requires it). Worth turning on before merging
anything else that touches the Linux boot path.

### Related

- radon#118 — produces the `simulator-server-argent-linux` artifact this
PR consumes.
- argent-private#11 — cross-builds the iOS Simulator dylibs on Linux.
Strictly orthogonal: those dylibs are macOS-runtime-only regardless of
where they're built, and this PR keeps them under `dylibs/` with a clean
error path on Linux callers.

</details>

Fixes #246
2026-06-03 15:55:39 +02:00
Paweł Fornagiel a96f1f1578 feat: refactor android describe to not rely on uiautomator (#237)
## Summary

- `formatDescribeTree` was branching on `source === "uiautomator"` to
pick nested rendering; the newer `android-devtools` source fell through
to flat mode and lost every node below depth 1. Extends the predicate to
cover both Android sources so the helper-service path renders with the
same hierarchy as the legacy `uiautomator dump` path.
- Bumps `packages/argent-private` to pick up the persistent Android
describe service that emits `source: "android-devtools"`, plus its
build-and-sign CI workflow.
- Wires the Android helper APK + `manifest.json` into the published
`@swmansion/argent` package via `bundle-tools.cjs`, and adds
`pack:mcp:local` / `build:android-binaries` so a local tarball can be
produced without going through `download:native-binaries`.
- Minor: tightens a couple of comments in `android-helper-install.ts`
and `format-tree.ts`.

This is connected to argent-private PR:
https://github.com/software-mansion/argent-private/pull/8
2026-05-27 11:46:12 +02:00
Ignacy Łątka 2180dd8e6a ci: typecheck test files in CI (#202)
We've had lots of type errors in our tests, scripts and other non-core
code despite all CI being green. This PR fixes that.

This PR also removes some redundant features like `@ts-check` inline
directives which have been replaced by proper ts config files.

<details>

## AI Summary

Audit of every CI workflow against every package's `package.json` and
`tsconfig` surfaced several gaps where CI did partial coverage. This PR
closes them and fixes every pre-existing type error the new gates
surfaced.

## Gaps closed

| # | Gap | Fix |
|---|---|---|
| 1 | `tsc --build` only covers `src/**`; test files (`test/**`,
`tests/**`) never typecheck. Vitest transforms tests with esbuild —
strips types instead of checking them. | New `tsconfig.test.json` per
test-bearing package (`composite: false`, `noEmit`, `rootDir: "."`,
includes `src/**` + `test/**` (or `tests/**`) + `vitest.config.ts`). New
`typecheck:tests` npm script per package. New CI step `npm run
typecheck:tests --workspaces --if-present`. |
| 2 | Build/publish scripts (`scripts/*.{cjs,mjs}`,
`packages/argent/scripts/*.cjs`, `packages/skills/scripts/install.js`) —
JS, never typechecked. `sync-readme.cjs` runs in `prepack`;
`postinstall.cjs` runs on every install; `bundle-tools.cjs` is
build-critical. A typo only surfaces at runtime. | Root
`tsconfig.scripts.json` with `allowJs` + `checkJs` + `noImplicitReturns`
covers all script paths. Root `typecheck:scripts` npm script. New CI
step. Per-file `// @ts-check` comments removed — tsconfig drives
inclusion. |
| 3 | `argent-cli` had no `typecheck:tests` script. When tests get added
there they'd silently bypass the gate. | Added `tsconfig.test.json` +
`typecheck:tests` script. `--if-present` would have skipped it anyway,
so this is future-proofing. |
| 4 | Local `prettier --check .` walked into the `argent-private`
submodule and reported violations there. CI didn't see this because
`actions/checkout@v4` doesn't init submodules — so local `prettier
--check` did not match CI behavior. | New `.prettierignore` excludes the
submodule plus `dist/`, `node_modules/`, lock files, `tsbuildinfo`.
No-op in CI; aligns local with remote. |

## Test type errors fixed

These were already present. The `npm test` passed because esbuild
stripped the offending types:

- **argent-installer**: widen `scope` literal type for
dead-code-elimination guard; non-null `addAllowlist!`/`removeAllowlist!`
(optional methods on adapter).
- **argent-mcp**: add `.js` extensions for NodeNext module resolution.
- **registry**: broaden `StaticBlueprintResult` blueprint api from `{
id; deps? }` to `Record<string, unknown>` so existing tests can pass
arbitrary api shapes.
- **tool-server**:
- non-null `zodSchema!` on tool defs that always carry one
(`launchAppTool`, `restartAppTool`, etc.)
- cast sentinel `"ignored"` strings to `DeviceInfo` in factory-rejection
tests
- match the 4-arg `dispatchByPlatform<IosServices, AndroidServices,
Params, Result>` signature (tests still passed the old 3-arg shape)
- `assertFlowRunResult` type guard for the `FlowRunResult |
FlowPrerequisiteNotice` union before `.steps` access
- swap broken `typeof import("supertest").default` for static `import
supertest` (supertest is `export = supertest`)
- fill in the 4 fields missing from a `NativeProfilerSessionApi` mock
(`xctraceProcess`, `recordingTimedOut`, `recordingExitedUnexpectedly`,
`lastExitInfo`)
- cast a vitest `Mock` to a plain function at one direct call site where
the `Procedure | Constructable` union loses callability
- fix `update-checker.test.ts` reading `../../package.json` (off-by-one
— went past tool-server; was relying on a Vite resolver quirk)
- replace one cross-package `../../../registry/src/index` import with
`@argent/registry` so registry's private `services` field isn't compared
between src and dist declarations

All fixes are type-level. Full `npm test --workspaces --if-present`
still reports **991 passed across 87 test files**.

</details>
2026-05-20 17:03:34 +02:00
filip131311 6355620c4d refactor(mcp): split mcp package into argent, argent-cli, argent-installer, argent-mcp, argent-tools-client (#162)
## Summary

- Splits the monolithic `packages/mcp` package into five focused
packages:
- `argent` — the user-facing CLI entry (`argent` binary,
init/uninstall/update flows, bundled-paths, scripts).
- `argent-cli` — CLI runner internals (flag-parser, run, server, tools).
- `argent-installer` — `init`/`update`/`uninstall` flows, MCP config
adapters, skills installer.
- `argent-mcp` — MCP server itself (mcp-server, tool-mapping,
auto-screenshot, content adapters).
  - `argent-tools-client` — HTTP client + launcher for the tool-server.
- Adds Gemini agent definitions, VSCode MCP config, and per-package
`tsconfig.json` / `vitest.config.ts`.
- Updates root `tsconfig.json`, `scripts/dev.cjs`, CI workflows, and
`package.json` to wire the new packages together.

## Test plan

- [ ] `npm install` resolves the new workspaces cleanly
- [ ] `npm run build` succeeds across all packages
- [ ] `npm test` passes (renamed test files keep their assertions)
- [ ] `npx argent init` still produces the expected MCP config in a
sample project
- [ ] `npx argent update` / `argent uninstall` exercise the installer
package
- [ ] CI workflows (`publish.yml`, `unit-tests.yml`) run on this branch
2026-04-28 08:36:24 +02:00
filip131311 9fee899e5f chore: bump package versions to 0.5.3 for mcp, registry, skills, and tool-server (#149) 2026-04-20 17:51:14 +02:00
pFornagiel 066aad2452 fix: fix problematic CLI install, bump package versions to 0.5.2 for mcp, registry, skills, and tool-server 2026-04-16 17:47:25 +02:00
filip131311 3979e2780d bump version (#136)
* init

* chore: update package versions to 0.4.5 for registry, skills, and tool-server

---------

Co-authored-by: pFornagiel <pawel.fornagiel2@gmail.com>
2026-04-16 14:15:44 +02:00
Filip131311 ea99e945f8 fix: default download:native-binaries tag to argent-main 2026-04-16 00:17:59 +02:00
filip131311 7db8305fff feat: pin native binaries to versioned release tags and publish to npm (#125)
- download-native-binaries.sh now requires an explicit release tag (e.g.
  argent-v0.4.3) and fails fast with a clear error if the release doesn't
  exist in argent-private-releases
- publish.yml is now manual-only (workflow_dispatch) with a ref input
  (branch/tag/SHA); derives the binaries tag as argent-{ref} and checks
  out the repo at that ref
- publish.yml now targets npm (registry.npmjs.org + NPM_TOKEN) instead
  of GitHub Packages
- download:native-binaries npm script auto-derives the tag from
  packages/mcp/package.json version for local pack:mcp usage
2026-04-15 16:26:55 +02:00
Filip131311 f30948fe46 feat: download native binaries from argent-private-releases instead of building locally 2026-04-15 11:41:10 +02:00
Filip131311 d082ea2fd2 feat: download simulator server 2026-04-14 15:09:58 +02:00
Filip131311 faf6911801 Fix: removeing other dylibs injected into the simulator (simCam compatibility) 2026-04-10 16:38:55 +02:00
Paweł Fornagiel 167ccf409d fix: move simulator-server binary to native-devtools-ios and fix esbuild bundling (#82)
* chore: update simulator-server path references across documentation and scripts

* feat: add native-devtools-ios entry to bundle-tools script
2026-04-07 18:25:48 +02:00
filip131311 d9dc17d0d5 fix: move describe to native devtools (#83)
* fix: move describe to native devtools

Replace the AX describe path with an app-scoped native hierarchy adapter so coordinate discovery stays on the supported injection flow.
Also add safer targeting and restart guidance to reduce ambiguous Home and cold-start recovery paths.

Made-with: Cursor

* init

* update sim server

* Delete NATIVE_DEVTOOLS_DESCRIBE_MIGRATION_NOTE.md
2026-04-07 17:36:56 +02:00
Filip131311 4e514c1435 chore: rename package from @software-mansion/argent to @swmansion/argent 2026-04-03 17:16:39 +02:00
Filip131311 8c2a313c76 chore: format codebase with Prettier
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-03 14:53:58 +02:00
Filip131311 47b66b3eed feat: add native devtools iOS — view hierarchy, network inspection, passive simulator watcher
- Adds native-devtools-ios package with pre-built dylibs (libNativeDevtoolsIos, libKeyboardPatch, libInjectionBootstrap)
- Passive simulator watcher injects dylibs into all booted simulators on tool-server startup, eliminating manual restart requirement
- Lazy network activation: NSURLProtocol interception is opt-in per bundleId via Control socket command, re-activated on reconnect
- New tools: native-find-views, native-full-hierarchy, native-network-logs, native-devtools-status
- dev script: non-fatal argent-private submodule (uses pre-built dylibs when source unavailable)
- Adds Prettier config and format script
- Updates CONTRIBUTING.md with npm run dev workflow and project structure

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-03 14:53:45 +02:00
Filip131311 d4b4ebfdee chore: move native-devtools-ios ObjC source to argent-private submodule
- Add argent-private as git submodule at packages/argent-private
- Remove Sources/ from packages/native-devtools-ios (now in submodule)
- Update build.sh to resolve SRC_DIR from submodule path with clear
  error message if submodule is not initialised
- Update scripts/dev.cjs to run git submodule update --init before
  building dylibs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-03 14:53:32 +02:00
filip131311 2ba7ab6bba feat: add dev mode script for local development without packing (#72)
Adds `npm run dev` which:
- Builds MCP TypeScript (tsc only, skips esbuild tool-server bundle)
- Copies simulator-server binary and skills/rules/agents into mcp package
- Patches ~/.claude.json to point argent MCP at the local dist/cli.js
- Starts tool-server from source via ts-node, writes ~/.argent/tool-server.json
- On exit: restores ~/.claude.json and stops the tool-server
2026-04-02 13:02:45 +02:00