619 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Hubert Gancarczyk	4a11317706	chore: bump to 0.20.0 (#765) ... Version-only bump of the workspace to 0.20.0, same shape as #741 (the 0.19.0 bump). ## What changed - All 16 packages under `packages/*` → `0.20.0` - `package-lock.json` — regenerated with `npm install --package-lock-only --ignore-scripts` on Node 24, matching what the Lockfile job runs. Version-only: 16 insertions, 16 deletions, nothing else. - `server.json` — the MCP registry manifest, in **both** places it carries a version: the top-level `version` and `packages[0].version`. It lives outside `packages/*`, so a workspace sweep leaves it behind and the registry then rejects the publish because `packages[].version` names a version npm has never seen. ## Verification - `npm run check:versions` → "All workspace packages and server.json are at 0.20.0." - `npx prettier --check` clean on every touched file - No `"0.19.0"` string left anywhere in the repo No code changes. v0.20.0	2026-08-10 19:36:14 +02:00
Ignacy Łątka	c876fe8dbc	ci: add knip anti-dead-code gate (#572) ... ## What Adds a CI gate for dead code, using [Knip](https://knip.dev). We didn't have one. ESLint's `no-unused-vars` only sees unused locals inside a module, so whole files nobody imports, exports nobody uses, and dependencies declared but never required all slipped through. Knip covers that gap in one tool (roughly `ts-prune` + `depcheck` + `unimported` combined). The new `.github/workflows/knip.yml` workflow ("Dead Code", job `Knip`) runs `npm run knip` on push and PR to `main`. `CONTRIBUTING.md`'s pull-request checklist gains it as a step, with the reason it has to run against a tree with no build output. Knip reads source directly, mapping each package's `dist` entry back to `src`, so the job needs no workspace build. It installs with `--ignore-scripts`: an analysis that only reads `package.json` and source gains nothing from the seven packages here that carry a native install script, six of which reach this runner (`fsevents` is darwin-only). Five try a prebuilt binary before compiling, and `dtrace-provider` always runs node-gyp but ends in `|| node suppress-error.js` — and off mac/solaris/freebsd its `binding.gyp` picks a stub target with no sources, so on ubuntu-latest node-gyp builds nothing for it and `node-pty` is the only package that reaches a compiler at all — an `optionalDependency`, whose failure npm ignores. The exposure is a bump that leaves a package with no prebuild for the runner, which is exactly what `dependabot.yml` pins `tree-sitter` against (0.25.0 ships no Node 24 prebuild and its compile breaks `npm ci`). While adding it I checked whether any other installed checker was missing a gate. None were: ESLint (`lint.yml`), Prettier (`format.yml`), `tsc` build + `typecheck:tests` + `typecheck:scripts` + vitest (`unit-tests.yml`), the workspace-version / focused-test / merge-marker checks (`repo-hygiene.yml`) and lockfile sync (`lockfile.yml`) are all already enforced. Dead code was the only gap. ## How it's enforced `npm run knip` is two invocations: ``` knip --exclude exports,types && knip --include exports,types,classMembers,nsExports,nsTypes --max-issues 206 ``` Every issue type knip reports is an error — that is its default severity, so `knip.jsonc` sets no `rules` block at all. The pre-existing backlog is bounded by the `--max-issues` ceiling on the second pass instead of being hidden behind a non-failing severity: it cannot grow unnoticed, and the number comes down as follow-up passes pay it off. When a change legitimately adds an export nothing imports yet, the number moves up in the same commit — that bump is what review should see, rather than the contributor deleting someone else's backlog to make room. Two PRs making that identical one-line edit merge without conflicting, so `main` lands one short of the truth and its push-triggered run is what catches it. Splitting the passes also keeps a real failure out from under the backlog — the enforced classes report on their own, and the second pass only runs once the first is clean. The second pass names `classMembers`, `nsExports` and `nsTypes` explicitly because they sit in knip's own `defaultExcludedIssueTypes`: `--exclude exports,types` never covered them and `--include exports,types` did not ask for them, so a new unused class member cost nothing and moving an export behind opaque namespace use took it out of the ceiling's scope entirely. `classMembers` has 12 findings on the unbuilt tree the ceiling counts (9 on a built one); the other two have none today. None of the three is gated at zero — all five parked classes share the one ceiling, so a new `nsExports` finding passes if something else in the backlog is retired in the same PR. Naming `classMembers` also turns knip's `isSkipLibs` off (`create-options.js:122`), which makes it search declaration files for external references and **suppresses** 44 export/type findings the same run reports without it — 57/181 becomes 44/150. That, not the 12 findings it adds, is why the ceiling is lower than the two counts alone would suggest; `--include-libs` with no `classMembers` reproduces 44/150 exactly. The ceiling counts what CI counts, against a tree with no build output. Whether a workspace's `main`/`types` path resolves to an existing file changes the result, and not by way of anything in that file — a zero-byte `packages/registry/dist/index.js` moves the count just as a real build does, while a zero-byte `dist/zzz.js` does nothing. A built tree reports fewer issues — 43/147/9 = 199 built, against 44/150/12 = 206 unbuilt — so `npm run knip` passing after a local `tsc --build` is not evidence the job will pass, which is how the Knip job first went red on 199. The ceiling is 206 and it is exact: 206 passes, 205 fails. Remove every `dist/` before trusting a number from it. The ceiling is one total across those classes, so it bounds the backlog rather than each class: paying off one parked export licences one new one elsewhere in the same PR. Knip has no per-class ceiling. `treatConfigHintsAsErrors` is on, so an exemption that stops suppressing anything fails the job rather than printing a hint below the backlog. ## Config `knip.jsonc` documents the false positives so the gate is both green and meaningful. - **`@swmansion/argent`** is a thin dispatcher whose real payload is an esbuild bundle of the other workspaces (`packages/argent/scripts/bundle-tools.cjs`). Knip can't follow imports across that boundary, so packages consumed only by the bundled-in code read as unused; 11 are exempted by name here, though not all for that reason — the http3-quiche transport is pulled in by `@fails-components/webtransport` itself at runtime, and no first-party file imports it. They're exempted **by name**, not behind a catch-all: `ignoreDependencies` also suppresses `unlisted`, and a pattern that matches everything would take this workspace's own `src/` and `scripts/` out of that check — where an import with no matching declaration is a genuine bug rather than a bundler artifact. (It reaches `unresolved` only nominally: knip matches the list against a package name parsed out of the specifier, and an unresolvable relative path yields none, so even a catch-all leaves those reported. `ignoreUnresolved` is the option that does silence them.) - **`tool-server`** reaches a few packages in ways static analysis can't see. The exemptions cover two distinct classes, kept apart in the config so each stated reason matches the finding it silences: - *declared but loaded too dynamically to detect* (would be reported as **unused dependencies**): `tree-sitter` / `tree-sitter-typescript` via dynamic `_require`, and the http3-quiche transport as a runtime companion of `@fails-components/webtransport`; - *imported but deliberately absent from that `package.json`* (would be reported as **unlisted dependencies**): `sharp` (kept out so the ~30 MB native binary isn't a cost every consumer pays) and `electron` (an `optionalDependency` of the published package at one call site, the Electron main-process API at the other). - Root `scripts/`: knip resolves those from `package.json` scripts **and** from `run:` steps in `.github/workflows` via its github-actions plugin — multi-line `run: |` blocks included. What the plugin doesn't parse is a PowerShell body, so `scripts/ci/windows-chromium-e2e.mjs` — invoked at `windows-e2e.yml:176`, in the `windows-latest` job where PowerShell is the default shell — is the one entry declared by hand. ### Known limits - `ignoreDependencies` exempts a package **name across its whole workspace**, in both directions; knip has nothing narrower. While `sharp` is listed for `tool-server`, an undeclared `import "sharp"` anywhere in that workspace passes too. The list is held to packages that genuinely cannot be declared, for exactly that reason. - Knip analyzes JavaScript and TypeScript only (`DEFAULT_EXTENSIONS`), so **only 11 of the 30 files under `scripts/` are in the gate at all**. The other 19 — 15 `.sh` files, `e2e-full/lib/report.py`, `ci/e2e-chromium-page.html`, and e2e-full's `README.md` and `results/.gitignore` — are invisible to it however they are reached — including the whole `scripts/e2e-full/` harness (13 files), invoked from nothing outside its own directory, its only mention elsewhere being a `.prettierignore` entry. - Reachability's widest hole is the workspace entry. Whatever `src/index.ts` re-exports is reached, so a single `export { x } from "./dead-module";` takes that module out of `files` altogether — verified against a module reported as an unused file one run earlier. The same line applied to parked exports retires them from the count without deleting anything: re-exporting `equalsCI`, `includesCI` and `Settled` from `tool-server`'s entry takes 206 to 203 and frees three ceiling slots. The ceiling bounds what knip can see, not what is dead. - Test files are entry points, so a production module kept alive only by its own test still passes `files`. Being reached by a test says nothing about being reachable in production, which is what the check approximates, and two modules sit in exactly that gap: `tools/paste` is imported by two test files and registered by nothing, so it is unreachable over MCP and HTTP, and `utils/ios-profiler/lifecycle.ts` is a re-export barrel whose header keeps it "for source compatibility with existing imports", of which its own test is now the only one — its four production consumers all import `profiler-shared/lifecycle` directly. That single test import is exactly what keeps `files` from reporting it: repoint it and knip returns `Unused files (1)`. `knip --production` is the lens that reports the class; the repo isn't clean under it — 13 unused files (those two, seven root scripts, `packages/argent/scripts/{bundle-tools,observe,sync-readme}.cjs` and `packages/registry/tests/helpers.ts`) with the export/type backlog at 227/206 — measured with `classMembers` off, so comparable to the 57/181 above rather than to the ceiling's 44/150 — so a production-mode pass is a follow-up of its own. - The same entry treatment hides the test trees themselves. Root `package.json`'s `test:scripts` runs `node --test`, which is what makes knip's node plugin contribute its test entries, and that plugin registers four entry patterns in **every** workspace: `**/test/**`, plus `*{.,-,_}test.<ext>`, `test-*.<ext>` and `test.<ext>` matched anywhere. So none of the **380** JS/TS files under `packages/*/test/` can be reported unused, and under `packages/*/tests/` only a name none of those three match can — of today's 15 that is `registry/tests/helpers.ts` alone, which is why it is the lone `tests/` entry in the `--production` list above. A `tests/test-utils.ts` or `tests/test.ts` would be just as invisible. A dead `zz.test.ts` planted in `packages/registry/tests/` passes; a dead `zz-helper.ts` in the same directory fails. - `knip@5.88.1` is the last 5.x release ever published — 6.0.0 landed the next day and `latest` is now 6.x. The pin is deliberate: knip 6 removed the `classMembers` issue type outright, and the shipped script hard-errors on it (`ERROR: Invalid issue type: classMembers`, exit 2). Upgrading is therefore not a version bump — it means re-deriving the ceiling, rewriting the paragraph that explains it, and resolving the pass-1 findings knip 6 adds (six unlisted binaries: `tail`, `plutil`, `xctrace`, `netstat`, `pkill`, `pgrep`; two unresolved imports: `/usr/bin/osascript`, `/usr/libexec/PlistBuddy`). Dependabot's `npm-minor-patch` group cannot move it, since no 5.x exists above 5.88.1; the `npm-major` group will keep offering the 6.x jump. - Knip folds `optionalDependencies` into the declared set, so importing `electron` or `node-pty` is not `unlisted` — they pass because it read them. What it never reports is an optionalDependency nothing uses, so only that direction is outside the gate, along with `argent-cli`'s runtime `req("node-pty")`, resolved through a `createRequire(import.meta.url)` in `lens-pty.ts`, which knip can't see either. ## Dead code this found - `@types/semver` and `@argent/telemetry` from `packages/argent` — both were being suppressed by exemptions whose stated bundler-boundary reason could not apply. esbuild strips types and never resolves `@types/*`, and `packages/argent` names `semver` nowhere; `@argent/telemetry` is aliased to an absolute source path by `bundle-tools.cjs`, so the declaration bought nothing. Removing them is proved safe by building: four of the five bundles still emit and telemetry is inlined in each with no runtime `require` of it. The fifth, preview-window's main, sits behind a downloaded native asset absent from a script-free install, so it is covered by mechanism rather than observation — `bundle-tools.cjs` passes the same `alias` map to every build, and esbuild's `alias` bypasses node resolution entirely: deleting `node_modules/@argent/telemetry` outright leaves all four emitted bundles byte-identical. Removed alongside, but **not** gate-reported: the `benchmark` npm script in `packages/argent`, whose `scripts/benchmark.cjs` does not exist. A dangling script target is a blind spot of this gate — restoring the script leaves knip at exit 0. - Four files with no importers: the `render-hook` injected script, react-profiler's `source-map` + `snippet-extractor`, and `scripts/setup-in-workspace.cjs` (referenced nowhere, and its own usage string names a `scripts/setup-project.cjs` that doesn't exist). - `superagent` in `tool-server`'s `test/http-streaming.test.ts` — imported for a type but declared in no manifest in the repo, resolving only through npm hoisting of supertest's own dependency. Reverting that import to `superagent` reports `Unlisted dependencies (1) … :3:32` and exits 1; `@types/supertest/index.d.ts:10` re-exports the identical type (`type Response = superagent.Response`). - `@clack/prompts` and `@moq/lite` from `tool-server`'s `package.json` — declared, imported nowhere. (`@moq/net` is the one actually used; `@moq/lite` isn't a peer of it.) Also deleted here, but **not** something this gate reported: `src/tools/simulator/simulator-server.ts`. It sat in the `files` blind spot above — `interaction-messages.test.ts` imported it, which is enough to count as reached. `setup-registry.ts` never registers it, so it was unreachable over MCP and HTTP, and that catalog roll-call was its only reference; the import and the catalog count (77 → 76) went with it. ## Lockfile Adding knip moves two transitive packages: `@napi-rs/wasm-runtime` 1.1.5 → 1.1.6 and `@tybys/wasm-util` 0.10.2 → 0.10.3. Knip pulls in `@oxc-resolver/binding-wasm32-wasi`, which floors `@napi-rs/wasm-runtime` at `^1.1.6`, lifting the hoisted entry that had resolved to 1.1.5 for `@rolldown/binding-wasm32-wasi`'s `^1.1.4`, which then cascades to `@tybys/wasm-util`. No runtime impact — both are support packages for the wasm32-wasi fallback bindings, which don't get installed where a native binding exists; `npm ls` reports both extraneous on darwin-arm64. Those two are the only resolved versions that change in the diff; the rest is knip's own tree (49 packages added) and `@moq/lite` removed. ## Verification - `npm run knip` exits 0. - **Coverage of the enforced set.** With no `rules` block, knip's included set is 15 issue types: ten enforced at zero tolerance by the first pass, plus the five the second pass names under the ceiling. Eleven mutations cover all ten, each flipping the gate to exit 1 and back to 0 on revert: | class | mutation | | --- | --- | | `files` | a throwaway `.mjs` in `scripts/`; a throwaway `.ts` in `packages/argent/src` | | `dependencies` | a bogus declared dep in `packages/tool-server/package.json` | | `devDependencies` | a bogus declared devDep in the same manifest | | `optionalPeerDependencies` | `bytes` moved to an optional `peerDependency` there | | `unlisted` | an undeclared import in `packages/argent/src/cli.ts` | | `unresolved` | an unresolvable relative specifier in the same file | | `binaries` | a root `package.json` script invoking a nonexistent binary | | `duplicates` | `export default sleep` alongside the existing named export in `utils/timing.ts` | | `enumMembers` | an unused member on an enum a second module imports and uses | | `catalog` | a `catalog` block in the root `package.json` naming a package nothing uses | One nuance worth recording: `duplicates` is narrower than it looks. It reports a symbol exported both by name and as `default` (`sleep|default`); a second *named* alias of the same symbol is not a duplicate, and a freshly added symbol nothing references is not reported at all — so several naive probes read as a miss. `enumMembers` needed a shape the repo does not currently contain, not an unreachable one. The only exported enum here, `ServiceState`, is re-exported through `@argent/registry`'s entry, where knip counts exports as used, so an unused member added to it is **not** reported — but an enum in a non-entry module that another module imports and uses does report one (`Blue ZzColor …/zz-enum.ts:3:3`, exit 1; deleting the unused member returns exit 0). That is the ninth of the ten covered. `catalog` is reachable here despite the repo using npm workspaces: knip's catalog loader falls back to `manifest.catalog` when there is no pnpm or yarn workspace file, so a `catalog` block in the root `package.json` reports `Unused catalog entries (1)` and exits 1. - The two new mechanisms discriminate as well: one extra unused export takes the backlog to 207 and fails the ceiling, and a stale `ignoreDependencies` entry produces `Configuration hints (1) … Remove from ignoreDependencies` and exit 1. - `npm ci --ignore-scripts` in a clean worktree, followed by `npx knip`, is green — the job doesn't need any of the postinstalls it now skips. - `tsc --build`, `typecheck:tests` (all workspaces), `typecheck:scripts`, `test:scripts`, `eslint . --max-warnings 0`, `prettier --check .` and the full workspace vitest suite (4711 passed, 3 skipped across 13 workspaces; 3070 of those are `@argent/tool-server` alone) are green; `package-lock.json` is untouched by the review pass.	2026-08-10 17:09:16 +02:00
Hubert Gancarczyk	8623965b8a	perf(flow): stop the recorder returning the whole flow file per step (#715) ... ## What changes `flow-add-step` and `flow-add-echo` returned the **entire growing YAML** on every call. Across measured recording sessions that was the majority of all tool-result text, and it grows quadratically with step count. - Both tools now return `stepCount` — how many steps the flow has — instead of `flowFile`. - `flow-add-step` additionally returns `recorded`: the one summary line that was appended. - The full file still comes back once, from `flow-finish-recording`. ## Why The cost is not just tokens. With the whole file echoed per step, context pressure during a long walkthrough visibly changes the artifact being produced — checks get trimmed from a test to buy back room. Returning a line per step removes that pressure without removing any information the author needs *at that moment*. ## Supporting change `summarizeStep` is lifted out of `flow-finish-recording`'s per-file loop so the recorder can render its one line exactly the way the final summary does — one spelling, not two that can drift. That line now also carries `times`, `duration` and `delayMs`. They change what replays, and since the recorder no longer returns the YAML, this line is the author's only per-step view of what landed in the file. ## Compatibility This is a **breaking change to the tool result shape**: `flowFile` is gone from `flow-add-step` and `flow-add-echo`. `savedTo` is unchanged and is still the only field naming the destination in remote-client mode. `flow-finish-recording`'s result **shape** is unchanged, but its `summary` strings are not: taps now render `×N`, long-presses `for Nms`, and tool steps `(after Nms)`, so the two surfaces spell a step the same way. Nothing outside the agent reads `summary`, so that is not a break. ## Tests Recorder tests now assert against the flow **as persisted on disk** rather than a returned string, which is the surface that actually matters. --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-08-10 15:54:47 +02:00
Hubert Gancarczyk	74cddedf50	docs(skills): split argent-create-flow into references and add argent-qa-flows (#730) ... > Now rebased directly onto `main`. #728 and #574 have landed, so this PR carries only its own 13 documentation commits; it still does not sit on the #715-#726 chain. > > Guidance describing behaviour owned by that chain is therefore **not** carried here: the recorder's refusal of a vacuous `hidden` and of `await-screen-idle` (#724), its cross-tree re-probe warning (#717), the per-step `recorded` line (#715), and `type:` failing on unconfirmed focus (#726). Each of those paragraphs states what this base actually does instead; they are worth restoring when that chain lands. ## `argent-create-flow`: ~5,000 words to ~900 plus three references Every word of that skill loaded on every invocation, whether the task was replaying a two-step fragment or authoring a full regression test. The core skill now routes to three references read on demand: - **`live-authoring.md`** - recording a walkthrough - **`flow-yaml.md`** - the file format and selector vocabulary - **`reliability-and-recovery.md`** - what to do when a step or a replay goes wrong Nothing is dropped in the split - it moves behind a router. ## New skill: `argent-qa-flows` Turning a **test case, ticket, or acceptance criteria** into a repeatable regression test is a different job from recording a path worth replaying, and it was being done by a skill that never asks for acceptance criteria. It orchestrates `argent-create-flow` as its engine, records the first walkthrough live, requires every requested screen and state to be proved with stable evidence, and completes only after the unchanged flow passes **twice consecutively**. ## Proving a navigation Spelled out here for the first time, as two element-level checks rather than a route read: ```yaml - await: { visible: { id: settings-screen } } # identity: WHICH screen - await: { idle: true } # readiness: it stopped moving ``` Neither implies the other - a dropped tap leaves the source screen perfectly idle, and the destination's elements enter the tree while the transition still animates over them. Both belong after every screen change, including the one `launch:` performs. ## Routing The three skills were being confused by name alone, so the rules now draw the line: | you want | skill | | --- | --- | | a one-off interactive check, nothing saved | `argent-test-ui-flow` | | a saved, replayable path | `argent-create-flow` | | a saved test with acceptance criteria and two-pass proof | `argent-qa-flows` | "Record a flow" also stops being ambiguous with **screen recording**, which is video. ## Tests Every bundled skill's YAML frontmatter is parsed in a test, so a malformed new skill fails here rather than at install time. The `idle` doc guards that came with the warn-instead-of-fail change follow the prose into `references/flow-yaml.md`: they pin the documented defaults, settle cost and minimum timeout to the constants the parser enforces, and hold the tool description and the reference to the same account of what `idle` does. All 43 anchored internal links resolve. --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-10 14:41:23 +02:00
filip131311	4975ff490c	fix(debugger): cut debugger tool fail rate — structured not-connected results, classified CDP failures, narrow recovery (#610) ... ## Why PostHog (30d) shows the debugger tool family is argent's least reliable surface: | tool | fail rate | dominant causes | |---|---|---| | `debugger-status` | **37%** | METRO_NO_TARGETS / SERVICE_INITIALIZATION_FAILED / zod | | `debugger-connect` | **24%** | SERVICE_INITIALIZATION_FAILED (~7k, >1k users), METRO_NO_TARGETS | | `debugger-log-registry` | 41.7% raw / 4.2% excl. storms | one user retry-looped it **12,742×** on NO_TARGETS | Three design issues drive this: (1) "app/Metro not ready yet" — an expected precondition — is reported as a tool *error*, which both inflates fail rates and invites agent retry storms; (2) every CDP init/lifecycle fault lands in telemetry as the catch-all `REGISTRY_SERVICE_INITIALIZATION_FAILED` with `error_kind=unknown`; (3) ~600 users/30d hit zod validation failures with zero visibility into which parameter was wrong. ## What **1. Classified failures** (`d71efa43`) — six former plain-`Error` sites in `cdp-client.ts` / `target-selection.ts` now carry dedicated `DEBUGGER_CDP_*` / `DEBUGGER_TARGET_DEVICE_MISMATCH` signals (messages byte-identical). The registry's two causeless "terminating window" `ServiceInitializationError`s get their own `REGISTRY_SERVICE_TERMINATING` code. **2. Narrow recovery** (`ff9c1955`) — `recoverable()` on both debugger blueprints, scoped to windows where retry is provably safe: Metro recovers only `DEBUGGER_CDP_NOT_CONNECTED` (request never left the host; a detected socket death already self-heals for the *next* call via the terminated cascade — recovering `CONNECTION_CLOSED` there would be dead code, see the blueprint comment). Chromium also recovers `CONNECTION_CLOSED` for the tab-switch reconnect window. `REQUEST_TIMEOUT` stays unrecoverable (double-execution risk). A reachability test proves the retry actually fires for a RUNNING node. **3. Structured not-connected results** (`48b91ffd`) — `debugger-status` and `debugger-log-registry` become factories that return `{ status: "not_connected", reason, detail, guidance }` for classified preconditions (`metro_not_running | no_app_connected | device_mismatch | cdp_unreachable | stale_connection | reconnecting`) and still throw on unexpected faults. `detail` preserves the original error text for string-matching agents. `debugger-status` gains a socket-state gate (Metro: dispose stale node; Chromium: report transient reconnect, no dispose — the client belongs to the ChromiumCdp dependency). log-registry keeps **no** gate on purpose: captured logs are readable over a dead socket, and disposal would destroy the post-crash log file. Flows/run-sequence map a not_connected result to a **failed step** (mirrors `isUnmetUiWaitResult`) so recorded connectivity gates don't silently pass. `debugger-evaluate` re-codes agent-expression throws as `DEBUGGER_EVALUATE_EXPRESSION_THREW`. **4. Telemetry + guidance + docs** (`7602a70d`) — `tool:fail.invalid_params` (schema-declared names only; unknown keys → literal `"unrecognized_keys"`; capped emit-side at 16), new `debugger:tool_outcome` event (coded enum; joins `tool:invoke`/`complete` via `tool_invocation_id` — `ai_client`/`duration` live on the joined row), anti-retry-loop guidance appended to the Metro discovery errors (first sentences verbatim — skills match on them), six skills updated, Telemetry.md bumped to v1.03 with the new coded diagnostics disclosed. ## Dashboard migration note `debugger-status` / `debugger-log-registry` precondition failures stop emitting `tool:fail` — track them via `debugger:tool_outcome` (`outcome != 'connected'`). Per-AI-client breakdowns of those outcomes need a join on `tool_invocation_id` against `tool:complete`. ## Testing - Full suites green: tool-server 3,142, telemetry 290, registry 87. New coverage: recoverable truth tables (incl. non-Error inputs), status not-connected matrix (Metro/Chromium/terminating-window/stale/dispose-side-effect/fall-through-rethrow), log-registry rows, zod `invalid_params` derivation + emission-level guard, flow gate, evaluate rewrap, sanitize-level enum checks. - Live-tested against a booted iOS simulator + RN app and an Electron target (details in PR comments). Plan hardened through 3 adversarial review rounds (46 confirmed findings addressed) before implementation. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-08-10 11:51:45 +02:00
Hubert Gancarczyk	96ea94f87a	feat(flow): add an idle condition that waits for the screen to stop moving (#728) ... > Stacked on #574. Replaces the route-fingerprint approach that #727 and #729 carried; both are closed. > > Rebased off the #715-#726 chain so it can merge on #574 alone. That chain owns `DirectiveOutcome.warning` / `StepReport.warning` and the `indeterminate` scoring rule; the small amount of each that this condition needs is carried here, scoped to `idle`, and will conflict textually with #724/#725 for whichever lands second. ## One check a selector cannot express ```yaml - await: { idle: true } ``` Has the screen stopped moving? Flows were substituting fixed `wait:` steps for it, which either wait too long on every run or too little on the run that mattered. ## Why it reads pixels as well as the tree Each signal is blind to what the other sees. The tree cannot see presentation-layer motion. An iOS push or modal dismissal commits its hierarchy up front and then animates a layer for a few hundred milliseconds; a cross-fade or a scrim moves no node at all. Measured on a real Android emulator: a page animating a large element continuously reads as a single static `WebView` node, and the existing `await-screen-idle` tool called it settled in **465 ms, five times out of five**. On a deterministic Chromium page whose DOM is byte-identical across the animation, that tool returned `settled: true` in **407 ms** while this condition failed 8/8. Pixels alone would not do either: they cannot see a tree still churning behind an unchanged surface, and anything animated forever (a video, a shimmer) would make a pixel-only settle unsatisfiable on a screen the tree calls ready. Unlike that tool, this cannot be silently stepped over. Readiness is not an acceptance criterion — the verdict belongs to the identity and outcome checks around it — so a screen that never settles still **passes**, carrying a `warning` that says what the green actually bought. That is what makes it safe to persist in a flow: it reports on every run without ever failing one over a video or a shimmer. The only thing that stops a run is a tree source that cannot be read. ## The rule the design follows The absence of evidence is never evidence. - A capture that did not arrive cannot complete the hold. - Neither can a single agreeing pair. Every animation that reverses has a turning point, and two samples straddling it come back identical while the screen is moving. On a live 3 s cross-fade that passed a default-shaped step on roughly **one run in three**; settling now takes two consecutive still intervals, so `minStableMs: 0` still means three reads. - A round is never started without the budget to observe it with. A round begun with nothing left neither captures nor reads, and both absences were otherwise recorded as facts about the device. - No verdict comes from a latch. A screen that settles and then moves again, or goes blank, has not settled. ## `timeout:` is a real bound No describe path takes an abort signal, so the tree read is raced against what is left of the budget. Without that, a wedged ViewInspector RPC ran 2.25 s past an 8000 ms budget, and a busy Android screen ran 5.4-6.0 s against a 3000 ms one. A read that runs out of step budget is the step ending, not the source failing, and only the latter is reported as an environment problem. ## Failure modes stay apart They call for opposite responses. A screen that never stopped moving is a verdict about the app. A tree that could not be read is not, and it names the foreground check first, because a backgrounded app reads identically to an uninstrumented one and relaunching is the wrong repair. A run whose captures never produced a comparable pair still settles on the tree and says so in a warning, rather than passing off half a proof as the whole one. ## Capture routing and tolerance Captures route exactly as the `screenshot` tool routes them, so tvOS and Vega, which have no simulator-server backend but are perfectly screenshottable through `xcrun` and the emulator console, are covered rather than written off. The pixel comparison owns its tolerance rather than borrowing `screenshot-diff`'s. That one holds a baseline stored across sessions, machines and OS versions against a live capture and must absorb real drift; this one holds two captures one poll apart from a single session, where a static screen reads back byte-identical (measured: zero changed pixels across five consecutive pairs on an idle simulator). The margin is load-bearing, because uniform change is all-or-nothing: at the baseline tolerance any cross-fade slower than ~2 s counted zero changed pixels and read as settled mid-animation. ## Not a screen check A dropped tap leaves the source screen perfectly idle. It belongs after the element check that names the destination, never instead of one. #730 documents the pairing. ## Verification - 3307 tests pass; typecheck, `typecheck:tests`, lint and prettier clean. Eleven mutations applied to the source, all eleven caught by tests. - Driven end to end against a real iOS simulator, a real Android emulator, and a Chromium app through a tool-server built from this branch: static screens pass with no warning (proving captures ran), animating screens pass carrying the warning that names the motion, and the pre-existing tool disagrees exactly where it should. --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-08-10 11:50:16 +02:00
Hubert Gancarczyk	afd722924f	feat(flow): support concurrent flow recordings (#574) ... ## Problem Only one flow could be recorded at a time, anywhere on the machine. Recording state was three module globals in `flow-utils.ts` — harmless in a per-client process, but the tool-server is a **host-wide singleton per install bundle**. Every MCP client, every subagent sharing a parent's connection, every `argent` CLI call and every project on the machine attaches to the same process, the same `Registry`, and those same three globals. HTTP requests are served concurrently with no serialization. Three failures followed, all reproduced against the pre-change code: 1. **A second recording clobbered the first.** `startRecordingSession` just overwrote, so the first agent's next `flow-add-step` silently appended into the *other* agent's flow file — in a different project. 2. **Any agent could finish any agent's recording.** `flow-finish-recording`'s schema was literally `z.object({})`. 3. **Replay stomped recording.** `resolveFlowFilePath` called `setActiveProjectRoot`, so a `flow-execute` in project B rebound the global root mid-recording of project A. Already worked around, locally only, in `flow-add-step`. ## Approach Recording state is now a `Map` keyed by the resolved flow file path, `<project_root>/.argent/flows/<name>.yaml` — the identity of the artifact being built, so "same key" means "same output file" and two sessions on one key are a genuine collision rather than an accident of scoping. `flow-add-step`, `flow-add-echo` and `flow-finish-recording` each take `name` + `project_root`, so every call is self-contained; required rather than optional-with-fallback, since a fallback would succeed while an agent is alone and fail only once a second recording exists. `resolveFlowFilePath` is pure, which fixes defect 3 and retires `FLOW_PROJECT_ROOT_REQUIRED` (the state it guarded can no longer exist). Rejected alternatives — keying by device udid (flows are deliberately device-portable, and `flow-add-echo`/`flow-finish-recording` have no device at all), an opaque `recordingId` (an agent that loses the token cannot recover), and per-caller identity from the transport (`ToolContext` carries no caller identity, and subagents share their parent's MCP connection, so it would fail the exact scenario being fixed). ### Locking, and why readers needed more than a lock Every mutation of one flow file runs under a lock keyed by that path — appends, `flow-start-recording`'s truncate-and-register, and `flow-finish-recording`'s read-summarize-clear. Keyed by file rather than owned by the session, because a restart *replaces* the session: a session-owned lock cannot exclude the operation that supersedes it. Per file, not global, so two recordings never queue behind each other. An append whose session was finished, restarted or evicted meanwhile fails with `FLOW_NO_ACTIVE_RECORDING` rather than writing into a different take. The lock serializes writers only, and no reader of a flow YAML can join it — `flow-execute`'s own load, its `run:` fragment load, `flow-read-prerequisite`, `flow-add-step`'s sibling-fragment check, and the `argent` CLI reading from another process entirely, where an in-process lock cannot reach. Both writers used a plain `fs.writeFile`, which opens `O_TRUNC`, so a reader could land between the truncate and the write. That case is silent: `parseFlow("")` returns `{ steps: [] }` with no error and `summarize` derives `ok` from "no failures", so a `flow-execute` racing an append reports a **top-level PASS having replayed zero steps**. Both writes now go through a sibling temp file and a rename, so a reader sees either the whole old file or the whole new one — and unlike the lock, that also holds cross-process. ### `stop-all-simulator-servers` gets a `devices` scope Every agent is told to call this at session end, and unscoped it walked the whole registry — so agent A wrapping up tore down agent B's devtools mid-recording, degrading B's flow to brittle coordinate taps. It now takes an optional `devices` scope and reports `unmatched` ids, so a mistyped id doesn't read as a clean machine. Which namespaces count as "owned by a device" turned out to be the load-bearing part, because nothing cascades to most of them and `unmatched` turns any omission into an actively wrong diagnosis — a correct device id reported as a typo. The set now covers `AXService` (the in-sim ax daemon, spawned `--timeout 3600`; an iOS session that only ran boot/launch/describe owns this and nothing else), `ScreenRecordingSession` (an ffmpeg child plus the touch-visualizer overlay it enabled on the device), `NativeProfilerSession` (an xctrace child, or an on-device perfetto process and its trace file) and `JsRuntimeDebugger` (a bound loopback server, the CDP socket to Metro, a log handle). The debugger URNs interpose the Metro port — `<ns>:<port>:<deviceId>` — so the matcher understands both shapes, consuming only the first colon so a wireless adb serial after the port still compares whole. `stop-simulator-server` shared none of this and had drifted: it looked URNs up with an exact, case-sensitive `services.get()`, so a lower-cased UDID silently no-op'd there while the scoped stop-all reaped it. Both tools now share one matcher. Their namespace *sets* stay deliberately different, documented where they are defined — `stop-simulator-server` is also the documented recovery for a wedged transport, and widening it to devtools/AX would make a routine retry drop the native-devtools connection another agent's recording depends on, which is the hazard the `devices` scope exists to prevent. ## Verification **End-to-end, three devices, one shared tool-server** (iPhone 16 Pro, iPhone 17 Pro, Pixel 3a): three interleaved recordings, plus an unrelated `flow-execute` in a fourth project while all three were live. Both iOS devices held their own live native-devtools concurrently (two per-UDID sockets, `connected: true`). Each YAML contained exactly its own steps in order, taps captured as selectors rather than coordinates, and neither picked up another's or the third root's path. All three replayed green — also through the **released 0.17.0 CLI** against the modified server, which checks wire compatibility. A device-scoped teardown of sim 1 left sim 2 and the emulator running. Re-run against the final build: - Two interleaved recordings across two project roots on one device, with a **separate process** polling one of the flow files as fast as it could for the duration. Zero empty or truncated observations; each YAML held exactly its own steps; both replayed green (6/6 and 5/5). Against the pre-change in-place write the same reader observes zero-step reads interleaved among the real ones — the silent-PASS input. - A scoped teardown of the iPhone reaped `AXService` and `NativeDevtools` for that UDID only, reported no `unmatched`, and left the second sim and the emulator running. Lower-casing the UDID matched; a genuinely bogus id was still reported. - With a screen recording live on the Pixel 3a, a scoped teardown reaped `ScreenRecordingSession` and the **ffmpeg child actually exited** (1 → 0). Before the fix it survived to the 180 s cap while the tool called the correct serial a typo. The pre-change repros were re-run afterwards to confirm observed behavior changed, not just the tests. `npx vitest run` in `packages/tool-server` (295 files, 3154 tests) and `packages/argent-cli` (280), `tsc --build`, `tsc --noEmit` on both test projects, eslint and prettier — all green. ## Review notes Several rounds of review found real defects in this change; each was reproduced before fixing and re-verified after. Worth knowing when reading the diff, since the tests exist to pin them: - `flow-start-recording` truncated the `.yaml` outside any lock, so a step from the take being discarded landed in the freshly reset file and was reported as success (26-29 of 200 runs). - `flow-finish-recording` never took the lock; its `await fs.readFile` was a yield during which a concurrent append committed, leaving the reported summary disagreeing with disk (11-16 of 200). - A failed finish destroyed the recording. Hand-editing the `.yaml` mid-recording is a documented workflow, so `parseFlow` can legitimately throw — and the error told the agent to call `flow-start-recording`, which truncates the very take it would recover. - `unmatched` reported a device this session had just stopped, because `disposeService` returns a node to IDLE and keeps it. - A bare IP claimed every wireless-adb device at that address, since an adb serial is itself `ip:port`. - The eviction backstop stamped with `Date.now()`; ties at millisecond resolution let it drop the session that was just used. - `requireRecordingSession` ended with "Call flow-start-recording first" — reached for a key that was finished, superseded or evicted as well as one never started, and on those branches the file on disk is fully populated while that call truncates it and reports no `restarted`. - The atomic write regressed twice on its own terms: its scratch name was derived from the flow file's basename, which has no length cap, so a long flow name that appended fine before failed `ENAMETOOLONG`; and the cleanup guard began after the temp file was created, leaking a scratch file into a committed directory on any write that failed after opening. - Two tests passed against implementations they claimed to reject: the `stop-simulator-server` narrowness guard used a udid that classifies as *android*, where the iOS-only services it guards can never appear, so widening the iOS branch kept it green; and the client-mode finish asserted only the *shape* of `savedTo`, never its content, though that directive is the only thing that lands the file in client mode. ## Notes for reviewers - Client/remote persist mode is the least-exercised path, and now has its own concurrency tests (overlapping appends, and a restart superseding an in-flight one) — it previously had none. The `project_root` key is byte-stable across the file boundary (`kind: "probe"` passes it through unchanged), which is what keeps a remote recording addressable from `flow-add-step`, which declares no file input. - Two spellings of one flow file still mint two sessions — and two independent locks — over one file, which bypasses every guarantee here. Two ways in: the root spelled two ways (a symlink, or a case variant on APFS), and the likelier one, the *name* cased two ways on a case-insensitive volume. Neither is normalized away because the correct normalization is the filesystem's: case-folding the key would wrongly merge two distinct flows on ext4, and resolving symlinks is impossible in client mode, where the root does not exist on this host. Documented at `getFlowPath`. - No recording tool can detect that another agent took its key. `flow-add-step`, `flow-add-echo` and `flow-finish-recording` all re-resolve the key on every call, so after a takeover they act on whichever take is now live and report success: an append lands in the other agent's file, and a finish finishes and clears it. The only case that fails loudly is the narrow one the liveness check covers - an append already IN FLIGHT when the restart lands. There is no caller identity to key ownership on (see the rejected alternatives above), so this is spelled out for the agent under "Pick a name unique to your task" in `argent-create-flow` rather than mechanized. - Skew hazard in one direction only: a *new* client against an *older* tool-server. Zod strips the undeclared `name`/`project_root`, and the old server falls back to its module globals — so concurrent recordings silently collapse onto one session. The other direction (released 0.17.0 CLI against this server) was verified end-to-end. - `packages/argent-private/docs/reference.md` still describes `flow-add-step` as appending to "the current recording", and names two tool ids that do not exist (`flow-insert-echo`/`flow-run` are file names; the ids are `flow-add-echo`/`flow-execute`). It is a submodule, so the fix cannot ride on this branch. - A restart reported `discardedSteps` from the superseded session's in-memory flow while it truncates the *file*. Hand-editing the `.yaml` mid-recording is documented, so the two diverge: four steps wiped, one reported. It now counts the file, and reports no number at all when that file cannot be read or parsed - 0 is the answer a genuinely empty take gives. Client mode still counts from memory (this host cannot see the client's file), and the description now says which mode it is promising. - `ChromiumJsRuntimeDebugger` cascades from `ChromiumCdp` and was therefore already being torn down - silently, while `NetworkInspector` and `ReactProfilerSession` cascade the same way and were named. `stopped` is documented as the services that were live and got shut down, so it is listed now too. - The client-mode case named for a superseded in-flight append restarted *between* calls, so the next append re-resolved the key and succeeded - the guard was never reached, and neutering it left the whole file green. It now parks an append in its live sub-tool call across the restart. - `stop-tools.test.ts`' mock handed the tool the live service map where the real `getSnapshot` copies, so a cascade rewrote the state the sweep was still reading and the answer depended on map insertion order. The mock copies and recurses like `_teardown` now, and the cascade case runs under both orders. - Prose the code contradicted, each verified against source before rewriting: Vega owns `JsRuntimeDebugger`/`NetworkInspector` once the debugger has run (and can match anything at all through an ERROR node); "nothing here cascades" was denied twice in its own file; an iOS boot/launch/describe session owns `NativeDevtools` too; `:tcp` is mintable but unreached in production; the tool-server is a singleton *per install bundle*, not per machine; `argent flow list` does enumerate `.argent/flows`; the stop-tools comments narrated a "before" that only existed on this branch. Found while verifying, out of scope, worth their own issues: a `tool: flow-execute` self-reference recurses past both cycle guards (`run:` is guarded, a nested `flow-execute` reseeds the stack); react-profiler artifacts collide host-wide on a second-resolution filename; `stop-metro` is port-scoped and kills another agent's bundler; `debugger-reload-metro`'s HTTP fallback broadcasts to every attached client. A tool's `capability` is enforced only by the HTTP layer, not by `registry.invokeTool`, so a call that reaches the registry another way - `flow-add-step` takes `command` as a bare string - can resolve a service the tool declares it does not support, leaving an ERROR node behind (e.g. `SimulatorServer:<vega serial>`). --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-08-10 10:54:58 +02:00
Juliusz Wajgelt	67a4481868	refactor(tool-server): use @swmansion/argent-cloud-sdk for MoQ + input protocol (#743) ... ## Summary argent and the other client of the same device backend each hand-rolled the same MoQ client and `DataChannelCommand` protobuf encoder, with no shared source — argent carried a vendored copy of the `.proto` as an inline string that could silently drift from the server schema. Both now use `@swmansion/argent-cloud-sdk`, where the encoder is checked byte-for-byte against the canonical `.proto` in tests, so drift fails the build instead of shipping. - **`utils/moq-client.ts`** (195 → ~50 lines) is now a thin adapter over the SDK's `MoqDeviceSession`. It keeps the `MoqClient` interface exactly (`sendControl`, `screenshot` → `Buffer`, `close`), so `blueprints/simulator-server.ts` and `createMoqTransport` are untouched. argent still resolves its endpoint through `sim-remote moq-info` (preserving daemon-based session sharing) and still publishes on its own `"argent"` broadcast path. - **`utils/datachannel-proto.ts`** (201 lines → re-exports) so its ~10 import sites keep working unchanged. - **Dependencies:** drops `protobufjs` and `@moq/lite`, both now unused. `@moq/net` is kept and still dedupes to a single copy. No behavioural change intended. ## Verified end-to-end against live infrastructure Ran the migrated client against a real remote simulator (iPhone 17 Pro, iOS 26.5) on a live staging machine, driving this branch's `moq-client.ts` through the published SDK **0.1.2**: | Check | Result | |---|---| | MoQ session open (WebTransport, cert-pinned) | **PASS** | | Screenshot | **PASS** — valid PNG 1206×2622 | | Sequential screenshots | **PASS** | | Concurrent screenshots | **PASS** | | Input round-trip (tap an app icon) | **PASS** — tapping Settings opened Settings, confirmed visually in the returned screenshot | | Screenshot after concurrent burst | **PASS** | | Session teardown | **PASS** | Plus the existing suite: full workspace `tsc --build`, **3372 tests passing** (1 skipped), and `npm ls @moq/net` confirming a single deduped copy. ### A bug this surfaced, now fixed Concurrent `screenshot()` calls hung under SDK **0.1.0**. The server answers one screenshot per session at a time; 0.1.0 had dropped the serialisation the old hand-rolled client had, so a second in-flight request was never answered and its promise never settled — a regression against pre-SDK behaviour that only showed up under live traffic, with no unit test catching it. Fixed in SDK 0.1.1 (queue requests, plus a 30s request timeout so a dropped response errors instead of hanging). This PR depends on `^0.1.2` — a republish of 0.1.1 through the release workflow carrying npm build provenance, whose shipped `dist` is byte-identical (verified by SHA256), so the live results above hold for it unchanged. ## Notes for reviewers - The SDK subscribes to the screenshot track lazily, so a video-only consumer doesn't open a subscription it never reads. argent requests screenshots, so this path is exercised. - Screenshot responses are matched by the request id the server echoes, so a late or duplicate frame can't be handed to the wrong waiter.	2026-08-07 12:51:19 +02:00
Jakub Piasecki	6d907cc86b	feat(flow): boot a Chromium instance per launch step (#585) ... A `launch:` on Chromium now starts its own Electron instance, so nested e2e flows and mid-flow relaunches work as they do on iOS/Android. Previously the runner booted one app per run and rejected any second chromium launch. A chromium device id _is_ its CDP port, so a launch that starts an app from scratch has to boot one and move the run onto it: `state.device` became reassignable (steps already read it per call), and `state.owned` tracks what the runner booted, torn down in reverse at run end. An instance the runner did not boot — pinned via `--device`, or auto-detected — is attached to, never killed. Relaunching the same app kills its instance and waits for the process to actually exit, or the replacement loses a race against the app's single-instance lock. The runner needs a device before step 1, so `resolveRunDevice` follows a leading `run:` chain to find the launch the run _begins_ with: a fragment composing a chromium e2e flow now boots instead of failing with "No booted chromium device found". A launch declaring no app id for the run's platform stays an error — the run's platform is fixed start to finish. Also fixes a pre-existing teardown leak: the spawned handle is the npm `electron` node wrapper, not Electron, so signalling it left the app and its helpers running (reparented to launchd, still in the dock). Teardown now sweeps the child's process group. **Behavior change:** a fragment leading with `run:` into a chromium e2e flow now boots a fresh instance where it previously attached to a running one; pass `--device` to keep attaching. ## Tests Cover the boot/attach/settle three-way, teardown ordering and the group sweep, device rebinding after a nested launch, same-app retire ordering, cancellation mid-boot, both "no chromium app declared" guards, and the leading-`run:` chain (multi-hop, cyclic, no-launch). The guard, exit-wait and sweep tests are mutation-checked.	2026-08-06 07:28:27 +02:00
filip131311	566663bde4	chore: bump to 0.19.0 (#741) ... Version-only bump of the workspace to 0.19.0, same shape as #603 (the 0.18.1 bump). ## What changed - All 16 packages under `packages/*` → `0.19.0` - `package-lock.json` — regenerated with `npm install --package-lock-only --ignore-scripts` on Node 24, matching what the Lockfile job runs. Version-only: 16 insertions, 16 deletions, nothing else. - `server.json` — the MCP registry manifest, in **both** places it carries a version: the top-level `version` and `packages[0].version`. It lives outside `packages/*`, so a workspace sweep leaves it behind and the registry then rejects the publish because `packages[].version` names a version npm has never seen. ## Verification - `npm run check:versions` → "All workspace packages and server.json are at 0.19.0." - `npx prettier --check` clean on every touched file - No `"0.18.1"` string left anywhere in the repo No code changes. v0.19.0	2026-08-05 20:13:09 +02:00
Filip131311	3c98e3fe84	chore(submodule): advance argent-private to main (v0.19.0)	2026-08-05 20:00:34 +02:00
Jakub Piasecki	22033e1418	feat(flow): allow to run every flow in a directory from flow run (#576) ... `argent flow run` now accepts a directory and runs every flow in it sequentially: ``` argent flow run .argent/flows --recursive ``` Discovery uses the same acceptance rules as `flow list` (lowercase `.yaml`, safe stem, readable file), sorted for a deterministic run order. `-r`/`--recursive` walks subdirectories too, skipping dot-directories and `node_modules` and never following directory symlinks. A directory named `foo.yaml` still batches (stat wins over extension). Batch output is failures-only: each flow prints a `[i/n] <path>` header, its failing/errored steps — numbered exactly as a single-mode rerun would number them — and a per-flow summary; the run ends with a `PASS/FAIL — N flows: X passed, Y failed, Z skipped` verdict. `--json` prints one aggregate object with per-flow reports. Exit code reflects the batch (0 pass, 1 any failure, 2 discovery/config errors). `--output` exports keep the single-run `<output>/<flow>/` layout, keyed by subdirectory in recursive runs so same-stem flows in different directories can't clobber each other's snapshots. **Failure semantics:** a flow failing its steps — or one the tool-server rejects as invalid (bad YAML, unparseable step) — fails alone and the batch continues. An infra error (transport death, unclassified failure) stops the batch and counts the remaining flows skipped. To make that distinction possible, tool-error responses now carry the server's failure signal (`error_code`/`error_kind`) on the wire — on the streaming terminal error line and on JSON error bodies — and the tools client surfaces server-answered errors as a typed `ToolInvocationError`. Against a pre-signal server this degrades to stopping on any throw. Not breaking: single-file runs behave exactly as before.	2026-08-05 14:51:42 +02:00
Jakub Piasecki	cc2f907ce9	feat(flows): resolve run: directives from explicit YAML paths (#567) ... A `run:` value is now a YAML path resolved against the directory of the flow file that contains the step, matching the CLI cutover: ```yaml - run: login.yaml - run: ../shared/login.yaml # cross-directory composition now works ``` Bare names fail at parse with a migration hint (`run: login` → `run: login.yaml`). The filename stem stays the fragment's report attribution; reports also carry the as-written path as `target`. Cycle detection compares realpath-canonical paths (same stem in two dirs ≠ cycle; same file via symlink/spelling = cycle). Snapshot baselines are unchanged — keyed by and stored beside the root flow. An uploaded root flow with `run:` steps is now rejected up front with a contract error instead of a mid-run temp-dir ENOENT (its referenced files stayed on the client, and relative resolution in the temp dir could reach host files); same for a relative chromium app path. The recorder emits the sibling form (`run: <name>.yaml`). **Breaking:** existing `run: <name>` directives fail at parse until renamed to `run: <name>.yaml`. Snapshot baselines for a saved flow whose YAML is a symlink now anchor beside the real file (the symlink target's directory) — move any committed `.argent/flows/__baselines__/<flow>/` set beside the real file once, or the next run fails with a missing baseline. A `run:` target must also resolve inside the project root or beside/below the flow file that references it (arbitrary host YAML is no longer reachable), and an uploaded flow with a `snapshot` step is rejected up front (its baselines cannot anchor to the per-call temp dir).	2026-08-05 14:51:42 +02:00
Jakub Piasecki	a644d52d39	feat(flows): run CLI flows from explicit paths (#568) ... `argent flow run` now takes an explicit YAML path instead of a saved-flow name: ``` argent flow run .argent/flows/checkout.yaml --platform ios ``` The path may point anywhere on the local filesystem; the filename stem names the run's report/artifacts, so it must be `[A-Za-z0-9_-]+` with a lowercase `.yaml` extension. A bare name fails with the exact replacement command, and `argent flow list` prints paths instead of names. Because sibling `run:` files and `__baselines__` resolve beside the YAML, the CLI now requires the auto-started local tool server and refuses `ARGENT_TOOLS_URL` / `argent link` routing. The `flow-execute` tool stays remotely callable via `name` + `project_root`, and gains a `flow_path` parameter (exactly one of `name`/`flow_path`) that is only honored when the file-input boundary verified it as a real host path — uploads and boundary bypasses are rejected, so it can't execute arbitrary server YAML. Also: the flow-name pattern is now a single shared export from `@argent/registry`, and the recorder composes a `flow_path` call into a `run:` step when it targets a sibling of the recording.	2026-08-05 14:51:42 +02:00
Jakub Piasecki	2df2fac43d	fix(flow): error a no-windows iOS read instead of a false hidden pass (#519) ... ## Problem On iOS, a flow `assert hidden` (and an unmet `when` guard) could pass / green-skip against a screen where the element was plainly visible — whenever the target's view hierarchy can't be read. A flow that launches a non-injectable app (e.g. an Apple system app, where library validation blocks injection) runs to exit 0 with a false pass while `describe` still shows the element on screen. ## Root cause `queryFullHierarchyTree` returned an empty tree as a *successful* read when `getFullHierarchy` yields no windows, and the iOS source never sets a degraded-read hint. The runner's blind-read guard then falls back to `everMatched` alone, so an element that was *never seen* leaves the empty tree "trusted" — and an empty tree is exactly what a `hidden`/absent check accepts as satisfied. ## Fix Throw on a no-windows read instead of adapting it to an empty tree — consistent with the existing unavailable / requiresAppRestart guards. The caller's retry loop rides out a transient blank; a persistent one surfaces as the step's failure (`assert` fails, `when` guard errors) with an actionable reason, never a false pass. Keys on raw `windows`, not the flattened child count, so a genuinely sparse screen (windows present, no matching leaves) still resolves to "not found". ## Tests - New `flow-ios-tree-no-windows.test.ts`: throws on empty / missing windows, returns a tree when windows are present, still throws on an explicit RPC error. - `flow-hidden-blank-reads.test.ts`: adds the never-seen case (element never matched + dark source) → `hidden` fails with the outage reason, not a pass.	2026-08-04 15:48:40 +02:00
filip131311	c56cfa47f6	fix(installer): write rules and agents through a symlinked target (#731) ... Fixes #701. ## The problem `copyRulesAndAgents` copied the bundled trees onto each managed directory with one recursive `fs.cpSync`. When the destination is a symlink at a directory elsewhere, `fs.cp` refuses to overwrite it: ``` Could not copy agents to /repo/.claude/agents: Error: Cannot overwrite non-directory /repo/.claude/agents with directory .../@swmansion/argent/agents ``` Everything else installs, so it reads as a success apart from one warning line — but `argent-environment-inspector` never lands, while `rules/argent.md` keeps telling the agent to use it. The layout is the point of the report: keep one canonical, harness-neutral copy of the definitions and symlink each vendor path at it, so there's one definition to edit and no drift. The host tools already support that — Claude Code documents symlinks for `.claude/rules/`, and loads argent's skills through exactly such a link today. `rules/` had the same defect; the reporter just hadn't symlinked it. ## It is worse than the report on Node 20 `fs.cp`'s handling of a symlinked destination depends on which runtime you are on — Node 20 and Node 22 ship different implementations, and they disagree exactly here. Measured on both: | destination | Node 20 (JS impl) | Node 22 (C++ impl) | |---|---|---| | top-level symlinked dir | refuses (the reported bug) | refuses (the reported bug) | | nested symlinked dir | refuses | writes through | | symlinked **file** | **replaces the link, canonical left stale** | writes through | | dangling / unwritable dir | catchable throw | **aborts the process, exit 134** | So on Node 20 — which is what this repo's own CI runs — released 0.18.1 silently leaves a symlinked agent file stale on every `update`, with no warning at all. Verified against the released CLI: ``` released 0.18.1, Node 20: symlinked file: canon=stale link=intact (0 warnings) nested symlinked dir: files-in-canonical=0 ``` ## The fix The copy walks the tree itself, resolving each destination as it goes, and copies leaves with `fs.copyFileSync`. Nothing is delegated to `fs.cp`: that is what makes the behaviour the same on every supported runtime, and `copyFileSync` reports each failure — dangling link, loop, unwritable directory, a link pointing at a directory — as a plain catchable errno. The fix lives in `copyDir` rather than in a new helper because that function was already an exact, unused copy of the same buggy shape. Routing `copyRulesAndAgents` through it covers rules and agents, init and update, and removes the latent duplicate. That also settles the case that kills the install outright on Node 22: `fs.cp` does not throw when it cannot create a directory, it aborts through an uncatchable C++ `std::filesystem` exception. Two reachable shapes, both verified against released 0.18.1: - a broken `.claude/agents/references` link (the bundled agents tree really does have that subdirectory) — `argent init` dies at exit 134 mid-run; - **a read-only `.claude/agents`** — same abort, and this one needs no symlink at all, so it hits users of today's release. Both are now ordinary reported warnings. Resolution follows a link chain to its end, resolves each hop against the directory the link really lives in (not the lexical parent — they differ when a parent is itself a link into another subtree), and completes a dangling link only into a directory that already exists, so a link can't conjure a tree at an arbitrary path. Where the bytes land can now differ from the configured path, so the output names both — in the same shortened form the rest of the managed-content output uses: `Copied agents to .claude/agents -> .agents/agents`. A destination outside the workspace stays absolute, so an escape stands out. Nothing changes for a layout with no symlinks. ## Verification Reproduced against released 0.18.1 and a local build, then re-run against the fix. Every row below was executed end-to-end through the real `argent init`: | scenario | before | after | |---|---|---| | `.claude/agents -> ../.agents/agents` (the report) | warning, no files | both files land, link intact | | single symlinked `.md` file inside a real `.claude/agents` | worked | still works (regression test added) | | symlink chain to a real directory | warning, no files | resolves, files land | | link created before its target directory | warning, no files | target completed, files land | | dangling `.claude/agents/references` | **abort, exit 134** | files land, no warning | | **read-only `.claude/agents`, no symlinks** | **abort, exit 134** | `EACCES` warning, install completes | | link at a file / dangling into a missing tree | warning | warning, victim untouched, nothing created | | destination outside the project | writes there | writes there, disclosed absolutely | | `.gemini` / `.opencode` / `.cursor` targets, global `~/.claude` scope | refused | land correctly | | uninstall over a symlinked layout | — | removes only argent's files; user's files, link and directory kept | | re-run over a populated layout (`update`) | — | clean, no warnings | | symlinked file, **Node 20** | **silently stale, no warning** | canonical updated, link intact | | nested symlinked dir, **Node 20** | refused | writes through, link intact | | no symlinks anywhere | works | unchanged, no redirect in output | Every row was run under **both Node 20 and Node 22**, with identical outcomes on each. 15 new unit tests; 552 green on both runtimes. Build, eslint, prettier and the test typecheck all pass. Symlink tests skip on Windows, matching `update.test.ts`. `argent update` is not run here because it acts on the real global npm install, but its call site is byte-identical to init's and the same function was exercised directly in both scopes. ## Deliberately not in this PR - **The summary still prints a green "Rules & agents copied" when a copy failed** (`copiedRules: copyResults.length > 0` in `init.ts`). The issue calls this out as why the failure was easy to overlook. Fixing it honestly means giving `copyRulesAndAgents` a structured return and deciding whether a failed copy should make `argent update` exit non-zero — a separate change, and the general case isn't symlink-specific. Happy to follow up. - **Uninstall leaves both halves of a symlinked layout.** It removes argent's files through the link correctly and never touches the user's link or their other files; it just can't prune the emptied canonical directory (`rmdir` on a symlink fails `ENOTDIR`, swallowed). Not a regression, and arguably right. - **Writing through a link can leave the workspace.** That is pre-existing — `fs.cp` already wrote through symlinked files and nested symlinked directories before this change — and confining it would break the very layout the issue asks for. What this PR does add is bounded: a dangling link is completed only where its parent already exists.	2026-08-04 15:33:52 +02:00
filip131311	19782f4e1a	feat(secrets): resolve {{secret:…}} from .env files, not just the environment (#734) ... ## Why A credential could only reach a typing tool through an `ARGENT_SECRET_<NAME>` environment variable. That is the right CI contract — and stays the highest-priority source — but interactively it is poor ergonomics: an editor-hosted tool-server inherits the environment its editor was launched with, so changing a credential means restarting the editor, and there is nowhere to write one down that survives a reboot. "Is loading these from a `.env` file on the roadmap?" is the recurring ask. ## What `{{secret:NAME}}` now resolves through an ordered chain, first match wins, re-read on every resolution — so editing a file applies to the next keystroke with **no server restart** (the same runtime-read design `ios.additionalDeviceSets` already uses). | # | Source | Which keys it exposes | | - | ------ | --------------------- | | 1 | `ARGENT_SECRET_<NAME>` in the environment | prefixed variables only — unchanged, still wins | | 2 | `<project>/.argent/secrets.env` | every key (gitignore it) | | 3 | `<project>/.env.local`, then `<project>/.env` | **only `ARGENT_SECRET_`-prefixed keys** | | 4 | `~/.argent/secrets.env` | every key, any project | Covers `keyboard`, `paste`, `run-sequence` keyboard steps, and flow `type:` steps — all four already funnel through the same resolution point, so nothing else changed. Also adds `argent secrets [list] [--json]` so a setup is verifiable without an agent: names only, never values, shadowed names marked, and a warning when a linked remote tool-server means resolution happens on a different machine. ## The one design decision worth reviewing **Row 3 is why this isn't just "read the `.env`".** An agent composes the tool call, so anything reachable through `{{secret:…}}` is exfiltratable by a prompt-injected one — type it into a field, then read the field back. "Which values may argent type" has to stay an allowlist the user wrote deliberately. So: a *dedicated* argent secrets file exists for no other purpose and therefore **is** the allowlist; a file the app *shares* keeps the prefix as the allowlist, exactly as the process environment does. Your `STRIPE_SECRET_KEY` sitting in the same `.env` stays unreachable — verified live, not just asserted. Adding `ARGENT_SECRET_` to one line is the deliberate act that exposes it. Two smaller calls, both reversible: - The chain lives in `@argent/configuration-core`, not the tool-server, so the CLI reads the same code; the tool-server keeps only the placeholder engine. - The project scope is discovered by the **cwd walk-up every other project-scoped configuration uses**. A per-request project root (header → `ToolContext`) was considered and rejected as new protocol surface across two client paths; reusing flows' module-global `setActiveProjectRoot` was rejected as a cross-project leak on a reused server. The cost is that a tool-server whose cwd sits outside the project skips project files — mitigated by the failure now listing every path it consulted, and by `~/.argent/secrets.env` needing no project at all. ## Verification Unit + integration: tool-server 3122, configuration-core 122 (19 new), argent-cli 370 (6 new). Lint and prettier clean. All four publish bundles build. Live, driving this branch's tool-server on :3001 against iPhone 17 Pro / iOS 26.4 and the Electron testbed: - global `~/.argent/secrets.env` → typed correctly; the response echoed only the placeholder - prefixed key in the project `.env` → typed; **unprefixed `STRIPE_SECRET_KEY` in that same file → rejected** - `.argent/secrets.env` beat `.env`; the environment variable beat both - edited a secrets file mid-session → applied on the very next call, same server pid - a chromium flow with `type: { text: "{{secret:APP_PASSWORD}}" }` → 3/3 pass, asserting on the greeting the app rendered, so the *correct value* provably landed - an undefined name inside a flow surfaces the full source list in the step report Sample of the failure output (no value ever appears): ``` Unknown secret "LOGIN_PASSWORD" — no source on the machine running the tool-server defines LOGIN_PASSWORD. Available secrets: APP_USER. Sources consulted, first match wins: 1. environment (ARGENT_SECRET_*) — 0 secrets 2. /path/to/project/.argent/secrets.env — found, 1 secret 3. /path/to/project/.env.local — not found 4. /path/to/project/.env — found, 1 secret 5. /Users/me/.argent/secrets.env — not found To make it available, ask the user to add `LOGIN_PASSWORD=…` to … ``` ## Out of scope No general flow variable/parameter system — a non-secret external value (a test account's email, a staging tenant id) can use the same placeholder, but argent treats every such value as a secret: redacted from errors, never echoed, so it is not for something a report should show. No config key pointing at an arbitrary secrets file. Remote (`argent link`) still resolves on the remote host, which the CLI now says out loud.	2026-08-04 15:23:12 +02:00
filip131311	5a64e4e7de	fix(flows): stop a recorded sub-flow pinning the device it was recorded on (#696) ... Fixes #607. ## The bug `DEVICE_BIND_KEYS` covered `udid` and `device_id`, but `flow-execute`'s own device parameter is named `device`. So a recorded `tool: flow-execute` step kept the record-time id in the YAML, and on replay the runner never rebound it. Reproduced live on 0.18.1. Recording a cross-project `flow-execute` step: ```yaml steps: - tool: flow-execute args: name: b-only project_root: /…/projB device: BC0026C7-AAE0-490E-8BD4-A345CF65FB1B # survived stripDeviceKeys ``` Then editing that id to one that does not exist — what a flow committed on one machine looks like on another — and replaying against a real booted device: ``` flow-execute { name: pinned, device: BC0026C7-… } [1] ✓ flow-execute { "device": "00000000-0000-0000-0000-000000000000", <-- baked in, not rebound "ok": false, "errored": 1, … "reason": "…[SimulatorServer:00000000-…] simulator-server exited…" } PASS — 1 passed, 0 failed, 0 errored ``` This contradicted three things the code and docs already claimed: that flows store no device id and are portable, that the runner is authoritative on device and drops any id stored in a step, and that device ids are stripped at record time. It also made the raw `tool:` form behave differently from `run:` composition, which has always inherited the run device. That matters more than it looks: the recorder falls back to the raw form whenever the target is not a resolvable sibling, **and always uses it for a remote recording**, since `run:` composition is host-resolved. Remote recordings are therefore the main producer of the affected artifact. ## The fix Add `device` to `DEVICE_BIND_KEYS`. That fixes both halves at once — `stripDeviceKeys` keeps it out of new recordings, and `bindDeviceArgs` replaces an id already committed to a flow file. Safe because `bindDeviceArgs` only injects keys the target tool declares, and across all 75 registered tools **exactly one** declares `device` — `flow-execute` itself. (24 use `device_id`, the rest `udid`; both were already bound.) ### `platform` is deliberately not stripped There is a test pinning this so a later symmetry edit fails loudly. Two independent reasons: 1. It is read **only when no device was given** — `resolveFlowDevice` returns on `opts.device` before touching it, and the chromium boot spec is gated on `!params.device`. Once `device` is bound it is inert. 2. The strip is schema-blind, and `platform` is *not* device-specific on every tool: `react-profiler-analyze` declares its own. Stripping it would silently rewrite a recorded Android profile analysis into an iOS one. ## Verified live ``` replay, flow bakes in the all-zeros id, run given a real device: before -> nested device 00000000-…, ok false, errored 1 after -> nested device BC0026C7-…, ok true, errored 0 record leg -> recorded YAML has no `device:` key ``` Five new tests, each confirmed to fail before the change and pass after: the two unit cases, an integration case through the runner, the record-side case (using a **non-resolvable** target — a resolvable sibling records as `run:`, which carries no args and so could never show this), and the remote-persist case. Full tool-server suite 3091 passing; lint, prettier, `typecheck:tests` and the tool-description gate clean. ## Behaviour changes worth knowing - **Flows with a baked-in `device` stop pinning** and follow the run device. That is the fix and the documented contract, but it is a silent semantic change for anyone who noticed the pinning and relied on it. No migration is possible or desirable — the baked-in id is host-specific and is exactly what made those flows non-portable. - **A nested `flow-execute` can no longer target a device other than its parent's.** Verified nothing in the repo does this (no tracked `.argent` flows, no flow references in the e2e scripts or workflows), and `run:` already behaved this way. The loud case is a nested Chromium flow that used to boot its own Electron instance; the quiet case is a nested step with a different `platform`, which now silently runs on the parent's device. ## Related - #606 (a nested flow's failure does not fail the outer step) is what let the transcript above report `PASS`. Independent in code — #606 is in the result inspection after dispatch, this is in the args before it — so they do not conflict. Worth knowing when reviewing #606 that the "nested run errored" symptom there had a second, separate cause, fixed here. - `extractDeviceArg` in `http.ts` keeps its own parallel `udid`/`device_id` list and also omits `device`, so a top-level `flow-execute` call gets no platform attribution in telemetry. Not touched here — it is telemetry-only, and unifying it would create an import direction that does not exist today. - `project_root` / `flow_file` are absolute host paths baked into recorded steps — the same portability family, but they cannot simply be stripped (the nested call needs them) and rebinding needs a design decision. Filing separately. Co-authored-by: Filip131311 <f.kaminski2000@gmail.com>	2026-08-04 14:00:34 +02:00
filip131311	e24b7cac20	fix(flows): let a nested run's verdict reach the run that contains it (#697) ... Fixes #606. ## The bug A flow step that runs another orchestrator reported `pass` whatever the nested run did. `case "tool"` treats any non-throwing result as a pass, and both `flow-execute` and `run-sequence` report failure *in their result* rather than by throwing. Reproduced live — the **same flow**, run two ways: ``` run DIRECTLY -> ok=False passed=0 failed=1 run NESTED via raw `tool: flow-execute` -> OUTER ok=True passed=1 failed=0 step status = pass sub-report right there: ok=False failed=1 ``` The failing verdict was sitting inside the very object being reported as a pass. A second shape lost the verdict entirely: a sub-flow whose `executionPrerequisite` was never acknowledged returns a notice, runs **zero** steps, and also reported a green pass. ## The fix Two shapes, mapped to statuses the runner already has: | nested result | status | why | |---|---|---| | `ok: false` | **fail** | the composed flow ran and its assertions failed — what an inline `run:` composition already produces | | `notice`, zero steps | **error** | never runnable as written; the class the runner already uses for an unreadable fragment or a cyclic reference | | `aborted: true` | **skip** | matches the runner's own rule that a cancelled step is a skip, never a failure | Both fail and error hard-stop — in this runner *every* fail and error does (`state.stopped`), and there is no continue-on-failure concept: a per-step `optional:` is rejected at parse time because `when:` already expresses it. ### `run-sequence` had the same hole Found while reviewing the plan. `run-sequence` has **no verdict field at all** — every failure path (disallowed tool, unsupported operation, unmet `await-ui-element`, a tool that threw) pushes an `error` entry, `break`s, and returns normally. So a flow step whose sequence stopped at step 1 of 8 also reported a pass. Fixed here rather than left as a known identical bug on the same line. ### Why not a blanket `ok === false` rule There is no `ok` contract in this codebase to generalise. The only other soft-verdict tool spells it `success` (`await-ui-element`), `run-sequence` spells it neither way, and `case "tool"` dispatches tools whose results are typed `unknown` or `Record<string, unknown>` — several carrying app-derived payloads. A blanket rule would silently bind all of those, and every tool added later, to "a key called `ok` decides my flow's verdict". `isUnmetUiWaitResult` set the precedent for naming the tool instead. **There is a test pinning this**, so a future blanket refactor trips. Verified exhaustively: exactly one registered tool returns a top-level `ok` in its result — `flow-execute`. In particular `settings-permissions` does *not*; its `{ok: false}` is a private per-`pm`-invocation type that either throws (already an error) or returns `applied`/`skipped` (a legitimate pass). ## Verified live ``` raw-b-fail direct -> ok=False failed=1 nested -> OUTER ok=False failed=1, step fail reason: flow "b-fail" failed: 0 passed, 1 failed, 0 errored (await: …) sub-report attached raw-b-prereq nested -> OUTER ok=False errored=1, step error reason: flow "b-prereq" did not run — its execution prerequisite was not acknowledged: Settings must be open. Add prerequisiteAcknowledged: true to the step's args, or compose with run: instead. ``` The direct and nested verdicts now agree, which is the exact discrepancy in the issue. 11 new tests in a new file; the 5 behavioural ones each confirmed to fail before and pass after. Full suite 3097 passing; lint, prettier, `typecheck:tests` and the tool-description gate clean. ## Reporting shape One failing step carrying a summarising `reason` **plus the whole sub-report in `result`** — the pass path already attached `result`, so this is the same shape with a non-pass status. The nested steps are deliberately *not* spliced into the outer `steps[]`: `run:` can expand inline only because it shares one `ExecState` (one index sequence, one depth base, one device, one baseline dir), whereas a raw `tool: flow-execute` is a separate runner invocation. Splicing would mean renumbering indices and re-homing artifacts — a wire-format change for a bug fix. Nothing is lost: MCP renders `result` for any step that has one, and the CLI (which renders only `reason`) gets the sub-flow's own first failure inside the reason string. No new `StepReport` fields, no wire-format change; older clients render the new line unchanged. ## Merge ordering and conflicts - **Merge #696 (#607) first.** Before it, a nested `flow-execute` could run against a stale baked-in device and legitimately report `ok: false`; with this landed that becomes a parent failure and would read like a regression *caused by* this PR. No code-level conflict — #696 touches `flow-device.ts` and three other test files. - **#578 rewrites this exact block** (`case "tool"`, adding an `evidence` code to each return) and adds `StepReport.failure`/`durationMs`. Whichever lands second should give the new branches an evidence code — likely `nested-flow-failed` / `nested-flow-prerequisite-unacknowledged` / `nested-flow-aborted` — so its CI diagnostics classify the composition case. **#677** also edits `execLeafStep` and adds `StepReport.warning`. ## Behaviour change worth knowing Previously-green flows containing a nested composition that was silently failing will now go red. That is the fix, but it surfaces pre-existing breakage on upgrade. The likeliest one is a recorded raw step missing `prerequisiteAcknowledged`, which becomes a hard error instead of a silent no-op — the reason names both remedies. ## Follow-up, deliberately not bundled Raw `tool: flow-execute` nesting has **no cycle or depth guard**: `MAX_RUN_DEPTH` and the run-stack cycle check cover only `run:`, so a flow whose raw step names itself recurses through fresh runner invocations. Independent of this issue; filing separately. --- > **Stacked on #649** (`filip/flow-deviceless-and-counts`). Both edit the import block of `packages/tool-server/src/tools/flows/flow-run.ts` — #649 widens the `./flow-device` import for its device-optional run, this one adds `./flow-nested-outcome`. Rebased on #649, so review only the top commit. GitHub retargets it to `main` when #649 merges. Co-authored-by: Filip131311 <f.kaminski2000@gmail.com>	2026-08-04 10:32:57 +02:00
filip131311	2278115028	fix(flow): run a device-less flow without a device, and say why its counts are zero (#649) ... Fixes #644. ## Before ``` $ argent flow run echo-only # .argent/flows/echo-only.yaml: two `echo` steps [Tool:flow-execute] No booted device found. Pass a device id or platform explicitly. Available devices: 18BE573F-… (ios, Shutdown), <70 more> ``` and when a device *was* booted: ``` PASS on 18BE573F-… — 0 passed, 0 failed, 0 errored, 0 skipped ``` ## After ``` $ argent flow run echo-only # nothing booted Flow "echo-only" › first step › second step PASS — 0 passed, 0 failed, 0 errored, 0 skipped (no test steps) ``` ## Symptom 1 — the device Whether a device is needed is decided from the flow's own steps, before resolution. Classification is **per step kind and defaults to needing one**, so a kind added later inherits today's behaviour instead of silently running against no device — and the `never` binding makes leaving a new kind unclassified a compile error, backed by a test keyed on the step union so it fails to build rather than to run. Two classifications worth calling out: - **`when` always needs a device**, whatever its body contains, because the guard reads one itself (its platform, or its view tree). A `when: {platform: ios}` block containing only narration still resolves. - **`run` needs one without the fragment being read here.** The named flow is resolved at run time; resolving it a second time would duplicate that lookup and could disagree with it if the file changed in between. A deliberate over-approximation: composing a narration-only fragment still resolves a device. Stated in the code and in the tool description. For `tool:` steps the tool's own schema decides, via a superset of the keys the runner injects — `device` is included because a nested `flow-execute` step drives a device without being handed the run's own. A tool that declares no input at all (`stop-all-simulator-servers`) is device-free, and reading its absent schema must not throw — it previously would have. `ExecState.device` is nullable, which is the safety mechanism rather than just typing: the compiler enumerates every site that acts on a device, each of which now says so via one `deviceEnv()` narrowing. `flow-actions.ts`, `flow-visual.ts` and `ActionEnv` are untouched. The executor also re-checks each step against the **same predicate**, so if the scan and the executor ever disagree the step reports it instead of failing obscurely deeper in. A run that resolved no device reports `device: ""`, and neither renderer claims it ran somewhere. ## Symptom 2 — the counters Reproducing this showed the issue's framing is slightly off: `--json` doesn't only disagree with the text, it disagrees with **itself** — counters all zero while its own `steps[]` marks both steps `"pass"`. So the CLI text was a faithful rendering of the server's counters. **The counters are right and stay as they are.** Narration is deliberately excluded, and it should be: after a hard stop every remaining step *including narration* is reported skipped, so counting it would inflate `skipped` on every real failing run — a worse and far more common lie than the vacuous one being reported here. It would also contradict the JUnit model in #578, where echo is `<system-out>` and not a `<testcase>`. What was missing was saying *why* zero. Both the CLI and the MCP renderer now append `(no test steps)`, and only on a pass — a cancelled run can be a FAIL with all four counters zero, where "no test steps" would read as though the failure had no cause. No wire format changes: `steps[].kind === "echo"` already distinguishes counted from uncounted steps, so the JSON is self-describing without widening the status enum. ## Conflicts, for whoever lands second This is a hot area — seven open PRs touch `flow-run.ts`. - **#578** rewrites the CLI summary line I also change (one hunk, one 7-line function, semantically independent — resolution is keep-both), and its `attachFailureDiagnostics` dereferences `env.device.platform`. **It will need a null-device guard**, or every failure in a device-less flow silently degrades to `read-failed` diagnostics. Happy to rebase onto it rather than the reverse. - **#585** rewrites `resolveRunDevice` at the same seam and edits the same description sentence. - **#538** rewrites `flow-device.ts`. Worth noting it makes `resolveFlowDevice` reject a physical iPhone for flows — skipping resolution means a narration-only flow now passes on a physical-iPhone-only host, which I think is desirable but is a second behaviour change on this seam. All new tests are in new files, since every existing flow test file in the blast radius is rewritten by an open PR. ## Behaviour changes a reviewer should weigh - A device-free flow no longer prints `PASS on <id>` **even when a device is booted**. That is the point: otherwise an identical flow reports differently in CI (no simulator) than on a laptop with one open. An explicit `--device` is still honoured and still named. - `--platform` is ignored for a device-free flow, where it previously errored with nothing booted. - A 0.18.1 CLI against a newer server would print `Flow "x" on ` — cosmetic version skew. ## Verification Live against the built CLI: device-less flow with nothing booted (the reported bug) and with a device booted (identical output, no ambient attribution); `--json` showing `device: ""`; explicit `--device` still naming it; a mixed flow and a composed flow both still demanding a device; a mixed flow with a device booted still reporting `PASS on <id> — 1 passed`. 24 new tests. tool-server 3103 passed, CLI 284 passed, MCP 78 passed — no existing test modified. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- > **#697 is stacked on this branch** — it conflicts with this one in `flow-run.ts`'s import block, so it was rebased on top. Merge this first.	2026-08-03 16:14:30 +02:00
filip131311	81de9f66a6	fix(config): say what a rejected value should have been, and prune the group it left behind (#650) ... Fixes #643. ## Before / after ``` $ argent config set ios.additionalDeviceSets "~/Library/Developer/Radon/Devices" --scope project Error: Invalid value for config key "ios.additionalDeviceSets". # ← the whole message ``` ``` $ argent config set ios.additionalDeviceSets "~/Library/Developer/Radon/Devices" --scope project Error: Invalid value for config key "ios.additionalDeviceSets": expected an array of strings. Did you mean: argent config set ios.additionalDeviceSets '["~/Library/Developer/Radon/Devices"]' --scope project Run `argent config list` to see each key's expected value. ``` ``` $ argent config unset ios.additionalDeviceSets --scope project $ cat .argent/config.json { "ios": {} } → {} ``` ## Symptom 1 — the message The issue names three gaps: no expected type, no example, no pointer to `config list`. All three are closed, plus one the issue didn't ask for and I think is the actual fix. **The suggestion carries the user's own value.** A generic `Example: … '["~/DeviceSets/ci"]'` would hand someone a path they have never heard of and expect them to spot the difference. Where the correction is mechanical — a single item offered to a list-valued key — the error offers back what they typed, wrapped, shell-quoted, and **on the scope they asked for**. Pasting it works; verified live that it lands in project scope, not global. Nothing is coerced. The issue floated accepting a bare string and storing it wrapped; I didn't, because `config set` persists, gets committed, and is read by other machines, so a value silently stored in a shape the user didn't type is expensive to debug and `coerceCliValue` already has one silent fallback. Showing the corrected command costs the user one keystroke and keeps the store honest. If a reviewer prefers coercion, this is the smaller change to build on. **The expected shape comes from the validator, not a per-key string** — `asStringArray` *is* "an array of strings", so swapping a key's `parse` swaps its description with it and the two cannot drift. A bespoke validator can override via a new optional `expected`, and degrades to today's exact message if it says nothing. **`config list` states it too.** The issue notes `list` doesn't close the gap either — its description describes the *elements* but never says the value is a list. It now shows `value: an array of strings, e.g. ["~/DeviceSets/ci"]`, and `list --json` carries both fields. `telemetry.enabled` deliberately gets no example: `setConfigValue` refuses it before validating (it's managed by `argent telemetry`), so a "here's how to set it" hint would point at a command you may not run. A schema guard test keeps that honest as keys are added — and asserts every example **actually round-trips through its own validator**, so an example can never ship that reproduces the error it exists to fix. ## Symptom 2 — the empty group Deleting a key now removes any container the delete emptied, stopping at the first ancestor that still holds something. **This is file hygiene, not a behaviour change.** I traced every raw reader of the document — `readConfigObject`, and telemetry's `consent.ts` and `notice.ts`, which each do their own read — and an empty parent is already indistinguishable from an absent one: `getAtPath` returns `undefined` either way, before `parse`, so the merge layer never sees a container at all. The only observable difference is the bytes on disk, which is exactly the complaint. Guards worth noting: - **The root always survives** — an emptied config is `{}`, never a deleted file. `.argent/` is itself a project-root marker, so removing the file wouldn't restore the prior state anyway, and `updateConfig` is a shared publish path with a lock/rename protocol that has no defined meaning for file removal. - **A no-op unset still doesn't touch the file.** That fast path exists so an unset that removes nothing can't materialise a project config and dirty git status; pruning lives inside the mutate callback, which the fast path returns before reaching. So a group someone deliberately left empty by hand is never tidied away — pinned by a test asserting the file stays byte-identical. - Siblings at any level stop the unwind, including an empty array value, which is preserved. **Two existing tests asserted the old behaviour and now assert the new one** — `config-access.test.ts:47` and `config.test.ts:109` (the latter is `clearRememberedAgent`'s sibling-preservation test, so it doubles as proof siblings survive). ## Known same-class case, not fixed here `resetFirstRunNotice` (`packages/telemetry/src/notice.ts`) does its own inline delete and still leaves `{"notices":{}}`. It doesn't go through `deleteAtPath`, so it doesn't inherit this. It's a one-line change to route it through the same function — happy to do it here or as a follow-up, but it pulls the telemetry package into a config PR so I left it out by default. ## Verification Live against the built CLI: the reported case; pasting the suggestion back and confirming it lands in **project** scope; unset leaving `{}`; a sibling key surviving; a non-array key correctly getting the generic example and *no* bogus "Did you mean"; `config list` output; `telemetry.enabled` still delegating unchanged. configuration-core 103 passed, argent-cli 285 passed. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-08-03 14:56:40 +02:00
filip131311	3fff176a32	fix(cli): let a boolean flag take the value you typed (#699) ... Fixes #586. ## The bug `--includeChildren false` left the flag **true** and silently dropped the word, so the tool ran with the opposite of what was asked. The reporter spent a while chasing a filter bug that did not exist — the output was simply the unfiltered kind. Reproduced on 0.18.1, device-free, with a discriminator that shows the flag's value directly: ``` $ argent run screenshot-diff --udid U --baselinePath A.png --currentPath B.png --captureCurrent=false { "summary": "Screenshot diff summary … pixel_mismatch: 1.79% …" <-- false, diff runs $ argent run screenshot-diff --udid U --baselinePath A.png --currentPath B.png --captureCurrent false [Tool:screenshot-diff] Provide either currentPath or captureCurrent, not both. <-- stayed TRUE ``` That message is the whole problem in one line: the user typed `false`, and the tool refuses on the grounds that they asked to capture. ## Why the old behaviour existed, and why it does not need to The parser deliberately declined to look ahead: > `// Bare boolean flag: true. Don't consume next argv to avoid surprising behavior when a positional follows.` That protects a case which does not exist. `parseFlags` has exactly one caller, and **`positional` is never read anywhere in the repo** — every bare token is collected and dropped, for booleans and for every other type. There is no positional to steal. The module header had documented the space form as legal all along: ``` // --name value (string / number / integer / boolean-with-value) ``` This matters because several tool booleans **default to true**, so `false` is the only way to say what you mean: `screenshot.includeImageInContext`, `network-request.includeBody`, `debugger-component-tree.onScreenOnly`, `debugger-inspect-element.resolveSourceMaps`. ## What changed A boolean flag consumes a following token when it is exactly the word `true`/`false`, case-insensitively. `1`/`0` and everything else are left alone — a bare number after a switch is more likely an unrelated argument than a value, and guessing at an ambiguous token is what caused this bug. `--flag -- false` still keeps the word positional. Three consequences are handled here rather than left to bite later: - **`argent run` now reports arguments it ignored.** This is what stops `--flag 0` and `--flag yes` being the same silent inversion, in a syntax users are now being taught works. stderr, so `--json` stays parseable. - **`--no-flag true` / `--no-flag false` now name the positive form** instead of quietly meaning false. One is a contradiction, the other a double negative. - **`--flag=True` used to throw** while `--flag True` would now work — the inline coercion was case-sensitive and did not trim. Both paths now share one helper, so a single word cannot get two answers one function apart. The **flow** and **lens** parsers take switches that accept no value at all, and would have silently ignored the syntax their users just learned elsewhere. They now refuse it — in the flow parser that also stops `false` being swallowed as the flow name, since its first bare token is the flow to run. `--help` gains **one legend line** for tools with a boolean field, rather than widening the flag column on every field of every tool: ``` Booleans: --flag or --flag true sets true; --flag false, --flag=false, or --no-flag sets false. ``` It deliberately does not begin with `--`. `scripts/e2e-full/lib/discover-tools.sh` reads any such line in the `Flags:` section as a flag row and takes the first `--token` as its name, so a legend starting with a flag would inject a phantom flag into every tool model it builds. **There is a test pinning that shape**, verified live: running the harness's awk over the new help output yields exactly the real flags and no legend row. ## Verified live ``` --captureCurrent false -> diff runs (was: "Provide either currentPath or captureCurrent, not both.") --captureCurrent=false -> unchanged --captureCurrent 0 -> "Note: ignoring unused argument(s): 0. Pass values as --flag <value> or --flag=<value>." screenshot --help -> legend present; harness awk parses only the real flags ``` 25 new tests; 8 confirmed to fail before and pass after. The rest pin what deliberately did *not* change: bare flag still true, a following flag or non-boolean word left alone, `1`/`0` still positional in the space form, `--` escape, string and unknown fields untouched, `--no-flag` semantics, boolean arrays, last-write-wins. The parser had **zero** boolean coverage before — which is why this survived. Full CLI suite 298 passing; lint, prettier and `typecheck:tests` clean. ## Scope and neighbours Only `flag-parser.ts` (the schema-driven parser for `argent run`) changes semantics. The other CLI parsers take fixed, default-off switches where `false` has no coherent meaning; three of them already reject a stray token outright, and the two that did not now do. `flow.ts`'s deliberate rejection of `--flag=value` is untouched. **#648** also edits `flag-parser.ts` (rewrites `renderFlagName`, inserts helpers at the same anchor). Behaviourally compatible — it keeps boolean → bare `--name` — so this is a textual conflict only, and either order works. ## Found while here, not fixed `scripts/e2e-full/lib/discover-tools.sh:60-64` detects flag kinds with `\bboolean\b` in awk, where `\b` is **backspace**, not a word boundary — so it never matches and every non-enum flag is typed `unknown`. That makes the `bad-type` validation case downstream unreachable. Pre-existing and unrelated to this fix; filing separately. --- > **Stacked on #648** (`filip/run-cli-validation-messages`). Both rewrite the same region of `packages/argent-cli/src/flag-parser.ts` — #648 factors out `flagNameFor`/`isJsonField` and rewrites `renderFlagName`, this one adds `booleanWord` beside them and a boolean legend to `formatSchemaUsage`. Merging them independently conflicts; this branch is rebased on #648, so review only the top commit. GitHub retargets it to `main` when #648 merges. --------- Co-authored-by: Filip131311 <f.kaminski2000@gmail.com>	2026-08-03 12:18:11 +02:00
filip131311	a6f1bef632	fix(cli): stop argent mcp --help starting the server, and argent tools --help dumping the catalogue (#656) ... Fixes #638. ## The report understated it The issue says `argent mcp --help` "does not hang and does not start the server — it simply prints nothing". Reproducing it showed the opposite: ``` $ argent mcp --help < /dev/null ; echo $? 0 # 0 bytes ← what the reporter saw $ argent mcp --help # stdin open (a pipe, an editor, a CI step) # HANGS — I hit a 300s timeout ``` `--help` **starts the MCP stdio server**, which blocks reading JSON-RPC from stdin. The "prints nothing, exit 0" is just the server getting an immediate EOF from a closed stdin. So this is the same "`--help` runs the real command" bug that was fixed for the installers — `mcp` was missed. `startMcpServer` takes no argv at all (`mcp-server.ts:96` — its only parameter is `{ paths }`), so a help flag was *structurally* unreachable; there was nowhere downstream that could have handled it. ## Fix `mcp` joins the existing guarded set rather than growing a second help path. That set's defining property is "the handler never inspects its argv", and `mcp` qualifies more strongly than the installers do — it isn't merely ignoring `rest`, it is never handed it. **It cannot fire on a real launch.** The guard tests `rest`, which is `[]` for every generated editor config (all four shapes end at `mcp`, and `isArgentManagedEntry` requires exactly that), and `[].some(...)` evaluates no predicate. This is pinned in both directions, because a guard that *did* fire here would print usage onto the JSON-RPC channel and break every editor at handshake — the highest-cost regression in this file, and it was untested. The top-level table's `mcp` row now reads the same metadata as its own help, so the two cannot drift. **No rename.** An earlier draft renamed the module and six identifiers (`installer-help` → `command-help`, etc.) on the grounds that its comments had become false. They hadn't quite: fixing `mcp` *and* `tools` makes "every other subcommand parses `--help` itself" true again — I checked all ten siblings. Since #530 and #568 both edit `cli.ts`, a rename would have forced them through churn for no behavioural gain. Prose-only. ## Second commit: `argent tools --help` The issue mentions it in passing; it's a different mechanism and a genuinely dead branch. `argv.filter((a) => !a.startsWith("--"))` dropped `--help` before the subcommand was read, so it fell through to listing all 77 tools — **contacting the tool-server, starting one if none was running**, to do it. Only `-h` ever reached the existing help branch. Scoped to the subcommand position on purpose: a global check would have broken `argent tools describe <name> --help`, which today prints *that tool's* flags. It still does — there is a test. Separate commit so it can be reverted alone. ## Verification Live, against the built CLI: | | before | after | |---|---|---| | `mcp --help`, stdin open | hangs indefinitely | usage, returns immediately | | `mcp --help`, stdin closed | 0 bytes, exit 0 | 849 bytes of help, exit 0 | | `mcp` (real launch) | works | works — verified with an actual `initialize` handshake, server replies with its capabilities | | `mcp --metro-port 8082` | starts server | starts server (guard keys on help, not on "any argv") | | `tools --help` | 77-line catalogue | usage | | `tools describe gesture-tap --help` | tool's flags | tool's flags | | `argent --help` `mcp` row | — | byte-identical | argent 80 passed, argent-cli 284 passed. ## Note on the help text Every claim in it is sourced (stdio/JSON-RPC transport, stdout ownership, the `~/.argent/mcp-calls.log` default and `ARGENT_MCP_LOG` override, auto-spawn vs `ARGENT_TOOLS_URL`/`argent link`). It deliberately does **not** suggest running `argent mcp` by hand to check an install — that is the reported hang — and instead says it will look like it has hung and why. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-08-03 12:14:24 +02:00
filip131311	d08c3a3cfc	fix(cli): name the missing flag instead of printing a Zod dump (#648) ... Fixes #645. ## Before ``` $ argent run gesture-tap [ { "expected": "string", "code": "invalid_type", "path": ["udid"], "message": "Invalid input: expected string, received undefined" }, ... two more ] ``` ## After ``` $ argent run gesture-tap Error: missing required flags --udid, --x, --y argent run gesture-tap [flags] ...full per-flag help... ``` ## Scope The issue names the missing-flag case. Reproducing it showed the same raw dump has a second trigger: any constraint the tool itself rejects, which the CLI cannot know in advance. | invocation | before | after | |---|---|---| | `gesture-tap` (no flags) | raw issue array | `missing required flags --udid, --x, --y` | | `screenshot --scale 99` | raw issue array | `--scale Too big: expected number to be <=1` | | `gesture-scroll --udid X --x .5 --y .5` (zero deltas) | raw issue array | `Pass a non-zero deltaX and/or deltaY …` | | `run-sequence --udid X` | raw issue array | `missing required flag --steps-json` | | `gesture-tap --x abc` | already good | unchanged | | runtime failure | plain message, exit 1 | unchanged | Fixing only the first row would have left the CLI's error voice less predictable than before, so both go through the same renderer. ## How it works Two sources, one output. Missing flags are found locally from the tool's published schema — a **presence** check only, never a type or range judgement, which stays exclusively the server's. That is sound because the schema generator excludes defaulted fields from `required` (`debugger-connect` requires `device_id`, not `port`), so it cannot reject a call the server would have accepted. Everything else is read off the rejection the tool-server already returns. Both produce one report rendered by one function and reported through one call site, so the wording, the output channel and the exit code cannot drift between them. Recognition of a rejection is **structural** — the shape of the issue list plus the tool's own schema — never the wording of a message, which is not ours to depend on and changes between Zod releases. Four gates must all hold, and any failure leaves the existing handling untouched. Details worth a reviewer's attention: - **A rule spanning the whole payload reports no field.** `gesture-scroll` and `gesture-rotate` validate at the object level, and `screenshot-diff` is `.strict()`. Treating those as field-scoped would print `Error: --undefined …`; they are stated on their own instead. - **A rule firing on an *optional* field is not called a missing required flag.** `await-ui-element` refines on `expectedText`, which it does not require — reporting that as missing would contradict the help block printed directly underneath, where the field carries no `(required)` marker. - **The check runs after `--args` is merged**, so `argent run gesture-tap --args '{...}'` is not falsely reported as missing everything. Verified live: it reaches the device and taps. - Presence uses own-key semantics, so a required field named e.g. `constructor` is not treated as supplied by the prototype chain. ## Behaviour changes a reviewer should weigh - **A tool-rejected value now exits 2 instead of 1.** `run.ts` already uses 2 for bad input and 1 for a failed run; a Zod 400 means the tool never ran. No in-repo consumer tests for a specific code — `scripts/e2e-full` only checks zero/non-zero. Note this does **not** make 2 universal: an unknown tool name still exits 1, which is untouched here. - **`--json` gains a defined failure shape** (it had none — the blob printed regardless). One object on stderr, nothing on stdout, so `--json | jq` on a failed run reads an empty stream and a non-zero status: ```json { "error": "missing required flags --udid, --x, --y", "missing": ["--udid", "--x", "--y"], "issues": [] } ``` `issues` carries the tool's list verbatim, which is the raw data the issue asked to keep. - **One adjacent fix**, called out because it is not the reported bug: the global-option parse ran *outside* the error handler, so `argent run screenshot --out` printed a raw stack trace. Same bad-input-to-ugly-output defect, one line above the fix. ## Not done - The unknown-flag hole (`argent run gesture-tap --bogusflag 1` is silently accepted and the tool runs). The pass-through is deliberate — see the comment in `flag-parser.ts` — and belongs with #586, the same CLI-argument family. - `argent flow` builds its own payload and has its own required check; it cannot reach this path. - The server still returns the issue list verbatim, so MCP and other clients are unaffected. All prettifying is CLI-side. ## Verification Live against the built CLI for every row of the table above, plus: `debugger-connect` asks only for `--device_id` and never `--port`; `--args '{...}'` reaches the device and taps; a thrown runtime error still prints its bare message with no `Error:` prefix, no help block, exit 1. 43 new tests. `run-telemetry.test.ts` passes **unedited** — it already asserted the non-Zod passthrough, which makes it the regression guard for the untouched path. Full CLI suite 320 passed, registry 87 passed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- > **#699 is stacked on this branch** — it conflicts with this one in `flag-parser.ts`, so it was rebased on top rather than left to collide at merge time. Merge this first.	2026-08-03 11:42:26 +02:00
filip131311	c52f2dba9e	fix(screenshot-diff): say when two screenshots were resized to be compared (#689) ... Fixes #617. ## Reproduced three ways **The reported false green**, using an image whose 2× downscale is lossless so no resampling noise can mask it: ``` baseline 800x1600 current 400x800 (exact half) → status: unchanged pixel_mismatch: 0% (size difference never mentioned) ``` **Worse on real screenshots** — a 1152×648 capture vs its aspect-preserving half: ``` → status: changed pixel_mismatch: 0.35% changed_areas: 8 Text changes: Disappeared "General" / Appeared "Remotes and Devices" ``` The content is identical; every region and both OCR "text changes" are resampling artifacts. So the tool doesn't merely stay silent about the size difference — it manufactures a plausible-looking regression report out of noise. **The control shows the asymmetry is the bug** — aspect-*breaking* correctly reports `dimension_mismatch: expected=1,152x648 actual=1,152x400`. Sizes are disclosed exactly when the tool refuses to compare, and never when it silently rescales. ## The fix `PngDiffResult` gains `sizeNormalization` (both original sizes plus the size compared at), populated only when a rescale actually happened — so its presence *is* the signal that the comparison carries resampling uncertainty. The summary prints it directly under the status: ``` - status: resized_no_change - size_normalized: baseline=800x1,600 current=400x800 compared_at=400x800 - the inputs share an aspect ratio but not a resolution, so the larger was downscaled before comparing; any pixel or text differences below may include resampling artifacts, and the diff images are at compared_at rather than full size - re-capture the baseline at the same scale as the current image (screenshot with scale: 1.0) to compare without resampling ``` **`resized_no_change` is checked last**, after the difference checks, so a rescaled comparison that *did* find something is still a plain `changed` — a caveat status must never soften a real regression. There's a test pinning that ordering specifically. **The name matters.** `unchanged_after_resize` contains `"unchanged"`, so the one naive gate this protects — `summary.includes("status: unchanged")` — would still read it green. `resized_no_change` contains neither `"unchanged"` nor `"changed"`. ## What I deliberately did not do **Report `dimension_mismatch` whenever sizes differ** — the issue's primary suggestion. The normalization is deliberate: `screenshot-diff/index.ts:292-294` states that same-aspect normalization *"keeps a scaled capture diff-compatible with a baseline saved at any scale"*. The `screenshot` tool defaults to **0.3** for iOS/Android while the diff's live capture is full-res, and on Android the live side falls back to the default scale when full-res capture fails. Hard-failing would break the case this code exists for. **Expose the internal `normalizeSizes` flag.** `screenshot-diff-tool.test.ts:10-23` is titled *"rejects public tuning options so defaults stay internal"* and asserts exactly this kind of param is refused. And the better remedy is the one the summary now gives — re-capture the baseline at the same scale, which the skill already instructs and which removes the resampling rather than reporting it. Worth reconsidering separately if a user actually wants "green means byte-identical". *(An earlier draft proposed exposing it via `z.coerce.boolean()`. Worth recording why that was dropped beyond the posture conflict: in zod, `z.coerce.boolean("false")` is **`true`** — so a user asking for strict comparison would have silently got normalization anyway, re-creating this exact bug through the fix's own escape hatch.)* **Suppress the OCR noise.** Text analysis runs on each *original file at its own resolution* and only rescales the resulting bounds, so a diff-time filter cannot separate "OCR read a 576px image badly" from a real text change. Suppressing it whenever normalization occurred would silence text analysis on every scaled comparison — a new, worse false green of exactly the class this issue is about. Disclosure is the honest treatment; running OCR on the normalized pair is a separate change to `text-diff`'s contract. ## Tests Three new cases plus three strengthened ones: the false green now discloses and no longer reads `unchanged`; a normalized comparison with a real change still reports `changed` *and* still discloses; equal sizes say nothing; and the retina case still compares rather than hard-failing. Mutation-verified both halves — removing the status fails the #617 regression test; moving its check before the difference checks fails the ordering test. Full tool-server suite green — 3089 passed. No existing assertion needed changing, which is also why this survived: nothing pinned it. ## Notes for the reviewer - The flow snapshot path is untouched and **cannot** reach normalization: full-screen baselines key on the capture's own pixel dimensions (`flow-visual.ts:212`), so a differently-sized capture misses the baseline entirely and fails; `cropOn` diffs already pass `normalizeSizes: false`. - `SummaryStatus` is unexported and nothing switches on it — the status reaches the outside only as text inside `summary`, which the MCP layer passes through verbatim. No skill doc, flow assertion or e2e script matches on a status string. - The diff artifacts were already produced at the normalized size; that was simply never stated. The caveat now says so.	2026-08-03 11:16:29 +02:00
filip131311	35ab44dc03	fix(chromium-tabs): say why a tab cannot be opened in an Electron app (#659) ... Fixes #634. ``` before: [Tool:chromium-tabs] Not supported after: [Tool:chromium-tabs] Could not open a tab: Not supported. An Electron app has no browser-level target creation, so a tab cannot be made from outside it — have the app open one (e.g. `window.open()` through `debugger-evaluate`) and it will show up in `chromium-tabs list`. The list, select and close actions are unaffected. ``` The old text is Electron's own CDP answer to `Target.createTarget`, passed through untouched. The limitation is real, but the message names neither the cause nor a way forward, and reads as though the tool were unavailable — when only this one action is. ## Two things I checked before writing the message - **The workaround actually works.** On the Electron testbed I ran `window.open('about:blank')` via `debugger-evaluate` and the new window appeared in `chromium-tabs list` as `t2`. I didn't want to ship advice I hadn't run. - **The rest of the tool really is fine** — `list` returns correctly both before and after. ## Why the cause is kept rather than matched on `Target.createTarget` can fail for reasons other than Electron's missing browser shell — a dropped connection, or a real Chrome refusing the url. The CDP client keeps only the error's `message` and discards its numeric code, so branching would mean matching on the string `"Not supported"`, which breaks the moment Electron rephrases it. Instead any failure is wrapped, with the original message embedded. The explanation is worded as a property of Electron rather than a claim about *this* failure, so it stays accurate when the cause is something else — and the underlying error is still right there to read. ## Verification Live against the Electron testbed (Electron 42 / Chrome 148): the message above is the real output, and `list` still works alongside it. tool-server 3087 passed, plus a new test asserting the message keeps the original cause *and* carries the reason, the workaround, and the note that the other actions are unaffected. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-08-03 11:09:11 +02:00
filip131311	9489a220b0	chore(argent): declare the Node version the package actually needs (#653) ... Fixes #640. ```json "engines": { "node": ">=20.12.0" } ``` ## Why 20.12.0 and not 20 My first answer was `>=20` — CI's lowest tested line, satisfied by every runtime dependency, and the version the issue reporter (and I) installed successfully. **It is wrong**, and shipping it would have told npm it is safe to install onto runtimes where the CLI cannot execute a single line. The mistake was looking at first-party `src/`. The published artifacts are four esbuild bundles with their dependencies **inlined**, and that is where the real floor lives. Confirmed in `packages/argent/dist/`: | blocker | added in | consequence below it | |---|---|---| | `import { styleText } from "node:util"` (via inlined `@clack/*`, in `installer.mjs` and `cli-cmds.mjs`) | **20.12.0** | a **link-time** `SyntaxError` — `argent init` and every CLI subcommand fail to load | | `import.meta.dirname` at module scope (`bundled-paths.js`, `cli.js`) | 20.11.0 | `TypeError` during module init | So 20.12.0 is the lowest version on which the package actually loads. It is also the lowest line CI exercises (7 workflows pin Node 20), and it satisfies every runtime dependency's own constraint (`@fails-components/webtransport` `>=20`, `@modelcontextprotocol/sdk` `>=18`). `electron`'s `>=22.12.0` does **not** apply — it is an `optionalDependency`, and npm does not engine-check those. Verified: installing `0.18.0` on Node 20.19.5 produces no `EBADENGINE` at all and electron is never materialized. I did not go to `>=22` on EOL grounds: it would refuse a runtime seven CI workflows still test and that users demonstrably run. ## Docs corrected alongside - `README.md` said **20.11** — one minor below where the package can load. - `CONTRIBUTING.md` said **Node.js 18+** — predates what the repo's own lint toolchain needs (`eslint@10` requires `^20.19.0 || ^22.13.0 || >=24`). `packages/argent/README.md` is generated by `scripts/sync-readme.cjs` on `prepack`, so it follows automatically and is deliberately untouched. ## What a reviewer should know **Enforcement is package-manager-dependent**, which matters because `argent init` installs with whichever PM the project uses: | | behaviour on too-old Node | |---|---| | npm (default) | `npm warn EBADENGINE` — installs anyway | | npm + `engine-strict=true` | hard error, install aborts | | **yarn 1.x** | **hard error by default** | | **pnpm** | **silently ignores it** | So this achieves the issue's "warn" but only conditionally its "refuse". There is no `.npmrc` in the repo, so contributors are unaffected either way. **Nothing in the repo validates `engines`** — `repo-hygiene`, `check-workspace-versions.mjs` and `pack-mcp.cjs` all ignore unknown manifest keys, and `next-canary-version.mjs` round-trips the manifest through `JSON.parse`/`stringify`, which preserves the new field and stays prettier-clean. **One inconsistency I did not resolve:** `packages/argent/scripts/bundle-tools.cjs:383` builds with `target: "node22"`, which now sits above the declared floor. In practice the node20→node22 syntax gap is negligible (tsc targets ES2022 and I found no post-20 *syntax*), so I left the shipped artifact alone rather than change build output in a manifest-only PR — but it should either be lowered to match or consciously kept. Your call. ## Separate bug found while verifying this `ios-remote` is broken on the **entire Node 20 line**, independent of this change: the inlined `@moq/*` client uses `Promise.withResolvers` (Node 22+) ungated — 6 occurrences in `dist/tool-server.cjs`, reached from `utils/moq-client.ts` via the simulator-server blueprint. Reproduced on Node 20.19.5: `TypeError: Promise.withResolvers is not a function`. I deliberately did **not** raise the floor to `>=22` to paper over it — that would refuse Node 20 for every user to hide one broken path. `moq-client.ts` already has an `ensurePolyfill()` that polyfills `WebSocket`/`WebTransport`; this belongs there. Filed separately. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-08-03 11:01:09 +02:00
filip131311	ead834897b	build: keep the local daemon builds universal, matching CI (#655) ... Companion to software-mansion/argent-private#29. Together they fix #639. ## Split of work The binaries users install are **not** built by this script — they come from `build-native-binaries.yml` in `argent-private`, which duplicates the compile commands inline. So the load-bearing fix is [argent-private#29](https://github.com/software-mansion/argent-private/pull/29); this PR keeps `build.sh` (the `pack:mcp:local` path) in step, which that workflow's own comment promises it mirrors. Letting them drift is how the gap appeared. ## The change The three daemon compiles built arm64 only while the dylibs five lines away already cross-compile both slices, so a locally packed tarball had a universal `simulator-server` next to an arm64-only `ax-service`. `-target` is kept and `-arch arm64 -arch x86_64` added — identical output, but the platform stays pinned by something that fails loudly rather than by a flag whose loss would silently yield a macOS binary that packs fine and only dies at `simctl spawn`. `tvos-hid-daemon` also gets `-mmacosx-version-min=14.0`. It was the only compile with no deployment target, so its floor was whatever Xcode built it — on a current machine that is macOS 26, which would exclude nearly every Intel Mac the x86_64 slice exists for. 14.0 is what the shipped binary already reports, so this is a no-op against the current artifact. ## Verification Ran this script end to end against the real private sources: ``` ax-service x86_64 arm64 IOSSIMULATOR 17.0 (both slices) tvos-ax-service x86_64 arm64 TVOSSIMULATOR 17.0 (both slices) tvos-hid-daemon x86_64 arm64 MACOS 14.0 (both slices, was 26.0 locally) libNativeDevtoolsIos.dylib x86_64 arm64 (unchanged) ``` **Not verified:** that the x86_64 slices *run* — no Intel Mac and no x86_64 simulator runtime here. This restores a build pattern the repo already uses everywhere else; it is not a tested claim of Intel support, and no doc is changed to say otherwise. ## Note for whoever merges Merging both PRs is still not enough for the fix to reach an npm release: `publish-npm.yml` pins a frozen `argent-<ref>` binaries tag built by a manual dispatch of the private workflow. After argent-private#29 lands, that workflow needs a dispatch against the tag the next release will use.	2026-08-03 10:58:35 +02:00
filip131311	a6422a3f5f	chore: align argent-private submodule to its main (#612) ... Bumps the `packages/argent-private` submodule pointer from `2d6c292` to `c3fa4fc`, the current tip of its `main`. Newly included commits: - `c3fa4fc` Merge pull request #28 — @Filip131311/self-hosted-macos-builds - `cc189e6` ci: build and sign native binaries on the self-hosted Mac 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-31 16:37:10 +02:00
filip131311	23b9ae5c87	chore: bump to 0.18.1 (#603) ... Patch bump. First release whose npm tarball carries `mcpName` (added in #598), which unblocks the MCP registry publish (`mcp-publisher publish` rejects `0.18.0` because its manifest predates the field). `server.json` is bumped alongside `packages/*` for the first time — `scripts/check-workspace-versions.mjs` now enforces it (`All workspace packages and server.json are at 0.18.1` locally). | Commit | Change | | --- | --- | | `eea6c27c` | `style`: fix prettier formatting on main (#604) | | `7e4ece69` | `chore`: server.json for the official MCP registry (#598) | | `fc09612f` | `feat(config)`: `recordings.directory` — configurable save location for screen recordings (#601) | | `cccda92d` | `feat(config)`: `ios.additionalDeviceSets` — additive array of extra CoreSimulator device sets (#542) | Version-only diff, rebased on main as of #604. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-31 15:49:05 +02:00
filip131311	11f384d37f	fix(chromium): make Electron apps testable without window focus (#605) ... ## Problem Field report: *"electron needs focus to be tested."* Analysis confirmed three focus/visibility dependencies in the Chromium/Electron surface: 1. **Hidden window → throttled renderer.** A minimized/occluded window's compositor throttles: mouse-input acks stall **~5 s per event** (they wait on hit-testing), wheel scrolls hang (`gesture-scroll` already refused), `visibilityState` flips to `hidden`. 2. **Unfocused window → focus-gated app logic breaks.** `document.hasFocus()` is `false` before any input arrives; focus/blur-dependent UI misbehaves. 3. Focus-gated web APIs (clipboard already worked around it locally with `Page.bringToFront`). ## Fixes **1. CDP focus emulation on every page session** (`primePageSession`, né `enableCoreDomains`): `Emulation.setFocusEmulationEnabled(true)`, best-effort, re-applied automatically after tab switches (Playwright precedent). Live A/B on the Electron testbed: identical boot→unfocus→probe sequence gives `hasFocus:false` on main, `true` on this branch. Bonus verified by A/B (kill server → same minimized window flips to `hidden`): in current Chromium/Electron, emulation also pins reported visibility to `visible` and keeps input unthrottled while genuinely minimized — minimized tap latency **~5 s → ~65 ms**, wheel scroll works. *Trade-off (same as Playwright ships):* while attached, the app can observe neither real blur nor real hidden, so pause-on-hidden behavior is unobservable during a test. **2. Anti-throttling switches for every argent-booted Electron app** (`--disable-background-timer-throttling --disable-backgrounding-occluded-windows --disable-renderer-backgrounding`). Sessions are created lazily and die with the idle-exiting tool-server while the app outlives it — the flags cover that detached lifecycle plus emulation-less runtimes. Also covers flow-booted apps via the shared `bootElectronApp`. **3. Hidden-window fail-fast extended from gesture-scroll to gesture-tap + gesture-drag** via shared `assertChromiumWindowVisible`, under a new dedicated `CHROMIUM_WINDOW_HIDDEN` failure code (scroll migrated; `failure_stage` strings unchanged so telemetry stays joinable — analytics keyed on `CHROMIUM_INPUT_INVALID` + `chromium_scroll_window_hidden` should switch to the new code). With emulation active the guard is a backstop that fires only where emulation couldn't be applied — exactly where the stall is real. Keyboard/button stay unguarded on purpose: measured live, key events skip hit-testing and stay fast on hidden windows. ## Verification - Unit: 3075 tool-server + 87 registry tests green, `typecheck:tests` green, new tests for emulation priming, boot args, and the guard (all three tools). - Live matrix on the Electron testbed (dev tool-server): focused baseline; unfocused (`hasFocus` true pre-input, typing lands); minimized booted app (tap 69 ms, scroll works, typing + slider drag land, screenshot fine); minimized **external** un-flagged app on :9222 (tap 63 ms, scroll 460 ms — emulation alone suffices while attached). - Plan + implementation went through three adversarial review rounds (scope, CDP semantics, failure-code taxonomy/telemetry compat, doc accuracy). 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-31 15:36:36 +02:00
filip131311	05f53502ab	feat(tool-server): drive iOS simulators from additional CoreSimulator device sets (#600) ... > Recreates #543 — GitHub auto-closed it when the base branch of the stacked PR (#542) was deleted on merge, and a closed PR cannot be reopened after its head is force-pushed. Same commit, rebased onto main. ## Summary **Was stacked on #542** (now merged). Consumes the `ios.additionalDeviceSets` configuration across the iOS tool surface, so simulators living in extra CoreSimulator device sets — e.g. **Radon IDE's** `~/Library/Caches/com.swmansion.radon-ide/Devices/iOS` — are discovered by `list-devices` and drivable by every tool, by UDID, exactly like default-set devices. ### The core piece: a UDID → device-set map (`utils/ios-device-sets.ts`) simctl scopes every operation to one device set, so each per-device call must know the target's owning set. The new module owns that mapping and the single argv choke point: - **Learned from discovery**: `list-devices` and the simulator watcher enumerate the default set + every configured set and `rememberDeviceSet` each sighting — any tool used after a list hits a warm cache. - **Lazy fallback**: an unknown UDID triggers one `simctl list` probe per configured set, cached on hit. An unknown-everywhere UDID resolves to the default set **without** caching (a device that appears later is found on the next call) — preserving pre-feature behavior and error messages byte-for-byte. - **`simctlArgsForUdid(udid, args)`** builds the full argv (`--set` injected only for additional-set devices); `simctlPrefix` covers multi-call sites that resolve once; sync variants (`simctlArgsForUdidSync`, `cachedDeviceSetForUdid`) cover the `ChildProcess`-returning and `execFileSync` call sites, each of which runs after an async step that warmed the cache. - Config is **read off disk per call** (the #534 design): editing `ios.additionalDeviceSets` applies without a tool-server restart. - **Guard**: a configured set whose directory doesn't exist is never queried — simctl would materialize the dir, turning a config typo into a stray directory. ### Wired consumers - **`list-devices`** — enumerates every set, tags additional-set devices with `deviceSet`, documents the field. - **`simulator-watcher`** — polls every set (a set failing to list throws → the tick is skipped, so a transient error still can't mass-dispose tracked services); warms the map for the injection path. - **simulator-server blueprint** — passes `--device-set` (the same flag Radon IDE passes to this shared binary); default-set spawns unchanged. - **`ax-prefs`** — the pre-boot AX plist now lands under the owning set's device dir (previously hardcoded to the default `CoreSimulator/Devices` root, so the write would land nowhere for these devices and describe would degrade). - **`boot-device`** — set-aware shutdown/boot/bootstatus; skips the Simulator.app attach + `CurrentDeviceUDID` write for additional-set devices (one Simulator.app instance displays one device set — these run headless and are streamed, exactly how Radon IDE presents them). - **Every remaining per-device simctl call site**: launch-app / restart-app / reinstall-app (both the direct impls and the `SimctlBackend` shared handlers), open-url, settings-permissions (privacy + install probe), the tvOS screenshot fallback, status-bar pin/restore, device-shutdown, the ios-host `simctl spawn` family behind describe/native-devtools, tv-control's AX daemon, and the native profiler's sync helpers (cache warmed at `startNativeProfilerIos`). **Out of scope**: `sim-remote` — the remote orchestrator host has its own device-set story. ## Testing - **Unit**: new `ios-device-sets.test.ts` (mapping, lazy probe + caching semantics, no-probe fast path with no sets configured, discovery enumeration/tagging/dedup, missing-dir guard); blueprint test asserts the `--device-set` argv; the profiler shell-injection mock extended for the new import. Full tool-server suite green: **276 files / 2752 tests**; `tsc --build`, eslint, prettier clean. - **Live, end-to-end on a real Radon IDE device** (iPhone Air, iOS 26.4, in `com.swmansion.radon-ide/Devices/iOS`), driving this branch's tool-server over HTTP with the config set at project scope: - `list-devices` → the Radon device appears, tagged with its `deviceSet` (invisible to argent 0.16.1 on the same machine); - `boot-device` → boots headless in its set, no Simulator.app window; - `screenshot` → real home-screen PNG via simulator-server spawned with `--device-set`; - `launch-app` (Settings) → launched via `simctl --set launch`; - `describe` → full accessibility tree from the Settings screen, i.e. the AX bootstrap (set-aware prefs + in-sim ax-service daemon via `simctl --set spawn`) works; - clean shutdown; default-set devices meanwhile listed and behaved exactly as before.	2026-07-31 15:14:03 +02:00
filip131311	eea6c27cf0	style: fix prettier formatting on main (#604) ... ## Summary `npx prettier --check .` fails on current `main` with one violation: `packages/argent-tools-client/test/artifacts.test.ts` (a single `expect` line wrapped where it now fits the 100-char print width). This PR is the `prettier --write` output for that file — whitespace-only, no behavior change. ## Test plan - [x] `npx prettier --check .` passes on the branch (was failing on `main`) - [x] Diff is whitespace-only (1 file, 1 reflowed statement) 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-31 13:13:12 +02:00
Ignacy Łątka	7e4ece697d	chore: add server.json for the official MCP registry (#598) ... Adds the two files the official MCP registry (`registry.modelcontextprotocol.io`) needs in order to list argent. Argent is not in that registry today (`?search=io.github.software-mansion` returns 0 servers). The registry is metadata-only — it points at the npm package rather than hosting anything. ### `server.json` Describes the server and how a client starts it: `npx @swmansion/argent mcp`. The `mcp` positional is the dispatcher case in `packages/argent/src/cli.ts:137`, and it is the same subcommand `argent init` writes into every client config — `packages/argent-installer/src/mcp-configs.ts:130-141` spells it four ways (`node <path> mcp`, `yarn argent mcp`, `npx --no-install argent mcp`, and the global `{ command: MCP_BINARY_NAME, args: ["mcp"] }`, where `MCP_BINARY_NAME` is `"argent"`). None of them is literally the npx-by-package-name form in `server.json`, which is the form the registry expects a client to construct from `identifier` + `runtimeHint` — so it is verified below by running it, not by matching a string. `repository.id` is the GitHub repo id (`1164885639`); the schema recommends it so a registry can detect a delete-and-recreate of the repo. ### `mcpName` in `packages/argent/package.json` The ownership proof. The registry fetches the npm package and requires `mcpName` to equal the server name in `server.json` before it will accept a publish under `io.github.software-mansion`. It is the field the registry's own quickstart names for npm packages. Nothing in the publish path drops it: `scripts/pack-mcp.cjs` shells out to `npm pack` without rewriting the manifest, and the canary path's `scripts/next-canary-version.mjs --write` round-trips the JSON and only sets `version`. ### Namespace `io.github.software-mansion/argent` — GitHub OAuth auth. The registry grants an org namespace only to org **Owners**; plain membership is no longer enough. If no Owner is available to run the publish, the alternative is DNS auth via an apex TXT record on `swmansion.com`, which changes the name to `com.swmansion/argent` (and the `mcpName` value with it). ### Version sync `server.json` sits at the repo root, outside `packages/*`, so a release bump would have left its two `0.18.0`s untouched — and the registry rejects a publish whose `packages[].version` names a version npm has never seen. `scripts/check-workspace-versions.mjs` (already run by the Static checks job) also holds `server.json` to the version in `packages/argent/package.json` — the package it names — and pins the two cross-file links a publish depends on: `name` == `mcpName`, `identifier` == the published package name. Both halves are reported in one run, so a half-finished bump doesn't cost a CI round-trip per file. ### Not in this PR - **Publishing.** Nothing has been published; this is just the manifest. Publishing is `mcp-publisher login github && mcp-publisher publish`, and it can only succeed **after** a release whose npm tarball carries `mcpName` — i.e. not against `0.18.0`, which is `latest` (published 2026-07-30) and predates `mcpName` — `curl registry.npmjs.org/@swmansion%2Fargent/0.18.0 | jq .mcpName` → `null`. The first publishable release is the next bump. - **`github.com/mcp`.** A separate, manual step. Onboarding a new server there is hand-curated: per [github/github-mcp-server#1257](https://github.com/github/github-mcp-server/discussions/1257) (trent-j, a collaborator on that repo, 19 May 2026) "onboarding a new server is still a manual curation process today. Once a server has been onboarded, newly published versions from the OSS registry should sync from there." The nomination channel is `partnerships@github.com`, from GitHub's [registry how-to post](https://github.blog/ai-and-ml/generative-ai/how-to-find-install-and-manage-mcp-servers-with-the-github-mcp-registry/) (24 Oct 2025) — that address does not appear anywhere in the discussion thread. ### Verification - `mcp-publisher validate` (v1.8.0, which posts the file to the live registry) → `server.json is valid`. Mutation-checked what that actually covers: removing `transport` fails it and removing `$schema` is a 422, but a foreign namespace (`io.github.anthropics/argent`), a nonexistent npm identifier and a `99.0.0` package version all still pass — so it checks shape only, and the ownership/existence checks happen at publish time. Also validated offline against the pinned `2025-12-11` schema. - Ran the entrypoint the file declares. `npx -y @swmansion/argent@latest mcp` fed an `initialize` over stdio answered `{"protocolVersion":"2024-11-05","capabilities":{"tools":{}},"serverInfo":{"name":"argent","version":"0.18.0"},"instructions":"…"}`, with stderr empty. - Mutation-checked the new version guard: drifting `server.json` `version`, drifting `packages[0].version`, deleting `mcpName`, and typo-ing `identifier` each fail `check-workspace-versions.mjs` with the specific line; the pre-existing packages/* drift check still fires on its own. - Asserted across the two files: `mcpName` == `server.json` `name`, `identifier` == package `name`, all three versions equal, description 89 chars (registry limit is 100). - `prettier --check` and `eslint` clean on both changed files. - Mutation-checked the guard as a whole — 22 mutants, 1 survivor. Killed: reverting the self-invocation guard to a raw `argv[1]` compare and making it never match; `exit(1)` → `exit(0)` on the failure path; dropping either `failed = true`; exiting early on `packages/*` drift instead of collecting; `packages.slice(0, 1)`; hardcoding `packages[0]` in either message; dropping the no-packages-array guard, the non-object-entry guard, the non-object-JSON guard, and the presence checks for `version`, `name` and `mcpName`; `continue` → `break` in the `packages/*` scan (both arms) and in the entry loop; dropping the on-message read, the `git checkout` hint, and the per-version name accumulator. The survivor is the `packages/*` readdir sort: it fixes the scan order rather than any output, so no single run can observe it — it is there so a scan that stops early drops the same packages on every machine instead of a filesystem-dependent set. - Reproduced the symlink no-op on the real repo: with `packages/registry` at `9.9.9` and `server.json` at `0.17.0`, `node /tmp/argent-alias/scripts/check-workspace-versions.mjs` now exits 1 and prints both problems, where before it exited 0 with no output at all. - Closed the vacuous-pass class in `serverJsonMismatches`: all four comparisons it makes were bare `!==`, so a field absent from *both* sides compared equal and passed. A `packages/argent/package.json` with no `version` printed `All workspace packages and server.json are at undefined.` and exited 0; with no `name`, a `server.json` naming nothing installable was reported in sync. `version` and `name` now require presence, as `mcpName` already did. - `readTrackedJson` guarantees an object, not just parseable bytes — a `server.json` holding `null` used to reach the field reads and throw, and `"0.18.0"` or `[]` produced mismatch lines that never said the file was the wrong shape. - Full CI parity locally: `test:scripts` 85/85, `tsc -p tsconfig.scripts.json`, `eslint --max-warnings 0`, `prettier --check`.	2026-07-31 12:36:44 +02:00
filip131311	fc09612f30	feat(config): recordings.directory — configurable save location for screen recordings (#601) ... ## Summary Adds a `recordings.directory` configuration entry (built on the #534 config system) that lets users choose where finished screen recordings (mp4) are saved, instead of the fixed `<project>/.argent/recordings/`. - **Schema entry** (`configuration-core`): scopes `project` + `global`, `prioritize-local` merge (project wins when both are set). Picked up automatically by `argent config set/get/unset/list`. - **Value semantics**: absolute path, `~`-prefixed (expands to home), or relative — relative anchors at the project root (home dir when not in a project). - **Client-side wiring** (`argent-tools-client` `durableSaveTarget`): the wire `saveDir` hint stays `.argent/recordings` (wire contract unchanged → old/new client-server interop). Past the existing client allowlist, the recordings hint is remapped to the configured directory. ## Security model unchanged The allowlist exists because `saveDir` arrives from a possibly-hostile remote tool-server. That gate still runs first and is unchanged: a server can only *select* the sanctioned recordings category, never name a destination. The configured directory comes from the client's own trusted config files. Exclusive never-overwrite writes (`name (2).mp4`), the durable download size caps, and the symlink real-path guard all remain; for a configured dir the realpath check degenerates to "the dir resolves to itself" (a user-chosen symlink is legitimate, matching how the default base may be symlinked). A test asserts hostile wire values (`.git`, `..`, absolute) stay rejected even with config set. ## `argent link` interaction Materialization always runs on the **client** (co-located → copy from `hostPath`; remote → download over `/artifacts/:id`), and the config is read there at materialize time — so with a remote tool-server the mp4 lands on the client host per the *client's* config, on both paths. ## Testing - New unit tests: 10 in `argent-tools-client` (absolute/`~`/relative resolution, local-vs-global precedence, both materialization paths, hostile wire values with config set) + 4 in `configuration-core` (scope writes, trim, precedence, validation). - Suites green: tools-client 188, configuration-core 87, tool-server screen-recording 62, CLI config 16; full workspace vitest 4727 pass (3 pre-existing `scripts/*.test.mjs` node:test files don't parse under root vitest — unrelated). - **Live E2E** on an iPhone 15 Pro simulator via the branch tool-server: real recording captured, then materialized through the branch client with configs written by the real `argent config` CLI in a sandboxed HOME/project. 7 scenarios green: default location, global-only (`~` path), project-only (relative), both-set → project wins (co-located **and** remote download with Bearer auth), unset fallbacks, `argent config list` rendering. Remote-downloaded mp4 ffprobe-verified (byte-exact size, 4.17 s duration); name-collision suffixing observed live. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-31 12:36:31 +02:00
filip131311	cccda92d15	feat(config): ios.additionalDeviceSets — additive array of extra CoreSimulator device sets (#542) ... ## Summary **Stacked on #534.** Adds the first *additive* configuration entry on the new schema — `ios.additionalDeviceSets`, a list of extra CoreSimulator device-set directories — delivering the config side of the #401 discussion on the #534 machinery. ### The schema entry One registry entry in `config-schema.ts`: - key `ios.additionalDeviceSets`, scopes `project` + `global`, `parse: asStringArray`, **`merge: "union"`** Unlike `lens.agent` (project *shadows* global), this is additive: the effective value is the **union of both scopes** — the global baseline first, the project's extras appended, deduplicated. A repo can commit the device sets its tooling needs to `.argent/config.json` and they *extend* (never replace) the user's global list. `argent config list/get/set/unset` handle the array key with zero command changes — arrays already round-trip through the JSON value coercion. ### The runtime reader `getAdditionalIosDeviceSets(options): string[]` in `config-access.ts` — the surface iOS tooling should call. It reads each scope separately so paths normalize against the base the author meant: - `~` / `~/…` expands to the home directory (honoring the sandbox `homeDir` override) - relative **project** entries resolve against the **project root**, relative **global** entries against **home** - output order matches the `union` preset (global first), with **post-normalization** duplicates dropped — `~/DeviceSets/ci` in global and its absolute spelling in the project file collapse to one entry - empty array when unset Support refactor: `resolveHomeDir()` extracted in `paths.ts` (the HOME/USERPROFILE + override logic previously inlined in `argentHomeDir`/`configDir`). ### Out of scope (follow-up) Actually pointing simctl at the extra sets. There is no central simctl wrapper today (~25 call sites build `xcrun simctl …` argv inline, and `ax-prefs.ts` hardcodes the default set's `CoreSimulator/Devices` path), so consuming this reader in `list-devices`/boot/etc. is its own change — the runtime-read-off-disk design here means that consumer needs no CLI/env threading, matching how #534's description framed the split. ## Testing - `configuration-core`: 5 new tests (82 total, green) — additive union across scopes + per-scope file isolation, non-array rejection + entry normalization, reader `~`/relative resolution per scope, post-normalization dedup, empty-when-unset - `argent-cli`: new real-filesystem CLI round-trip (274 total, green) — `config set` an array at each scope, effective `get` shows the union while `get --scope` shows only that scope's entries - `tsc` build + `typecheck:tests` + eslint clean on touched packages	2026-07-31 10:27:25 +02:00
Ignacy Łątka	646f726bc4	chore: bump to 0.18.0 (#596) ... bump debated myself whether this should be a QoL patch or a minor release and i think the latter makes more sense | Commit | Change | | --- | --- | | `7189333a` | `feat(tool-server)`: human-readable log message (#582) | | `5ac69349` | `feat(flow)`: CSS combinator scopes for flow selectors (#569) | | `e23f0e28` | `feat(flow)`: snapshot `cropOn` for single-element visual diffs (#516) | | `6a53e66a` | `feat(flow)`: rotate directive (#546) | | `68ceb314` | `ci`: cut over npm publishing to the unified trusted-publishing workflow (#571) | | `2546788a` | `ci`: unified publish-npm workflow (#565) | v0.18.0	2026-07-30 23:47:28 +02:00
Stanisław Chmiela	7189333ac4	feat(tool-server): add human-readable log message (#582) ... ## Summary Adds tool-owned, human-readable interaction messages for every tool definition. Each tool defines its own started, completed, and failed wording (as implemented in https://github.com/software-mansion/argent/pull/367. Messages can use validated parameters, successful results, and safe failure codes. They are to avoid sensitive input such as typed text, cookie values, JavaScript expressions, and URL credentials. Bridges the gap between current Argent <img width="954" height="493" alt="Screenshot%202026-07-28%20at%2015 48 37" src="https://github.com/user-attachments/assets/2c689a93-8ba8-4208-b35e-66eb53c96671" /> and current agent-device <img width="1188" height="476" alt="Zrzut ekranu 2026-07-28 o 20 14 42" src="https://github.com/user-attachments/assets/6aed0f38-625b-4748-83e5-d384e79266c2" /> ## Test Selected fields from the generated JSONL: ```jsonl {"level":30,"type":"tool.invoked","toolId":"list-devices","toolInvocationId":"decb864f-51c1-41ee-91fc-15bc2494e268","msg":"Listing devices","time":"2026-07-29T10:37:19.896Z"} {"level":30,"type":"tool.completed","toolId":"list-devices","toolInvocationId":"decb864f-51c1-41ee-91fc-15bc2494e268","durationMs":96.77333299999918,"msg":"Listed 22 devices and 0 AVDs","time":"2026-07-29T10:37:19.993Z"} {"level":30,"type":"tool.invoked","toolId":"screenshot","toolInvocationId":"8504abb8-83ed-4490-8210-3355d6c638c9","msg":"Capturing screenshot","time":"2026-07-29T10:37:24.503Z"} {"level":50,"type":"tool.failed","toolId":"screenshot","toolInvocationId":"8504abb8-83ed-4490-8210-3355d6c638c9","failureSignal":{"error_code":"SIMULATOR_SERVER_READY_EXITED","failure_stage":"simulator_server_spawn_ready","failure_area":"tool_server","error_kind":"subprocess","failure_command":"simulator_server","failure_exit_code":1},"durationMs":750.4639999999999,"msg":"Failed to capture screenshot: SIMULATOR_SERVER_READY_EXITED","time":"2026-07-29T10:37:25.254Z"} {"level":30,"type":"tool.invoked","toolId":"screenshot","toolInvocationId":"63b938fc-3d86-4eb5-9972-a008474ffb01","msg":"Capturing screenshot","time":"2026-07-29T10:38:01.107Z"} {"level":30,"type":"tool.completed","toolId":"screenshot","toolInvocationId":"63b938fc-3d86-4eb5-9972-a008474ffb01","durationMs":25.509208000003127,"msg":"Captured screenshot argent-screenshot-chromium-cdp-9222-1785321481132-81477.png","time":"2026-07-29T10:38:01.133Z"} ```	2026-07-29 13:07:49 +02:00
Hubert Gancarczyk	5ac69349f0	feat(flow): CSS combinator scopes for flow selectors (#569) ... ## What Adds the CSS combinators to flow selectors, read off frames. Any action or condition can now scope its matches by where an element sits relative to another: ```yaml - tap: { text: Delete, within: { id: profile-card } } # CSS "A B" — inside that container - tap: { role: Switch, next: { text: Wi-Fi } } # CSS "A + B" — the nearest follower - assert: { visible: { role: Button, after: { text: Danger zone } } } # CSS "A ~ B" — any follower - assert: { hidden: { any: true, within: { id: empty-state } } } # CSS "*" — the universal selector ``` | Scope | CSS | Reads as | | --- | --- | --- | | `within: <sel>` | `A B` | the match's frame sits inside the scope element's frame | | `after: <sel>` | `A ~ B` | the match follows the scope element in reading order | | `next: <sel>` | `A + B` | as `after`, narrowed to the nearest follower of each anchor | Plus `any: true` — the CSS `*` universal selector, accepted only alongside a scope and never beside `text`/`id`/`role`. The child combinator `>` has deliberately **no analog**: flow trees flatten to leaves under one synthetic root, so parent/child structure does not survive to replay. ## How - **Geometric, not ancestry-based.** Frames are what survives flattening, so every scope is resolved from them by `findAll` in `ui-tree-match`. The synthetic root never counts as a scope element, and every relation requires a *distinct* node (nothing scopes itself). - **Reading order is row-band aware.** An element follows the anchor when it starts below the anchor's bottom edge, *or* shares its row band and sits entirely to its right. That band rule is what makes `{ role: Switch, next: { text: Wi-Fi } }` resolve the Wi-Fi row's own switch even though the taller switch's frame starts a couple of pixels *higher* than the label's — a raw top-y comparison gets that backwards. `followKind` is the single classification both the follower test and the nearest-pick grouping read, so they cannot disagree. - **`next` is the union over anchors**, exactly as CSS `A + B` is: three rows on screen give three switches, one per label. - **Scopes compose and nest**, capped at `MAX_SELECTOR_SCOPES` (6) counted across the whole relation *tree* — a size bound, not a depth bound, because three relations branch and the runner's loose-alternative expansion is exponential in the bare-string scopes it finds. - **Flow-YAML only.** `await-ui-element`'s strict schema rejects `within`/`after`/`next`/`any`, so a scope can never be silently dropped on that path. - Adding a fourth relation is a **compile error**: `RELATION_RESOLVERS` is a `Record` keyed by the relation union, so a new scope cannot be wired through parse/serialize/reporting and then ignored by the match engine. ## Testing - **Unit:** full tool-server suite green — **3006 passed / 1 skipped**. New coverage in `ui-tree-match.test.ts`, `flow-utils.test.ts`, `flow-selector-scopes.test.ts` (renamed from `flow-within-selector.test.ts`, now covering all scopes) and a new `flow-skill-docs.test.ts` that parses every YAML snippet in the skill so the docs cannot rot. Each fix is mutation-checked: reverting it fails at least one test. - **Real-device E2E** (Pixel 3a, Bluesky on Metro 8081, via a branch tool-server): three flows, 20 steps, all green — geometric ordering both ways, two `text` asserts that pin `next`'s pick *by identity* out of five candidates, empty-scope `hidden` semantics, and two observable taps (`next`-scoped and `any: true`-scoped) that navigated to the *adjacent* topic rather than the anchor's own. - **iOS not covered.** Native-devtools does not attach on either booted simulator in this environment, so *every* flow selector fails there — an unscoped `{ text: "Video" }` fails identically. Verified environmental, not caused by this branch: the globally-installed argent reports the same, direct `SIMCTL_CHILD_DYLD_INSERT_LIBRARIES` injection also fails to connect, and it survives a forced simulator reboot on both iOS 18.3 and 26.5. Worth re-running the iOS leg once that is healthy. ## Notes for reviewers Two deliberate deviations from CSS, both documented in the skill: - **`next` is looser than `+`.** Where `A + B` matches nothing unless the immediate sibling is a `B`, `next` keeps looking and returns the nearest match further on. That is what makes it survive the wrapper and spacer nodes a flattened tree is full of — but a row *missing* the control you asked for resolves to the next row's rather than failing. Happy to switch to strict `+` semantics if the loud failure is preferred. - **"follows" is not transitive.** Against a tall anchor an element can follow something that follows the anchor without following it directly, so nesting `after` scopes is not the same as chaining `~`. Other things worth a look: - `nearestAfter` and `afterTester` are direct scans, not indexed. An earlier sorted index was a *second* implementation of the nearest rule and the two disagreed on frames near the tolerance; it also measured 2-13x slower than the scan on realistic shapes, with a prune that provably never fired. The quadratic tail is documented with measured numbers: 1.2 ms for a single-anchor `next` at the 12k-node tree cap, ~555 ms only when the anchor selector matches essentially every node. - A bare-string *anchor* cannot self-correct the way the loose fallback usually does — a decoy `testID` matching your anchor text wins the identifier pass whenever anything sits after it. The skill tells authors to spell sibling anchors as maps. - `MAX_SELECTOR_SCOPES = 6` replaces the previous `MAX_SELECTOR_WITHIN_DEPTH = 10`, so a `within` chain deeper than 6 that parsed on the earlier commits of this branch now rejects. Nothing released depends on it. --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-07-27 18:25:27 +02:00
Jakub Piasecki	e23f0e285e	feat(flow): snapshot cropOn for single-element visual diffs (#516) ... `snapshot` gains an optional `cropOn: <selector>` — diff one element's region instead of the whole screen: `- snapshot: { name: cart-total, cropOn: "Order summary" }` - The selector resolves like a directive target: settled tree, auto-wait, the standard not-found failure. Selector-only — no point form (a point has no extent to crop to). - The frame resolves **before** the capture, and the **cropped** image is what gets compared, stored as the baseline, and reported as the `current` artifact. The pixel rect is the normalized frame × capture size; the crop is a pngjs `bitblt` written to a swept scratch dir and registered only when a failure returns it. - The baseline key stays on the **full** capture's WxH (device-class identity), so the previously unreachable `dimensionMismatch` guard becomes load-bearing: an element that grew or shrank fails with "nothing was compared" plus a crop-fixed-size-containers hint. Deliberately strict — a 1px layout drift fails on dimensions, not content (the same tradeoff as Playwright element screenshots). - A tree-source outage while resolving `cropOn` errors the step instead of degrading to a full-screen compare against a cropped baseline. - No tool-schema changes; `snapshotKey` and `--output` behavior ride along unchanged. Tests: parse/serialize round-trip + rejection cases, and runSnapshot coverage with real PNGs (crop dims, key vs. content, artifact retention/sweep, unresolved selector, abort, size-drift message, sub-pixel region).	2026-07-27 10:07:49 +02:00
filip131311	68ceb314d6	ci: cut over npm publishing to the unified trusted-publishing workflow (#571) ... ## Why Cutover step 3+4 of the plan from #565. `publish-npm.yml` is now registered on npmjs.com as the **trusted publisher** for `@swmansion/argent` (2026-07-24) and the OIDC publish path is verified live: `v0.17.1-next.1` published with a SLSA provenance attestation via a workflow_dispatch of the unified workflow. Extra urgency: token-based publishing died on 2026-07-24 (~11:20–15:20 UTC window; npm's 2FA-bypass token restriction) — **`publish-next.yml`'s canary now fails on every main push** (identical E404 on the registry PUT, e.g. run 30104847558). The old workflows aren't just redundant, they're broken; only the trusted-publisher workflow can publish. ## What - Delete `publish.yml` and `publish-next.yml` — `publish-npm.yml` is the only publish path. - `sync-next-tag.yml`: drop the two retired workflow names from the `workflow_run` trigger. - `publish-npm.yml`: remove `NODE_AUTH_TOKEN` from both publish steps (OIDC authenticates; the rest of the job already ran token-less) and update the header — the cutover plan is replaced by a note that this file's name is load-bearing for the npmjs.com registration. - `NPM_TOKEN` stays as a secret: `sync-next-tag.yml` still needs it for dist-tag mutation, which OIDC doesn't cover. Note the token may currently be dead for dist-tag writes too — the post-publish sync run succeeded but had no mutation to make; if a real re-point fails, a fresh granular token is needed. ## After merging Set the repo variable to enable canary-on-push (kept as the canary kill-switch): ``` gh variable set PUBLISH_NPM_UNIFIED --repo software-mansion/argent --body "1" ``` Ordering is safe: until the variable is set, a main push publishes nothing (the old canary workflow is gone, the new one skips) — no double-publish window exists. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-24 17:54:00 +02:00
filip131311	2546788a8d	ci: unified publish-npm workflow (provenance-ready, trusted-publishing prerequisite) (#565) ... ## Why npm provenance / trusted publishing for `@swmansion/argent` requires registering **one** workflow file as the package's trusted publisher on npmjs.com — but we currently publish from two (`publish.yml` for stable, `publish-next.yml` for canaries/prereleases). This PR adds their unified successor, **`publish-npm.yml`**, without touching the old workflows: they keep running unchanged until cutover. ## Design One file, mode derived from the trigger and the version — nothing is spelled twice: | Trigger | Behavior | |---|---| | `push` to `main` | Canary: version derived + stamped (`<base>-next.<k>`), published under `next` with the same registry-race retry loop as before. **Dormant until the `PUBLISH_NPM_UNIFIED` repo variable is set to `1`** — otherwise every main push would publish two canaries while `publish-next.yml` still exists. | | `workflow_dispatch` | Publishes the ref's `package.json` version verbatim. Dist-tag derived from the version itself: prerelease → `next`, stable → `latest`. The optional `channel` input is an assertion (fails on mismatch), not a selector — you can't accidentally ship a prerelease to `latest` or vice versa. `dry_run` runs the full build + bundle verification + pack without publishing. | Provenance is on from day one: the job has `id-token: write` and every publish path uses `--provenance` (repo is public, `repository.url` matches — the two things the registry verifies). Token auth (`NPM_TOKEN`) is kept for now; npm ≥ 11.5.1 is ensured in the job so the OIDC exchange takes over transparently once the trusted publisher is registered. `sync-next-tag.yml` now also triggers on the new workflow, so `next` keeps getting re-pointed after publishes from either generation. (Until cutover a main push may trigger the sync twice — once per publish workflow — which is harmless: the script is idempotent.) ## Cutover checklist (follow-up, also documented in the workflow header) 1. Test: `workflow_dispatch` with `dry_run`, then a real prerelease dispatch (`v*-next.N` ref). 2. On npmjs.com, register `publish-npm.yml` as the trusted publisher for `@swmansion/argent` (org `software-mansion`, repo `argent`). 3. Set repo variable `PUBLISH_NPM_UNIFIED=1` and delete `publish.yml` + `publish-next.yml` in the same PR (also dropping their names from `sync-next-tag.yml`'s `workflow_run` trigger). 4. Remove `NODE_AUTH_TOKEN` from the publish steps (`NPM_TOKEN` stays for `sync-next-tag.yml` — dist-tag mutation isn't covered by OIDC); optionally restrict the package's publishing access to the trusted publisher only. ## Behavior differences vs the old pair - Stable dispatch now publishes with an explicit `--tag latest` (same effect as the old default) and `--provenance`. - The old `publish-next.yml` dispatch guard ("refuse non-prerelease versions") is generalized: a stable-version ref dispatched here goes to `latest` instead of erroring. Set `channel: next` if you want the old refusal behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-07-24 17:20:18 +02:00
Jakub Piasecki	6a53e66a96	feat(flow): rotate directive (#546) ... # feat(flow): rotate directive Adds a selector-targeted two-finger rotation directive to flows: `rotate: { on?, by }`. `on` resolves like `tap` (settled tree + auto-wait); omitted, the rotation centers on the screen. `by` is degrees, positive = clockwise. Distinct from `tool: rotate`, which changes device orientation — the docs and step labels call this out. ## Design **One continuous gesture, fixed pace, no duration option.** Fingers orbit at ~90°/300 ms; duration derives from the angle. Unlike pinch there is no sub-gesture chaining — any angle dispatches as one sweep, so the angular delta is exact with no settle delays. `by` is parse-bounded to ±3000°: the largest sweep the 10 s single-gesture envelope delivers at that pace, derived from the constants rather than picked as policy — rejected at parse, never silently clamped. A consequence embraced here: frame count always scales with the angle, so a speed-sensitive recording stays a raw `tool:` step (the create-flow skill's polish list covers the conversion). **Fixed centroid, constant physical radius.** Drifting the centroid would couple pan into the turn; growing the radius would couple pinch. Both stay fixed — the trade is a smaller orbit near edges and small targets, in exchange for a pure rotation. **Physical circle, not a normalized-space ellipse.** A single normalized radius on a non-square screen traces a physical ellipse, coupling an unintended pinch into every turn. `gesture-rotate` grows optional per-axis `radiusX`/`radiusY` (fractions of width/height with `radiusX·W = radiusY·H`), and the flow tree adapters surface screen dimensions via an optional `screen` field on `DescribeTreeData` so the directive can compute the aspect. Missing dimensions fail soft to the legacy single-radius call rather than erroring. **Placement reuses pinch's edge model.** The initial finger axis (horizontal or vertical) is scored against the same per-platform system-edge guards, preferring a start parallel to the nearest violated edge and staying inside the target's axis-matching span. Guards only score placement — they never shrink the radius, and a violated guard is attempted, never rejected. The only geometry failure is no positive on-screen orbit radius. **Abort correctness.** `gesture-rotate` now honors `ctx.signal`: each frame checks it, lifts the synthetic fingers with a terminal Up (so a cancelled run doesn't leave them held on the glass), then throws with name `AbortError`; the directive maps a mid-gesture abort to the run's aborted skip rather than a step failure. **Explicit `inputSchema`.** The radius requirement is a zod `.refine()` cross-field rule, which `z.toJSONSchema` drops — so the tool ships a derived schema plus an explicit `anyOf` (`radius` or `radiusX`+`radiusY`) to keep invalid calls rejected at the schema layer. **Platform rejections.** Vega is remote-driven (no touch) and Chromium has no rotate-gesture idiom at all — both fail upfront with guidance to drive the app's rotate controls directly. ## Tests Unit tests cover the geometry module (duration derivation, candidate construction, aspect handling, edge-guard ranking), the directive end-to-end against a fake device, and `gesture-rotate`'s per-axis radii and abort path.	2026-07-24 13:18:23 +02:00
Ignacy Łątka	a0460eeefe	chore: bump workspace packages to 0.17.0 (#554) ... ## What Lockstep-bump all 16 `packages/*` workspace packages (and the matching `package-lock.json` entries) from `0.16.1` → **0.17.0**. Diff is exactly 32 version-field swaps across 17 files — no formatting or dependency churn. ## Verification - **Lockfile CI gate reproduced locally on the exact CI toolchain** (Node 24.14.1 / npm 11.11.0, matching `lockfile.yml`'s `node-version: "24"`): `npm install --package-lock-only --ignore-scripts` on top of the commit, then `git diff --exit-code -- package-lock.json` → **clean**. npm's own regeneration reproduced the hand-edit byte-for-byte, so none of the `"peer": true` churn that #513/#539 had to keep out reappears here. - `node scripts/check-workspace-versions.mjs` → `All workspace packages are at 0.17.0.` - Remaining `repo-hygiene.yml` checks pass: no conflict markers, no focused tests. - `prettier --check` on all 17 changed files → `All matched files use Prettier code style!` - **Runtime version actually reports 0.17.0**: `getInstalledVersion()` (`argent-installer/src/utils.ts:384`) and `getGloballyInstalledVersion()` (`topology.ts:94`) both read `version` straight out of the resolved package root's `package.json` at runtime, so the bump is the whole mechanism — no generated constant or second version file to keep in sync. Exercised that resolution against the bumped tree: `PACKAGE_ROOT` → `packages/argent` (`@swmansion/argent`) → `0.17.0`. - Verified all 16 lockfile hits belong to workspace packages (parsed the lockfile's `packages{}` and matched every `0.16.1` entry to a `packages/*` path) — no unrelated dependency happened to sit at 0.16.1. - No test asserts a release version string; the only `0.1x.x` matches under `*.test.ts` are historical references inside comments. ## Note `packages/argent-private` needs no pin change here. Its release version is carried purely as a git tag — the repo has no version field on the release line (its only `package.json` is a `private: true` research CLI on an independent `0.1.0` line) — and `v0.17.0` is now tagged at `2d6c292`, which is **already** the commit this superproject pins. `v0.16.0` and `v0.16.1` sit on that same commit, so the submodule entry is correct as-is and stays out of this diff. v0.17.0	2026-07-23 23:33:49 +02:00
Ignacy Łątka	98ae93145b	feat(screen-recording): save recordings to the project's .argent/recordings (#551) ... Stacked on #549 (touch visualizer). ## What Screen recordings now land durably under **`.argent/recordings/`** on the client, instead of the disposable temp-artifact cache — and for a remote `argent link` tool-server they land on the **client** host, not the server. ## Why Two rough edges in the recording flow: 1. The finished video was only ever written to the OS temp-artifact cache, so it was easy to lose and not obviously part of the project. 2. Over `argent link`, the video was downloaded into that same temp cache; there was no project-scoped home for it. ## How - New optional `saveDir` hint on the artifact wire contract (`@argent/registry` + `@argent/tools-client`). - `screen-recording-stop` tags its video with `saveDir: ".argent/recordings"` and drops the internal `argent-` temp prefix from the filename. - The client materializer honours `saveDir`: **copies** the file when the tool-server is co-located, **downloads** it over `/artifacts/:id` when it is remote. Either way the bytes end up in the durable directory. - `saveDir` resolves against the client's **project root** (nearest ancestor with `.git`/`package.json`/`.argent`), falling back to the global **`~/.argent`** when run outside a project — matching how the rest of argent scopes per-project config. Adds `findProjectRoot` to `configuration-core` (a null-returning variant of `resolveProjectRoot`) to drive that choice. - Hardened: absolute paths and `..` escapes are rejected, directory bundles excluded — a tool-server can't steer a write outside the base directory. ## Verification - Unit: 28 cases in `argent-tools-client/test/artifacts.test.ts` (co-located copy, remote download, project-root anchoring incl. from a subdirectory, global fallback, path-traversal rejection, integrity). configuration-core suite green (55). - E2E against a booted iOS 18.5 sim + a worktree tool-server, driving the real client materializer path: - in a project, co-located server → `<projectRoot>/.argent/recordings/…mp4` (valid h264) - in a project, forced remote download (`GET /artifacts/:id`) → same durable dir, temp cache left empty - outside a project (HOME redirected) → `~/.argent/recordings/…mp4` - Root `tsc --build`, prettier, and eslint all clean. MoQ / `sim-remote` recording support lands in a separate PR stacked on this one. --------- Co-authored-by: Ignacy Łątka <claude-hubert.gancarczyk@swmansion.com>	2026-07-23 22:43:01 +02:00
Ignacy Łątka	f85d3843aa	feat(screen-recording): draw touch gestures into the recording (#549) ... ## What Draws the touch gestures argent drives directly into the screen recording, so a captured video shows **where** each interaction landed instead of a UI reacting to an invisible finger: - a pulse marks each **tap** - a fading comet trail follows a **swipe / drag** - paired markers show the two contact points of a **pinch / rotate** On by default; pass `showTouches: false` to `screen-recording-start` for a raw, overlay-free capture. ## How simulator-server already has a touch visualizer behind its `pointer` cargo feature (the binary argent bundles ships with it). It renders the markers into the very MJPEG stream this recording pipeline already consumes — nothing is composited host-side, and it's driven by the touch events argent already sends. This PR just turns it on for the life of a recording: - `screen-recording-start` builds a small `PointerControl` from the resolved simulator-server handle and hands it to `startCapture`. - The overlay is enabled right after the encoder goes live (so the first interaction is already drawn) and **restored to off** when the capture ends — stop, time-limit cap, encoder crash, or session teardown all run the same disable path, so it never outlives the recording it was enabled for. - Two thin client helpers (`setPointerVisible`, `setPointerTrail`) POST to simulator-server's `/api/pointer`. They're best-effort: a build without the feature (or a remote sim) just yields a `warning` at stop and a normal overlay-free video, never a failed recording. ## Verification - **Unit** — `screen-recording.test.ts`: 49 pass, incl. 10 new covering enable-at-start / disable at stop, cap, unexpected exit, and dispose; the `pointerFailed` warning; the no-control path; and the `/api/pointer` wire format (`{show}` / `{trail}`, ok/error/throw handling). - **Static** — `tsc` (src + `tsconfig.test.json`), eslint, prettier, full workspace build all clean. - **E2E** — drove the real tool-server → the bundled (production) simulator-server on an iOS 18.5 sim: - `showTouches: true` + a slow swipe → a translucent pointer bubble is rendered onto the screen at the touch point in the recording. - identical swipe with `showTouches: false` → paging still happens, **no bubble** — proving both the toggle and that stop restored the overlay to off (the control clip never re-enables it, yet draws nothing). --- Stacked on #548 (static-frame trimming) → base branch `feat/screen-recording-trim`. Review after that merges. --------- Co-authored-by: Ignacy Łątka <claude-hubert.gancarczyk@swmansion.com>	2026-07-23 22:32:51 +02:00
Ignacy Łątka	82f6eb16b0	feat(screen-recording): trim static stretches out of the recording (#548) ... > Stacked on #517 (`feat/screen-recording`). Review/merge that first; this PR's diff is against that branch. ## What Trims stretches where the screen doesn't change out of the recorded video. A recording left running across waits, slow navigation, or agent thinking time used to come back padded with dead air; now a long session with only brief activity comes back as a short clip. ## How The frame pump already re-emits the last frame on a wall-clock schedule to keep the timeline honest. This adds a change detector on top: - Each frame is compared to the last one **byte-exact** (a reference check short-circuits the common "no new frame arrived" case, so only genuine arrivals pay the compare). Exact byte equality is stronger than a hash — no collisions — and flags a change down to a single pixel, matching "something moved, even by a couple of pixels." - The first second of every still stretch is kept (pauses read naturally), then unchanged frames are dropped until the screen moves again. - Active stretches still play at real time: the wall-clock pacer is re-anchored whenever a dead stretch is skipped, so the gap yields no output frames rather than a catch-up burst of the next frame. On by default. `trimStatic: false` on `screen-recording-start` keeps a faithful real-time recording (e.g. to measure how long something took on screen). `screen-recording-stop` now returns the trimmed length as `durationMs`, plus `wallClockMs` and `trimmedMs` when trimming removed anything. ## Verification - **Unit**: 39 tests in `screen-recording.test.ts` (7 new trim cases: default-on, grace-window kept, no-trim control, resume-the-moment-the-screen-changes, byte-identical-new-frame-stays-trimmed, duration reporting, fields-omitted-when-off). Full tool-server suite green (2817). `tsc --build`, `typecheck:tests`, eslint, prettier clean. - **E2E on a live tool-server (iOS 18.6 sim)**: - 23s idle-heavy session (motion → 15s idle → motion) → **1.47s** clip, `trimmedMs` 21930, with the real transitions intact (44 distinct frames — above the 30-frame grace floor, proving post-grace motion is kept). A `trimStatic:false` control of the same pattern returned the full 23s / 691 frames. - 8s continuous burst-motion session → **2.1s** across multiple motion bursts (not collapsed to the 1s floor). - **Cross-check**: ran ffmpeg's own `mpdecimate` (duplicate-frame detector) over the same untrimmed footage — it kept ≤ what this trim keeps, confirming the trim is faithful, never over-aggressive (byte-exact is deliberately more conservative than mpdecimate's near-duplicate heuristic, so smooth animation is preserved while only frozen stretches are dropped).	2026-07-23 21:45:42 +02:00
Ignacy Łątka	2251c7cf46	feat(screen-recording): screen-recording-start/stop tools with per-call stop reminders (#517) ... ## What Adds a `screen-recording-start` / `screen-recording-stop` tool pair (naming follows `native-profiler-start/stop`) that records the device screen to an mp4 and returns it as a downloadable artifact. Capture runs through **simulator-server** — the same backend `screenshot` and every interaction tool already drive. simulator-server publishes the device screen as an MJPEG stream (`/stream.mjpeg`), so recording needs no platform-specific recorder: the tool-server subscribes to that stream, paces the frames onto a fixed 30 fps timeline, and pipes them into a single ffmpeg process that encodes — and watermarks — straight to the final mp4. The stream is multi-subscriber, so a preview window can stay open on the same device while recording. - **One path for every device simulator-server drives** — iOS simulators, Android emulators, and physical Android devices. - **Native resolution, constant 30 fps** — h264, `+faststart`, no post-processing pass. - Session state lives in a per-device `ScreenRecordingSession` blueprint (mirrors the native-profiler session): time-limit cap with auto-finalize, unexpected-exit recovery (a capture that ended on its own is still retrievable via stop), always-startable state reset on stop, and dispose-on-shutdown cleanup through the registry. ## Why the frames are paced host-side A device only emits a frame when the screen **changes**. Pointing ffmpeg straight at the stream therefore produces a video whose length is the sum of the moments something moved — a still screen collapses to a fraction of a second — and an ffmpeg blocked on a silent socket does not answer a stop signal promptly either (measured: 12 s late). Re-emitting the last frame on a wall-clock schedule fixes both: **video duration always equals real elapsed time**, and stop is a stdin EOF that finalizes in ~60 ms. Duplicate frames cost almost nothing once encoded (a 10 s recording of a static screen is ~1 MB). ## Finishing pass folded into capture The Argent watermark (logo + `By @swmansion`, bottom-left, sized relative to the frame) is stamped by the same ffmpeg invocation that encodes the capture, so there is no second encode and no temp intermediate. Its colour is chosen **per pixel** from the background luma underneath it — the white logo over dark areas, a runtime-derived near-black copy over light areas — so it keeps maximum contrast against whatever is behind it. It sits at 20 % opacity and is opt-out via `argent disable video-watermark` (read when the recording starts). Frame geometry comes from the first JPEG's SOF header, so no `ffprobe` is needed. ffmpeg is now the recorder rather than an optional finisher, so a missing ffmpeg fails `screen-recording-start` up front with an install hint instead of silently degrading the output at stop time. ## Reminder plumbing While a recording is running — or ended on its own but was never retrieved — **every** tool result carries a `note` telling the agent the recording is still going and how to stop it. It rides the same `note` field as the update reminder but is per-call and unsuppressed; both notes compose when they coincide. The note text switches to "already ended, retrieve the file" once the cap fires or the capture dies. ## Skill New `argent-screen-recording` skill (+ routing stanza in `rules/argent.md`): documents the start → interact → stop workflow and instructs the agent to schedule its own reminder/wakeup for the expected end of the recording immediately after starting one (harness reminder tool if available, otherwise a background `sleep` whose completion notification pulls it back). Scores 10/10 on `scripts/grade-skills.mjs`. ## Verification - Unit: 45 tests across `screen-recording.test.ts`, `mjpeg-stream.test.ts`, `watermark.test.ts`, `http-recording-note.test.ts`; full tool-server suite **2808 tests / 280 files green**; `tsc --noEmit -p tsconfig.test.json`, root `tsc --build`, eslint, prettier all clean. - E2E on a live tool-server (`POST /tools/...`), driving real interactions between start and stop: - **iOS sim** (iPhone 16 Pro Max, iOS 18.5): 1320×2868 h264, `r_frame_rate=30/1`, 338 frames, container duration 11.27 s vs reported `durationMs` 11314 — **within 50 ms of wall clock**. Extracted frames show real screen content (mid app-switcher animation) with the watermark rendered light-over-dark. - **Android emulator** (Pixel 3a, API 34): 1080×2220 h264, 30 fps, 353 frames, duration 11.77 s vs `durationMs` 11793 — within 30 ms. 512 KB for a mostly-static home screen. - **Static screen** (no interaction at all, 10 s): 300 frames, duration exactly 10.000 s — the case the previous `recordVideo` implementation reduced to ~0.03 s. - **Long silence then activity** (15 s untouched, then an app launch): stream survived, no warning, frames after the silence show the new screen. This caught a real bug during review — the connect timeout was also acting as a socket *inactivity* timeout and tore the stream down at 10 s of stillness; fixed and covered by a regression test that fails without the fix. - **Time-limit cap**: `timeLimitSeconds: 5` self-finalized at 4.97 s / 149 frames, the per-call note flipped to "already ended (it hit its 5s time limit)", and a stop issued 11 s later still returned the video with the matching warning. ## Not supported (capability-gated with clear errors) tvOS simulators, physical iPhones, Chromium apps, Vega, and remote (`remote:`-prefixed) simulators — none of them expose a readable frame stream. --------- Co-authored-by: Ignacy Łątka <claude-hubert.gancarczyk@swmansion.com>	2026-07-23 21:14:37 +02:00
Ignacy Łątka	19995985d3	docs(scripts): stop citing flow-add-step's since-reworded description in a fixture comment (#527) ... Follow-up to #436. The escape-cooking fixture's comment cited flow-add-step.ts as a currently-shipping example of a `\n` description; main's flow rewrites reworded that description, so the citation is stale. The comment now leans on the fixture alone instead of any live tool's wording. No code change - `npm run test:scripts` still 59/59.	2026-07-23 19:40:07 +02:00
Aayush Gautam	3795cb84fb	fix(ios-profiler): stop native-profiler-stop being aborted mid-packaging (#529) ... ## Problem `native-profiler-stop` fails with `No active native profiling session found. Call native-profiler-start first.` while xctrace is still recording — #248, #250, #330. Those were closed on the 0.12.1 `--all-processes` workaround (#380), which fixed trace finalization but not this. The tool isn't declared `longRunning`, so the MCP adapter applies its per-request timeout: ```js fetchTimeoutMs: meta?.longRunning ? null : FETCH_TIMEOUT_MS, // mcp-server.ts:222 ``` A stop that runs past 30s is aborted and retried (`MAX_RETRIES = 4`). The retry runs against the session state the first call has already cleared, hits the `!api.profilingActive` branch and throws. The message points at session tracking; the actual failure is the timeout. Every failed stop I have logs for lands at 30–34s: `durationMs: 33639` in [#330](https://github.com/software-mansion/argent/issues/330), and 30276 / 30605 / 31019 / 31914 locally. Stop can't fit in 30s on Xcode 26.4+. Since #380 the capture is host-wide rather than app-scoped, so the bundle is much larger — ~55MB per 10s of recording here, 385MB for 90s — and xctrace needs ~22s to package even the 10s case before the four export passes start. `STOP_GRACE_MS` has the same problem from the other side: at 30s xctrace is SIGTERM'd mid-package, and the bundle it leaves behind fails to export. ## Fix Declare the tool `longRunning`, and raise `STOP_GRACE_MS` to 300s. The grace is a ceiling rather than a fixed wait — a fast package still returns as soon as xctrace exits — so this only changes captures that were previously killed. With both applied a 90s capture stops and exports cleanly in ~2–4 min, and a 40s capture returns inline. ## Note The abort-and-retry sequence is inferred rather than captured directly. What's observed: `MAX_RETRIES = 4`, the uniform ~30s durations, and `tool-server.log` showing the `did not respond to SIGINT` warning — written after the throw site — logged 184ms *before* the failure was returned. One execution can't produce that ordering. ## Testing - `npm run build` and eslint clean on changed files - `npm test -w @argent/tool-server` — 2727 tests / 273 files pass - Verified on Xcode 26.5 / iOS 26.5 simulator against a bridgeless RN app: start → interact → stop → analyze now returns a populated report with `errors: {}`	2026-07-23 18:32:44 +02:00