mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
fix/flow-batch-invalid-verdict
714 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Ignacy Łątka
|
476c07ecbd | chore: retrigger CI | ||
|
Ignacy Łątka
|
90e062c562 |
docs(flow): split the aggregate error sentence in two
Keep the condition and its examples as separate sentences, under the page's one-idea-per-sentence limit. |
||
|
Ignacy Łątka
|
a821baaaa0 |
docs(flow): note that a batch --json entry holds error for any no-report call
The --json aggregate paragraph said an entry holds error when Argent "got no report", which reads as only the no-run-report outcome. A rejection and a run error produce no report too and carry error (and error_code/error_kind); a flow that produced a report - passing or failing its steps - carries report, not error. Enumerate the three no-report cases and pin report to a produced report. |
||
|
Ignacy Łątka
|
f6daf3e2b6 | Merge branch 'main' into fix/flow-batch-invalid-verdict | ||
|
Sebastian Flajszer
|
1068430b91 |
feat(ios-device): small fixes related with ios-device support (#1031)
Follow-up to #944: the fixes made after the review that did not make the squash. ## What changes - **Signing note**: delivered only to a call whose device argument resolves to a physical iPhone, including `flow-execute`'s `device`, and held (not dropped) across other calls. The label is now `Apple Development (KEYID)`; the certificate CN's Apple ID no longer reaches agent context or logs. - **Flow device resolution**: the no-device-matched refusal names the cabled phone and the exact `device <udid>` / `--device` spelling. A physical iPhone is never picked automatically. - **`gesture-swipe`**: the description states the hardware semantics (edge gesture needs `fromX` 0, `durationMs` sets drag speed, `momentum:false` does not damp). - **Skills**: `argent-ios-simulator-setup` skips `kind: "device"` entries; `argent-ios-device-setup` and `-interact` state the runner build timing, the full list of tools that do not exist on hardware, the backgrounded-target behaviour of each read tool, `actionButton` only on models that have one, and the swipe and `open-url` rules; the flow, Metro and profiler skills each carry one line on physical-iPhone limits. - **Docs**: `physical-ios-devices`, `flows`, `flow-yaml` and `tools` corrected to match the behaviour above; README platform row; runner `PROTOCOL.md`. ## Verification - tool-server suite 400 files / 5173 tests, `tsc`, lint, prettier and the docs build clean. - Live re-check against 0.23.0 on three simulators and a physical iPhone 15: no regression; the note lands once on the first phone call with no e-mail address, and flow auto-detection is byte-identical to 0.23.0 with 0, 1 and 3 simulators booted.v0.24.0 |
||
|
Ignacy Łątka
|
f106e486e0 |
fix(tools-client): make the server identity guard survive a clipped ps and a sanitized PATH (#713)
## Problem
`processCommandMatches` reads a tool-server's command line to confirm a
pid is one of ours before signalling it — the guard that stops
kill-before-respawn from killing an unrelated process that inherited a
recycled pid.
It read that command line with:
```ts
execFileSync("ps", ["-p", String(pid), "-o", "command="], …)
```
Two things are wrong with that call, and they fail the same way.
**1. procps-ng truncates `ps -o command=` to `$COLUMNS`.** When the
recorded bundle path does not fit that width, the command line comes
back clipped, never matches its own marker, and the function returns
`false`.
**2. A bare `"ps"` resolves through `PATH`.** An MCP server launched
from a GUI / launchd context inherits a sanitized `PATH` that omits
`/bin`, where the spawn `ENOENT`s — and the `catch` turns that into the
same `false`, on *every* call rather than only a long path.
`false` is the fail-safe answer — *don't kill something you can't
identify* — which is right when `ps` is genuinely missing or
unsupported. In both cases above it is wrong, and it fails in the
expensive direction: the kill is skipped, a second server spawns, and
**the live one is orphaned**.
Measured on Linux (procps-ng 4.0.4, stdout piped, same pid):
```
COLUMNS=<unset> plain_len=157 ww_len=157
COLUMNS=40 plain_len=40 ww_len=157
COLUMNS=80 plain_len=80 ww_len=157
```
and, replicating the guard's own `execFileSync` with
`PATH="/usr/local/bin"`:
```
bare "ps" → throws ENOENT
"/bin/ps" → OK
```
## The twin already had both
`packages/tool-server/src/utils/vega-process.ts` pins the width flag in
both its arg sets:
```ts
export const PS_ARGS = ["-A", "-ww", "-o", "command="] as const;
export const PS_ARGS_WITH_PID = ["-A", "-ww", "-o", "pid=,command="] as const;
```
…and resolves the binary absolutely:
```ts
export const PS_BIN = ["/bin/ps", "/usr/bin/ps"].find((p) => existsSync(p)) ?? "ps";
```
That `PS_BIN` pin landed in #508, which fixed the identical
sanitized-`PATH` failure across all four `ps` spawn sites in
`tool-server` (a VVD silently listed twice). `launcher.ts` lives in a
different package and was the one site left behind, so this PR takes
both halves. `grep '"ps"'` over the tracked sources now returns no
bare-`ps` production call site anywhere in the repo.
## Why no existing test caught it
Six tests across `launcher-sweep`, `launcher-duplicate-spawn` and
`launcher-version-gate` do fail without `-ww` — but only when `COLUMNS`
happens to be narrow in the environment running them. Nothing sets it,
so on a normal-width terminal or in CI the whole suite is green either
way. Nothing at all covered the `PATH` half.
## The tests
Four, all in `launcher-sweep.test.ts`. Each spawns a real `node <bundle>
start` keepalive, writes a state record, and runs
`sweepDeadStateFiles()`.
**`terminates a live server whose bundle path is longer than the
terminal is wide`** narrows `COLUMNS` to 40 for the duration (the
`execFileSync` child inherits it) and restores the ambient value
afterwards, distinguishing "was unset" from "was set to something".
It **skips where `ps` ignores `$COLUMNS`**. BSD `ps` clips only to a
*terminal* width, and the guard hands it `stdio: ["ignore", "pipe",
"ignore"]` — never a terminal — so on macOS this assertion holds against
the un-hardened guard too. `psClipsToColumns()` probes that with the
guard's own binary and options against the live pid, rather than reading
`process.platform`, so the test says "this platform cannot observe the
bug" instead of passing vacuously. On CI (`unit-tests.yml` →
`ubuntu-latest`) it runs: the job log shows `test/launcher-sweep.test.ts
(10 tests)`, none skipped.
Its fixture is also sized past **132**, not just past `COLUMNS`.
procps's single `-w` widens to `max($COLUMNS, 132)` rather than lifting
the limit, so a shorter command line cannot tell `-ww` from a
one-character regression to `-w` — straight back to the orphaning bug,
green. The test asserts `` `node ${bundle} start`.length >
PS_SINGLE_W_FLOOR ``, so a future fixture rename that shortened the path
fails rather than quietly making it vacuous.
**`terminates a live server when PATH omits every directory holding
\`ps\``** points `PATH` at a directory that does not exist for the
duration and restores it the same way. It discriminates on every
platform, macOS included.
**Two look-alike tests** cover the guard's structural match, which is
what `-ww` makes load-bearing: command lines the width limit would have
cut off before they could collide now reach the regex whole, and neither
half of it was pinned — both could be deleted with the suite green.
- `does not signal a process that merely mentions the bundle path` — a
live process carrying the recorded path in its argv without running it.
Red when the trailing `` ` start` `` requirement is dropped, which would
SIGTERM a user's `tail -f <bundlePath>`.
- `does not signal a different install whose bundle path ends with ours`
— a second install nested under a longer prefix. Red when the leading
`(?:^|\s)` boundary is dropped, which would SIGTERM a live server
another session is using.
Both record a bundle path that is **not** on disk and assert it, because
`sweepDeadStateFiles` short-circuits on one that is and would never
reach the guard. That assertion is the pin: with a shared fixture path
and only declaration order keeping them apart, swapping the two blocks
made the first pass against a guard that had lost the exact behaviour it
names.
## Verification
Six mutations, each redding **exactly one** test and leaving the other
nine green — which is why each test had to be added. Linux runs are
`node:24-bookworm`, procps-ng 4.0.2 (baseline there: `10 passed (10)`).
| mutation | red test | where |
|---|---|---|
| drop `-ww` | `…longer than the terminal is wide` | Linux |
| `-ww` → `-w` | `…longer than the terminal is wide` | Linux |
| `PS_BIN` → bare `"ps"` | `…PATH omits every directory holding \`ps\``
| Linux + macOS |
| drop the regex's `(?:^\|\s)` anchor | `…install whose bundle path ends
with ours` | macOS, in **both** test orders |
| drop the regex's `` ` start` `` tail | `…merely mentions the bundle
path` | macOS, in **both** test orders |
| ps `timeout: 2_000` → `1` | 3 tests (macOS) / 4 (Linux) | both |
- Unmutated: Linux `10 passed (10)` — nothing skipped, so CI gates all
of it; macOS `9 passed | 1 skipped (10)`.
- The probe was checked against the environment it exists for: with
`PATH` holding only node's directory (no `ps`, `/bin/ps` present), the
suite is `9 passed | 1 skipped`; resolving the probe through `PATH`
instead of `PS_BIN` there gives `Error: spawnSync ps ENOENT`, `1
failed`.
- Package suite: `12 files, 194 passed | 1 skipped (195)` on macOS.
- `eslint --max-warnings 0` 0, `prettier --check` 0, `tsc --build` 0,
`tsc -p tsconfig.test.json --noEmit` 0, `npm run knip` 0 — exit codes
read directly, after `npm run build` (the type-aware lint rules report
spurious errors against an unbuilt workspace).
## Note on the tests' timeout
`waitForExit`'s window exceeds vitest's 5 s default, so the tests carry
an explicit `20_000`. That is what makes a mutation fail as `expected
false to be true` — an unterminated server — rather than as an
uninformative bare timeout.
## Found alongside, filed separately
Out of this PR's scope, all reproduced: #832 (on Windows,
`ensureToolsServer`'s kill-before-respawn is the one of three call sites
without the `win32` guard, so the wedged server is never killed before
the respawn), #833 (`LOCK_WAIT_TIMEOUT_MS`'s budget comment counts one
`ps` where the critical section runs three, so its "worst case ≈ 26s" is
not a bound), and #834 (`stop-metro` swallows a missing `lsof` into
`stopped: false`, reporting "port is free" when it could not look — the
same PATH-resolved-silent-failure class this PR fixes for `ps`).
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Bug Fixes**
- Improved detection of the correct background server process, including
long command lines and shell-wrapped processes.
- Prevented unrelated or similarly named processes from being stopped
during cleanup.
- Improved recovery when an automatically started server becomes
unresponsive or cannot be verified.
- Added more reliable handling for restricted environments, unusual
installation paths, and processes with similar names.
- Improved diagnostics when process verification encounters command-line
inspection errors.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Sebastian Flajszer
|
1e69c90981 |
feat(ios-device): add physical iOS support (#944)
# Physical iOS device support Try it now (cabled iPhone, unlocked, Developer Mode on): ```bash argent run list-devices # copy the 25-char UDID argent run launch-app --udid <UDID> --bundleId com.apple.Preferences ``` First launch builds and signs the on-device runner: 3 to 5 minutes, keep the phone unlocked. Every later start: seconds, from cache. No signing setup needed if an Apple ID is signed into Xcode; the team is detected from the keychain. ## What this adds - Physical iPhone support, on by default: a cabled iPhone simply appears in `list-devices`. iPhone-only: physical iPads are skipped at discovery. - 15 tools ported to hardware. Flows replay on device. - Zero-config signing from the keychain; `ARGENT_IOS_TEAM_ID` is an override, nothing is persisted. - Self-contained npm install: the runner's Swift sources ship in the package. New package `@argent/ios-device-runner` (an XCUITest bundle hosting an HTTP command server on the device), new tool-server backend under `src/utils/ios-device/`. - The one always-on cost: `list-devices` on macOS now runs physical discovery in parallel with the other platform probes, under the existing 8s devicectl timeout; off-macOS it early-returns. ## Suggested review order Rough estimate: half a day for the full pass. 1. `packages/ios-device-runner/PROTOCOL.md`, then `README.md` (wire contract and reliability model, ~15 min) 2. `packages/tool-server/src/utils/ios-device/` (devicectl, usbmux, team-detect, runner-build, runner-commands: the core) 3. `packages/tool-server/src/blueprints/ios-device-runner.ts` (service lifecycle, crash recovery) 4. Per-tool device arms and the capability gates (`ios-physical-device-classification.test.ts` is the ratchet) 5. Docs page, the two new skills, the Swift runner sources ## Requirements 1. macOS host with full Xcode (iOS platform installed); uses `xcrun devicectl` and `xcodebuild`. 2. An Apple ID signed into Xcode; automatic signing mints the runner's provisioning profile. 3. USB cable only. Wi-Fi is deliberately unsupported: the transport is usbmux, and a Wi-Fi-only device lists as `paired`, not `connected`. 4. iPhone paired with and trusting the Mac, Developer Mode on, A12/2018 or newer (modern 8+16-hex UDID, the CoreDevice floor). Legacy 40-hex UDIDs are not classified as iOS. 5. Signing resolves as: `ARGENT_IOS_TEAM_ID` in the tool-server's environment (explicit override, CI) -> keychain detection (newest identity first, memoized per server run, never persisted). One certificate: nothing to do. Several teams: newest wins; the first tool result names the pick, the runner-ups, and the override. No certificate: the error walks through Xcode > Settings > Accounts and Manage Certificates > + > Apple Development, then retry; it never names the env var, since a team id alone cannot sign. Bundle ids derive per team (`com.argent.runner.t<teamid>`). `ARGENT_IOS_RUNNER_PROJECT` overrides the project path for unusual layouts. Build artifacts cache under `~/.argent/ios-device-runner`, keyed by runner sources, Xcode/SDK version, and signing. A team with no registered devices self-heals: the failed generic build retries against the connected phone, which registers it. ## How it works 1. `xcrun devicectl` (CoreDevice) is the control plane: discovery, readiness, app lifecycle. Readiness gates on the wired transport, so an unplugged phone is refused before any build starts. 2. A signed XCUITest runner is built lazily with the resolved team, started via `xcodebuild test-without-building`, and parks as an HTTP command server for 24h. On an npm install the Xcode project is the copy shipped inside the package. 3. The listener binds to the device's loopback interface; the host reaches it over usbmuxd with a dependency-free `node:net` client. 4. Mutating commands are journaled by `commandId`; a lost reply is resolved by asking `status` for the journaled fate, never by replay (at-most-once delivery). 5. Routing hangs off `resolveDevice` UDID-shape classification plus a per-tool capability allowlist. CLI and MCP layers are untouched; outside tool-server, docs, and the runner package the diff is failure-code constants (registry), two agent skills plus rule updates (skills), the pack step (packages/argent), and the e2e smoke script. ## Supported tools | Tool | Status | Notes | | ----------------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | | launch-app | Supported | Plain `devicectl` launch. System UI (SpringBoard/Spotlight) registers a session only; no launch attempt. Carries a note when signing is not resolvable yet. | | restart-app | Supported | System UI bundle ids rejected before any device contact. | | reinstall-app | Supported | Same system UI pre-flight. | | screenshot | Supported | Served through the runner; device-wide, needs no registered app. (No host-side devicectl screenshot exists on any toolchain; that dead route was removed.) | | describe | Supported | AX tree snapshot via the runner. Snapshots the session app's process, so another process's dialogs are not in the tree. A backgrounded session app fails with an actionable error instead of being silently re-fronted. Focus polling is off for this source; typing uses a fixed settle. | | keyboard | Supported | Types into the focused element; typing with nothing focused is refused with "tap the text field first". `enter` and `backspace` are the named keys (both exist as software-keyboard characters; there is no per-keycode HID surface on hardware). Whitespace-only keys rejected, not silently "typed". | | gesture-tap | Supported | `clickCount > 1` dispatches as one native multi-tap gesture, not separate taps. | | gesture-swipe | Supported | Single-finger drag between two points. | | gesture-custom | Supported | Two shapes only: press-hold and straight single-finger drag. Second finger or `Move` waypoints rejected with authoring guidance, before any runner starts. | | button | Supported | `home`, `volumeUp`, `volumeDown`, `actionButton` (volume buttons are simulator-unavailable in the SDK, not device-unavailable). `power`/`appSwitch` have no public XCUIDevice API and reject before device contact; a button the model lacks gets a typed refusal via `hasHardwareButton`. | | await-ui-element | Supported | Polls the same runner snapshot describe returns; waits and taps see identical frames. Degrades to `cause: "unreadable"` with a note on a backgrounded app, never a raw throw. | | await-screen-idle | Supported | Same describe path; resolves not-settled instead of throwing, so a follow-up screenshot still runs. | | run-sequence | Supported | Outer gate only; each step pre-flights its own capability. | | open-url | Supported | devicectl launches the receiving app with the URL as its launch payload. http(s) defaults to Safari; any other scheme needs the new optional `bundleId` param (hardware cannot resolve a scheme's handler system-wide). | | screenshot-diff | Supported | Live captures through the runner (full-resolution, device-wide, no registered app needed); the diff itself runs on the host. Baselines are per device model/resolution; `rotation` is ignored on hardware. | Flows run through these tools; the device flow tree gets the same `subtreeText` hoisting as every other platform, so container-scoped `text` asserts work. ## Not supported (400 at the capability gate) 1. gesture-pinch, gesture-rotate: no two-finger coordinate API on hardware. Pinch/rotate flow directives also fail upfront with a clear error. 2. screen-recording-start/stop (video): simulator-only capture pipeline. 3. rotate, paste, shake, settings-permissions: simulator-only backends, not ported (paste also hits the iOS 16+ cross-app paste permission prompt). 4. native-devtools, debugger, profiler families: simulator-side instrumentation (DYLD injection does not exist on hardware). 5. Physical iPads: filtered at discovery for now. A test ratchet (`ios-physical-device-classification.test.ts`) keeps `apple.device` declarations in sync with the ported list. ## Core contracts 1. Observation never mutates the screen: a backgrounded session app makes `describe`/`await-*` fail with an error naming the three ways out (screenshot, launch-app, or launch-app com.apple.springboard). Mutating commands do re-front, and their result carries `reactivated: true`. 2. The runner listener is unauthenticated; mitigation is the loopback-only bind. 3. `list-devices` states: `connected` (cabled now, usable) vs `paired` (unreachable, never auto-bound). Wired transport alone decides; a Wi-Fi-migrated CoreDevice tunnel does not count. 4. The multi-team signing pick rides the first affected tool result as a one-shot note (the pre-existing notes channel). The `list-devices` result shape is unchanged from main: devices and avds, nothing else. 5. Rejections are typed 400 validation errors end to end, so a batch flow run skips one failing flow instead of aborting. Also: the runner is a first-class device-owned service (`stop-simulator-server` and `stop-all-simulator-servers` reap it, detached xcodebuild child included; a crashed runner respawns on the next call). A passcode-locked phone refuses launches; the devicectl 10002 hint also covers a pending system prompt on an unlocked phone. Build failures surface xcodebuild's own `error:` lines with signing hints (no registered devices, free-team app-id cap, missing provisioning, the `errSecInternalComponent` partition-list fix). An app XCTest reports as `.unknown` (a swipe-killed foreign app) is refused with launch-app guidance. Status-bar pinning is skipped on hardware; pixel-settle masks the top band. ## Testing - Unit coverage, hardware-independent: UDID classification and capability routing; usbmux framing and transport; devicectl list/hints parsing incl. the silent iPhone-only filter; keychain detection against real certificate fixtures; runner build/cache self-heals (poisoned cache, concurrent checkout, fresh-team retry); crash/journal recovery; the backgrounded-observation and reactivated contracts; keyboard guards incl. the not-focused refusal; system-UI pre-flight; per-tool device arms. - Manual smoke on a real device: `scripts/e2e-ios-physical-device.mjs`. ## Docs and skills 1. `features/physical-ios-devices.mdx`: the user-facing walkthrough (zero-config signing with the sign-in fallback, prerequisites, app-scoped model, troubleshooting). 2. `fundamentals/supported-platforms.mdx`: physical iPhones in the platform matrix (iPads not supported yet). 3. `reference/tools.mdx` and `reference/configuration.mdx`: list-devices states and the `ARGENT_IOS_TEAM_ID` override. 4. Two skills ship with the package via `argent init`: `argent-ios-device-setup` (setup ladder plus a troubleshooting table keyed to real error texts) and `argent-ios-device-interact` (the app-scoped hardware contract, supported subset with alternatives, gesture recipes, pitfalls). The agent rule and sibling skills route to them. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added experimental USB-connected physical iPhone support. * Physical iPhones now support discovery, app lifecycle actions, URL opening, accessibility snapshots, screenshots, taps, swipes, custom gestures, keyboard input, waits, flows, and screenshot comparisons. * Added automatic signing-team detection with optional `ARGENT_IOS_TEAM_ID` configuration. * Added clear handling for paired or unavailable devices and unsupported hardware actions. * **Documentation** * Added setup, troubleshooting, capability, and physical-device usage guidance. * Clarified that physical iPads and several simulator-only tools and gestures remain unsupported. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Ignacy Łątka
|
0d5757b803 | test(flow): tighten the wording above the batch stop rows | ||
|
Ignacy Łątka
|
cf3edabc41 |
test(flow): pin the batch stop on a rejection carrying no kind
The batch's stop test only covered a kind other than validation, so the "or none at all" half of the rule was unpinned: letting a signal-less rejection continue the batch left the suite green. The single-flow --json paragraph claimed stdout stays empty when a run produces no report, which a flag error breaks by printing the usage text there (#1025). |
||
|
Ignacy Łątka
|
11380710e2 |
docs(flow): name the stop rule by the mark the server did not send
A ToolInvocationError whose errorKind is undefined stops the batch as surely as one marked subprocess - tools-client passes the wire field straight through, so a non-OK reply without error_kind produces exactly that. "A rejection of any other kind" excluded it; "a rejection the server does not mark as validation" is the guard. Three more on the same page: --json does not keep stdout to JSON on an argument error (#1025), so the sentence claiming it does is gone; a skip entry carries neither report nor error, since Argent never called for that flow; and the --json-stream help row called its terminal record an "outcome", the page's name for the line that mode does not print. Also the isFlowReport rationale's twin in the test file, which still said a primitive step element makes the run report PASS - that comes from the value's own ok, which those rows set. |
||
|
Ignacy Łątka
|
32193401ae |
fix(flow): pin the no-report branch's ordering and entry, and name what stops a batch
Self-review follow-ups. An unrestricted "any other failure stops the batch" also claims the artifact export at flow.ts:1146, which sits outside the loop's try: a throw there escapes runFlowDirectory with no outcome line, no skip entries and no aggregate (#1026). The help text, the runFlowDirectory docstring and flow-yaml.mdx name the three that do stop it instead. The no-report branch was reordered alongside the catch, but only the catch was pinned: reverting its order, or adding a bogus error_code to its aggregate entry, left the suite green. The ordering test covers both branches now, and the --json no-report test asserts the entry exactly, so its lack of a failure signal is a fact rather than an omission. Also: --help called --json a "raw JSON report" where a directory run prints an aggregate, and did not mention that --json-stream cannot join it; the docs never named the aggregate's top-level fields, nor that a single-flow --json failure writes nothing at all to stdout. |
||
|
Ignacy Łątka
|
63eee31416 |
docs(flow): a flow failing its steps keeps a directory run going
The batch stops on a failure the tool call raises, not on a flow whose steps fail - that one is pushed as `status: "fail"` with its report and the loop carries on. Both the `flow run --help` blurb and flow-yaml.mdx said any failure but a rejection stops the run. Also on the same page: the aggregate entry for a step-failing flow carries a report and no `error`; `--json` suppresses the headers and the flow summary as well as the outcome lines; the printed step list includes a passing step that carries a warning; and `--json-stream` cannot be combined with `--json`. |
||
|
Ignacy Łątka
|
b2d2f67135 | docs(flow): a batch aggregate entry only holds a report when the flow produced one | ||
|
Ignacy Łątka
|
5d3ab0e44b |
fix(flow): carry a batch rejection's failure signal into --json, and correct the ledger's prose
Review follow-ups on the directory-run verdict:
- The --json aggregate dropped the classification the human verdict is chosen
from, so an invalid flow file and an unresolvable device were the same
`{"status":"fail","error":"<prose>"}` entry. BatchFlowResult now carries
error_code/error_kind under the spelling --json-stream already publishes,
via a failureSignal() helper shared with writeJsonStreamError.
- The batch wrote the stderr detail before the stdout verdict while the
single-flow runner wrote the verdict first. Both now print the verdict under
the header and the detail after it, so a merged log reads the same way.
- Comments claiming PASS/exit 0 come from the step elements (they come from the
report's own `ok`), that both runners print a step line for an unreadable
element (renderFailedSteps prints none), and that only an unclassified
failure stops the batch (any kind other than validation does) now describe
what the code does.
- Two test assertions read as stronger controls than they are: a failed step
line carries the same ` ✗ ` prefix as a verdict, so an indented-line match
could not tell them apart. They match the outcome lines exactly, and the
✗-count control says what it cannot distinguish.
- New coverage for the two guards nothing exercised: a directory --json run
over a validation rejection, and the --json-stream half of the single-flow
verdict guard.
- flow-yaml.mdx documented the pre-change batch behaviour, listed none of the
verdict strings and omitted --json-stream.
|
||
|
mjq2020
|
23181baaa8 |
fix(network): report UTF-8 response byte size (#783)
## Summary - measure React Native response bodies from a cloned Blob instead of a decoded JavaScript string - report one consistent metric: decoded response entity bytes - avoid Content-Length because HEAD/304 and compressed responses give it different semantics - keep response text capture independent, and leave the size unknown if Blob conversion is unavailable ## Root cause body.length counts UTF-16 code units. Re-encoding the decoded text as UTF-8 fixes ordinary UTF-8 text, but it overcounts replacement characters from non-UTF-8 and binary responses. Content-Length is also not a safe fallback: it can describe an unsent GET body for HEAD/304 or the compressed wire representation while the response body is decompressed. Response.blob().size measures the bytes in the decoded entity supplied by the React Native response. The same measurement therefore applies to text, binary, empty, cached, and compressed responses. ## Tests - network interceptor regression: 23/23 passed - covers Unicode JSON, HEAD, 304, non-UTF-8 replacement characters, compression, binary bodies, and unavailable Blob conversion - npm run build - npm run typecheck:tests -w @argent/tool-server - targeted ESLint and Prettier checks on both changed files Fixes #662 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved reported network response sizes to reflect the actual decoded response data. - Corrected size calculations for binary content, compressed responses, and text containing non-ASCII characters. - Added reliable handling for `HEAD` and `304` responses, which now report zero size. - Prevented failed size calculations from incorrectly reporting a zero-byte response. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Ignacy Łątka
|
1409195138 |
test(run-sequence): pin the one observation a sequence returns (#934)
## What `run-sequence` tells agents: > One screenshot is captured automatically after the whole sequence (not per step) The MCP-side half of that promise is already pinned — argent-mcp's auto-screenshot tests name `run-sequence` explicitly, twice. The tool-server half is pinned by nothing: this adds the gate that pins it, plus the renderer-side pin in argent-mcp. **No behavior change.** ## Why it is unpinned The steps stay silent only because no tool `ALLOWED_TOOLS` admits returns an image or an artifact handle. Nothing checks: - which tools the allow-list admits, - that every admitted name still resolves to a registered tool (a stale name still reads as allowed and stays advertised in the description, but every step naming it dies on `ToolNotFoundError` and halts the sequence), - that none of them declares `outputHint: "image"`. Admitting a frame-producing tool would go unnoticed: a sequence result reaches `toMcpContent` whole, and its `materializeArtifacts` walk collects handles from anywhere inside it, `steps[].result` included, appending one image block per handle. That walk recurses into arrays with `Promise.all`, so those frames arrive in **completion order, not step order** — the agent could not even tell which one is the screen the sequence ended on. Both halves are verified against the code, not assumed: - Putting an artifact handle in one step's result makes `toMcpContent` emit an image block for it, confirming the walk reaches `steps[].result`. - Materializing three step handles whose fetches complete fastest-last yields `[s2, s1, s0]` — exactly reversed from step order. ## What is gated **tool-server** — `test/run-sequence-observation-gate.test.ts`, against the real registry (`createRegistry()`, no device). `ALLOWED_TOOLS` is exported for it: - the allow-list admits exactly the reviewed set of 14 step tools - every name still resolves to a registered tool - none of them declares `outputHint: "image"` **argent-mcp** — `test/content.test.ts`: - a multi-step sequence result renders with no frame of its own, against a `fetchImpl` that resolves and is asserted never to be called (without a resolving fetch the assertion would hold equally for a sequence whose step frames merely failed to download, since `materializeArtifacts` swallows a failed download and renders the handle as nothing) - the positive control that pin rests on: a handle on `steps[].result` does surface as an image block, so the walk ceasing to reach steps cannot turn the pin vacuous The membership of `run-sequence` in `AUTO_SCREENSHOT_TOOLS` was already pinned by existing assertions and needed no new test. ## Verification Every test was mutation-checked — each fails on the broken code, not just passes on the good: | Mutation | Fails | | --- | --- | | add `screenshot` to `ALLOWED_TOOLS` | reviewed-set + `outputHint` checks | | rename an allow-list entry the registry lost | reviewed-set + registration checks | | give a step result an image artifact handle | the frameless-sequence check, on both its fetch-call and its image-block assertion | | stop `materializeArtifacts` recursing into arrays | the step-carried-handle check, and nothing else in the file | Locally green: `prettier --check`, `tsc --build`, `typecheck:tests` for both touched packages, the full argent-mcp suite (82 tests), and the gate test (3 tests). ## Scope note The allow-list is spelled out rather than counted, so adding a step tool means answering the question the file exists to ask — does it put a frame of its own in front of the agent? The gate deliberately sits on the allow-list rather than on the renderer: a tool that would break the guarantee is caught before it can be dispatched as a step, rather than having its frames filtered afterwards. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> |
||
|
filip131311
|
927bfc630f |
chore: bump version to 0.23.0 (#987)
Bumps all workspace packages from 0.22.1 to 0.23.0. - All 16 `packages/*/package.json` bumped via `npm version 0.23.0 --workspaces` - `package-lock.json` regenerated on Node 24 / npm 11 (`npm install --package-lock-only --ignore-scripts`); diff is version-only - `server.json` updated in both places (top-level `version` and `packages[0].version`) - `npm run check:versions` passes No docs update needed — version-only change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VGjTuE8XKfrmmhJEjchBFq <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Updated the release version across all packages and the server manifest from 0.22.1 to 0.23.0. - Ensures package metadata and server configuration consistently reflect the new release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->v0.23.0 |
||
|
filip131311
|
397dce87f6 |
feat(installer): allowlist.enabled config — opt out of allowlist refresh on update (#986)
A minimal opt-out for the allowlist refresh in `argent update`: a new config key **`allowlist.enabled`** (project + global scopes, restrictive merge — `false` in either scope wins, so a committed project opt-out holds for the team). **Nothing changes by default.** The key has no default value; while it is unset, `argent init` and `argent update` behave exactly as today. Only an explicit ``` argent config set allowlist.enabled false ``` changes anything: `argent update` then skips re-applying editor auto-approve allowlist rules entirely (rules already written stay) and prints a dim one-liner saying why. `argent init` is untouched — it still asks its auto-approve question and applies the answer as before. ### Docs `reference/configuration.mdx` gains the key's row and a short behavior note. `docusaurus build` and `npm run format` are clean. ### Tests - `allowlist.enabled` merge semantics (restrictive, no default) in configuration-core. - Suites: installer 555, configuration-core 125; tsc, eslint --max-warnings 0, prettier, both knip gates clean. - Live-tested from a packed tgz in an isolated HOME: `init -y` and `update` with the key unset are byte-identical to main's behavior; with `allowlist.enabled false`, `update` re-imposes nothing and prints the skip note. Supersedes #787. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_014b5AyEqWd16k1s8xvve3dS <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added an `allowlist.enabled` setting to control editor auto-approve allowlist updates. - Allowlist updates now run when enabled or unset, while an explicit disabled setting prevents changes. - **Documentation** - Added configuration reference details, including scope precedence and unset behavior. - **Bug Fixes** - Preserved non-blocking handling when individual editor allowlist updates fail. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Ignacy Łątka
|
a2ed83e041 |
fix(tool-server): await the simulator-server command ack so a lost tap fails (#985)
## The defect
`sendCommand` returns `void`. It writes the JSON command onto the
WebSocket — or queues it behind `once("open")` — and returns without
ever reading the reply:
```ts
export function sendCommand(api, cmd): void {
const ws = getOrCreateWs(api);
const payload = JSON.stringify({ id: String(++cmdId), ...cmd });
if (ws.readyState === WebSocket.OPEN) ws.send(payload);
else ws.once("open", () => ws.send(payload)); // queued, never awaited
}
```
simulator-server *does* acknowledge every command; argent just never
looked. Probing the shipped binary's `/ws` endpoint:
| sent | reply |
|---|---|
| `{"id":"1","cmd":"touch","type":"Down",…}` |
`{"id":"1","status":"ok"}` |
| `{"id":"7","cmd":"button","button":"notARealButton"}` |
`{"status":"error","message":"parse error: unknown variant
`notARealButton`…"}` |
Because the return type was `void` there was no ack to check and no
error to surface, so `gesture-tap` ended in an unconditional `return {
tapped: true, timestampMs }`.
**Repro:** shut a simulator down underneath an open session. The server
sends no reply at all and `gesture-tap` still reports `{ tapped: true
}`. Every transport failure — that one, a dead socket, a wedged device —
looks identical to success from the agent's side.
This is argent's own bug, and it is what turns an Apple regression into
a silent one: awaiting the reply would have made #932 an actionable
failure on day one instead of a silent no-op.
## The fix
`sendCommand` now returns a promise that resolves on `{"status":"ok"}`
and rejects when:
| condition | `error_code` |
|---|---|
| server answers `status: "error"` | `SIMULATOR_COMMAND_REJECTED` |
| socket fails or closes with the command in flight |
`SIMULATOR_COMMAND_TRANSPORT_FAILED` |
| reachable server never answers (5s) | `SIMULATOR_COMMAND_ACK_TIMEOUT`
|
Every WebSocket call site awaits it: `gesture-tap`, `gesture-swipe`,
`gesture-custom`, `gesture-pinch`, `gesture-rotate`, `button`, `rotate`.
Two details of the wire protocol shaped the implementation:
- **A success reply echoes the command id; an error reply carries no id
at all.** So a rejection can only be matched positionally, against the
oldest outstanding command. Since every send is awaited, that is
normally the only one in flight.
- **The abort paths in `gesture-swipe` / `gesture-rotate` lift the
finger before unwinding, and that lift is now fallible.** It is
therefore best-effort: a refused lift rides along as the `AbortError`'s
`cause` rather than replacing it — matching the existing pattern in
`gesture-drag`, because `format-error.ts` and `cdp-session.ts` key on
`err.name === "AbortError"`.
## Verified on a real simulator
Before, after shutting the device down mid-session: `{"tapped": true}`.
After:
```
result: {"tapped":true,"timestampMs":…} ← healthy tap, unchanged
--- simulator shut down underneath the open session ---
tap FAILED as it should:
code: SIMULATOR_COMMAND_TRANSPORT_FAILED
msg : simulator-server did not accept the 'touch' command: connect ECONNREFUSED
127.0.0.1:62818. The command was NOT delivered to the device. …
```
The failure signal is fully populated for telemetry (`error_kind:
"network"`, `network_failure: "connection_reset"`, `failure_command:
"simulator_server"`).
## Cost
An ack is a localhost round-trip behind an already-open socket: **0.06ms
p50 / 0.17ms max**, measured over 200 sends against a booted iOS sim.
Awaiting one per event adds a flat **~30ms to a swipe of any duration**
— against the 36–142ms of `setTimeout` skew the 16ms frame loop already
carries:
| `durationMs` | wall before | wall after | added |
|---|---|---|---|
| 200 | 236ms | 266ms | +30ms |
| 500 | 559ms | 591ms | +31ms |
| 1000 | 1142ms | 1178ms | +35ms |
Flagging it explicitly since the swipe cadence is deliberately tuned. If
reviewers would rather keep the cadence byte-identical, the alternative
is to pipeline within a gesture and await all acks at the end; that
costs more code and defers failure to the end of the gesture, so I kept
the simpler per-send await.
## Deliberately out of scope
- **The MoQ (`ios-remote`) transport** is unchanged — it has no
equivalent ack.
- **The stdin `pressKey` channel**, used by `keyboard` and paste's ⌘V,
is untouched: simulator-server never replies on stdin, so it has the
same blind spot with no local remedy. The WebSocket protocol *does*
expose an acked `key` command
(`{"cmd":"key","direction":"Down","code":4}` → `{"status":"ok"}`), so
moving those onto it would close the gap — a separate, riskier change to
the text-entry path that deserves its own PR.
## Tests
`packages/tool-server/test/simulator-command-ack.test.ts` — 8 tests
against a fake `/ws` server that mirrors the real wire format, **all 8
fail on `main`**: ack resolves, `status: error` rejects, socket death
rejects in flight, silent server times out, no phantom success,
`gesture-tap` throws instead of claiming a rejected tap, and the aborted
swipe still reports `AbortError`.
Full suite: 369 files / 4840 tests pass, `npm run lint` clean, `tsc
--build` clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_011bEvNDV7o3CpKdFmzjVBYF
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Simulator commands now confirm successful delivery before completing.
* Added clear errors for rejected commands, connection failures, and
acknowledgement timeouts.
* **Bug Fixes**
* Button, rotation, tap, swipe, pinch, and custom gestures now report
failures instead of silently succeeding.
* In-progress gestures handle connection interruptions and cleanup
failures more reliably.
* **Tests**
* Added coverage for command acknowledgements, rejections, timeouts,
transport failures, and gesture error handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: latekvo <f.kaminski2000@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
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 --> |
||
|
Ignacy Łątka
|
2cc1f5281c |
test(android-profiler): drop an assertion that cannot fail (#933)
Audit of every assertion in the repo that pins shipped agent-facing text, looking for more of the `/restart/i` shape that reddened main in #871 — an assertion satisfiable only by making the product string restate itself. **The class is essentially absent. One instance found.** ## The one `tool-server/test/android-perfetto/hang-stacks-offcpu.test.ts` ```ts expect(out).toMatch(/No usable on-CPU stack samples/); // L156 // The thread was on-CPU/executing — this is genuine CPU work, not a wait. expect(out).toMatch(/on-CPU/); // L158 ← removed expect(out).toMatch(/executing/); ``` `/on-CPU/` is a substring of the literal asserted two lines above, so `L156 ∧ L158 ≡ L156`. It cannot fail while L156 passes. Mutation-checked rather than argued: changing the pipeline's sentence from `_No usable on-CPU stack samples were captured…` to `_No usable stack samples were captured…` still fails the test — ``` × explains an empty sample set as un-unwound CPU work when the thread was executing AssertionError: expected ... to match /No usable on-CPU stack samples/ ``` — so nothing this line was catching goes uncaught. `/executing/` is **not** implied and stays; the comment above it still reads correctly. ## What the audit covered, and what it found instead | stage | count | |---|---| | assertions pinning shipped agent-facing text | 707 | | …in the `/restart/i` shape (short generic literal beside stronger siblings on the same value) | 44 | | …surviving hand-verification | **1** | The 43 rejected are doing real work. A sample, so the reasoning is auditable: - `"844x390"` / `"390x844"` — dimension-mismatch test; the point is argument order. - `"(2×)"` / `"(1×)"` — different dedup counts. - `/wait/` beside `/No on-CPU stack samples/` — the sibling literal contains no "wait"; it pins the wait-vs-CPU explanation. - `"localhost:8081"` / `"127.0.0.1:8081"` — two hosts the interceptor must filter. - metro `discovery.test.ts` `"Do not retry in a loop"` — a *different* test from the one asserting the full sentence. More importantly, the 707 is not a pile of redundancy. It is mostly the only way to test a function whose output is a string — `formatTraceFreshness(NOW - 5*60*MIN, NOW)` can only be checked by asserting `/5 hours ago/` — plus assertions that are load-bearing in their own right, e.g. `expect(description).not.toContain("hunter2")` in `configuration-core/tests/secrets.test.ts`, the sole guarantee a secret value never reaches a source description. Stripping the category wholesale would empty 74 tests into unconditional passes. ## Two adjacent findings, not changed here - `tool-server/test/native-profiler-missing-trace.test.ts:90` — `expect(result.report).toMatch(/missing|not found|unreadable/i)` is meant (per its comment) to check the wording tells the user the file is missing rather than the export empty. It passes vacuously: the fixture is named `missing_cpu.xml`, so `/missing/i` matches the filename the assertion two lines up already pins. It enforces nothing. The fix is a discriminating regex or a renamed fixture, not deletion. - `tool-server/test/capability.test.ts:58-59` — `toContain("demo-tool")` and `toContain("android")` are both substrings of `"tools/demo-tool/platforms/android.ts"` asserted at L60, so they are provably implied. Left in place deliberately: the test is named *"composes a uniform message with toolId, platform, and the file"*, and deleting them would make the name over-claim relative to what the body checks. ## Verification Branched off `366f17c61`. | gate | result | |---|---| | `vitest run` (tool-server) | **358 files, 4401 passed, 1 skipped**, exit 0 | | `tsc --build` | clean | | `typecheck:tests --workspaces` | clean | | `eslint . --max-warnings 0` | exit 0 (with `npm ci` in `packages/docs`, as CI does) | | `prettier --check` | clean | |
||
|
Juliusz Wajgelt
|
bf38e9069b |
fix(sim-remote): pass the udid before -b in simctl bootstatus (#982)
## What `simctlBootstatus` in `packages/tool-server/src/utils/sim-remote.ts` built the command as `simctl bootstatus -b <udid>`, placing the flag before the device. simctl's grammar puts the device first — `bootstatus <udid> -b`. ## Why Newer Argent Cloud deployments are more faithful to how the local `simctl` handles its arguments, so they now reject `-b` in that position as an unknown flag. The result is that **every remote boot fails**, on every device: ``` [tool: mcp__argent__boot-device] -> invalid option -- b ``` The local `xcrun` path already used the correct order (`boot-device.ts:461`), so this only affected sim-remote / Argent Cloud. ## Notes for the reviewer - I checked the other `sim-remote` wrappers for the same flag-before-udid shape. `injectDylib` is the only other one with an optional flag and it already appends after the udid, so `bootstatus` was the sole offender. - `packages/tool-server/test/boot-device.test.ts` passes (23/23), but it only exercises the local `xcrun` path — there is no existing test for the remote wrapper, so **this fix is not covered by tests**. It's worth a manual check against a Cloud device before merging. - `tsc --noEmit` reports pre-existing errors in this package from a stale `@argent/registry` build; `sim-remote.ts` itself is clean and none of them are related to this change. - No docs update needed: this is an internal argument-construction bug with no user-facing surface change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed simulator boot status checks by correcting the order of command-line arguments. * Ensured the simulator identifier is processed correctly when the optional boot flag is used. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
filip131311
|
23b9a07760 |
feat(mcp): auto-describe — element tree after interaction tools + relaxed tapping rule (#958)
## Why
The rule's `<tapping_rule>` ("Before **every** tap, you MUST call a
discovery tool") makes agents loop `tap → describe → tap → describe`:
over 35 baseline runs, `describe` was called 90 times against 86 taps —
one round-trip per tap, each a full model turn. The screen after an
action is already captured by the auto-screenshot; the element tree is
the only thing the agent still had to fetch.
## What
- **MCP** (`packages/argent-mcp`): after the action settles, call
`describe` and append `--- Elements after action (describe) ---` + the
tree to the tool result. Follows the same tool allow-list as the
auto-screenshot, skips `describe` itself, and sits inside the non-secret
branch so a `{{secret:…}}` value never re-enters context as text.
Spy-logged as `auto_describe` (duration, chars). MCP `instructions`
reworded.
- **Flag** `disable-auto-describe` (opt-out, mirrors
`disable-auto-screenshot`). Auto-describe has its own tool list
(`AUTO_DESCRIBE_TOOLS`) and its own branch, so the two features are
independent; the secret-placeholder skip and the screen-idle wait cover
both.
- **Rule + skill**: `tapping_rule` now says take coordinates from the
returned tree, call a discovery tool only when there is no fresh tree
for the screen (first action on a screen, after a wait, target not
listed); the two-failed-taps rule and the "never from pixels"
prohibition stay. `argent-device-interact` best-practice #6 and the
general-rules line updated to match.
- **Docs**: `reference/configuration.mdx` flag row,
`features/interacting-with-apps.mdx` discovery section.
- **Tests**: `autoDescribeEnabled` (global / project scope) and
`shouldAutoDescribe`.
## Evidence
A/B with `claude -p --model sonnet`, one build (variant env-gated during
the experiment), two sandbox projects differing only in rule/skill text
and the toggle; 7 tasks × 2 arms × 5 reps = 70 runs on the iPhone 17 Pro
simulator (iOS Settings ×3, Expo `my-app` ×2, Contacts create verified
in sqlite) and the Electron testbed.
| | main | this PR | Δ |
|---|---|---|---|
| Task success | 35/35 | 35/35 | = |
| Turns / run | 10.4 | 8.2 | −21% |
| Wall / run | 36.5 s | 28.0 s | −23% (new won 28/34 pairs) |
| Cost / run | $0.28 | $0.23 | −17% (27/34) |
| `describe` calls / run | 2.2 | 0.24 | −89% |
| Cache-read tokens / run | 622k | 421k | −32% |
(34 clean pairs; one pair excluded for external interference on the
simulator — with it included: wall −5%, cost −10%, turns −16%.) Every
iOS task moved 15–30% in favour of the new arm; Electron was neutral
(both arms describe once after `boot-device`, then a single
`run-sequence`). The appended tree costs ≈2.4k chars (~700 tokens) and a
median 73 ms per action; per-turn latency was unchanged, the saving is
the removed model round-trip.
Full report:
https://claude.ai/code/artifact/279528cf-ea92-4f52-9276-4f0475ba3e3c
## Caveats
- Sonnet only, short tasks, small screens. On feed-heavy apps with
10–20k-char trees the per-action overhead grows; a follow-up could cap
or filter the appended tree.
- Flags are read once at MCP start (same as `disable-auto-screenshot`),
so toggling needs an MCP restart.
## Checks
`vitest` argent-mcp (85) + configuration-core (122) green; `eslint
--max-warnings 0` on changed packages; `prettier --check .`; `npx
docusaurus build` green.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Interaction tools now return an accessibility element tree alongside
the latest screenshot.
- Agents can use fresh element data for follow-up actions without an
additional discovery step.
- **Configuration**
- Added the `disable-auto-describe` feature flag to turn off automatic
element trees.
- **Documentation**
- Updated interaction guidance and configuration references to explain
the new behavior and flag.
- **Bug Fixes**
- Secret-placeholder handling now correctly accounts for both
screenshots and element trees.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
filip131311
|
e7c6f3324c |
fix(tool-server): frame device sockets on \n only — U+2028 in a label stalled describe for 15 s (#975)
## Problem
`describe` returned an empty tree after ~15 s whenever the screen had an
accessibility label containing U+2028 (LINE SEPARATOR) — e.g. any
Element/Matrix reply preview, or rich text pasted from the web. Nothing
was wrong on the device: ax-service answered in ~170 ms with a full,
valid tree.
The tool-server framed the ax-service socket with
`readline.createInterface`, which splits on the four ECMAScript line
terminators (`\n`, `\r`, **U+2028, U+2029**). JSON permits U+2028
unescaped inside a string and `NSJSONSerialization` emits it raw, so one
valid frame became three fragments, none parsed, each was dropped by
`catch { return }`, and the pending RPC waited out its 10 s timeout —
then the native-devtools fallback waited out its 5 s. On top of that,
the timeout was reported as *"this simulator was not booted through
argent … call boot-device with force=true"*, sending the agent to reboot
a healthy simulator.
Minimal repro (no device): `JSON.stringify({label:"a
b"})+"\n"` through
`readline` → 2 lines, 0 parsed.
## Fix
- **`utils/ndjson-socket.ts`** (new): `attachNdjsonReader` frames on the
`\n` byte only (`0x0A` can never occur inside a multi-byte UTF-8
sequence, so this is exact) and reports every unparseable frame to
stderr instead of swallowing it.
- Wired into the three device-facing JSON sockets: `ax-service.ts`,
`native-devtools.ts`, `android-devtools-client.ts` (the Android helper's
`org.json` and the iOS dylib's `NSJSONSerialization` both emit U+2028
raw, so all three were exposed). The two remaining `readline` uses read
process **stdout** for readiness markers — no JSON, no user text — and
are left alone.
- **`describe/platforms/ios/index.ts`**: a throw from `axApi.describe()`
(timeout, RPC error) now carries the error message in the hint and uses
the resolved api's real `degraded` flag, instead of being reported as
the boot caveat. Separate commit so it can be reverted independently.
## Verification
- 6 new unit tests for the reader (U+2028/U+2029 in a string, chunk
split mid-multibyte char, several frames per chunk, CRLF/empty lines,
dropped-frame reporting, trailing frame on `end`) and 3 for the hint
(`describe-tool.test.ts`). Full tool-server suite: 4816 passed.
- Live A/B on iPhone 16 / iOS 18.5, Safari page with `<meta
charset=utf-8>` and a U+2028 inside a `<p>`:
- argent 0.22.1 (`main`): `describe` **12.6 s**, `source:
native-devtools`, web text missing, false "not booted through argent"
hint.
- This branch (`dist` against installed binaries): **50 ms**, `source:
ax-service`, label `"In reply to Dave
Loving it so far"` present, no
hint, 0 dropped frames logged.
## Docs
No docs update needed — no tool, CLI, config or flow surface changes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01VwqqHRhfuHXbKr6LYskxhJ
|
||
|
Ignacy Łątka
|
43b7d79a2a |
fix(boot-device): stop passing -crash-report-mode to emulators that reject it (#557)
## Problem
`boot-device` cannot launch an Android emulator on builds that do not
accept
`-crash-report-mode`. The emulator rejects the unknown option and exits
before
the device ever comes up:
```
$ argent run boot-device --args '{"avdName":"Pixel_6_API_34"}'
emulator binary exited with code 1 before the device booted
```
## Cause
`-crash-report-mode` is undocumented and missing from many emulator
builds, so
#306 added a feature-detect gate:
```ts
const crashReportArgs = (await emulatorSupportsFlag("-crash-report-mode"))
? ["-crash-report-mode", "never"]
: [];
```
That commit was a pure addition. It never removed the flag from the
shared
`LAUNCH_HARDENING_ARGS` constant, which still reads:
```ts
const LAUNCH_HARDENING_ARGS = [
"-no-boot-anim", "-netfast",
"-crash-report-mode", "never",
"-no-metrics",
] as const;
```
So the gate is dead code. Every launch gets the flag unconditionally
through
the constant, and builds that do support it are handed it twice.
## Proof
On Android emulator 36.1.9.0, whose `-help` does not list the flag:
```
$ emulator -version
Android emulator version 36.1.9.0 (build_id 13823996)
$ emulator -help | grep -c -- -crash-report-mode
0
$ emulator -help | grep -c -- -no-metrics
1
```
`-list-avds` and an unknown `-avd` both short-circuit before option
validation, so the rejection only surfaces once a real AVD resolves.
Against
one, with everything else held equal:
```
$ emulator -avd Pixel_6_API_34 -check-snapshot-loadable default_boot \
-crash-report-mode never
INFO | Found systemPath .../android-34/google_apis/arm64-v8a/
unknown option: -crash-report-mode
please use -help for a list of valid options # exits, never boots
$ emulator -avd Pixel_6_API_34 -check-snapshot-loadable default_boot
INFO | Ok: System requirements to run avd: `Pixel_6_API_34` are met
```
The launch dies at option parsing, after the AVD resolves and before any
device work. `hardeningArgs` reaches the snapshot probe, the hot boot
and the
cold boot alike, so on such a build all three spawns abort.
## Fix
- Drop `-crash-report-mode never` from `LAUNCH_HARDENING_ARGS`, leaving
the
`crashReportArgs` gate as the only source of the flag.
- Spread `crashReportArgs` into the `-check-snapshot-loadable` probe's
`extraArgs`. The probe is itself an emulator spawn, so it gets the same
crash-dialog suppression as the two boots; once the flag left the
constant
it would otherwise have been the only site missing it. This one is not a
snapshot-parity requirement - unlike `-noaudio` and `-netfast`,
`-crash-report-mode` selects crash reporting rather than qemu devices,
so a
mismatch would not invalidate the saved snapshot.
- Leave the gate where it is, after the already-running reuse fast path,
so
the `emulator -help` probe is still skipped when nothing will be
spawned.
- Fix the prose that had drifted: the comment above
`LAUNCH_HARDENING_ARGS`
still listed `-crash-report-mode` among its flags and claimed they all
need
no detection, and both it and the `emulatorSupportsFlag` docstring said
the
flag is "only present in newer emulator releases (~36.x and late 35.x)"
-
which 36.1.9.0 not having it contradicts.
`bootAndroidImpl` is the only emulator launch path in the repo, and all
three
of its spread sites are covered.
Making the gate load-bearing put weight on a probe that was only ever
advisory, so `emulator -help` detection is hardened alongside it:
- `emulatorSupportsFlag` memoized its verdict for the process lifetime
even
when the probe learned nothing - a transient spawn failure, a timeout,
or a
silent zero-exit run pinned "unsupported" until restart. An empty
capture is
now reported to stderr and left uncached, so the next boot retries.
- A killed probe is no verdict at all: the flag may sit in the part of
the
listing the child never transmitted, so partial output from a timeout is
discarded rather than read as absence.
- The three emulator probe spawns (`-help`, `-list-avds`,
`-check-snapshot-loadable`) pass `killSignal: "SIGKILL"`, matching
`runAdb`.
`execFile`'s default SIGTERM can be ignored by an emulator wedged in
Qt/graphics init, which leaves the promise unsettled past every caller's
deadline.
## Tests
A new `-crash-report-mode feature gate` block in
`packages/tool-server/test/boot-device-hotboot.test.ts` asserts the real
spawned argv and the probe's `extraArgs`:
- gate false: zero occurrences at the probe, hot-boot and cold-boot
sites;
- gate true: exactly one occurrence at each, always followed by `never`.
The
count matters - asserting mere presence passes on the double-pass code
and
would guard nothing;
- plus a test that `-help` detection is skipped entirely on the
already-running
reuse fast path, pinning why the computation sits where it does.
`emulatorSupportsFlag` is stubbed in that file's existing
`vi.mock("../src/utils/adb")` factory, because the real one memoizes its
`-help` result per binary for the process lifetime and both branches
could not
otherwise be exercised in one file. The two pre-existing
launch-hardening
assertions were updated to the four remaining flags.
`packages/tool-server/test/emulator-supports-flag.test.ts` is new and
covers
the detection hardening: which outcomes are memoized (a verdict read
from a
non-zero exit's captured listing, and a normal zero-exit run) and which
are
not (spawn failure, timeout with empty streams, zero exit with no
listing,
timeout with partial output), the stderr warning, and `SIGKILL` on all
three
spawns.
Discrimination check - both source changes reverted (flag back in
`LAUNCH_HARDENING_ARGS`, `crashReportArgs` dropped from the probe's
`extraArgs`), tests kept:
```
Tests 5 failed | 26 passed (31)
x omits the flag from the snapshot probe and the hot-boot spawn when the emulator rejects it
x omits the flag from the cold-boot spawn when the emulator rejects it
x passes the flag exactly once to the snapshot probe and the hot-boot spawn when supported
x passes the flag exactly once to the cold-boot spawn when supported
x keeps exactly one flag occurrence on the probe-reject cold fallback when supported
AssertionError: expected 1 to be +0 (x2 - the flag reaches a site that must not see it)
AssertionError: expected 2 to be 1 (x3 - the flag arrives twice)
```
Fix restored: `Tests 31 passed (31)`.
## Verification
- `npm run build` (`tsc --build`): clean.
- `npx vitest run` in `packages/tool-server`: clean.
- `npm run typecheck:tests --workspaces --if-present`: clean. This is
the CI
gate that plain `tsc` + vitest does not cover.
- `npx prettier --check .` and `npm run lint` (`eslint . --max-warnings
0`):
clean.
- End to end on a real emulator (36.1.9.0, the build that rejects the
flag).
The gate reads the binary correctly:
```
emulatorSupportsFlag(-crash-report-mode) = false
emulatorSupportsFlag(-no-metrics) = true
emulatorSupportsFlag(-no-boot-anim) = true
```
and `boot-device` against `Pixel_6_API_34` returns
`{"platform":"android","serial":"emulator-5554","booted":true}` in 4.8
s.
The live process argv shows the flag dropped and the four remaining
hardening flags intact:
```
qemu-system-aarch64 -avd Pixel_6_API_34 -force-snapshot-load
-no-snapshot-save \
-gpu auto -noaudio -no-boot-anim -netfast -no-metrics
```
`-force-snapshot-load` means the hot-boot path ran, so the snapshot
probe
accepted its flag-free argv too.
## Docs
No docs update needed: `-crash-report-mode` is an internal launch
argument
that no reference page, config key or tool parameter exposes.
|
||
|
Jakub Piasecki
|
d4a712c401 |
refactor(gesture): rename the momentum-free flag to momentum (#732)
`settle: true` collided with the runner's other `settle` — waiting for the tree to quiesce — which is the very thing a swipe step does after every dispatch, whichever way the flag is set. One sentence of the flow skill used both senses at once. This names the physics instead: `momentum: false` is the momentum-free gesture, and the natural fling is the default the prose already described. ### Renamed | surface | before | after | | --- | --- | --- | | `swipe` flow directive | `settle: true` | `momentum: false` | | `gesture-swipe` param | `settle` | `momentum` | | `gesture-drag` param | `settle` | `momentum` | The directive passes the flag straight through to the tools, so all three move together. `momentum: true` is the default and normalizes to absent, mirroring what `settle: false` did — parse/serialize stay exact inverses. ### Also - Recording summary label `(settle)` → `(momentum-free)` - Internal constants → `MOMENTUM_FREE_EASE_EXPONENT`, `MOMENTUM_FREE_MIN_STEPS` - `flow-run` tool description and `argent-create-flow` skill updated ### Testing `tsc -b`, eslint (`--max-warnings 0`) and prettier clean; `test/flows` + `test/tools` pass 671/671, including a new case covering the legacy-spelling rejection. |
||
|
Jakub Piasecki
|
e314b3ef4a |
feat(flow): swipe directive (#520)
Add one semantic finger flick for dismissing cards, paging carousels, opening drawers, and other gesture-driven actions distinct from goal-seeking scroll-to. Support bare directions plus from with exactly one of direction, to, or by, optional settle, and positive finite duration. Resolve selector or normalized-point anchors through the shared target path. Direction uses the Maestro geometry presets and now rejects anchored gestures that would collapse or travel opposite to the requested direction. Relative by deltas saturate supplied axes to [0, 1] and reject any effective zero or sign reversal. Resolved starts must remain on-screen, while explicit endpoints stay author-controlled. Keep parse and serialization invariants aligned for programmatic steps, improve unknown-key hints and selector diagnostics, surface swipe targets in run reports and recording summaries, share positive-millisecond validation, and reuse the common flow test harness. Dispatch gesture-swipe on touch platforms and gesture-drag on Chromium, reject touch directives on Vega, and document when to prefer scroll-to or a raw system-edge gesture. |
||
|
Hubert Gancarczyk
|
2740a3ec38 |
chore(knip): clear the parked dead-code backlog and drop the ceiling to 0 (#770)
> Built on top of #769, so this PR carries that commit too — the workflows only run for PRs targeting `main`, which is why it is based there rather than on the branch. #769 on its own is the minimal, low-risk fix for the currently-red job; if it merges first this shrinks to its own commit, and if this one merges first it closes #769 as well. ## What this does #572 landed the gate with 215 findings parked under `--max-issues`. That bounded the backlog but left it in place, and the first thing to touch it (#663) pushed the total to 218 and turned the job red. This clears the backlog and sets the ceiling to **0**, so the next unused export fails the job instead of consuming headroom nobody knew was there. | | before | after | | --- | ---: | ---: | | Unused exports | 47 | 0 | | Unused exported types | 156 | 0 | | Unused exported class members | 12 | 0 | | `--max-issues` | 215 | **0** | ## Scope check first Every workspace holding a finding is `private: true`. `@swmansion/argent` — the one published package — had **zero**, so nothing here moves a public API. ## Two shapes of fix **Still referenced inside its own file → drop `export`** (146 types, 36 exports). The declaration stays put; it just leaves the module's public surface. `tsc` keeps emitting these into the `.d.ts` as local declarations that the exported signatures reference. For the 146 **types** that is the whole story: they are erased, so the emitted `.js` is byte-identical. For the 36 **values** it is not. Dropping `export` from a `const`/`function`/`class` removes its `exports.X` binding from the emitted `dist/*.js` — 36 declarations across 26 files here. `packages/tool-server/dist/utils/device-info.js` no longer carries `exports.REMOTE_PREFIX` or `exports.VEGA_SERIAL_PREFIX`; `telemetry/dist/posthog.js` loses `exports.POSTHOG_PROJECT_TOKEN`; in the ESM-emitting `argent-installer`, `isTempRunnerPath` / `resolveLocalArgentDir` / `PACKAGE_ROOT` leave the module namespace object entirely. Nothing in the repo or in the `packages/argent-private` submodule at the pinned revision reads any of them by path, so nothing breaks. Flagging it because a `require()` of a built `dist/*.js` loses a dropped `export` exactly the way it lost the deleted `__resetAndroidDevtoolsInstallCache` this branch had to restore — so this half of the diff does deserve runtime scrutiny, not a skim. **Referenced nowhere → delete.** Orphaned test seams (`__test`, `_testValidators`), stale compatibility re-exports, and a few real orphans. The ones worth a reviewer's eye: - **`localSimctl`** — the `SimctlBackend` strategy is only ever constructed with `remoteSimctl`. What keeps the local iOS impl off it is the handler signature: `buildIosLaunchHandler` / `buildIosRestartHandler` read `services.nativeDevtools`, and `ios-remote` is the only platform that declares that service, so the local impl — which resolves native-devtools per device inside its own handler — has nothing to hand them and shells out to `xcrun` itself. - **`sendCharInsert`** — removing the `__sendCharInsert` test alias exposed the underlying helper as unreferenced. It is superseded, not unwired: the Chromium keyboard impl dispatches its own `type: "char"` event inline (`keyboard/platforms/chromium.ts:58`), which is exactly what this helper did. Chromium typing is unaffected. - **Error metadata fields** — `ServiceNotFoundError.serviceId`, `ToolNotFoundError.toolId` and their two siblings were never read. The `e.serviceId` in `registry-error-events.test.ts` is a field on the event object the handler pushes, not on the error class; the `err.toolId` in `http.ts` is `NotImplementedOnPlatformError`, a different class that keeps its fields. Flagging these in case they are wanted as deliberate diagnostics. Deleting `SourceMapsRegistry.toGeneratedPosition` / `findMatchingSource` cascaded into their only helper (`buildSourceCandidates`) and then into `projectRoot`, which had no remaining reader — so the constructor parameter goes too. `StubSourceMapsRegistry` existed only to pass `super("")`, so it goes with it. ## The three cross-workspace members `ArtifactStore.register`, `ArtifactStore.list` and `TypedEventEmitter.off` are all called from other workspaces — the artifact route and screenshot tools in `tool-server`, and `registry-listener` in `telemetry`. They are reported unused anyway, and the reason is the **unbuilt tree** the gate runs against: every workspace resolves `main`/`types` to a `dist/` that does not exist, so `@argent/registry` resolves to nothing and the cross-workspace edge never forms. Build the tree and all three findings disappear on their own; a control member that really is dead is still reported in the built run, so the pass is live either way. Nothing about this is specific to `classMembers` — every cross-workspace reference is invisible the same way. Each is exempted at the declaration with a `@public` JSDoc tag rather than by name in `knip.jsonc`, for one reason: **scope**. `ignoreMembers` matches a name across the whole workspace, so it would also hide a future dead `register`/`list`/`off` on any other class in `packages/registry`; `@public` binds to the member it is written on. What `@public` does **not** buy is a staleness check, and neither does `ignoreMembers`. Measured on a clean unbuilt worktree: a bogus `ignoreMembers` name and a redundant `@public` each leave the run at exit 0 with no hint, while a bogus `ignoreDependencies` name exits 1 with `Remove from ignoreDependencies`. `treatConfigHintsAsErrors` is live and simply has no producer for either exemption style — so when one of the three stops being earned, nothing says so, and the JSDoc at each member is the only record a re-audit has. Each one names its callers, and `off` records how to re-derive them (rename the member and read the `Property 'off' does not exist` errors; grepping `.off(` over-reports, since 14 of the 38 call-site hits are Node `EventEmitter`s). One correction to the commit message on that change: it says 35 hits where the exact count is 38 — 14 Node `EventEmitter`, 24 `TypedEventEmitter` (21 production, 3 in a test). The 14 and the 21 are right. `knip.jsonc`, `CONTRIBUTING.md` step 5 and the workflow's failure-explanation step all described a parked backlog and a ceiling to stay under. All three are rewritten for a gate that must simply come back empty, and all three now name the `@public` escape hatch — the remedy for a symbol whose only caller is out of knip's reach. ## Verification Run against a clean worktree with **no build output**, the way CI counts: - `npm run knip` — **exit 0, prints nothing**. Both passes empty, no config hints. - `npx tsc --build` — clean. - `npm run lint` (`eslint . --max-warnings 0`) — clean. - `npx prettier --check .` — clean. - `npm test --workspaces` — tool-server 329 files / 3857 passed; registry, telemetry, update-core, tools-client, mcp, cli green. - `npm run typecheck:tests --workspaces` — 13 workspaces define the script and all 13 are clean, but **the command itself exits 1**: npm errors `Missing script: "typecheck:tests"` for the three that do not define it rather than skipping them. Worth knowing before re-running this bullet and reading the exit code as a failure. End-to-end on a tool-server built from this branch, driven over its HTTP API: - **Chromium** (Electron smoke app) — `describe`, `screenshot`, `gesture-tap` (counter advanced 0 → 1), `POST /api/clipboard/text` read back out of the renderer, the WS `clipboardSync` command, and `debugger-connect` / `debugger-status` / `debugger-evaluate` (`sourceMapReady: true` with `StubSourceMapsRegistry` gone). - **iOS** — `boot-device`, `launch-app`, `describe` via ax-service, `screenshot`. - **tvOS** — `boot-device`, `launch-app` on an Apple TV udid, focus-driven `describe`. The registry snapshot then carries `NativeDevtools:<tvOS udid>`, confirming injection **is** resolved on tvOS through the local impl. - **Android** — `boot-device` on a dedicated AVD, `describe` via android-devtools, `gesture-tap`. - **Metro source maps** — the registry driven against a real 12.5 MB Metro `.map` over loopback, the way `Debugger.scriptParsed` drives it: allowlist passes and `waitForPending()` resolves on both, while retained heap after gc drops from 17.3 MB to 3.6 MB. ## Review follow-ups Five commits on top, one per finding. The base is back on `main`, so the workflows run again. - **`docs(launch-app)`** — the services comment explained the empty shape through `LaunchAppAndroidServices`, deleted here, and skipped `LaunchAppVegaServices`, which is still there. Same edit in `restart-app/types.ts`. - **`docs(registry)`** — the `@public` tag on `ArtifactStore.register` named 2 of 7 callers. Derived the full set the way the sibling tag on `off` prescribes: renamed the member, read the 16 `TS2339` errors `tsc --build` reports across flow-visual (5), screenshot (4), native-profiler-stop (2), screenshot-diff (2), native-profiler-analyze, react-profiler-analyze and screen-recording-stop. The re-derivation recipe is recorded too — `.register(` greps as badly as `.off(`. - **`docs(knip)`** — "the build, the tests and this gate all stay green on a wrong delete" held for argent-private and not for the cross-workspace case that justifies three of the four `@public` tags. Deleting `ArtifactStore.register` gives 16 `TS2339` errors and `9 failed | 2 passed` in `test/artifacts.test.ts`; only the gate stays green. Split in the workflow comment, the failure annotation and `CONTRIBUTING.md` step 5. - **`perf(debugger)`** — `doRegister`'s `data:` branch still base64-decoded and `JSON.parse`d a payload it dropped, inside a `catch` that swallows. Both arms returned the same `void`, so the branch collapses to an early return; `scriptUrl` / `scriptId` go with it, since their last reader was the deleted `this.maps.push(...)` and `doRegister` is private. - **`test(debugger)`** — the allowlist check moved to a top-level early return with nothing pinning it: delete the line and both SSRF files still pass 17/17. Two tests now assert `doRegister` consults it — four rejected URLs reach no `fetch`, and the loopback `*.map` Metro emits still does. Re-verified after: `npm run knip` exit 0 and empty on a `tsc --build --clean` tree, `tsc --build`, `eslint . --max-warnings 0`, `prettier --check .` all clean, and `npm test --workspaces` with tool-server at 329 files / 3854 passed. The one red test, `argent-installer`'s `globalPath returns ~/.config/opencode/opencode.json`, fails identically with these commits stashed — it reads the real `~/.config/opencode`, which holds a `.jsonc` on this machine. End-to-end on a tool-server run from this branch over its HTTP API, on every platform: - **Chromium** — `launch-app`, `describe`, `screenshot` (returned an artifact handle, `GET /artifacts/<id>` served the 137542-byte PNG back), `gesture-tap` (counter 0 → 1), `debugger-connect` / `debugger-status` (`sourceMapReady: true`) / `debugger-evaluate` (read `taps: 1` out of the page). - **iOS** — `launch-app`, `describe` via native-devtools, `restart-app`, `screenshot`. - **tvOS** — `launch-app`, focus-driven `describe` (12 focusables in TVSettings), `tv-remote`, `restart-app`. - **Android** — `launch-app`, `describe` via android-devtools, `restart-app`, `screenshot`. - **Vega** — `boot-device` on the `tv` VVD, `describe` via the automation toolkit. - **Metro** — the real `debugger-connect` / `debugger-status` / `debugger-evaluate` tools against a stand-in Metro (HTTP `/json/list` + CDP WebSocket) emitting three `Debugger.scriptParsed` events: a loopback `*.map` on a counting server, a 1.4 MB inline `data:` map, and `http://169.254.169.254/latest.map`. `sourceMapReady: true`, `loadedScripts: 3`, evaluate 42, the loopback map fetched exactly once and the metadata URL never. A malformed inline payload gives byte-identical output, which is the finding. ### Merge with `main` Putting the base back on `main` made CI resolve the merge, and the Unit Tests job went red on a conflict no side could see on its own: `#771` fixed the same red gate this branch's first commit did, the other way round — it made `component-names.test.ts` import `StrippedName`, `ComponentAnnotation` and `ComponentNameResolution` instead of dropping their `export`. Merged `main` in, reproduced it (`tsc --noEmit -p tsconfig.test.json` → three `TS2459`s) and took main's resolution: the three exports are back. The gate stays green because knip treats test files as entry points, so a type a test imports is not an unused export. All nine workflows are green on the head commit. |
||
|
Ignacy Łątka
|
2e19c97de0 |
fix(cli): accept --help after a telemetry subcommand (#959)
Found while differentially regression-testing the 0.22.1 release against a bundle built from `v0.22.0`. ## The problem `argent telemetry status --help` exits 2 with `Error: Unknown flag: --help`. So do `-h`, and the same after `enable` / `disable`. On 0.22.0 all of them exited 0. ``` $ argent telemetry status --help 0.22.0 telemetry: / state: enabled / device id: not created (rc=0) 0.22.1 Error: Unknown flag: --help (rc=2) ``` `telemetry` is the only subcommand that behaves this way. Checked against every sibling on both versions — `server status|logs|start`, `config list|get|set`, `flow list|run`, `secrets`, `flags`, `enable`, `link`, `unlink` all return 0 on both. And the top-level help tells the user to run exactly this: "Run `argent <command> --help` for command-specific help." ## Cause `TELEMETRY_OPTIONS` declares only `scope`, and `parseCommandArgs(args.slice(1), …)` reads everything after the subcommand — so `--help` arrives as an unknown flag. The `case "--help"` arm in the switch only fires when help is `args[0]`, which is why bare `argent telemetry --help` still works. Introduced when `telemetry` was converted to the declarative parser (#952, extended by #953). ## The fix Hoist the help check above the parser, matching what `config.ts:220` and `flags.ts:92` already do; `server`, `link` and `lens` instead declare `help` in their option spec. Telemetry did neither. Ordering of `telemetryInit` / `telemetryShutdown` around the help path is unchanged, so a help invocation behaves exactly as `argent telemetry --help` did before. ## Tests Six cases added to `telemetry-command.test.ts` covering `--help` / `-h`, bare and after each subcommand, asserting usage is printed and no config file is written. Mutation-checked: with the source change reverted, 4 of the 6 fail (the 2 bare-help cases pass either way, since those already worked) — so the test pins the actual defect rather than the behaviour around it. `@argent/cli`: 498 passed, 3 skipped. `tsc --build` clean, prettier clean. ## Docs No docs update needed — this restores previously documented behaviour rather than changing a documented surface. Co-authored-by: Ignacy Łątka <f.kaminski2000@gmail.com>v0.22.1 |
||
|
filip131311
|
efb87f1917 |
chore: bump version to 0.22.1 (#956)
Bumps every workspace package, `package-lock.json`, and both version fields in `server.json` from 0.22.0 to 0.22.1. - `npm version 0.22.1 --workspaces --no-git-tag-version --workspaces-update=false` - lockfile regenerated on Node 24 / npm 11 (version-only diff, 16 lines each way) - `npm run check:versions` passes No docs update needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01EjrDqoeJa5X8Ry2ND1P714 |
||
|
filip131311
|
26405e68c7 |
refactor(cli): one declarative option parser for every hand-written subcommand (#953)
Follows #952 (merged); rebased onto main. ## Why `argent-cli` had **seven** copies of the same argv loop — `server start`, `link`/`unlink`, `flow run`, `lens`, `config`, `enable`/`disable`, and (from #952) `telemetry` — each hand-rolling `--x value` / `--x=value` / `-x` / unknown-flag / missing-value handling with slightly different messages and gaps (e.g. `lens` silently ignored unknown flags). Adding an option meant another `if (tok === "--x") … else if (tok.startsWith("--x="))` pair. ## What `packages/argent-cli/src/command-args.ts` (introduced in #952) now covers the whole subcommand surface, and every command declares its options as a spec: ```ts const START_OPTIONS = { help: { kind: "boolean", alias: "h" }, port: { kind: "value", alias: "p" }, … } as const satisfies OptionSpecs; ``` Parser features: `--name value`, `--name=value`, single-letter aliases, boolean switches, `--` end-of-options, `choices` validation, "don't swallow a following flag as a value", and the guard against a stray `true`/`false` word after a switch (previously duplicated in `flow` and `lens`). Value *semantics* (port ranges, host wildcards, flag-name charset, link targets) stay in each command — the parser only shapes argv. `flag-parser.ts` is untouched: it is the schema-driven `argent run <tool>` payload builder (JSON Schema → tool args, `--args`/stdin hatches), a different job. ## Behavior changes (bad-input paths only) | Command | Before | After | |---|---|---| | `lens --bogus` / `lens extra` | silently ignored | `lens: Unknown flag: --bogus`, exit 2 | | `lens --terminal x` | `--terminal expects "iterm" or "terminal", got "x"` | `--terminal must be "iterm" or "terminal", got "x"` | | `flow run --platfrom=ios` | `unknown flag --platfrom=ios` | `Unknown flag: --platfrom=ios` | | `unlink foo` | `Unknown flag: foo` | `Unexpected argument "foo"` | | `config … --scope x` | `--scope must be "global" or "project", got "x"` | unchanged wording | | `enable/disable`, `server start`, `link` | — | unchanged wording (tests pinned) | Happy paths are byte-identical; `server`/`link` still throw `StartFlagError`, `flow` still throws `FlagParseException` (wrapped from `UsageError`), so callers and tests keep their contracts. ## Tests `argent-cli` 489/489 (26 files): rewritten `command-args.test.ts` (aliases, `-` positional, empty-token value, true/false guard, choices message), pinned-string updates only where the table above says the wording changed (`flow.test.ts` ×4, `parse-link-flags.test.ts` ×1). `tsc --build`, `typecheck:tests`, eslint, prettier, knip green. Smoke-tested every command's error path through the built `packages/argent/dist/cli.js` (see the "Behavior changes" table — all exit 2 with usage where the command prints one). ## Docs No docs update needed: the pages document flags and their meaning, not error strings; no flag was added, removed or renamed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01BkkAihA8Kp26hHS4uhkNGu |
||
|
Jakub Piasecki
|
2e182f7971 |
fix(flows): pin iOS tree reads to the launched app instead of auto-resolving every connection (#772)
## The problem Flow runs on CI iOS fail with `ViewInspector RPC timed out: Application.getState` at seemingly random steps - mid-flow after typing, on the first read after a fresh launch, in a probe flow 25 s after the original failure - while the app under test is provably healthy: spindumps taken at failure time show its main thread idle in the runloop, display links firing, and the injected devtools queue empty. The same flows pass locally. The chain: argent's iOS injection is simulator-wide (`launchctl setenv DYLD_INSERT_LIBRARIES` inside the simulator's launchd), so every process the simulator spawns afterwards loads the devtools dylib, not just the app under test. The dylib's constructor connects unconditionally (no UIKit or `com.apple.*` guard) and the tool-server registers whatever bundle id shakes hands, so Spotlight workers (`mdworker_shared`), background-launched system apps doing first-boot work (`MobileCal`), and bare daemons (nil bundle id, registered as `"unknown"`) all become connections indistinguishable from the app under test. Flow tree reads auto-resolved the target on every read: `queryFullHierarchyTree` passed `undefined` to `resolveNativeTargetApp`, which `Promise.all`s `Application.getState` over every connection. The dylib serves `getState` with a `dispatch_sync` onto the main thread, and a background-launched or suspended process never services its main queue, so its `getState` parks until the 5 s RPC timeout and the `Promise.all` rejects. One poisoned sibling connection fails every tree read in the run; the app under test is never even asked. That one mechanism explains everything the failures did. CI spindumps caught the poisoner red-handed (`MobileCal` parked in `HandleViewInspectorRequest -> CurrentApplicationState`) next to a healthy app. "Permanent wedge" vs "recovery" was just poisoner lifetime: a Spotlight worker exits and its connection drops with it, so reads recover; Calendar lingers and the whole job stays poisoned, fresh app launches and probe flows included. And it never reproduced locally because a warm simulator did its first-boot indexing and migrations long ago, while CI boots a pristine device and does exactly that work during the test window - which also made the failures correlate with typing and tapping purely by timing. ## The fix Pin the target instead of resolving it per read. `runLaunch` records the launched app on the run state once the tree-source gate passes, as `ActionEnv.treeTarget` - a `FlowTreeTarget` carrying the `bundleId`, whether the runner still vouches for it (`pinned`), and whether its probe has ever answered in this run (`probeAnswered`). `fetchFlowTree` threads that target down to `queryFullHierarchyTree`, the only platform source that consumes it. A pinned read talks to the pinned app and nobody else: it never calls `resolveNativeTargetApp` at all, and runs four gates of its own instead (`flow-ios-tree.ts`). A `com.apple.*` id is refused first, terminally and before anything is spent - a system app is never the app under test. Then `isConnected`, a map lookup, so a dead pin reports the dropped connection instead of driving env repair from 300 ms read polls, which could otherwise latch the device's process-wide give-up. Then a probe of the pinned app's own `getAppState`, and only that app's: a timeout before this pin has ever answered is ridden out as the cold start it usually is, one after an earlier answer is refused as an app that stopped servicing its main queue. Finally a foreground check on the answered state, as lenient over one app as auto-resolve is, so a flow that left the app fails with the crisp diagnosis instead of describing a screen that is not on screen. A poisoned sibling connection is never touched. The pin is only trusted while the runner can vouch for what is on screen, and it comes off in two tiers. The five `FOREGROUND_CHANGING_TOOLS` (`launch-app`, `restart-app`, `reinstall-app`, `open-url`, `button`) drop the target outright, because the launched app may no longer be on screen at all; `launch-app` and `restart-app` then re-set it from their own `bundleId` arg, unpinned, since they name the app they switched to - which is how a recorded flow switches apps, the recorder keeping them as raw `tool:` steps. Every other raw `tool:` step demotes the pin to an unpinned hint, its effect on the foreground being opaque to the runner; a launch attempt drops the target outright until it succeeds. Either tier also retires a proven tree-outage memo: a verdict proven against the pinned branch's four gates says nothing about the path the next read runs. An unpinned read is the pre-pin behavior with one arbiter added. `resolveNativeTargetApp` fans out over every connection, and only when that fan-out times out - the CI condition this change exists for - does the hint take the read, and only after passing the same terminal `com.apple.*` policy gate, the connections check, and a foreground-checked probe of its own. A resolution that answers always wins, including its deliberate "single app but backgrounded" refusal, so the arbiter never overrides a guard that fired: it only rides out a probe a stalled sibling made unanswerable. Nested `run:` fragments share the run state, so they inherit the pin, its demotion and its clearing. E2e flows begin with a `launch` by definition, so every selector read in a CI run happens pinned. Only iOS consumes the target; the other platforms' tree sources are per-device and never auto-resolve. The review rounds also surfaced that the `isInjectableBundleId` "library validation, the dylib can never load" rationale was disproven by this PR's own spindump evidence, so that text - and every other home of the same claim, `unreadableHierarchyReason` and `treeSourceGate` included - is rewritten as the policy it actually is (system apps are never the app under test; their observed connections are background processes whose reads hang or lie) with the gates' behavior unchanged. ## What this doesn't cover Auto-resolve's own fan-out is still all-or-nothing, so every read that reaches it without a hint the gates accept stays poisonable: top-level fragment runs, `describe` without an explicit bundle id, and the stretch after a raw `tool:` step whose hint is refused or cannot answer. Recording (`captureTapSelector`) now passes the session's last `launch` id as an unpinned hint, so it arbitrates the same timed-out fan-out, but it never pins - the recorder cannot vouch for the foreground app the way a `launch:` step can. Closing the rest means hardening auto-resolve itself (exclude connections that don't answer instead of failing the fan-out) and refusing the connections at the source (dylib constructor bail for non-UIKit and `com.apple.*` processes, plus a server-side handshake filter to cover already-shipped dylibs). Both are follow-ups. ## Tests Three files, and every behavior below is mutation-proven: disabling the seam, dropping the target from any single call site, removing an invalidation, or removing any one of the four gates each fails at least one dedicated test. The reviewers' repros were turned into tests rather than argued away. `flow-ios-tree-pinned-target.test.ts` works at the tree source. It reproduces the failure shape - a healthy app plus a poisoned `com.apple.mobilecal` sibling whose `getState` only times out - and proves a pinned read probes only the pinned app while an unpinned read still fails. It then covers the states auto-resolve could never reach: the terminal `com.apple.*` refusal, including its position ahead of the connections gate so a disconnected system app is not sent into a restart loop; a dead pin, where the dropped connection is reported and no env repair runs; both sides of the `probeAnswered` split; and the foreground guard over the states a real device produces (`inactive` under a permission dialog, `inactive` with no scenes, `background` still holding a foreground-inactive scene, and `background` with none). A second block covers the unpinned arbiter: the hint read on a timeout, refused when its own probe answers from the background, and rethrowing the fan-out timeout when the hint is disconnected, cannot answer, or when the resolution failed for any other reason. `flow-launch-pins-tree-target.test.ts` proves the runner wiring through the real `fetchFlowTree` seam: every read after a `launch` carries the pin across all five read paths (assert, tap, type, rotate, idle), retry polls included; fragments inherit it; a foreground-neutral `tool:` step demotes it and a foreground-changing one that names no app drops it outright; a later launch re-pins on a fresh target with the ride-out re-armed; and a launch-free run keeps the fallback. `flow-tool-step-tree-target-e2e.test.ts` drives `createRunFlowTool` with only `resolveService` stubbed, so the real `fetchFlowTree` and `queryFullHierarchyTree` run. It pins what the two tiers cost against a wedged sibling: the launched app still arbitrating across a foreground-neutral `tool:` step, the outage memo being retired when that step demotes the pin, the `com.apple.*` and foreground refusals staying in force through the demote, and a `hidden:` assert failing after the demote instead of going green against a backgrounded app. |
||
|
filip131311
|
c2324f052d |
feat(telemetry): project-scope opt-out (restrictive merge) + init --local --no-telemetry (#952)
## Why
Teams that must not send usage data to a third party (a
strict-compliance customer asked) had no way to turn telemetry off for a
repository. `telemetry.enabled` was global-only
(`~/.argent/config.json`), so a committed local install (`init --local`)
still defaulted every teammate to **on** — `--no-telemetry` only opted
out the machine that ran `init`.
## What
- **`telemetry.enabled` accepts the project scope.** `consent.ts` now
reads both `~/.argent/config.json` and
`<project-root>/.argent/config.json` (root resolved from `cwd`, per-file
cache keyed by path) and merges **restrictively**: `false` in either
document wins. A committed project opt-out holds on every clone, and a
project file can never re-enable a global opt-out. Env overrides
(`DO_NOT_TRACK`, `ARGENT_TELEMETRY`) still outrank both.
- **`argent telemetry enable|disable --scope global|project`** (default
`global`). `status` names the document that decided (`config.json
(project)`, `(global)`, `(project and global)`). `enable` at one scope
while the other still opts out reports that telemetry stays disabled
instead of claiming success. Unknown scope / argument → exit 2, nothing
written.
- **`argent init --local --no-telemetry`** also writes
`telemetry.enabled: false` to `.argent/config.json`, and the "commit
these files" checklist lists it. Global mode is unchanged.
- `argent config set/unset telemetry.enabled` stays delegated to `argent
telemetry` (`manageCommand`), so the lifecycle work (draining the live
client) is still done by the dedicated command.
## Tests
Unit: `telemetry` (+8 project-scope cases: single-scope opt-out, both
restrictive directions, both-off, env precedence, live re-read of the
project file, malformed project file), `configuration-core` (schema
scopes), new `argent-cli/test/telemetry-command.test.ts` (real-fs, 6
cases), `argent-installer` — all green; eslint, prettier, knip,
`docusaurus build` green.
Live, from a real `npm pack` tarball of this branch (sandboxed `HOME`):
1. Fresh project → `init --local --yes --no-telemetry --from <tgz>`:
writes `.argent/config.json` `{telemetry:{enabled:false}}` + global
opt-out; summary lists `.argent/config.json` to commit.
2. `telemetry status` → `disabled`, source `config.json (project and
global)`. `telemetry enable` (global) → "stays disabled: config.json
(project) still opts out"; from `~` (no project) → `enabled`. `enable
--scope project` → enabled; `disable --scope project` → disabled.
`--scope team` → exit 2.
3. **Teammate simulation**: copied only the committed files to a fresh
clone with a fresh `HOME` (no global opt-out), bundle built with a dummy
OTLP token, `ARGENT_TELEMETRY_DEBUG=1`, drove `argent mcp` (initialize →
tools/list → `list-devices` call). Control clone without
`.argent/config.json`: tool-server emitted `toolserver:start`,
`tool:invoke`, `tool:complete`, `toolserver:stop`. Clone with the
committed opt-out: **0 events**, and the first-run telemetry notice is
not printed.
## Caveat (documented)
The project file is resolved from the working directory of the process.
Editors start the MCP server in the workspace root, so it and the
tool-server it launches read it. The tool-server is shared by every
editor on the machine and reads the project file of the workspace that
started it; the global opt-out or `DO_NOT_TRACK` still covers the
machine-wide case.
## Docs
Updated in this PR: `reference/configuration.mdx` (scopes + merge
wording), `reference/telemetry.mdx` (new "Opt out for a whole
repository" section), `reference/cli.mdx` (`init`, `telemetry` rows),
`fundamentals/installation.mdx` (`--no-telemetry` in local mode).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01BkkAihA8Kp26hHS4uhkNGu
|
||
|
Hubert Gancarczyk
|
50c9580e30 |
fix(tools): make a schema failure name the parameter that was sent (#718)
> Stacked on #717. ## The problem An invalid tool call returned Zod's raw issue array. Someone who wrote `flow_name` instead of `name` got back, depending on the tool: ``` Invalid params for tool "flow-start-recording": [{"expected":"string","code":"invalid_type","path":["name"]}] Invalid params for tool "flow-execute": [{"code":"custom","path":["flow_path"],"message":"Pass exactly one flow source: name or flow_path."}] ``` The first names the parameter the tool **wanted** and never the one the caller **sent**. The second — flow-execute's, whose `name` is optional so `flow_path` can stand in — names neither: it is anchored on the one source field the caller had no reason to send. Either way the mistake is invisible, and working it out costs a whole turn. ## What changes **1. `describeParamIssues` — one sentence per bad parameter.** Missing fields read as `` `name` is required (string) and was not provided`` rather than as a type error about `undefined`. Unknown keys are named and path-qualified, so a key nested in `selector` is not reported as a bare name that contradicts the top-level list printed beside it. The message closes with the caller's own keys — `You sent: \`flow_name\`, \`project_root\`.` — which is what makes the mistake self-evident. Key names only, never values: this string reaches logs, telemetry and the agent transcript, and a params object can carry a secret. Wired into both entry points: the registry's `invokeTool` and the HTTP boundary. **2. `flow-execute` accepts `flow_name` as an alias for `name`.** The tool is called `flow-execute`, so "the flow's name" spells itself that way; this is a mistake worth absorbing rather than rejecting. The name is resolved in code rather than by the schema — a Zod `required` failure would once again name only the field it wanted — and the error text names the spellings that are silently discarded (`flowName`, `flow`, `flow_file`), because by then Zod has already stripped them and the caller has no other way to learn their key was dropped. **3. A flow DIRECTIVE passed as `command` points at the tool that records it.** `command: "echo"` used to come back as a bare "Tool not found". It now names `flow-add-echo` — and says to call it **directly**, since routing it through the recorder would write the echo *and* a `tool: flow-add-echo` step that fails on every replay. The hint distinguishes directives the recorder **rewrites** from those it stores **raw** for the polish pass, because promising a rewrite that does not happen sends the author looking for a directive that is not in the file. `wait` and `long-press` get their own answers: neither has a recording tool at all. Only a genuine not-found is rewritten — a tool that ran and failed still reports its own error. **4. A recorder tool as `command` is refused.** `flow-add-echo`, `flow-add-step`, `flow-start-recording`, `flow-finish-recording` each mutate the recording itself, and nesting one reported success either way. Each is refused for its own reason: `flow-add-echo` would write the echo **and** a replay-breaking `tool:` step, `flow-start-recording` would **erase** this flow at replay, `flow-finish-recording` would end the take it is a step in, and `flow-add-step` cannot record itself. Now nothing is executed and nothing is written. --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com> |
||
|
filip131311
|
efae6a8fb6 |
feat(tools): opt-in emulator sound on boot-device + boot-sound default flag (#813)
Field report ([Slack thread](https://swmansion.slack.com)): when a session needs to test audio, it is very non-obvious that argent boots the Android emulator muted — the agent (and the user) burned significant time discovering it. The mute default is intentional (several agent-driven emulators all playing sound on the host would be worse), so this PR keeps it and instead makes it **discoverable and overridable**: ## 1. `sound` argument on `boot-device` (default `false`) - `sound: true` boots the emulator with audio output (drops the mute launch flag). - Applied consistently to the snapshot-loadability probe, hot boot, and cold boot — the qemu-device-topology parity invariant is preserved. A `default_boot` snapshot saved in the other audio mode fails the probe and falls through to a cold boot that re-saves it, so toggling costs one slower boot and nothing else. - iOS/Vega/Electron are unaffected (argent never mutes them); the argument documents itself as Android-only. ## 2. Agent-facing information: argument description only Per the thread, the info lives in the tool argument's description (no skill changes): it states the muted default, *why* (multiple agent-driven devices sharing the host's speakers), the `force: true` hint for an already-running muted emulator, the cold-boot cost of toggling, and the `boot-sound` flag. The underlying emulator flag is described behaviorally ("muted"), not by name, per our agent-docs convention. **Validated that the description alone is sufficient** by driving three models through the Cursor `agent` CLI with the schema excerpt and the exact scenario from the field report ("testing notification sound, hear nothing, emulator already booted with defaults"). All three — composer-2.5, gemini-3.7-flash, gpt-5.2 — correctly diagnosed the muted default and produced the exact fix `boot-device({avdName, sound: true, force: true})`, including the weakest model. Transcript excerpts in the PR discussion below if wanted. ## 3. `boot-sound` feature flag `argent enable boot-sound` defaults the `sound` **argument** to `true`. It only moves the argument's default, read live per call: an explicit `sound: false` on a call still boots muted, and without the flag the default remains muted. Registered in `FLAG_REGISTRY` so `argent enable`/`argent flags` pick it up with no further wiring. ## Tests - New `boot-device-sound.test.ts`: mute-by-default, `sound: true` dropping the mute flag from **both** probe and hot-boot spawn (parity), cold-boot spawn, flag-driven default, and explicit-`sound:false`-beats-flag. - The three existing Android boot test files now pin `isFlagEnabled` to `false` so a developer's real `flags.json` can't flip the args they assert on. - `tool-server` boot suites 62/62 green, `argent-cli` flags 33/33, `configuration-core` 122/122; lint, `typecheck:tests`, and knip all pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Ignacy Łątka
|
0cddeced33 | Merge origin/main into fix/flow-batch-invalid-verdict | ||
|
filip131311
|
8dda6aa1f9 |
docs: cover the paste tool on the interacting features page (#941)
The `paste` tool was the only capability in the **Interacting** tool group with no mention on the `features/interacting-with-apps` page — typing, gestures, shake, buttons and permissions are all covered there. It already has its row in the tools reference. This adds the minimal parity: - one sentence in **Gestures and text**: for long text, the agent puts the text on the clipboard and pastes it in one step; - the **Limits** bullet for the sim/emu-only restriction, merged into the existing shake bullet, which has the same restriction. No reference changes needed — `reference/tools.mdx` already lists `paste`. Checks: `npx docusaurus build` in `packages/docs` passes; `npm run format` from the repo root leaves the file unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01YPSUSz8gDxGm7tb9tA3b4D |
||
|
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>
|
||
|
filip131311
|
5d76ed4d66 |
feat: tag artifacts with a semantic kind at registration (#564)
Fresh, from-current-main take on the artifact-kind idea from #398 / #466. Knowing what an artifact *represents* (not just its MIME type) is what a UI needs to group and name artifacts (e.g. in EAS); this PR delivers exactly that — the tagged handle — without the declared-output-map machinery. ## Summary - Adds a closed `ArtifactKind` union to the artifact wire contract; `kind` is carried on `ArtifactHandle`, `ArtifactEntry`, and the `/artifacts` inventory items. - `ArtifactStore.register` now takes a single `{ hostPath, kind, ... }` options object, so the compiler forces every producer — current and future — to declare what it is emitting. - Tags **all** producers on main today, including the ones #398/#466 predate: screenshot (all four capture paths), screenshot-diff, **flow snapshot baselines/diffs** (`screenshot` / `screenshot-diff-context`), **screen recordings** (new `screen-recording` kind), and the native/React profiler exports and reports. - Types the native-profiler export keys at their source (`IosExportKey` from the `EXPORTS` table, `AndroidExportKey`) and maps them through a kind table that is **total over both unions** — a newly added export key fails to compile until classified, replacing the previous stringly-keyed loop (and the `?? "native-profile-trace"` silent-default this design would otherwise need). - Client-side (`@argent/tools-client`), the mirrored handle type keeps `kind` **optional and widened** (`ArtifactKind | (string & {})`): an older, already-running tool-server sends no kind, and a newer server may send kinds this client build predates. Consumers treat it as advisory metadata; unknown kinds are opaque, never an error. ## Differences from #398 / #466 (deliberate) - No `@argent/artifacts` package, no third `ToolDefinition` generic, no per-tool output-name registrar, no runtime throw for “undeclared output” — the kind is plain data at the registration call site, checked at compile time where it can be, and a wrong label can never fail a tool at runtime. If a pre-invocation `artifacts` declaration in tool-listing metadata is ever needed (an actual EAS consumer), it can be layered on later without changing this wire contract. - The two `ArtifactKind` definitions (registry / tools-client) stay deliberately separate — same as the existing `ARTIFACT_MARKER` must-match convention — because the client-side type is intentionally *looser* (optional + open) than the server-side one for version skew. - Extending the union stays backward-compatible on the wire. ## 2026-08-24 update Merged current main (0.22.0) into the branch. The flow-visual rewrite that landed since (cropOn crops, per-app snapshot-key dedupe, diff scratch-dir hygiene) is preserved verbatim; its five registration sites now carry kinds (`screenshot` for baseline/current, `screenshot-diff-context` for the annotated diff — the cropped `current` included). No producer added since the PR was opened needed a new kind: `paste`, `shake`, and the other new tools emit no file artifacts, and every register call site in the tree now goes through the typed options object. No docs update is needed: the docs site (added since this PR) does not document the artifact handle wire contract; `kind` is advisory wire metadata with no user-facing surface. ## Validation At the merge with main (0.22.0), 2026-08-24: - `npm run build` (full `tsc --build`) — clean - `npm run typecheck:tests` in registry, tools-client, tool-server, mcp, cli — clean - Full suites green: registry 90, tools-client 191, tool-server 4495 (+1 skipped), mcp 80, cli 472 - `prettier --check .`, `eslint . --max-warnings 0`, and the knip gate — all pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Kacper Kapuściak
|
e0fcf6c995 |
chore(docs): add Google Search Console verification meta tag (#939)
Adds the `google-site-verification` meta tag to the docs site via Docusaurus `headTags`, so docs.swmansion.com/argent can be verified as a property in Google Search Console. ## Test plan - `npx docusaurus build` in `packages/docs/`, confirm the meta tag renders in the built HTML `<head>`. - After deploy, hit Verify in Google Search Console. No further docs update needed: this is a site config change with no user facing capability change. |
||
|
Hubert Gancarczyk
|
f21811d069 |
fix(flow): re-probe a recorded wait against the tree the runner reads (#717)
> Stacked on #715. ## The problem `await-ui-element` evaluates against the tree it reads — the **accessibility hierarchy** on iOS and Android, the **CDP DOM** on Chromium, the **toolkit page source** on Vega. The `await:` / `assert:` directive that the polish pass converts a recorded wait into is evaluated against the **runner's** tree instead. They overlap, but outside Vega neither contains the other — an id present in one can be absent from the other, and on iOS even the role vocabularies are disjoint. Both directions have been hit in practice: an id visible to the runner but not the recorder, and text-field values visible to the recorder but not the runner. The consequence is that a check can pass live and fail once converted. That makes the core promise — *each step is executed live so you verify it works before it's recorded* — untrue at exactly the point it is relied on. ## What changes After a recorded `await-ui-element` succeeds, the recorder re-runs the same condition against the tree the runner resolves directives against, and reports the result on the step it just wrote. **It warns; it does not refuse.** What gets written is a raw `tool: await-ui-element` step, and at replay that tool reads the same tree it just passed against — so "this would fail every run" was never true of the form actually recorded. What the probe genuinely reports is whether the **polish-time conversion** is safe, which is why the warning says to act on it *then* rather than re-record now (re-recording would duplicate the step). Because polish begins after `flow-finish-recording`, each verdict is also carried there: it is repeated on its own step's `summary` line, and `message` counts how many steps carry one. A warning raised at step 7 of a 40-step recording is otherwise long out of reach by the time it is actionable. ## Three outcomes, kept distinct | Runner tree says | Reported as | | --- | --- | | condition holds | nothing — step recorded clean | | condition does not hold | recorded + warning naming the causes to rule out before converting | | tree unreadable, or too slow to answer | recorded + warning that the conversion is **UNKNOWN, not known-bad** | The middle row is deliberately not "the conversion will fail". The identical verdict comes back when the screen simply moved on between the live wait and the re-probe, and on `text` checks when the selector matches several elements and the two sides elect different ones — so the consequence stays conditional on the cause, and the message names the causes in the order worth ruling out. The third row matters: nothing was compared, so calling it a divergence would send the author off to rewrite a selector that may be perfectly good. It is also the injection-free case the flow skills explicitly sanction, so refusing there would block a supported way of working. On iOS the quoted reason ends "provide bundleId explicitly" — advice aimed at the native tools. The warning corrects it, because no directive carries a bundleId and forwarding one would make the probe answer a question the runner never asks. ## Platform-specific explanation The *why* differs per platform, and stating the iOS story everywhere makes the message wrong where someone is trying to act on it. iOS gets AX-vs-native-hierarchy; Chromium gets "the flow tree keeps only addressable nodes"; Android gets the importance trim; Vega gets "the trees agree, so the screen changed". The named tool for reading the runner's side follows the same rule — pointing an Android author at `native-find-views` points at a tool they cannot call. The remedy is per **condition** as well as per platform: on `hidden` a determinate verdict means the runner's tree still *has* the element, so "retarget at an id the full hierarchy carries" is the opposite of what helps. ## A failed live wait is reported by cause `await-ui-element` returns `{ success: false }` for three different things: a condition read and found false, a tree source that never answered, and a cancelled wait. Only the first judges the condition, so only the first is narrated as "the wait never held" and prescribes re-recording or deleting the step. The other two observed nothing and are reported as UNKNOWN — the same unknown-vs-known-bad distinction the probe draws for the runner's tree. A wait nested inside a recorded `run-sequence` gets neither warning: that tool reports its own shape, which carries no `success` key. The skill says so, and says to read `toolResult` there. --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com> |
||
|
filip131311
|
366f17c619 |
chore: bump version to 0.22.0 (#891)
Bumps every workspace package, `package-lock.json`, and both `server.json` version fields from 0.21.0 to 0.22.0 (18 files). Lockfile regenerated on Node 24 / npm 11 so the diff is version-only; `npm run check:versions` passes. Covers everything on main since `v0.21.0`: `paste` tool (#879), rotation-aware Android capture (#693), 0.25 default screenshot scale (#878), no-reboot describe on externally booted sims (#583/#877), NDJSON flow progress (#855), OTLP telemetry (#570), native-devtools restart_required derivation (#560), Android profiler empty-CPU explanation (#669), plus test/CI fixes. Full-surface live QA of this main (tgz vs npm 0.21.0) on iOS, Android, tvOS, Electron, CLI and Linux/remote found zero regressions. Two release notes: the published artifact must be built with `ARGENT_OTEL_INGEST_TOKEN` set (a local tgz without it compiles `readIngestToken()` to an empty string, making telemetry a silent no-op), and remember to push the `v0.22.0` git tag so online `init` skills installs resolve. No docs update needed: version-only change.v0.22.0 |
||
|
filip131311
|
172e4d3a64 |
fix(android): capture the screen the way it is actually oriented (#693)
Part of #609. ## What was wrong On a rotated Android device `screenshot` returned a portrait-framed image with the content lying sideways. `describe` frames and gesture coordinates were **already upright** — uiautomator measures against the rotated display — so the capture was the one surface out of step. An agent reading that image saw geometry that disagreed with every coordinate it was given. Measured on a landscape Pixel_9 (API 36, `mCurrentOrientation=1`): | | space | |---|---| | `describe` | upright — `"Search Settings"` centre `(0.530, 0.234)` | | taps | upright — tapping describe's `"Display & touch"` centre opens it | | `screenshot` (default) | **portrait-native, sideways** | ## The fix simulator-server already applies `rotation` correctly; nothing was telling it which one. The rotation is now queried from the device per capture, so `screenshot`, `screenshot-diff`'s live captures and the auto-attached "Screen after action" screenshot all land in `describe`/tap space. **Nothing is tracked.** A cached orientation could only be staler than the ~40 ms probe, and serving a stale one is exactly the failure being fixed. When the rotation cannot be read the request is left byte-identical to before rather than guessing — a wrong orientation is a confidently wrong image, which is worse than a sideways one. **iOS is deliberately untouched.** There the whole surface is consistently portrait-native, so rotating only the capture would *break* an agreement rather than restore one. The iOS half of #609 needs a different change and is not in this PR. ### Also fixed: the legacy uiautomator describe path Same root cause, worse symptom. `wm size` reports the **unrotated** size — on the landscape Pixel_9 it still answered `Physical size: 1080x2424`. Dividing rotated bounds by that does not merely squash frames: `isVisibleRect` prunes nodes whose left edge is past the screen width, so **the right-hand half of a landscape screen vanished from the tree**. The dump carries `<hierarchy rotation="N">`, so the divisor is oriented from bytes already in hand — no extra round-trip on the path that runs precisely when the helper could not be reached. ## On the rotation-name table Pinned by measurement, never by what the names mean — the reporter of #609 found the same names are 180° inverted on iOS. `adb exec-out screencap` is rotation-aware, so it is ground truth; each candidate was scored as mean absolute difference against it both as-is and rotated 180°, so an inverted mapping could not pass: ``` rotation 1 LandscapeLeft 2.19 (vs 6.54 for its 180° twin) rotation 3 LandscapeRight 2.19 (vs 6.34) rotation 2 PortraitUpsideDown 2.67 (vs 13.13) rotation 0 Portrait identity ``` This table is the **inverse** of simulator-server's own `90° → LandscapeRight` convention, because it compensates for a rotation that layer applies while decoding the scrcpy stream. That makes it a property of a particular simulator-server build rather than of Android — which is why a table test alone is not enough, and the delivered PNG's aspect is checked against the rotation requested, falling back to an unrotated capture if they disagree. ## Verified live Branch tool-server against a real emulator: - landscape, default capture → **2424×1080 upright**, MAD 2.19 vs ground truth (6.54 vs its 180° twin) - cropping `describe`'s frame for the toolbar out of that default capture shows exactly the "Display & touch" toolbar — the two spaces agree - rotation 0 → 1080×2424, MAD 2.82, unchanged - rotated **iPad** → still 2064×2752 sideways, i.e. iOS provably untouched Plus 27 new unit tests, including one that asserts the legacy path *drops* the right-hand node before the fix and keeps it after. Full suite 3125 passing; lint, prettier, `typecheck:tests` and the tool-description gate all clean. ## Not in scope - The iOS half of #609. - `rotate` on Android silently no-ops when auto-rotate is enabled (it returned `{orientation:"LandscapeLeft"}` while `dumpsys` still read `cur=1080x2424`). Filing separately. - `screen-recording` still records sideways — same root cause, different pipeline. Worth noting a session's screenshots are now upright while its video is not. - Existing visual baselines captured on a rotated Android device will stop matching, since the baseline key embeds capture dimensions and those now swap. This fails loudly as a missing baseline rather than silently comparing the wrong thing — and those baselines were sideways captures of an upright UI. --- > **#673 and #658 are both stacked on this branch** — each conflicts with this one (`screenshot/index.ts` and `screenshot-diff/index.ts` respectively) but not with each other, so both were rebased onto this branch as siblings rather than chained. Merge this first; GitHub then retargets both to `main`. --------- Co-authored-by: Filip131311 <f.kaminski2000@gmail.com> |
||
|
filip131311
|
d0b3356ebf |
feat(tool-server): paste tool (device clipboard + paste shortcut) (#879)
## Summary Registers a real `paste` tool. The `tools/paste/index.ts` from #506 was never registered and sent a `{cmd:"paste"}` WebSocket command simulator-server doesn't implement — dead on every platform. - **iOS simulator**: simulator-server `POST /api/clipboard/text` fills a private pasteboard and pushes it into the sim synchronously (host clipboard untouched); the tool then sends the ⌘V chord over the existing `key` command (the same `simulatorSendKeyEvent` path the keyboard tool uses). - **Android emulator**: same route over the emulator gRPC bridge, then `KEYCODE_PASTE` via `adb shell input`. - **ios-remote**: the MoQ transport's existing paste primitive, awaited. - Clean 400 for tvOS / Android TV (runtime probe), physical devices, Chromium (a renderer paste would clobber the *host* clipboard), Vega. - `{{secret:<NAME>}}` placeholders as in `keyboard`; allowed inside `run-sequence`; device-interact SKILL updated. - Description/skill tell the agent `paste` is **not** a `keyboard` substitute — only where a real user would paste (OTP/2FA code, long link/token, testing paste handling). ## Blocked on a simulator-server built with the `clipboard` feature: software-mansion/radon#185 adds it to the `argent` alias (no sim-server API change). Until a release + bin bump, the tool reports `PASTE_CLIPBOARD_UNSUPPORTED` (with an update hint) against the shipped binary. ## Test plan - 13 new unit tests (`test/paste.test.ts`) + secret-placeholder tests; full tool-server suite, lint, knip green (the 4 telemetry/bind test files that fail locally fail identically on `main`). - Live against a locally built simulator-server: iOS 26.5 Settings search (12 back-to-back pastes in order), Messages composer (multi-line/tab/emoji/diacritics preserved), `{{secret:EMAIL}}` never echoed or logged; Pixel_9 Settings search (ASCII + `Zażółć🔑`), Expensify RN login field; `run-sequence` paste steps; live 400s for tvOS sim / phone serial / chromium id. |
||
|
Kacper Kapuściak
|
5e81136096 |
docs: generate an Open Graph card for every page at build time (#889)
## What Every docs page now gets its own Open Graph card, rendered during the build. The card is `static/img/og-background.png` with a frosted glass panel in the middle: a 920x400 rounded rectangle with a 32px radius, the background blurred behind it (`backdrop-filter: blur(60px)`, an SVG `feGaussianBlur` of stdDeviation 30) and tinted with `#C3D3E033`. The white Argent logo sits centred on the panel, with the page title centred underneath in DM Sans Medium at `#FFFFFF`. Before, every page shared one static `img/logo-icon.png`. ## How `plugins/og-image/index.js` is a Docusaurus plugin with a `postBuild` hook. It: 1. Renders the background and the frosted panel once. That part is the same on every card, so it is reused as a bitmap and only the logo and the title are rendered per page. 2. Reads each built page's `<title>`, strips the ` | Argent` suffix, and lays out the logo and the title with satori. Long titles wrap and stay centred. 3. Rasterises the result with resvg into `build/img/og/<route>.png`. 4. Rewrites the `og:image` and `twitter:image` tags of that page to the new absolute URL, and adds `og:image:width` / `og:image:height`. Satori embeds every glyph as a path, so resvg needs no font lookup. DM Sans Medium is vendored in `scripts/og-assets/` (with its OFL license), so CI renders the cards exactly as a local build does. Layout, colours and blur are constants at the top of the plugin. Pages with no title of their own get a card with the logo alone. That covers the home page, whose title is the site title the logo already shows, and the search page, whose title the Algolia search theme renders as `[object Object]`. The shared `@swmansion/t-rex-ui` theme points doc pages at `img/og/<title>.png` and hardcodes a "React Native Reanimated" fallback. The plugin overwrites those tags after the build, so the naming is route based and consistent across doc pages, the home page, search and 404. ## Verification Against a clean `npx docusaurus build`, checked for all 20 built pages: the tag is an absolute site URL, the target exists in `build/`, it is a valid PNG at 1200x630, `twitter:image` matches `og:image`, the width/height tags are 1200/630, and `twitter:card` is `summary_large_image`. No unreferenced cards. Serving the production build over HTTP, every card URL returns `HTTP 200` with `content-type: image/png`. `npm run typecheck`, `npm run build`, and the root `npm run format` and `npm run lint` all pass. ## Notes - The cards add ~16MB to a 65MB build, about 845KB each. The background is a photograph and resvg only emits PNG. Re-encoding as JPEG would bring that down to roughly 2MB at the cost of a `sharp` dependency, if the deploy size becomes a concern. - New `packages/docs` dependencies: `satori` and `@resvg/resvg-js`. Both record all platform binaries in the lockfile, including `linux-x64-gnu` for the CI runner. - The branch also carries a `docs: update favicon` commit, which replaces `static/img/favicon.png`. ## Docs No user facing docs change: this is build internals. The build step is documented for contributors in `packages/docs/CLAUDE.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JovgRQBFngBsHnUYSoRF56 |
||
|
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> |
||
|
Ignacy Łątka
|
08f91fffd1 |
perf(screenshot): lower the default capture scale to 0.25 (#878)
Screenshots are billed to an agent's context by **pixel area**, not by file size — so PNG level, JPEG quality and codec choice buy nothing here, and the default scale is the only real lever. `0.3` had never been measured against what a model can actually read, so I measured it. ## What was run Bluesky (`bsky.app`) built from source and driven on an iOS 18.6 simulator, signed in against the repo's own seeded dev PDS (`yarn e2e:mock-server` + `e2eSignInAlice`) — a live feed, profile, notifications, Explore and Settings, not marketing screens. Every image came out of argent's own `screenshot` tool over the tool-server HTTP API, so the downscaler under test is the shipped one. **160 API calls**: 5 screens × 5 scales × 2 models × 3 reps, plus a blind control per screen per model. Questions were the things an agent actually needs off a screen — an `@handle` two rows down, a relative timestamp, a follower count, a badge number, which tab carries the underline, the label of the row above another row. Two controls decide whether the numbers mean anything: - **Blind pass.** Each model answered every question with *no image at all*. The 6 of 30 questions it still got right from priors alone (the screen title, the tab selected by default) were struck from scoring. 24 survived. - **Counting split out.** "How many rows between X and Y" was unreliable at *every* scale including full resolution — it measures arithmetic, not legibility — so it is excluded from the headline and reported separately. ## Result Text-reading accuracy, 66 graded answers per cell. Token counts are the API's own reported `input_tokens`, image column net of the ~345-token prompt. | scale | image tokens | Haiku 4.5 | Opus 5 | |---|---|---|---| | 1.0 | 2692 | 100.0% (66/66) | 100.0% (66/66) | | 0.3 *(old default)* | 451 | 98.5% (65/66) | 100.0% (66/66) | | **0.25** *(new default)* | **298** | **100.0% (66/66)** | **100.0% (66/66)** | | 0.2 | 196 | 98.5% (65/66) | 100.0% (66/66) | | 0.15 | 114 | 81.8% (54/66) | 97.0% (64/66) | 0.25 is the only scale that scored perfectly for both models on every repetition, and it costs **34% less context** than 0.3. ## The cliff, and why the margin Every wrong answer at 0.15: | question | truth | answered | model | | |---|---|---|---|---| | 2nd author's handle | `@bob.test` | `@carla` | Haiku ×3 | read the row above | | selected feed tab | `Following` | `Feeds` | Haiku ×3 | couldn't resolve the underline | | 4th profile tab | `Videos` | `Likes` | Haiku ×3 | drifted two positions | | 3rd settings row | `Moderation and content filters` | `Privacy and security` | Haiku ×3 | off by one row | | bell badge number | `1` | `UNREADABLE` | Opus ×2 | declined rather than guess | **Opus fails safe; Haiku fails silent.** Opus refused the unreadable badge. Haiku returned a confidently wrong *neighbouring* row — the same wrong answer three times out of three. For an agent that turns a read into a tap, a confident off-by-one row is worse than a refusal, because nothing downstream catches it. ## Why 0.25 and not 0.2 0.2 measured no worse than the old default and would have saved 57% instead of 34%. I didn't take it: the evidence is five screens of one app at one device size, and Bluesky sets fairly generous type. A denser interface — a table, a settings screen with subtitles, a smaller phone — reaches the floor sooner. 0.25 keeps two steps of margin above the cliff instead of one. `ARGENT_SCREENSHOT_SCALE=0.2` stays available for anyone who wants to spend that margin deliberately. ## Scope - **Chromium is untouched.** Its default is 1.0 and its `scale` is silently dropped without the optional `sharp` dep — that's #673's territory and I'm not duplicating it. Worth noting the two interact: once Chromium can downscale (via the in-repo `resizeDecodedPng`, as #673 suggests as a follow-up), it should pick up this default too, and it is currently by far the most expensive platform per capture. - **`ARGENT_SCREENSHOT_SCALE` is documented as `0.3` in `argent-private/docs/environment-variables.md`** — a submodule, so it needs its own PR to stay in sync. - Stale prose fixed in passing: `getScreenshotScale` carried a `// default: halve the resolution` comment next to a `0.3`, and `vega-screen.ts` documented the fallback as `0.3`. ## Checks `npm run build` clean, `tsc --noEmit -p packages/tool-server/tsconfig.test.json` clean, eslint `--max-warnings 0` clean, prettier clean. Shard 1/4 of the tool-server suite: 88 files / 1261 tests pass; the four files touching screenshot scale pass individually (22 tests). The default had **no test coverage at all** — the constant could be edited to any value and the suite stayed green. This adds one. Mutation-checked: reverting the constant to `0.3` fails 6 of the 8 new cases. ### On the red Unit tests check Inherited, not introduced. This branch's Unit tests job fails on exactly one test — `describe-tool.test.ts > states the limitation without ordering a reboot when the degraded read returned a tree` — which is the same single test failing on `main` itself (main has been red since #583, this branch's base). Already tracked as #871 and fixed by #877. `flow-composition.test.ts` passes in CI on both. Everything else is green: ESLint, Prettier, Knip, Static checks, SpiderShield, lockfile sync, Linux + Chromium E2E, macOS + Chromium E2E, macOS + iOS E2E. |
||
|
Ignacy Łątka
|
d43938e4fe |
fix(describe): drop the redundant restart assertion that reddened main (#877)
Fixes #871. Supersedes #872, which fixed this the wrong way round. ## The assertion was demanding a restatement `describe-tool.test.ts:793` asserted: ```ts // It still names the remedy and what the remedy costs. expect(result.hint).toMatch(/boot-device/); expect(result.hint).toMatch(/restart/i); ``` against `DEGRADED_STANDING_HINT`, which reads: > …If something you expect is missing, **boot-device with force=true reboots the simulator** with the full accessibility settings "reboots the simulator" is already there. So `/restart/i` could only ever be satisfied by a *second* clause restating the reboot — and that is precisely what #583 carried: ``` … with the full accessibility settings — at the cost of restarting it and everything running on it. ``` A review suggestion applied through the GitHub web editor trimmed that clause (`dd99a2625`, `07f39cea8`) at 07:41:49Z and 07:41:57Z. The squash-merge fired at **07:42:23Z** — 34 seconds later, so no CI ran on that head. The trim was right: it removed eight words that said the same thing twice. The test was what was wrong. #872 restored the redundant clause to turn CI green. This deletes the assertion instead, and leaves the shipped hint byte-for-byte as it is on main today. ## No coverage leaves with it The four remaining assertions are the ones that carry the test's purpose — #583 existed to stop the caveat *ordering* a reboot that kills the developer's Metro session: ```ts expect(result.hint).not.toMatch(/MUST/); expect(result.hint).not.toMatch(/\bnow\b/i); expect(result.hint).not.toMatch(/before continuing/i); expect(result.hint).toMatch(/boot-device/); ``` Mutation-checked. Restoring the pre-#583 wording — `"You MUST call boot-device with force=true now to restart the simulator … before continuing."` — fails the test on the first assertion: ``` × states the limitation without ordering a reboot when the degraded read returned a tree AssertionError: expected 'This simulator was not booted through…' not to match /MUST/ Test Files 1 failed (1) | Tests 1 failed | 36 passed (37) ``` So the regression the test guards is still guarded. The deleted line was pinning a redundancy, not a behaviour. ## Verification Branched off `dd5d82383` (current main). Reproduced the failure there first — `1 failed | 36 passed` on the untouched tree. | gate | result | |---|---| | `vitest run` (tool-server) | **351 files, 4313 passed, 1 skipped**, exit 0 | | `tsc --build` | clean | | `typecheck:tests --workspaces` | clean | | `eslint . --max-warnings 0` | clean | | `prettier --check` | clean | Diff is one test file, one assertion and half a comment: ```diff - // It still names the remedy and what the remedy costs. + // It still names the remedy. expect(result.hint).toMatch(/boot-device/); - expect(result.hint).toMatch(/restart/i); ``` The comment loses "and what the remedy costs" because that half described the assertion being removed. |
||
|
schicaelmott
|
dd5d823833 |
feat(flow): stream run progress as NDJSON (#855)
## Summary - add `argent flow run --json-stream` - stream flow progress as JSON lines - finish with a JSON result or error ## Why `--json` prints only after a flow finishes. `--json-stream` lets scripts and CI see live progress. ## Why this is a separate PR This is useful without the custom-tools feature in #856. ## Validation - `npm test -w @argent/cli -- flow.test.ts` — 106 tests passed - `npm run typecheck:tests -w @argent/cli` - `npm run build -w @argent/cli` --------- Co-authored-by: Filip131311 <contact@filipkaminski.com> |
||
|
Ignacy Łątka
|
da61d4f2eb |
test(tool-server): pin the ARGENT_* suite guard and correct what it documents (#859)
Post-merge review of #707. Three findings, all from that change's own surface; no behaviour outside the test suite moves. ## 1. Nothing pins the guard `test/setup/clear-argent-env.ts` strips the developer's `ARGENT_*` overrides so unit tests assert argent's shipped defaults. On a machine with nothing exported its loop body never runs, and CI is exactly such a machine, so no suite failure can report the guard being weakened. Measured at `9dc7c7cfc`, each mutation applied alone to the shipped file: | mutation | full tool-server suite | |---|---| | body replaced with `export {}` | 350 files / 4300 passed, **exit 0** | | `startsWith("ARGENT_")` -> `startsWith("ARGENT")` | 350 files / 4300 passed, **exit 0** | | `delete process.env[name]` -> `process.env[name] = ""` | `bind-failure-telemetry.test.ts` 3 passed, **exit 0** | | entry removed from `setupFiles` | suite green | All four survive every gate: `vitest`, `eslint`, `prettier`, `tsc`, `typecheck:tests`, `knip`. `test/clear-argent-env.test.ts` re-imports the module against sentinels planted at call time, so the loop runs on demand rather than only when the developer happens to have an override exported. Each mutation now kills exactly one test, and the mapping is distinct: ``` body removed -> removes an ARGENT_ key outright instead of blanking it prefix underscore -> keeps a name that shares the prefix without the underscore delete -> blank -> removes an ARGENT_ key outright instead of blanking it setupFiles entry -> is registered as a setup file, so it runs before any test module ``` `ARGENTINA_REGION` is the look-alike: it pins that the underscore is load-bearing. The `in` check rather than a truthiness check is deliberate -- `= ""` reads as cleared to `if (process.env.X)` while still reaching a spawned child as a set key. ## 2. The header named the wrong file, and argued from the smallest cases > `ARGENT_PORT` / `ARGENT_HOST` retarget the bind that the startup-telemetry tests inspect. `startup-telemetry.test.ts` exists in that directory, mocks `createHttpApp`, and asserts nothing about host or port. `bind-failure-telemetry.test.ts` is the file that inspects the bind, and it is the one #707's own table named. A reader following the comment opens the sibling that literally bears that name and finds no bind assertion. The example list also covered only the four variables worth 9 failures, while the widest one was absent. Guard removed, one variable, full suite: ``` ARGENT_AUTH_TOKEN=devtoken -> Test Files 10 failed | 340 passed (350) Tests 80 failed | 4220 passed | 1 skipped (4301) ``` It puts every HTTP route behind a bearer check (`src/http.ts:59`), so ten files fail on status code alone. That is the strongest argument for scrubbing the whole prefix instead of a hand-maintained list, and the paragraph defending exactly that choice did not have it. ## 3. A comment #707 silently falsified `malloc-stack-logging.test.ts:89-95` promised to "restore the ambient value afterwards", capturing `process.env.ARGENT_IOS_CAPTURE` at module scope. The guard clears that variable before the module is imported, so the capture is always `undefined` and the `else` arm of the `afterEach` restore is unreachable. Clearing in `afterEach` is what the file actually needs and all it now claims. `export {}` is appended to the setup file so `tsconfig.test.json` -- which includes `test/**/*` -- treats it as a module rather than a global script; without it `typecheck:tests` fails with TS2306. ## Verification Run at `d0cb84de5`, rebased on `9dc7c7cfc`. - Full `packages/tool-server`: **351 files, 4306 passed, 1 skipped, 0 failed** on a clean environment, and identical with `ARGENT_EMULATOR_GPU_MODE=host ARGENT_EMULATOR_NO_WINDOW=1 ARGENT_SIMULATOR_NO_WINDOW=1 ARGENT_PORT=9999 ARGENT_HOST=0.0.0.0 ARGENT_AUTH_TOKEN=devtoken ARGENT_IOS_CAPTURE=all-processes` exported. - Mutation matrix above re-run against the new test: 4/4 killed, intended test each time, control green. - `prettier --check .` 0, `eslint . --max-warnings 0` 0, `tsc --build` 0, `typecheck:tests` 0, `knip` 0 (206 issues against the 215 ceiling, unchanged -- the new file is not flagged). ## Checked and left alone - The four `it.each(PLATFORMS)` retitles from #707 render correctly (`on linux ... -gpu swiftshader`); all four sites were fixed and a sweep of all 77 `.each(` sites in the repo found no other transposition. - The guard is genuinely unnecessary in the other ten packages: the union of all 53 `ARGENT_*` names read anywhere in `packages/*/src`, exported in two opposing polarities, leaves every other suite green. `argent-tools-client` already has its own copy from #714. - No CI workflow exports an `ARGENT_*` variable into a vitest process, so nothing in CI is destroyed by the scrub. `wayland-e2e.yml:46` sets `ARGENT_EMULATOR_NO_WINDOW` at job level but runs no vitest. - The other vestigial ambient save/restore pairs (`window-shake`, `http-auth`, `http-upload`, `http-flow-path-boundary`, `update-argent-tool`, `boot-device`, `boot-device-hotboot`) carry no false prose and are harmless defence-in-depth, so they stay. |