software-mansion__argent

mirror of https://github.com/software-mansion/argent.git synced 2026-09-14 19:27:14 +08:00

Author	SHA1	Message	Date
Hubert Gancarczyk	fdd1774ca4	fix(flows): stop reporting what a script prints, on both sides A `script:` step used to bring its stdout and stderr back with it. The recorder returned `log`/`logTruncated`, the step report carried `scriptLog`/`scriptLogTruncated`, and the CLI and the MCP content builder each rendered them. Both surfaces land in an agent's context, and nothing in the flow file declares what a script prints, so nothing can decide in advance whether that text is safe to forward. The channel goes rather than being gated behind a parameter the agent would set itself. A script step now answers with its verdict and its `reason` only. Deleting the capture removes what existed to bound it: the 64 KiB per-step cap, the 256 KiB run-wide budget, the V8 frame collapser, the partial-marker guard and the streaming half of the secret scrub. Both pipes are still drained, and stderr is still scanned for V8's heap-exhaustion banner and for a watchdog that never armed.	2026-09-09 16:23:04 +02:00
filip131311	02fd22e9d6	chore: bump all versions to 0.25.0 (#1083 ) Bumps every workspace package, `package-lock.json`, and both version fields in `server.json` from 0.24.0 to 0.25.0. - `npm version 0.25.0 --workspaces --no-git-tag-version --workspaces-update=false` - lockfile regenerated with `npm install --package-lock-only --ignore-scripts` on Node 24 / npm 11 (version-only diff) - `npm run check:versions` passes No docs update needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01XdSDvFDvuU1iyz7HPjcHMV Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>	2026-09-09 15:00:24 +02:00
Arkadiusz	9d512ce933	optimization: simplify flow-run description (#954 ) # Why It is reported that benchmark results are getting worse and the `flow-execute` description has the largest optimization potential. The description field of MCP tools should describe what the tool does and when to call it, not reveal implementation details. <img width="1195" height="382" alt="Screenshot 2026-08-25 at 12 45 30" src="https://github.com/user-attachments/assets/d83ba9c5-5e4f-4c65-ba3a-4af0588e5faa" /> # Warning Not tested. I'd have to run this change twice against the benchmark repo but that feels like overkill. I am not sure about behavioral regressions, relying on CI here. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Documentation - Updated the Flow YAML reference to streamline directive documentation. - Revised the flow execution tool’s description with a concise overview of supported flow sources and per-step reports. - Clarified documentation around flow behavior, including warning handling and project-root configuration. - Removed outdated or overly detailed statements from the published guidance to keep the reference accurate and easier to scan. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-09 14:21:30 +02:00
Ignacy Łątka	82db9e2f9f	test: stop tests leaving files in the shared system temp directory (#1057 ) Three issues, one theme: tests and the shared system temp directory. - #858 / #854 were live leaks — three call sites captured a real temp path and never removed it. One reaches the cleanup its file already had; the other two get a cleanup the file lacked. - #836 turned out to be already fixed on `main` by #714, which scoped those three tests' `TMPDIR` to a directory they own. This PR verifies that rather than changing it — reproduction and mutation evidence below. - Sweeping the same shape across the two packages turned up six more test files doing it, worth 44 leftover entries per run. - A third shape, in two more files: `resolveFileInputs` hands its caller a `cleanup` for whatever it materialized — the dispatcher's job in production. Five call sites kept the result and never called it, stranding 5 dirs per run. A scoped-`TMPDIR` run of all 12 affected files leaves one entry behind: an empty `argent-chromium-media/`, production's own fixed staging dir. 173 tests, no other file or directory. Fixes #858 Fixes #854 Closes #836 <details> <summary>#858 — before/after temp scope</summary> ``` rm -rf /tmp/scope && mkdir -p /tmp/scope TMPDIR=/tmp/scope TMP=/tmp/scope TEMP=/tmp/scope \ npx vitest run --root packages/tool-server test/chromium-server-screenshot.test.ts ``` \| site \| before \| after \| \|---\|---\|---\| \| `chromium-server-screenshot.test.ts` \| `argent-chromium-media/argent-screenshot-test-1788803062089-38318.png` \| (no PNG) \| \| `chromium-cdp-blueprint.test.ts` \| `argent-chromium-media/argent-screenshot-chromium-cdp-52204-1788803091335-39401.png` \| (no PNG) \| Both files keep an empty `argent-chromium-media/` — that is production's fixed staging dir (`src/chromium-server/cdp-session.ts:218`), shared with any live chromium-server on the machine and not per-run garbage. A test `rm`ing it would be the machine-global-resource defect #714 removed elsewhere, so it is left alone deliberately. </details> <details> <summary>#854 — before/after temp scope</summary> ``` TMPDIR=/tmp/scope … npx vitest run --root packages/argent-mcp test/content.test.ts ``` Before (`34 passed`, two directories left): ``` argent-mcp-content-kPfgYa/context.diff.png argent-artifacts/test-temp-dir-hygiene-037c31/20260907-174453-39458/home-diff.png ``` After: empty. The `mkdtemp` moved onto a `beforeEach`/`afterEach` pair mirroring the sibling describe, and `flowRunToMcpContent` now pins `ARGENT_ARTIFACTS_DIR` the way the artifact describe above it already did. The unconditional `delete process.env.ARGENT_ARTIFACTS_DIR` is kept, not converted to a save/restore: it is what keeps any future unpinned write landing in `tmpdir` rather than a developer's configured artifacts directory. The pin is not hygiene-only — it is asserted. Removing `process.env.ARGENT_ARTIFACTS_DIR = root` fails the test: ``` - diff: /tmp/scopeM4/content-flow-artifacts-LZKPAl + diff: /tmp/scopeM4/argent-artifacts/test-temp-dir-hygiene-037c31/20260908-100837-89701/home-diff.png ❯ test/content.test.ts:627 expect(artifactText?.text).toContain(`diff: ${root}`) ``` </details> <details> <summary>#836 — why no code changed, and the evidence</summary> (This PR does change `file-inputs.test.ts`, but for the unrelated third shape above — not for the sites #836 cites.) The issue cites `readdir(os.tmpdir())` at `file-inputs.test.ts:180-197` and `flow-visual.test.ts:586-602`. On current `main` neither exists: #714 replaced both (and the `http-upload.test.ts:154-158` sibling the issue also flags) with a per-test scratch directory the test creates, points `os.tmpdir()` at via `redirectTmpdir`, and removes. There is now no `readdir` of `os.tmpdir()` anywhere in either package. The per-path assertion the issue asks for is not reachable at these three sites — every name is a `mkdtemp` random suffix or a server-side `randomUUID`, so the test cannot know the path. Scoping the tmpdir to a directory the test owns is the same guarantee, and is what landed. Reproduced the reported failure by reverting each site to the global-listing shape, then running two simultaneous instances in a loop: \| file \| pre-#714 shape \| current \| \|---\|---\|---\| \| `file-inputs.test.ts` \| 1 / 12 — `expected [ 'argent-file-input-00mPU7', …(1371) ] to deeply equal [ …(1370) ]` \| 0 / 12 \| \| `flows/flow-visual.test.ts` \| 12 / 12 — `expected [ 'argent-flow-crop-00R9XZ', …(4853) ] to deeply equal []` \| 0 / 12 \| \| `http-upload.test.ts` \| — \| 0 / 12 \| Mutation-tested all three assertions — each still catches a real leak; production cleanup broken, test run, source restored: \| assertion \| mutation \| result \| \|---\|---\|---\| \| `file-inputs.test.ts:249` \| drop `await cleanup()` from `resolveFileInputs`' catch path \| ✗ `+ [ "argent-file-input-amPJuK" ]` \| \| `flow-visual.test.ts:610` \| drop `cleanupDiffDir(cropDir, …)` from the outer `finally` \| ✗ `+ [ "argent-flow-crop-rWT9Bd" ]` \| \| `http-upload.test.ts:187` \| drop `rm(tarPath)` from the `req.on("close")` handler \| ✗ `+ [ "argent-upload-43d82021-….tar.gz" ]` \| </details> <details> <summary>Sibling sweep — the same two shapes across both packages</summary> Shape "assertion diffing a listing of `os.tmpdir()`": zero remaining. No `readdir`/`readdirSync`/`glob` over `os.tmpdir()`, `tmpdir()` or a literal `/tmp` in either test tree. Every literal `/tmp` string left is a fixture handed to a mocked API; the three that do touch disk were checked by hand and are clean (`native-devtools-socket-bind.test.ts` mkdtemps and removes, `flow-utils.test.ts` is pure path math, `native-profiler-external-device.test.ts` writes only inside its own per-test `mkdtemp`). Shape "a captured temp path never cleaned up": six files fixed. Each gets one recording choke point plus an `afterEach`. Running those six under a scoped `TMPDIR` (`48 passed` both times): ``` before: 44 entries after: 1 entry ``` \| file \| sites \| \|---\|---\| \| `screenshot-diff-tool.test.ts` \| 9 \| \| `react-profiler/ast-index.test.ts` \| 6 \| \| `event-log.test.ts` \| 1 helper (4 dirs/run) \| \| `screenshot-diff.test.ts` \| 1 helper \| \| `react-profiler/component-source-duplicates.test.ts` \| 2 \| \| `ios-instruments/combined-report-frozen-anchor.test.ts` \| 1 \| \| `flows/flow-visual.test.ts` \| production's crop + diff dirs (5/run) \| Left deliberately (1 entry): an empty `argent-chromium-media/` — production's fixed staging dir (`src/chromium-server/cdp-session.ts:218`), shared with any live chromium-server on the machine. A test `rm`ing it would be the machine-global-resource defect #714 removed elsewhere. Two further shapes turned up while checking that claim, both production dirs a test drove and then abandoned: \| site \| per run \| why the test could not just `rm` it \| \|---\|---\|---\| \| `screenshot-diff-tool.test.ts` \| 2 dirs + 2 PNGs \| `resolveOutputDir` mints `argent-screenshot-diff/<hex>` before anything validates the call, so the path exists before the test can learn it — and on a throw it never learns it at all \| \| `flows/flow-visual.test.ts` \| 5 dirs + 5 PNGs \| `runSnapshot` leaves the file it registered as an artifact inside its `argent-flow-crop-` / `argent-flow-diff-` scratch dir; from there the dir belongs to whoever consumes the artifact, which here is the test \| Both are fixed the same way, with `redirectTmpdir` (already used by seven other test files): point `os.tmpdir()` at a directory the test already owns and removes, so whatever production mints lands inside it whether the call returns or throws. The `<hex>` and `mkdtemp` suffixes are per-call and never shared, so nothing a concurrent tool-server owns is touched — verified with a decoy file planted under the shared root, which survives the run. Measured under a scoped `TMPDIR`, three consecutive runs each: `screenshot-diff-tool` 6 dirs + 6 PNGs → 0 + 0; `flow-visual` 15 + 15 → 0 + 0. The underlying production leak behind the first is still #790. `argent-mcp` is now clean end to end: every `mkdtemp` in the package has a matching `rm`. </details> <details> <summary>Checks</summary> Run from the worktree root, exit codes read unpiped: - `npm run format` → 0, no files rewritten - `npx tsc --noEmit -p packages/tool-server/tsconfig.test.json` → 0 - `npx tsc --noEmit -p packages/argent-mcp/tsconfig.test.json` → 0 - `npm run lint` → 0 - `npm run knip` → 0 - no `it.only` / `describe.only` in the diff Suites: `argent-mcp` 90 passed; `tool-server` run as six shards under a scoped `TMPDIR`. `lens-tools-platform-gate.test.ts` times out at the 5 s default under full-suite load and passes in isolation (`2 passed`, 3.14 s). It is not in this diff and #714 recorded the same behaviour before it. </details> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Tests - Improved test isolation by consistently cleaning up temporary files and directories after each test. - Added coverage verifying generated artifacts are stored under the configured artifacts directory. - Ensured screenshot, upload, event-log, profiling, visual-flow, and diff-test artifacts are removed reliably after execution. - Standardized temporary-directory handling and cleanup across test scenarios to reduce leftover test data and improve repeatability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-09 12:39:02 +02:00
Ignacy Łątka	86a2668fb0	test: stop the suite and the process environment leaking into each other (#1058 ) Test env leakage, both directions. Out — six `argent-tools-client` suites pointed `HOME`/`USERPROFILE` at a temp dir, deleted the dir, and never restored, so a `--no-isolate` run handed every later file a home that no longer exists. Lifted the redirect into a shared `test/helpers/home-redirect.ts`; the three suites that already hand-rolled the save/restore moved onto it too. In — `packages/telemetry` had no `setupFiles` at all, so its unit tests read the developer's shell. Added `test/setup/clear-telemetry-env.ts` (ARGENT_ prefix sweep + `DO_NOT_TRACK` + the cloud-agent detector's markers), wired via `setupFiles`, following the `@argent/tool-server` precedent — and dropped the per-file guards it replaces. Pinned, not just fixed — a `test/setup/assert-env-restored.ts` in each of the three packages fails any suite that leaves a watched variable changed, so dropping a restore can no longer pass. Each watches `HOME` and `USERPROFILE`, plus `PATH` (`argent-tools-client`, `tool-server`) or `CI` and the five `OTEL_EXPORTER_OTLP_` (`telemetry`) — the ones the sweep deliberately leaves ambient and individual tests then set by hand. Each of the three `clear--env` setup files has a pin test that re-imports it against planted sentinels; `packages/tool-server`'s existing copy gains the mixed-case one it was missing. `packages/tool-server` is in the diff because it has the same two gaps: it originated the prefix sweep (both pre-existing copies were case-sensitive, so they are fixed together) and it has the largest redirect surface of the three — 12 files hand-rolling the redirect plus 17 going through `scopeTempHome` — with no guard. It is already green under one; deleting `scopeTempHome`'s restore turns 17 files red. Two of its suites restored `HOME` by assigning back a saved value that can be `undefined`, writing the string `"undefined"`; the guard caught both on a host with no ambient `HOME`, and they are fixed here. `windows-e2e.yml` runs seven `tool-server` files through the config this branch edits, but neither that config nor `test/setup/*` was in its `paths:` filter — so the new setup file would have reached windows-latest unverified. Added both. No docs change: test infrastructure only, no user-facing behaviour. Fixes #1028 Fixes #839 Same-class problems found while reviewing this and filed rather than folded in: #1068 (`argent-cli` fails under `DO_NOT_TRACK=1`), #1069 (the `ios-device-runner` suites pin only `HOME`, so the redirect is inert on Windows — its second half, the restore that wrote the string `"undefined"`, is fixed here because the new guard caught it), #1070 (`useTempHome` shares one saved-`HOME` slot, so a nested `scopeHome()` restores a deleted dir), #1071 (`tool-server`'s sweep is prefix-only, so an exported `ADB_SERVER_SOCKET` reddens its suite), #1072 (`external-devices.test.ts` reads the developer's real `~/.argent/providers`) and #1073 (`flow-script-environment.test.ts` fails with no ambient `HOME`). <details> <summary>#1028 — probe results, six files</summary> `test/zz-home-probe.test.ts` from the issue body, run per suite. Vitest's sequencer orders by cached duration and falls back to file size, so the cache is cleared before each run to keep the tiny probe last: ``` rm -rf node_modules/.vite/vitest ../../node_modules/.vite/vitest npx vitest run test/<suite>.test.ts test/zz-home-probe.test.ts --no-isolate --no-file-parallelism ``` \| suite \| before \| after \| \| --- \| --- \| --- \| \| `launcher-state` \| 1 failed \\| 23 passed — `argent-state-test-… exists=false` \| 24 passed \| \| `launcher-spawn` \| 1 failed \\| 8 passed — `argent-spawn-test-… exists=false` \| 9 passed \| \| `launcher-version-gate` \| 1 failed \\| 7 passed — `argent-version-gate-test-… exists=false` \| 8 passed \| \| `launcher-duplicate-spawn` \| 1 failed \\| 10 passed — `argent-dup-test-… exists=false` \| 11 passed \| \| `launcher-exit` \| 1 failed \\| 1 passed — `argent-exit-test-… exists=false` \| 2 passed \| \| `link-config` \| 1 failed \\| 43 passed — `argent-link-config-test-… exists=false` \| 44 passed \| \| `launcher-sweep` (control, fixed in #713) \| passed \| passed \| \| `launcher-ps-guard` (control, fixed in #713) \| passed \| passed \| The probe itself is a diagnostic and is not part of the diff — `test/setup/assert-env-restored.ts` is the durable guard, and it needs no `--no-isolate` to fire. </details> <details> <summary>#839 — the stated repro is stale; here is what is actually left</summary> The issue was filed 2026-08-18. #714 landed 2026-08-19 and hand-enumerated the variables inside `index.test.ts` (a `CONSENT_ENV_KEYS` `beforeEach`) and `base-props.test.ts` (eight `delete`s). On `aa90873b5` all three named repros are green: ``` $ DO_NOT_TRACK=1 npx vitest run --root packages/telemetry → 319 passed $ ARGENT_TELEMETRY=0 npx vitest run --root packages/telemetry → 319 passed $ CURSOR_AGENT_WORKER_ID=x npx vitest run --root packages/telemetry → 319 passed ``` What remains is the structural gap the issue names: the protection is per-file and per-variable, nothing pins it, and `vitest.config.ts` still has no `setupFiles`. This PR moves the property to the setup file and deletes the two hand-rolled guards, so the setup file is what holds it — see the mutation section, where removing the `setupFiles` line reproduces the issue's 17 and 1 exactly. Post-change sweep, one hostile value per run, 329 tests each: `DO_NOT_TRACK=1`, `ARGENT_TELEMETRY=0`, `ARGENT_TELEMETRY_DEBUG=1`, `CLAUDE_CODE_ENVIRONMENT_KIND=byoc`, `CLAUDE_CODE_ENTRYPOINT=remote`, `CLAUDE_CODE_REMOTE_SESSION_ID=x`, `CURSOR_AGENT_WORKER_ID=x`, `CURSOR_WORKER_POOL_NAME=x`, `GITHUB_ACTIONS=true`, `GITHUB_ACTOR=copilot`, `GITHUB_WORKFLOW_REF=…copilot-swe-agent.yml@refs/heads/main`, `REPLIT_AGENT=1`, `ARGENT_HOST`/`ARGENT_PORT`/`ARGENT_AUTH_TOKEN`, `CI=1`, `CI=false`, `OTEL_EXPORTER_OTLP_ENDPOINT`, `OTEL_EXPORTER_OTLP_HEADERS`, the real CI combination, and all eleven at once — 329 passed every time. `GITHUB_ACTIONS` is a ci-info vendor input as well as a copilot-agent marker, so clearing it does move `isCi()` — but only its GitHub Actions branch, and an Actions run also sets `CI`, which `isCi()` answers ahead of any vendor check. `GITHUB_ACTOR` and `GITHUB_WORKFLOW_REF` appear in neither `ci-info/vendors.json` nor `ci-detect.ts`'s generic list, so they cost nothing. The rest of the ci-info surface and `OTEL_` are deliberately left alone; the setup file says so. </details> <details> <summary>Mutation testing — every guard, applied by hand, verified to fail, reverted</summary> \| mutation \| result \| \| --- \| --- \| \| drop `setupFiles` from `packages/telemetry/vitest.config.ts` \| `DO_NOT_TRACK=1` → 19 failed (17 in `index.test.ts`, matching the issue, + the two registration pins) \| \| drop `CURSOR_WORKER_POOL_NAME` from `CLEARED_ENV_VARS` \| drift test red \| \| move an `env.` read into `src/detect/vendor.ts` \| drift test red, naming `detect/vendor.ts` \| \| rewrite a read as `env["X"]`, `const { X } = env`, `const { X }: NodeJS.ProcessEnv = env` or `const { X = "d" } = env` \| drift test red \| \| add `cloud-agent-detect.ts` to `UNSWEPT_SRC_FILES` \| drift test red \| \| revert `toUpperCase()` in any of the three sweeps \| that package's mixed-case case red \| \| drop `restoreHome()` from one suite / from all nine \| 1 file / 9 files red, each naming its deleted dir \| \| drop `scopeTempHome()`'s restore in `tool-server` \| 17 files red \| \| drop the hand-rolled `CI` restore in `base-props.test.ts` \| red, naming that file (green before; under `--no-isolate`, 7 unrelated failures in `otel-endpoint.test.ts`) \| \| narrow either guard's filter to variables that were already set \| `assert-env-restored.test.ts` red \| \| replace the leaked half of the guard's message with a constant \| `assert-env-restored.test.ts` red \| \| rename one function's `env` parameter in `cloud-agent-detect.ts` \| drift test red — the read is found under the new binding \| \| narrow any guard's filter to a subset of `SCOPED_ENV_VARS` \| `assert-env-restored.test.ts` red — the pin plants every entry \| \| drop the hand-rolled OTLP restore in `otel-endpoint-live.test.ts` \| red, naming all four leaked variables \| \| move the guard's `AMBIENT` snapshot inside `assertEnvRestored` \| `assert-env-restored.test.ts` red in all three packages \| \| delete the guard's `afterAll(...)` registration \| `assert-env-restored.test.ts` red in all three packages \| \| drop `restoreHome()` from `artifacts.test.ts`'s two `afterEach` \| red — the file-level guard catches a per-test omission \| \| drop ps-guard's `PATH` restore \| red, naming the file \| \| empty `leakedEnvVars`, or drop a name from `SCOPED_ENV_VARS` \| `assert-env-restored.test.ts` red \| \| replace the helper's delete branch with a plain assignment \| `home-redirect.test.ts` red \| \| drop `USERPROFILE` from the helper \| `home-redirect.test.ts` red \| </details> <details> <summary>Local CI parity</summary> ``` npx vitest run --root packages/argent-tools-client 213 passed \| 1 skipped npx vitest run --root packages/argent-tools-client --no-isolate 213 passed \| 1 skipped npx vitest run --root packages/telemetry 329 passed npx vitest run --root packages/telemetry --no-isolate 329 passed npx vitest run --root packages/tool-server 6166 passed \| 1 skipped npx tsc --noEmit -p packages/{argent-tools-client,telemetry,tool-server}/tsconfig.test.json clean npm run format / npm run lint / npm run knip clean ``` </details> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Tests - Improved test isolation by consistently restoring temporary environment settings after test runs. - Added safeguards that detect leaked environment settings and flag incomplete cleanup. - Added coverage to ensure telemetry-related environment variables are cleared before tests and do not affect results. - Added cross-platform checks for environment variable handling, including mixed-case names. - Expanded validation for home-directory redirection and restoration behavior. - Added checks confirming environment cleanup runs in the correct order across test suites. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-09 12:38:51 +02:00
Ignacy Łątka	5bc9c7be4c	test: make four non-discriminating test sites fail on what they claim to pin (#1059 ) Four tests could not fail on the thing they name: an assertion implied by the one below it, an assertion satisfied by the fixture's own filename, a clearance poll with nothing to clear, and `it.each` rows whose rendered name described only argv[0] - two of them stating the opposite of what the row asserts. Each site now fails when its claim is broken, proven by mutation. Tests only, no source change, so no docs update. Fixes #943 Fixes #942 Fixes #841 Fixes #861 <details> <summary>#943 — capability: toolId/platform were substrings of the path assertion</summary> `toContain("demo-tool")` + `toContain("android")` are both substrings of `tools/demo-tool/platforms/android.ts`, asserted on the next line. Replaced by the prose sentence, which names both outside the path. Mutation - drop the sentence naming the tool and the platform: ```diff - `Tool '${opts.toolId}' is not yet implemented on ${opts.platform}. ` + + `Tool is not yet implemented. ` + ``` \| \| result \| \|---\|---\| \| before \| `Tests 7 passed (7)` - green \| \| after \| `AssertionError: expected 'Tool is not yet implemented. The cros…' to contain 'Tool \'demo-tool\' is not yet impleme…'` \| The sibling "works without a hint" assertions are load-bearing and untouched. </details> <details> <summary>#942 — native-profiler: the wording assertion matched inside missing_cpu.xml</summary> `/missing\|not found\|unreadable/i` matched the fixture filename, so any product copy passed. The wording now has to land before the backticked path (a negated character class cannot cross into it), which no path can satisfy. Mutation - reword the ENOENT copy: ```diff - if (code === "ENOENT") return `not found at \`${filePath}\``; + if (code === "ENOENT") return `absent at \`${filePath}\``; ``` \| \| result \| \|---\|---\| \| before \| `Tests 1 passed (1)` - green (also green with the state word deleted outright) \| \| after \| `AssertionError: expected '# iOS Instruments Analysis…' to match /-\s\\cpu\\:[^…](?:missing\\|not found\\|unreadable)/i` \| </details> <details> <summary>#841 — vega-cli: the output-cap reaps had nothing to observe</summary> `expect(await waitForClear(SENTINEL_OVERFLOW)).toBe(0)` reads 0 just as readily for a launcher that never spawned a worker. The cap trips on the first write, so there was no window to see one in: the fake now spawns its sentinel worker before the flood and holds the flood - and so the reap - until the test writes a gate file, having seen the worker. Both the stdout and the stderr twin. Gated rather than timed, so the observation is decided, not raced. Cost is ~+110ms and ~+50ms on the two tests (165/177ms to 278/226ms). Mutation - the flood branches' workers stop carrying the sentinel, reap path untouched: ```diff - const worker = spawn("sleep", [secs], { stdio: "ignore" }); + const worker = spawn("sleep", ["600.0"], { stdio: "ignore" }); ... - spawnSync("sleep", [secs]); + spawnSync("sleep", ["600.0"]); ``` \| \| result \| \|---\|---\| \| before \| `Tests 12 passed (12)` - green, and `pgrep -f '^sleep 600\.0$'` empty (the reap works; nothing observed it) \| \| after \| both overflow tests `AssertionError: expected +0 to be 1` \| Cross-check that the pair pins the production reap: with `reapOnce()` made a no-op in `vega-cli.ts`, both fail `expected 2 to be +0`. Flake runs: 30 consecutive passes (2 x 15, before and after the comment pass), `12 passed (12)` every time. </details> <details> <summary>#861 — four it.each rows named only argv[0] (plus a fifth site)</summary> A single `%j` against a multi-element row renders only the first element. Each row is now one argv, nested, so `%j` names all of it. `packages/argent-cli/test/tools-help.test.ts` ``` - prints usage for "--help" without contacting the tool-server <- duplicate - prints usage for "--json" without contacting the tool-server <- false: a bare --json prints no usage and does hit the server + prints usage for ["--help","--json"] without contacting the tool-server + prints usage for ["--json","--help"] without contacting the tool-server ``` `packages/argent/test/installer-help.test.ts` ``` - treats "--foo" as a help request <- rendered twice; a bare --foo is the opposite + treats ["--foo","--help"] as a help request + treats ["--foo","help"] as a help request - does not treat "--metro-port" as a help request + does not treat ["--metro-port","8082"] as a help request ``` Swept the class across every `it.each` array-literal table in the repo (154 sites). One more reproduces it - `packages/argent-cli/test/telemetry-command.test.ts`, fixed here: ``` - prints usage for status and writes nothing <- rendered twice - prints usage for disable and writes nothing <- false: a bare `disable` writes config + prints usage for ["status","--help"] and writes nothing + prints usage for ["status","-h"] and writes nothing + prints usage for ["disable","-h"] and writes nothing ``` Left alone: ~64 sites with a discriminating label column (`_label` first, house style) and 16 whose first column is a real discriminator - names unique and truthful in both groups. `providers-command.test.ts:440` drops a third "shape" column from its name but stays unique and true, so it is a legibility nit, not this defect. </details> <details> <summary>Checks run</summary> - `npm run format`, `npm run lint` (the latter needs `npm ci` inside `packages/docs` locally) - `npx tsc --noEmit -p packages/{tool-server,argent-cli,argent}/tsconfig.test.json` - `packages/tool-server`: `npx vitest run --shard=1/6` .. `6/6` - 6163 passed, 1 skipped - `npx vitest run --root packages/argent-cli` (619), `--root packages/argent` (80) - `vega-cli-timeout.test.ts` x30 consecutive, zero flakes </details> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Tests * Improved test parameterization so command-line help cases display complete argument lists accurately. * Strengthened error-message and profiler warning assertions to verify exact wording and placement. * Improved subprocess timeout and output-limit tests by synchronizing worker startup and cleanup checks. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-09 12:38:37 +02:00
Ignacy Łątka	c3bb2990ea	test(flows): stop pinning idle and type-focus waits to exact wall-clock boundaries (#1066 ) Real-timer assertions in the flows suite were pinned to exact wall-clock boundaries, so a batch run reddened them for reasons unrelated to whatever was being tested. Each now pins the same property with slack, or with no clock at all — every one was mutation-tested to confirm it still fails loudly when the behaviour it guards is removed. Test-only; no docs change. Fixes #1000 Fixes #795 <details> <summary>Per-assertion: what it pins, and what changed</summary> \| Site \| Pinned property \| Before \| After \| \| --- \| --- \| --- \| --- \| \| `flow-type-focus` "skips the focus poll…" \| the fixed 500ms settle still runs on a source that cannot report focus \| `gap >= 500` \| `gap >= 450` \| \| `flow-type-focus` "waits for the tapped field…" \| settle plus at least one 300ms focus poll \| `gap >= 800` \| `gap >= 720` \| \| `flow-idle-run` "holds a still screen…" \| `stableFor` is served, not ignored \| run timed, `[750, 2400)` \| span between first and last tree read, `>= 750` \| \| `flow-idle-run` "settles inside the smallest timeout…" \| the hold is counted across the polls, so the parser's floor is servable \| run timed, `[750, 1000)` \| read span `>= 750`; "inside the wait" carried by `warning === undefined` \| \| `flow-idle-run` "does not call a slow but answering source a wedged one" \| an abandoned read is not a wedge when the source's own slowest answer explains it \| 2300ms reads / 4600ms wait, 5s default test timeout \| 2800ms / 5400ms, own 20s timeout \| \| `flow-idle-run` "does not report a tree-only settle…" \| the tree-only settle owes the same hold \| churn window clocked from before the run \| clocked from the first read \| \| `flow-idle-run` H1/H2 "settled early and then moved / went blank" \| the tree-only verdict is not a write-once latch \| 4 quiet reads in a 2500ms wait \| 3 quiet reads in a 6000ms wait, reads asserted \| The two focus bounds had zero slack in the one direction that matters: a 500ms `setTimeout` measured on `Date.now()` spans 499ms roughly once in 200 runs (400 samples, idle machine: `499ms x2, 500ms x48, 501ms x289, 502ms x55, 503ms x2, 505ms x1`). 10% of slack keeps them failing by hundreds of milliseconds if either wait is dropped. The two idle `elapsed` bounds timed the whole run, so the flow's start-up — parse, device resolve — counted against the step's own budget. Timing between the reads the hold is actually measured across removes that, and the upper bounds went with it: every way out of the wait except the settle carries a warning, so `warning === undefined` already says it landed inside. The remaining three are fixtures, not assertions: each assumed a number of poll rounds, or a wall-clock window opened before the run, that a batch run does not serve. They now count reads, or anchor the window on the first read. </details> <details> <summary>Repro — before</summary> `npx vitest run packages/tool-server/test/flows` on `origin/main`: ``` FAIL flow-idle-run.test.ts > settles inside the smallest timeout the parser allows for the hold AssertionError: expected 2439 to be less than 1000 FAIL flow-idle-run.test.ts > does not call a slow but answering source a wedged one Error: Test timed out in 5000ms. FAIL flow-idle-run.test.ts > does not report a tree-only settle when the hold was never served AssertionError: expected 'settled on the UI tree alone — this s…' not to contain 'UI tree alone' FAIL flow-idle-run.test.ts > does not pass on a screen that settled early and then started moving again AssertionError: expected 'settled on the UI tree alone — this s…' to contain 'never held still' ``` #1000's own intermittency did not surface in 10 rounds of `npx vitest run --shard=3/3` (4 idle, 6 under a 10-way CPU load), so it was forced instead: the `setTimeout`-vs-`Date.now()` measurement above is the whole mechanism, and the assertion had no margin over it. </details> <details> <summary>Repro — after</summary> Six batch rounds of `npx vitest run packages/tool-server/test/flows`, on a machine carrying other work (load average 22-30 throughout). Rounds 2-6 were additionally narrowed to `--maxWorkers=3`, which is harsher than CI: the 13 `flow-composition` cases of #1067 pin one of the three workers for ~13 minutes and starve the rest. \| round \| workers \| result for the assertions this PR touches \| \| --- \| --- \| --- \| \| 1 \| default \| all clean \| \| 2 \| 3 \| H1 starved to 3 reads \| \| 3 \| 3 \| all clean \| \| 4 \| 3 \| all clean \| \| 5 \| 3 \| all clean \| \| 6 \| 3 \| H2 starved to 3 reads \| Every other failure in those rounds was the same 13 `flow-composition` timeouts (#1067), plus load artifacts in `flow-selector-scopes`, `flow-tool-step-tree-target-e2e`, `flow-launch-pins-tree-target` and `flow-gesture-settle` — none of whose assertions this branch touches. The two starved rounds were a real gap, not noise: H1 and H2 each need a fourth read to reach a verdict at all, and a 3500ms wait serves eighteen on an idle machine but three on a starved one. The case then saw a screen that never moved and passed the wait cleanly, so the assertion ran against `undefined`: ``` AssertionError: the given combination of arguments (undefined and string) is invalid for this assertion. ``` Each case now budgets for the starved run and asserts the reads it was served, so an environment that cannot reach the verdict says so: ``` AssertionError: reads served — under four, nothing moved to be judged: expected 3 to be greater than or equal to 4 ``` Confirmed by stretching each read to 1800ms, which starves the wait to three reads without touching the wait itself. </details> <details> <summary>Mutation tests (source broken, assertion must fail)</summary> Each mutation was applied to `flow-actions.ts`, the test run, then reverted. \| Mutation \| Assertion under test \| Result \| \| --- \| --- \| --- \| \| delete `sleepOrAbort(TYPE_FOCUS_SETTLE_MS, …)` \| both focus gaps \| FAIL — `expected 0 to be greater than or equal to 450`; `expected 303 to be greater than or equal to 720` \| \| focus poll sleeps `0` instead of `POLL_INTERVAL_MS` \| `>= 720` \| FAIL — `expected 504 to be greater than or equal to 720` \| \| drop `&& heldForMs >= stableFor` from the settle \| both idle read spans \| FAIL — `expected 403 to be greater than or equal to 750` (both) \| \| drop `slowestAnsweredReadMs +` from the hung check \| "slow but answering" \| FAIL — step errors, `expected false to be true` on `r.ok` \| \| drop `&& now - treeSince >= stableFor` from `treeSettledAtLastRead` \| "tree-only settle" \| FAIL — `expected 'settled on the UI tree alone…' not to contain 'UI tree alone'` \| \| `if (lastRead === "timeout")` without `&& treeReadHung` \| "still warns, rather than erroring" \| FAIL — `expected 'error' to be 'pass'` \| \| make `treeSettledAtLastRead` a write-once latch \| H1 "settled early then moved again" \| FAIL — reports the tree-only settle \| \| drop `treeSettledAtLastRead = false` from the blank-read branch \| H2 "settled early then went blank" \| FAIL — reports the tree-only settle \| The `treeReadHung` one did not fail at `timeout: 3000`: with the round period dividing the wait exactly, drift decided whether the closing read was abandoned at all. `3200` leaves the seventh read 200ms — less than the source needs — so the case ends on an abandoned read by arithmetic. </details> <details> <summary>Why the latch waits stop at 6000ms, and H3 is untouched</summary> The first attempt at these two used a 12000ms wait, which passed CI and broke the budget-tail case below them in the same file — that one has to fit three reads into a 2000ms window, and with 24s of polling ahead of it it saw two: ``` expected 'the screen came back with content on 2 reads in 2000ms…' to contain 'never held still' ``` Deterministic, not load: 4/4 runs on the branch, 0/4 on `origin/main`, and it passes in isolation. Bisected across the file: \| latch wait \| whole file \| \| --- \| --- \| \| 2500 (main) \| 46 passed \| \| 3500 \| 46 passed \| \| 6000 \| 46 passed \| \| 12000 \| budget-tail fails every run \| So the latch cases take 6000ms and no more. H3 itself is left exactly as it is on `main`: widening its wait scores its abandoned closing read as a wedged source (`HUNG_TREE_READ_MS`) rather than a step that ran out of time, and slowing its source starves it below the three reads a verdict takes — both were tried against CI and both moved the failure rather than removing it. </details> <details> <summary>Deliberately not changed</summary> - `flow-idle-run`'s `elapsed < 3000` bounds at "honours its timeout", "rides over a dropped capture" and "stops on abort": 1.8–2.2s of slack over a sub-second expectation already clears a scheduling delay of hundreds of milliseconds. - `flow-composition.test.ts` times out 13 cases at 60s each in the same batch run, on `origin/main` as well as here, while passing in 27s on its own. It is a batch artifact of a different kind and belongs in its own issue, not papered over from here. </details> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Tests * Improved idle-flow timing validation by measuring intervals between reads for more reliable settle detection. * Updated timing scenarios and thresholds to account for delayed reads, startup overhead, slow tree changes, and early-settle behavior. * Added checks to ensure sufficient reads occur before slow-flow warnings and to detect later movement or blanking. * Relaxed Android and iOS focus-wait timing thresholds to accommodate minor timer measurement jitter. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-09 12:36:01 +02:00
filip131311	aa90873b59	feat(android): read the web DOM inside a WebView in `describe` (#1052 ) Android `describe` rendered an `android.webkit.WebView` as a single opaque leaf labelled `[web-view]` and threw the subtree away. Chromium publishes the web DOM to the accessibility tree on both capture paths (the android-devtools helper and the `uiautomator dump` fallback), so the sentinel hid content the agent can act on. This keeps the WebView as a landmark and lets the normal trim run over the DOM beneath it. Supersedes #955, which reached the same diagnosis. The parser rules for the doubled in-app node, the web list clip and the text-run role are carried over from it; the rendered-output cap, the `truncated` flag plumbing and the flow-tree parity rework are not, so this stays confined to the Android parser and adapter (about 120 source lines) with no renderer, contract, skill or tool-description changes. ### What changes inside a WebView, and only there - Doubled node merged. An in-app WebView dumps as a nested pair: the app's view (resource-id, gesture flags), then Chromium's root web area under the same class carrying the page `<title>` as text. The inner half folds into the outer node, so the tree reads `WebView "<title>"`. A Chrome tab arrives as one node and is untouched. - Web list clip. A `<ul>` arrives as `android.widget.ListView` with `scrollable="false"`. Inside a WebView only the framework flag decides what clips, so a list never drops the items laid out around it, and the role is `List` rather than `ScrollView`. - Text runs. A labelled leaf `android.view.View` with no gesture flag is reported as `StaticText`. Outside a WebView a bare `View` keeps its role (Compose semantics nodes). - Headings survive. The WebView branch returns before the label dedupe, so an `h1` that repeats the page title is kept. ### The cold-read race is fixed, not documented Chromium builds a WebView's accessibility tree on the first request for it. Measured on API 35 / WebView 124: a read taken 6 s after page load still shows the WebView with no children; the page appears only after a read has asked for it — 10–400 ms later for a small in-app page, ~600 ms later for a Chrome tab over a long article. `describeAndroid` re-reads in 250 ms steps within a 1.5 s budget while the tree still has the unread shape, on both paths. A screen without a WebView never pays for it (one call, pinned by test). A WebView that stays empty after the budget is labelled `(no web content exposed)`. The unread shape is an `android.webkit.WebView` with no children, or — for a browser tab, which has no WebView view of its own before the page is published — Chrome's childless content view labelled "Web View" (English UI only; other locales see the tab the way they do today). ### Measured on a Pixel 9 / API 35 emulator with the branch tool-server \| Screen \| Before \| After \| \|---\|---\|---\| \| In-app checkout form, cold launch \| `WebView "[web-view] (no label)"` \| full form in one call, 1.2 KB, 0.6 s \| \| 120-paragraph page \| sentinel \| the ten visible paragraphs, 1.8 KB (existing scroll clip bounds it) \| \| Wikipedia article in Chrome, helper attached after the page loaded \| `FrameLayout "Web View"` \| 4.7 KB, heading, links, lists, 1.4 s \| \| Settings home (native) \| — \| byte-identical to the previous parser \| ### Testing - New `android-webview-describe.test.ts` on three live captures checked in as fixtures (in-app, Chrome, cold): merged landmark, `List` role, heading kept, re-read fires on the cold in-app shape and on Chrome's content-view placeholder, never on a native screen, and gives up within the budget. - `uiautomator-parser-v2-trim.test.ts`: the opaque-leaf case is replaced by the merge, the empty-WebView leaf, and a native `View` that must not be remapped. - Full tool-server suite, `tsc`, eslint, prettier, docs build. ### Docs No docs, skill or tool-description change. The page already names the accessibility tree as the source on Android; WebView content is now part of that tree and the agent needs no new instruction, since the cold read no longer produces an empty WebView. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01GZbkzeVC9faSxTFi4DbMuT <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * New Features * Android WebView content is now captured and presented with meaningful controls, text, lists, and links. * Web content is consolidated into a clearer, unified accessibility view. * Bug Fixes * Improved Android screen descriptions when WebView content is initially unavailable by retrying the accessibility hierarchy. * Added safeguards for WebViews that never expose content, including a clear fallback message. * Tests * Expanded coverage for in-app WebViews, Chrome tabs, cold captures, and native Android UI filtering. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Hubert Gancarczyk <hubert.gancarczyk@swmansion.com>	2026-09-07 16:35:42 +02:00
Hubert Gancarczyk	236db0077f	fix(tool-server): support ios-remote in the await tools (#1051 ) ## Problem The `await-ui-element` and `await-screen-idle` tools refused a remote simulator. The tool server sent this error: ``` Tool 'await-ui-element' is not supported on ios-remote simulator (no ios-remote support declared). ``` Both tools poll the same accessibility tree as `describe`. The `describe` tool reads that tree correctly on a remote simulator. Therefore the tree source was never the problem. ## Cause Each tool declares its platform support in a capability record. The `describe` tool declares `appleRemote`. The two wait tools did not declare it. The HTTP layer rejects the request before the tool starts. Three more branches test `platform === "ios"`. That test is false for a remote device. These branches select the tree source, the dependency preflight, and the tvOS probe. Nobody excluded the platform deliberately. Git history has no commit that adds `appleRemote` to either file. The `appleRemote` concept arrived on 2026-07-01. The `await-screen-idle` tool arrived two days later. ## Effect on an agent session The MCP layer calls `await-screen-idle` before each automatic screenshot. On a remote simulator that call failed. The fallback then slept for the full budget: - 1500 ms for each tap or gesture - 2000 ms for `open-url` - 3000 ms for `launch-app` and `restart-app` - 15000 ms for `run-sequence` The user saw no error. The fallback exists for an old tool server, not for a current server that refuses a device. ## Change This change adds three lines to each tool: 1. The capability record declares `appleRemote: { simulator: true }`. 2. The tree fetch sends a remote device to `describeIos`. Before, a remote device fell through to the Chromium branch. 3. The dependency preflight requires `sim-remote`, and not `xcrun`. The `describe` tool uses the same three settings. The ax-service blueprint sends the accessibility read through the sim-remote tunnel. ## Test result I ran the branch tool server against a leased Argent Cloud runner. The device was a remote iPhone 17 Pro on iOS 26.5. The app was Bluesky. The app read its bundle from a local Metro server through the sim-remote tunnel. \| ID \| Case \| Result \| \| --- \| --- \| --- \| \| T1 \| `visible` by text \| `success: true` \| \| T2 \| `exists` by identifier \| `success: true` \| \| T3 \| `exists` by a second identifier \| `success: true` \| \| T4 \| `text` condition matches the label \| `success: true` \| \| T5 \| compound role and text selector \| `success: true` \| \| T6 \| absent text gives `unmet` \| `success: false` \| \| T7 \| wrong expected text gives `unmet` \| note quotes the actual text \| \| T8 \| 5000 ms budget honoured \| `elapsed: 5003` \| \| T9 \| `hidden` on an absent element \| `success: true` \| \| T10 \| screen settles \| `settled: true, polls: 2` \| \| T11 \| screen honours `minStableMs: 2000` \| `waitedMs: 2265, polls: 8` \| \| T12 \| wait across an app restart \| `success: true, elapsed: 27301` \| \| T13 \| `hidden` after the restart \| `success: true` \| \| T14 \| flow record boundary \| UNKNOWN verdict \| All 14 cases passed. Two results give the strongest evidence: - Case T12 blocked for 27.3 seconds across an app restart. The app rebuilt its bundle, the element appeared, and the tool returned success. - Case T6 returned `cause: "unmet"`. A failed tree read returns `cause: "unreadable"`. Therefore the tool read the remote tree and judged the condition. ### Unit tests Five new unit tests drive both tools with a remote udid. They use a mock accessibility service. Each test fails against `main` and passes with this change. Local checks: - `npm run build`: clean - `npm test -w @argent/tool-server`: 431 files, 6153 tests, 0 failures - `npm run lint`: clean - `npx prettier --check`: clean ## Note on flow records The flow tools declare no capability. Therefore they accept a remote device today. This change lets the re-probe reach `fetchTree`, which has no support for `ios-remote`. The recorder catches that error. The recorder then keeps the step and marks the verdict UNKNOWN, not known-bad. An earlier test asserted the refusal. This change inverts that test and records the new behavior in its comment. ## Documentation This change needs no documentation update. The tool reference lists these two tools with a description only. That page has no platform column, and no page describes remote simulators. --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-09-07 14:42:23 +02:00
Hubert Gancarczyk	0f90d182b2	feat(flows): record a script step by running it (#940 ) #864 (the executor) and #865 (the `script:` step) are merged. This branch now sits directly on `main` as a single commit, so the diff and the commit list are this change alone. A `script:` step could only enter a flow by being typed into the YAML after the recording ended. The authoring skill said so in as many words, and listed it among the legal unrecorded insertions. That made it the one directive exempt from every rule the recorder exists to enforce, and it cost two things: - The script never ran while the flow was authored. It first executed during the closing replay, where a failure reads as a flow that was never right rather than a script that needs one more attempt. Worse, the device steps recorded after it were walked against backend state the author had established by hand — so the recording proved nothing about whether the script establishes that same state. - The author never saw what the script returned. A later release reads `{{output:user.id}}`; the only way to know that path exists is to have seen the document. This adds one MCP tool, `flow-add-script`, that runs a local `.mjs` and records the step it ran. No new flow syntax — the `script:` step is unchanged, and this is a second way for one to enter a flow file. ```text flow-add-script { name: "checkout", project_root: "/Users/dev/shop", path: "../../scripts/seed-order.mjs", timeout: 60000 } ``` ## What it adds - One shared execution path. Path resolution, the on-disk casing guard, the executor call and the verdict were private to `flow-run.ts`. They are now `runFlowScriptStep` in `flow-script-step.ts`, with the file-ref helpers in `flow-file-refs.ts`, and `runScriptStep` is that call plus its `ExecState` plumbing. This is the load-bearing change: "the recording runs the script exactly as replay will" is one function, not two code paths that agree today. - A failure records nothing. The opposite of `flow-add-step`, deliberately. A failed script did not establish the state the rest of the walkthrough would then be recorded against, so an appended red step costs every step after it, not just itself. The `message` also says whether a child actually ran — a path that resolved to no file forked nothing, while a script that threw halfway did not roll back — because the author's next move is a retry or a cleanup and they cannot pick without knowing. - The returned document, as JSON text. Text rather than an object because this is the first tool result carrying bytes the server did not author: every result is deep-walked for `__argentClientFile` and `__argentArtifact` directives, which match on shape alone and write or fetch files on the agent's machine. As a string it inherits none of that. Bounded at 64 KiB with `outputTruncated`, for the reason `log` is bounded. - `{{output:` refused by `parseFlow`. Surfacing the document invites an author to write a reference that has no meaning yet — it would be printed literally by a step that passes, green and wrong. The scan runs over exactly the field list the output release declares supported, so it is a seam that release converts from reject to resolve rather than scaffolding it deletes. Patterns are excluded: a `{{` in a regular expression is a quantifier the author meant. - Two boundaries. A `client`-mode recording is refused — its `.mjs` never left the agent's machine, the same anchor reason `run:`, `snapshot` and uploaded flows are refused. And the tool is `longRunning`, so the MCP adapter's 30s fetch abort cannot retry a call whose whole purpose is a side effect; the comment records what that flag does not close, and where the rest belongs. - `flow-add-step` says when the call already ran. The `{{output:` refusal is the one refusal this release adds on a path where the tool call has already fired, and its message asks for an edit-and-retry — which repeats the device action. Verified: a `keyboard` step recorded twice left `{{output:code}}{{output:code}}` in the field. That one refusal now carries what already happened. ## Documentation Rule 5 of `argent-create-flow` no longer lists `script:` as a legal unrecorded insertion — leaving it there would tell agents to keep doing the thing this PR removes. `live-authoring.md` gains the recorder contract, the return fields that are not self-evident, and the ordering rule: a setup script is recorded before the `restart-app` it prepares state for, because that is where it runs at replay. A leading script does not make the flow a fragment. `qa-flows` and `reliability-and-recovery` follow the count and the wording. ## Verification `npm run build`, `npx eslint . --max-warnings 0`, `npx prettier --check` over tracked files, and `npm run typecheck:tests -w @argent/tool-server` are green. `npm test -w @argent/tool-server`: 358 files, 4657 passed, 1 skipped. Exercised end to end against a live tool server built from this branch, not only in tests — recorded a flow, then replayed the saved YAML through `argent flow run` with no hand editing: ``` flow-add-script → status "pass", durationMs 124 log: "seeding order for the flow\nwarn: using the fake backend\n" outputJson: {"order":{"id":"ord_4417","total":39.5}, "user":{"id":"usr_9","email":"buyer@example.com"}} recorded: 1. script: ../../scripts/seed-order.mjs (timeout 30000ms) ``` ```yaml steps: - script: path: ../../scripts/seed-order.mjs timeout: 30000 - echo: order seeded; expect Home ``` ``` Flow "checkout" ✓ 1 script ../../scripts/seed-order.mjs │ seeding order for the flow │ warn: using the fake backend › order seeded; expect Home PASS — 1 passed, 0 failed, 0 errored, 0 skipped ``` The three refusals, live on the same recording: - A script that threw after creating records: `status "fail"`, `stepCount` unchanged at 1, and the message naming the side effects that were not rolled back. - A path resolving to no file: refused before any fork, no `durationMs`, and "Nothing ran, so there is nothing to clean up". - `flow-add-echo` holding `{{output:user.id}}`: refused with the author-facing message naming the step, the field and the later release. No device is involved at any point — a recording may open with a seeding script before it boots anything. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * New Features * Added the `flow-add-script` tool to run local `.mjs` scripts and record successful executions in flows. * Added script status, output, duration, logs, and failure details to recording results. * Added clearer handling for script cancellation, timeouts, missing files, and path casing. * Documentation * Updated flow guidance to clarify end-to-end classification and script setup or cleanup usage. * Documented script path requirements, failure conditions, and recording workflows. * Bug Fixes * Flows now reject unsupported output-reference templates with more precise diagnostics. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-07 12:59:29 +02:00
Hubert Gancarczyk	9c4bc9ef5b	feat(flows): run an external script as a flow step (#865 ) Stacked on #864, which adds the executor. This branch adds the `script:` step itself: the YAML directive, the runner integration, the CLI and MCP rendering, and the authoring reference. ```yaml - script: { path: ../../scripts/seed-order.mjs } - script: { path: ../../scripts/seed-order.mjs, timeout: 60000 } ``` A `script:` step runs an external `.mjs` file from the project, in a fresh Node process. What the script does is its own business — seeding an order, creating a test account, cleaning up what a run left behind are the reasons flows have wanted one. It drives no device, so a flow of nothing but scripts runs with nothing booted. ## What it adds - The directive. The value is always a map; `path` is required and obeys the same name rules a `run:` target does, spelled for `.mjs`. It resolves against the directory of the flow file containing the step, so a fragment reaches the same script whichever flow composed it. A mis-cased path is refused rather than run, because macOS and Windows would open it and Linux CI would not. - The report. The script's stdout and stderr come back on the step and print under the step line, on a pass as well as a failure — it is the only record of what the step did to the backend. Both renderers (CLI, MCP) carry it, live and buffered. - The fail/error split. A `fail` is the script's own answer; an `error` is the host's. That is what lets CI tell a regression from the machine it ran on. - Two boundaries. An uploaded flow carrying a `script:` step is refused before anything runs — its `.mjs` never left the client. A flow whose script step sits beside a `run:` step still resolves a device. ## Review follow-ups The last three commits answer a review of the three above them. - A launch behind a leading `script:` was not read as the flow's leading launch, so a Chromium flow that seeded a backend first hoisted no boot, bound to whatever browser happened to be up, and passed a launch it never performed. The same lead-in walked a fragment past the `executionPrerequisite` refusal its `echo:`-led twin is given. Both readers now share one exhaustively-switched predicate. - Coverage: the whole failure-kind table, the mis-cased arm CI never ran, the two unreached `scriptFileProblem` arms, symlink resolution for a script path, and the CLI live-progress closure that no test had executed. - Docs: the reference claimed the log was secret-redacted (it is not), never mentioned the environment allowlist, the working directory, the time-limit ceiling or the log caps, stated a fail/error rule that was not the code's, and was missing from its own table of contents. The skills stated a closed list of legal unrecorded insertions that excluded the only way this step can ever be written. ## Verification `npm run build`, `npx eslint . --max-warnings 0`, `npx prettier --check .`, `npm run knip`, `npm run typecheck:scripts` and `npm run test:scripts` are green, as are the three `tsconfig.test.json` typechecks. Suites: tool-server 357 files / 4552 passed, `@argent/cli` 469, `@argent/mcp` 83, `@argent/registry` 90. Reproduced end to end against a real tool server and a real Electron app, not only in tests: a flow of `script:` then `launch:` now boots its own Chromium instance and reports it, where before it attached to a stray browser and reported a green launch for an app path that did not exist. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Added support for local `.mjs` scripts as flow steps, including configurable timeouts, validation, cancellation, and device-free execution. - Script output is displayed in CLI runs and included in MCP flow results, with indentation and truncation notices. - Script failures now provide clearer diagnostics and verdicts. - Flow authoring tools recognize and guide creation of script steps. - Documentation - Added guidance for script syntax, execution limits, output handling, classification, and remote-run restrictions. - Updated flow creation and QA guidance to include script steps. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-09-07 12:08:13 +02:00
filip131311	c573e9c524	feat(telemetry): report device_kind next to platform on tool events (#1049 ) ## Why Telemetry cannot tell a physical iPhone from a simulator: `platform` is `ios` for both, and the tool-server's `kind` never reaches the events. Physical-iOS support shipped in 0.24.0 with no way to count its adoption except through `ios_device_` failure stages on `tool:fail`. Physical Android phones have the same gap. ## What `tool:invoke` / `tool:complete` / `tool:fail` gain `device_kind: simulator \| emulator \| vvd \| device \| app`, derived from the same device id as `platform` (single classifier in `telemetry-platform.ts`, which `debugger:tool_outcome` and `lens:` also go through for their platform). `platform: ios` + `device_kind: device` is a physical iPhone. The kind is emitted only for positively recognised id shapes, so the metric is not polluted by ids that are not devices: \| id shape \| platform \| device_kind \| \|---\|---\|---\| \| simulator UUID / `remote:` \| ios / ios-remote (tvos when the cache is warm) \| simulator \| \| physical iPhone UDID \| ios \| device \| \| `emulator-NNNN` \| android (android-tv when warm) \| emulator \| \| USB serial, 6–20 alnum with a digit and a letter \| android \| device \| \| non-loopback `ip:port` \| android \| device (best effort) \| \| loopback `127.0.0.1:port` / `localhost:port` / `::1:port` \| android \| emulator \| \| anything else non-iOS-shaped (Metro 40-hex handle, `booted`, `Pixel_7`, IPv6, mDNS) \| android \| omitted \| \| `chromium-cdp-<port>` / `amazon-…` \| chromium / vega \| app / vvd \| \| `avdName`-only boot \| android \| omitted \| A child sub-tool that names its own device replaces the parent's platform, kind and provider label wholesale, so an `avdName`-only child never reports the parent's `simulator` next to its own `android`. A logical-keyed Metro session strips the kind. `unknown` (in the registry `DeviceKind`) is rejected by the sanitizer like `platform: "unknown"`; no registry change. ## Querying - Physical iPhone adoption: `tool:invoke` where `platform = 'ios' and device_kind = 'device'`, distinct users over it. `isIosPhysicalUdid` is a positive match, so invoke is clean. - Physical Android: count on `tool:complete` (the device answered). `tool:invoke` / `tool:fail` still carry residual mistakes that happen to look like a serial (`Pixel7`), and a non-loopback `ip:port` may be a networked virtual device. ## Docs - `Telemetry.md` → 1.04, effective date 7 September 2026 (adjust to the release date on merge). Discloses the new category and, while touching that section, the external-provider label that #735 added without a notice line — happy to drop that second bullet if it should ship separately. - No docs-site change: `docs/reference/telemetry.mdx` links to `Telemetry.md` and has no per-property list. ## Notes - Pre-existing, unchanged: HTTP-edge failures carry no `tool_invocation_id`; `stop-all-simulator-servers` declares no capability, so its invoke row has neither field while its fail row has both. `device_kind` follows `platform` in each site. - Legacy 40-hex iPhone UDIDs already classify as `android` today; they now get no kind rather than a wrong one. - Design went through three adversarial review rounds before implementation and one on the diff; the fallback-bucket and child-attribution rules above came out of those. ## Verification - `packages/telemetry`: 319 tests, `packages/tool-server`: full suite 5670 passed / 1 skipped; `typecheck:tests` both packages; `eslint --max-warnings 0`; `knip`; prettier. - New: `test/telemetry-device-kind.test.ts` (the shape table above), plus cases in `http-tools-meta`, `http-platform-alias`, `sanitize`, `registry-listener`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01BSGpeAwvoCv2JAhbLsuto2 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * New Features * Telemetry now records a device category—such as simulator, emulator, physical device, virtual device, or desktop app—alongside the platform. * Device categories are inferred from device information and may include a short label for external device providers. * Device category information is included in tool invocation, completion, and failure telemetry when available. * Device identifiers themselves continue to be excluded from telemetry. * Documentation * Updated the Argent Privacy Notice (Telemetry) to version 1.04, effective 7 September 2026. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-07 11:55:57 +02:00
Hubert Gancarczyk	1a7e97fb14	feat(flows): run a local script in a bounded child process (#864 ) Adds the executor a flow `script:` step runs on: a fresh Node child per step, a protocol over its IPC channel, and the two watchdogs that make a hung or runaway script the runner's problem rather than the host's. The step itself is not here. This branch stops at the executor and its unit tests; `feat/flow-script-step` stacks the YAML directive, the runner integration, and the reference docs on top. ## What it does - One child per step. Spawned with an old-space heap limit, an explicit working directory, and an environment built from an allowlist rather than copied from the tool server. - A deadline and a lifeline. A separate watchdog holds the deadline, so a script that wedges the event loop still dies on time; a second one reaps the whole process tree when the parent goes away, so a grandchild cannot outlive the run. - A concurrency queue. Slots are bounded per server. A step that never gets one is refused with a message that says so, and an aborted run frees its slot immediately. - Log budgets. 64 KiB per step and 256 KiB per run, counted on the bytes the report keeps: redaction and V8 frame collapsing both run before anything is counted, so what the limits bound is the size of the report rather than the size of the script's writes. - Secret hold-back. The scrub walks a chunk and stops where a value could still begin, so neither half of a value split across two chunks is released on its own, and a shorter value is never taken where the longer one containing it has not arrived yet. - A failure taxonomy. Twelve kinds, split into what the script did (it threw, it did not load, it exited non-zero, it wrote an unusable `output`) and what the host did to it (a limit, a signal, a spawn that failed, a queue slot it never got). ## Docs `packages/docs/docs/reference/configuration.mdx` lists the two configuration keys this branch adds, `scripts.maxTimeoutMs` and `scripts.heapLimitMb`. The `script:` step itself is documented with the step, on `feat/flow-script-step`. ## Verification `npm run build`, `npx eslint . --max-warnings 0`, `npx prettier --check .`, `npm run knip`, `npm run typecheck:scripts`, the test typecheck across every workspace, the tool-server suite (4719 passed, 1 skipped) and `npm run test:scripts` (92 tests) are green, as is `npx docusaurus build`. The executor's own behaviour is covered by the eight `test/flows/script/` files added here, against real child processes, and the compiled `dist/` executor was driven against its copied runner assets over every path the review reached. Each review fix carries the run that reproduced it before the change and the mutation that proves the new test fails without it: the prefix-secret leak and all three npm `node-options` routes were reproduced end to end through the executor against real child processes, and the five coverage findings were confirmed by re-applying the exact mutation each thread named. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * New Features * Added reliable execution of trusted flow scripts with concurrency control, cancellation, timeouts, memory limits, logging, and detailed failure reporting. * Added safeguards for process cleanup, watchdog termination, output validation, and secret redaction. * Added global configuration for maximum script runtime and memory usage. * Bug Fixes * Improved handling of script failures, stalled processes, malformed output, and child-process termination. * Documentation * Documented script resource limits and global configuration behavior. * Chores * Updated builds and packaging to include required flow-script runtime assets. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-09-07 11:34:30 +02:00
Mateusz Aliyev	6e9e45c9f0	feat: attach to devices offered by external providers (#735 ) This pull request lets Argent drive a simulator or emulator another process is already running, attaching to its simulator-server rather than spawning a second one. A provider writes a JSON descriptor to `~/.argent/providers/` listing the devices it offers and the mechanisms Argent may use on each. They appear in `list-devices` with an `ext:` id and work with the existing tools. The file is re-read on every call, so a withdrawal or a narrowed grant applies immediately. `ios.additionalDeviceSets` (#600) already makes such a simulator reachable by UDID. What changes is ownership. `boot-device` and `stop-simulator-server` refuse these devices, anything the provider did not grant is refused with a message naming it and Argent uses only the endpoints its own simulator-server build serves. A grant binds to the device rather than to one of its names, so the real udid or serial is gated exactly like the `ext:` id. Android emulators are covered too and a device visible to both a provider and `adb`/`simctl` is listed once. The contract ships as `schemas/device-provider-v1.json`, validated by `argent providers check`, and is documented in `docs/reference/device-providers.mdx`. Includes unit tests and an e2e phase where the harness acts as its own provider. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * New Features * Added support for discovering and connecting to externally managed devices. * Added `argent providers` commands for listing, validating, publishing, withdrawing, and pruning providers. * Added provider-aware simulator, debugger, profiler, native tools, and device listings. * Added capability controls, endpoint validation, revocation handling, and provider-specific diagnostics. * Added automatic CLI discovery for provider integrations. * Bug Fixes * Improved service recovery and paused-runtime error reporting. * Improved simulator keyboard and paste command reliability. * Documentation * Documented provider descriptors, capabilities, lifecycle rules, and CLI usage. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-04 22:02:57 +02:00
Ignacy Łątka	4c645f1619	fix(chromium): correct the hidden-window guard's docs and pin its exemptions (#611 ) Follow-ups from a post-merge review of #605. Each one is a comment thread on that PR. No behaviour changes to what the guard does — the three emitted `failure_stage` strings and the `CHROMIUM_WINDOW_HIDDEN` code are byte-for-byte identical, so telemetry stays joinable. ## Changes Derive `failure_stage` from `action`. It was a second free-form `string` parameter sitting next to `action`, and all three call sites passed a strictly redundant pair — `("tap", "chromium_tap_window_hidden")` and so on. Two adjacent same-typed params meant transposing them or mistyping the stage compiled cleanly and read fine in review. That carries extra weight here because #605 deliberately froze `chromium_scroll_window_hidden` to keep pre- and post-migration rows joinable. `action` is now `"tap" \| "drag" \| "scroll"`, so a typo is a compile error, and the docblock spells out all three literals so they stay greppable in `src`. Pin the keyboard exemption. Key events skip hit-testing and stay fast on a hidden window, so `keyboard` is deliberately unguarded. That was correct but lived only in a comment: adding the guard to `keyboard` would have made it refuse input that demonstrably works, and the suite would have stayed green. Now covered by `keyboard-chromium-unguarded.test.ts`. Pin the `button` exemption too. The same file now asserts that `assertSupported` rejects a Chromium device for `button`. The claim that `button` can never reach the guard rested entirely on a capability literal no test read — adding `chromium: { app: true }` to it reddened nothing. Correct the same comment's claim about `button` — three times. The first pass replaced the wrong rationale (that `button` skips the guard because key events bypass hit-testing) with the real one: its capability omits chromium. The replacement was itself wrong about where Chromium's button surface lives. `sendButton` is reached only from `handleWsCommand`, i.e. the chromium-server WebSocket channel — the HTTP router exposes screenshot / clipboard / fps / navigate / reload / history / viewport / stream and no button at all. It also over-claimed: `sendButton` emulates `Back` alone as an Alt+Left chord and throws for every other button, so "chromium hardware buttons exist" contradicted `button/index.ts` ("Chromium apps have no hardware buttons"). The wording now agrees with both neighbours. A third pass dropped the last false note in it: the lockout test said the gate stops `button` "before it can reach the guard", but `button` has no chromium branch and never imports the guard — there is no guard on its path to reach. Give the keyboard measurement its missing condition. `Input.dispatchKeyEvent` at 1–14 ms against 5002–5005 ms per mouse move, and "focus emulation keeps input fast even for a genuinely minimized window", describe the same scenario with opposite outcomes — those numbers can only come from a window where focus emulation was not in effect. The docblock says so, and names focus emulation alone: `primePageSession` applies it to every session at connect, so the probe reads "hidden" only where the runtime refused it. The anti-throttling launch flags are not part of that condition and do not narrow the guard's reach. A window carrying all three, minimized without emulation, still reads "hidden" and still costs ~5 s per `Input.dispatchMouseEvent` — Electron 42.5.0 5001–5005 ms and Chrome 152 5002–5010 ms, 3 trials each — so an argent-spawned app is no less exposed than an externally launched one. `boot-electron.ts`'s `ANTI_THROTTLING_ARGS` docblock, where that claim originated, is corrected to match, along with its test. Two mechanisms that were stated unconditionally. `Page.bringToFront` un-minimizes on Chrome 152 (`windowState` minimized → normal, `visibilityState` hidden → visible, mouse dispatch 5002–5006 ms → 16–25 ms, 3/3) and not on Electron 42.5.0 (returns ok, stays `"hidden"`, stays 5001–5005 ms, 3/3), so `flow-run`'s comment calls the outcome runtime-dependent rather than denying it. And the capability gate that keeps `button` off a Chromium device is not universal: `assertSupported` runs at the HTTP edge and in run-sequence's pre-flight, but `Registry.invokeTool` skips it, so a flow step dies resolving `services()` with `SimulatorServer.factory does not support platform "chromium"` rather than an `UnsupportedOperationError`. Replace the try-boundary rationale. It said the property access had to stay inside the `try` because fakes "may lack `cdp`" — but for such an api `chromium.cdp` yields `undefined` without throwing; the TypeError comes from reading `.send`, which is inside the `try` wherever `chromium.cdp` is read. Verified: hoisting `const client = chromium.cdp;` above the `try` leaves every relevant test green. The placement would only matter if `chromium` itself were nullish, which no registry path produces. Cover the no-`cdp` probe path directly. The existing test claimed to cover "api objects with no `cdp` at all" but used a rejecting `cdp.send` — a different failure mode. The real shape was exercised only incidentally, by bare fakes in `chromium-drag.test.ts` and `tools/gesture-tap.test.ts`. Cover the other half of "a failed or empty read proves nothing". Only the throw paths were pinned. Flipping the comparison to `value !== "visible"` left the entire suite green, so a probe that resolves without a usable value — `Runtime.evaluate` against a torn-down document — would have started refusing tap/drag/scroll on a fully visible window. Pin the action word in the user-facing message. Hardcoding `Cannot tap:` for all three tools also stayed green, so a scroll refusal reading "Cannot tap" could have shipped unnoticed. Update `flow-run`'s comment. It still named gesture-scroll as the only tool that fails fast on a hidden window; tap and drag have since #605. It also over-promised "fail fast": on the flow path `frontChromiumPage` has just primed focus emulation, so the three tools proceed. They carry the guard; whether it fires depends on the mitigations. ## Verification - Full gate on the branch: `tsc --build`, `typecheck:tests`, `eslint --max-warnings 0`, `prettier --check`, and the whole tool-server suite sharded — 360 files / 4504 passed, 1 skipped, 0 failed. - Every empirical claim in the comments was re-measured on a live CDP testbed rather than carried over: an Electron 42.5.0 app and Chrome 152, each launched with and without the three `ANTI_THROTTLING_ARGS` plus `--remote-debugging-port`, minimized, probed for `document.visibilityState` and per-event `Input.dispatchMouseEvent` / `Input.dispatchKeyEvent` latency before any focus emulation, then after `Page.bringToFront`, then after `Emulation.setFocusEmulationEnabled`. Runs were sequential, one browser at a time, 3 trials per condition. - The `button`-on-chromium flow path was run, not just read: `registry.invokeTool("button", { udid: "chromium-cdp-9222", button: "home" })` with the simulator-server blueprint registered throws `ToolExecutionError … SimulatorServer.factory does not support platform "chromium"`. - The measurements behind the keyboard exemption were taken on a real minimized Electron window (v42.5.0): `Input.dispatchKeyEvent` 1–14 ms, against 5002–5005 ms for each of ten consecutive `Input.dispatchMouseEvent` moves, with `Runtime.evaluate` staying at 1–3 ms. - Mutation-tested every pin rather than assuming it bites. Each of these fails the suite: adding the guard to `keyboard/chromium`; adding `chromium` to `button`'s capability; making the probe rethrow instead of swallowing; drifting the derived stage template; collapsing the stage to one action for all three tools; flipping the comparison to `value !== "visible"`; and hardcoding the action word in the message. - No docs update needed: comments only, no user-facing behaviour or tool-surface change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Documentation - Clarified how window minimization, focus emulation, and anti-throttling behavior affect Chromium interactions. - Bug Fixes - Standardized hidden-window handling and error reporting for tap, drag, and scroll gestures. - Preserved keyboard input on hidden Chromium windows. - Tests - Expanded coverage for hidden-window gestures, keyboard input, missing browser capabilities, and varied visibility responses. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-04 14:43:54 +02:00
Ignacy Łątka	f05d17c1d3	refactor(flow): let each step kind own how it reads (#744 ) Stacked on #574 (merged; retargeted at `main`). ## Summary `summarizeSteps` (flow-finish-recording.ts) was a 50-line `switch` over `FlowStep["kind"]`. `stepTarget` (flow-run.ts) was a second one, over the same union, with its own selector and condition spellings. Neither could be found from the other, and `summarizeSteps`' `case "tool": default:` arm meant a step kind added without wording rendered as a `tool:` step instead of failing. Both surfaces now read from one definition per kind — the same shape as a tool owning its own log wording in `ToolDefinition.interaction` (#582): ```ts const FLOW_STEP_DEFINITIONS: { [K in FlowStep["kind"]]: FlowStepDefinition<Extract<FlowStep, { kind: K }>>; } = { "scroll-to": { summary: (step) => `${yamlSelectorLabel(step.target)} (${step.direction})`, target: (step) => { ... }, }, ... }; ``` Keying the record by `FlowStep["kind"]` is what replaces the default arm: a new kind is a compile error until it says how it reads on both surfaces. That also retires the `renderToolArgs` interpolation guard, which existed only because the default arm could hand it a step with no `args` field. `flow-run.ts` sheds a net 113 lines and five imports; `flow-finish-recording.ts` sheds a net 180 and is now just the tool. The two shared label templates (`conditionLabel` parameterized by selector spelling, and a new `whenLabel` for the platform-vs-UI guard arm) mean the recorder's `when:` tail and the runner's no longer duplicate the platform arm. ## Test The extraction is rendering-neutral: every summary line and report target the switches produced moved over byte-for-byte (with four later corrections, below). - Proved with a throwaway equivalence test that inlined both pre-refactor switches verbatim and compared them against the new definitions over 1264 generated steps: all 15 kinds, 10 selector shapes (loose, regex, scoped, quote-bearing), every wait condition × text-match mode × expectation, every scroll direction, and the cyclic-`args` path. Identical on all of them. Mutating two renderings (`equals` comparator, default scroll direction) failed it, so it discriminates. - Replaced by `packages/tool-server/test/flows/flow-step-definitions.test.ts`, kept: assertions pinning the exact summary line and report target of every kind, in a table keyed by `FlowStep["kind"]` so a new kind fails `typecheck:tests` here too (verified by deleting the `wait` entry: `TS1360`). Deleting a kind from the definitions record fails the build the same way (`TS2741`). - `npm run build`, `eslint --max-warnings 0`, `prettier --check .`, `npm run typecheck:tests --workspaces`: clean. - `packages/tool-server`: 329 files / 3906 tests passed. `packages/argent-cli`: 459 tests passed. ## Review follow-up (`9cf51322`) Four corners where the carried-over rendering misdescribed what replays, fixed with pinning tests: - `delayLabel`: a hand-edited `delayMs` above `setTimeout`'s 2³¹−1 ceiling sleeps a clamped 1ms tick but read `(after 3000000000ms)` — the summary now describes no delay there. - `scroll-to`: the `within` container is parsed, serialized, and changes what replays, yet neither surface rendered it; both do now. - `snapshot`: the summary now carries `cropOn` and `maxMismatch` (baseline key and tolerance). Only `cropOn` was on the report target; `maxMismatch` was rendered on neither surface (the run report surfaces the tolerance through the step's `reason`, not its target). - `type`: the summary JSON-quotes the text, the module's own convention for literals, so embedded quotes/newlines stay one unambiguous line. ## Review follow-up (`7c804907`) A second pass (7 lenses + a 32-mutation pass that also re-derived the neutrality claim against main over a 136-step corpus, and confirmed the mapped-type compile gate by adding a kind) found no HIGH or MEDIUM. One rendering defect this PR introduced, plus the boundaries the case table left open: - `scroll-to`'s report target spelled the scroll container ` within (…)` — exactly how `selectorLabel` spells a selector's own relational scope. `scroll-to: { target: Footer, within: { id: list } }` (scroll that container) and `scroll-to: { target: { text: Footer, within: { id: list } } }` (match only inside it) both rendered `"Footer" within (id=list)`; carrying both put the direction between two identical-looking scopes. It now reads ` in scroll container (…)`, the noun flow-actions already uses in its own "scroll container … is not visible" failure. `main` did not collide here only because it dropped `within` entirely. - New pinning cases, each verified to fail without the code it pins: `setTimeout`'s ceiling from both sides (`231-1` keeps its label, `231` loses it) and its 1ms floor; `maxMismatch: 0`, the one value separating `!== undefined` from a truthiness test; `cropOn` + `maxMismatch` together, which fixes their suffix order; and a multi-field selector, which fixes the report target's field order. - `Number.isFinite(ms) &&` in `delayLabel` became redundant the moment correction #1 added the ceiling — NaN and ±Infinity both fail the range test. Removed (proved a no-op by mutation). - Prose corrected where it had stopped matching the code: the point-gesture comment argued a criterion the file does not follow (`snapshot` and `scroll-to` are equally not recorder-built and this PR renders their fields anyway) and listed two of the four still-unrendered replay-affecting fields; `flow-tools.test.ts` still claimed to be the only coverage of the long-press rendering; and the new case table justified a no-op `as number` cast with a reachability claim that is wrong in both halves (`flow_add_step`'s `delayMs` is `z.number().int().min(0)` with no ceiling, so the recorder reaches these values too). Follow-ups filed rather than folded in, both pre-existing and unchanged here: #808 (an `echo` message or `tool` name containing a newline breaks the one-line-per-step summary contract, the same hazard correction #4 fixed for `type`) and #809 (`type.submit`, `await.timeout`, `idle.timeout`, `idle.stableFor` change what replays and render on neither surface). ## Review follow-up (`4560f412`) Round from @hubgan's review (11 inline) plus CodeRabbit's one. Nine acted on, two verified and rejected — the rejection reason is now in the code, so it stays answered rather than being re-raised. Rendering - `type`'s report target carried the typed text with no bound. Unlike the summary, that target is emitted on every replay — stdout, `--json`, the MCP blocks handed to the agent, and the appended `~/.argent/mcp-calls.log`, which has no cap or rotation — so a literal written into a flow file shipped its whole length to each. Now capped at 200 chars with the same `…(+N chars)` elision `parseFlow` uses on the entry it echoes (`MAX_ENTRY_RENDER_CHARS`). The summary keeps the text whole: it is also `stepAnchor`, so eliding there would collide two steps differing only past the cap and drop the recording's verdicts on both. Elides before quoting, so a cut cannot halve an escape. - `selectorLabel` and `describeSelector` interpolated `textMatches`, `identifier` and `role` raw — a newline in one splits a step across two report lines, an escape byte drives the terminal. That is the hazard `text`'s quoting already names, so it now covers its neighbours: new `escapeInline` (flow-utils) spells out control characters only. Not JSON escaping: doubling a backslash would print a regex source nobody could copy back into the .yaml. `describeSelector`'s literal arm moved from a raw `"${v}"` to `JSON.stringify` — byte-identical for ordinary values, unambiguous for a quote in the content. Types - `FlowStepDefinition`'s two renderers were method shorthand, and a method's parameters are checked bivariantly even under `strict`: a definition annotated narrower than its slot (one requiring `times` on `tap`) was accepted and then read a field the step need not carry, with a green typecheck. Property types now — the same assignment is `TS2322`. Tests — each verified against the mutation it exists to catch - Every case in the table goes through `serializeFlow` → `parseFlow` and is compared on its anchor. That round-trip stability is what `scroll-to.within`, `snapshot.cropOn` and `snapshot.maxMismatch` newly depend on, and nothing asserted it; deleting `yamlSelectorLabel`'s key sort now reddens the suite. The `when` case with `steps: []` became a two-step block — `parseFlow` refuses an empty guarded block, so no count below 1 is renderable, and `when #1` already pins the singular. - The case table's value type is a non-empty tuple of cases narrowed to the key's own kind. `"wait": []` and a `wait` slot holding an `echo` case both typechecked clean before; both are `TS2322` now. - The `when:` pluralization coupling only ran a two-step block, so hardcoding either surface's tail to `(N steps)` left everything green. Both counts run now, and against the whole skip reason rather than its suffix. - New pins for the escaping above, on both surfaces, each confirmed to fail without the line it pins. Prose corrected against the code - `stepAnchor`: there are two anchor comparisons, not three, and only the per-verdict one is unconditionally raw-vs-parsed — host mode re-parses both sides of the whole-file check, which is raw-vs-parsed only in client persist mode. - `delayMs` bounds: `.int()` admits nothing strictly between 0 and 1, so the 1 ms floor is hand-edit-only; the ceiling is recorder-reachable, as stated. - `0` is not a parse-only `delayMs`: `min(0)` accepts it and `serializeFlow` writes it back out. Rejected, verified - Escaping `snapshot.name` (CodeRabbit) would be dead code: `parseFlow` gates it on `FLOW_NAME_PATTERN` — letters, digits, `_`, `-` — because it becomes a baseline filename, and that parse is the only construction site of a snapshot step. A comment there now says so. - "`textMatches`' `/…/` delimiters are unpinned" no longer holds at this head: `flow-progress.test.ts`'s swipe-`from` target asserts `to (0.1, 0.5) from /^Card \d+$/`, and dropping the delimiters fails it. Measured by mutation. Verification: 371 files / 4950 tests in `packages/tool-server`; `argent-cli` 527, `argent-mcp` 87, `registry` 120. `npm run build`, `typecheck:tests --workspaces`, `eslint --max-warnings 0`, `prettier --check .`, `npm run knip`: clean. 13 mutations aimed at the new code, all killed. Docs: no update needed. `packages/docs/` documents flow YAML, the CLI and the tools; it does not specify how a step summary or a `StepReport.target` is spelled, so nothing there goes stale. That covers the two earlier rendering changes from `3e2244b3` as well — report-target text literals becoming JSON-quoted, and the `type` target carrying the typed text (now bounded, above). ## Review follow-up (`9f8a1e29`) The two threads left open after approval. - `typedTextLabel` compared and cut on `text.length`, which counts UTF-16 units. 199 ASCII chars plus one astral char is 200 characters but 201 units, so the target quoted a lone high surrogate and announced `(+1 chars)` for text that fits under the cap whole. It counts and slices code points; the boundary is pinned on both sides of the cap and each assertion is red without the fix. Two sibling truncations are deliberately left, both pre-existing and outside the step-summary code: `badEntry` slices an already JSON-stringified entry, and `cappedReason`'s cap bounds the EMITTED length, where counting code points would let the emitted string overshoot its own bound. - The `textMatches` delimiter thread, re-measured at this head: dropping the `/…/` fails `flow-progress.test.ts:231` and `flow-step-definitions.test.ts:451` - 2 failed \| 1571 passed of 1573. Restoring them returns the suite to green. Verification: `packages/tool-server` 371 files / 4950 tests; `eslint --max-warnings 0`, `prettier --check .`, `tsc --noEmit` on both `tsconfig.json` and `tsconfig.test.json`: clean. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Improvements * Flow step summaries and run reports now use consistent formatting across supported step types. * Type steps display their target identifier and entered text more clearly. * Selector, gesture, condition, delay, and snapshot details are rendered consistently, including safer handling of special characters. * Tool steps with cyclic arguments are handled safely in summaries. * Tests * Added comprehensive coverage for step rendering, flow numbering, conditions, edge cases, and report output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Hubert Gancarczyk <hubert.gancarczyk@swmansion.com> Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-09-04 12:54:13 +02:00
Sebastian Flajszer	1068430b91	feat(ios-device): small fixes related with ios-device support (#1031 ) Follow-up to #944: the fixes made after the review that did not make the squash. ## What changes - Signing note: delivered only to a call whose device argument resolves to a physical iPhone, including `flow-execute`'s `device`, and held (not dropped) across other calls. The label is now `Apple Development (KEYID)`; the certificate CN's Apple ID no longer reaches agent context or logs. - Flow device resolution: the no-device-matched refusal names the cabled phone and the exact `device <udid>` / `--device` spelling. A physical iPhone is never picked automatically. - `gesture-swipe`: the description states the hardware semantics (edge gesture needs `fromX` 0, `durationMs` sets drag speed, `momentum:false` does not damp). - Skills: `argent-ios-simulator-setup` skips `kind: "device"` entries; `argent-ios-device-setup` and `-interact` state the runner build timing, the full list of tools that do not exist on hardware, the backgrounded-target behaviour of each read tool, `actionButton` only on models that have one, and the swipe and `open-url` rules; the flow, Metro and profiler skills each carry one line on physical-iPhone limits. - Docs: `physical-ios-devices`, `flows`, `flow-yaml` and `tools` corrected to match the behaviour above; README platform row; runner `PROTOCOL.md`. ## Verification - tool-server suite 400 files / 5173 tests, `tsc`, lint, prettier and the docs build clean. - Live re-check against 0.23.0 on three simulators and a physical iPhone 15: no regression; the note lands once on the first phone call with no e-mail address, and flow auto-detection is byte-identical to 0.23.0 with 0, 1 and 3 simulators booted.	2026-09-03 14:53:25 +02:00
Sebastian Flajszer	1e69c90981	feat(ios-device): add physical iOS support (#944 ) # Physical iOS device support Try it now (cabled iPhone, unlocked, Developer Mode on): ```bash argent run list-devices # copy the 25-char UDID argent run launch-app --udid <UDID> --bundleId com.apple.Preferences ``` First launch builds and signs the on-device runner: 3 to 5 minutes, keep the phone unlocked. Every later start: seconds, from cache. No signing setup needed if an Apple ID is signed into Xcode; the team is detected from the keychain. ## What this adds - Physical iPhone support, on by default: a cabled iPhone simply appears in `list-devices`. iPhone-only: physical iPads are skipped at discovery. - 15 tools ported to hardware. Flows replay on device. - Zero-config signing from the keychain; `ARGENT_IOS_TEAM_ID` is an override, nothing is persisted. - Self-contained npm install: the runner's Swift sources ship in the package. New package `@argent/ios-device-runner` (an XCUITest bundle hosting an HTTP command server on the device), new tool-server backend under `src/utils/ios-device/`. - The one always-on cost: `list-devices` on macOS now runs physical discovery in parallel with the other platform probes, under the existing 8s devicectl timeout; off-macOS it early-returns. ## Suggested review order Rough estimate: half a day for the full pass. 1. `packages/ios-device-runner/PROTOCOL.md`, then `README.md` (wire contract and reliability model, ~15 min) 2. `packages/tool-server/src/utils/ios-device/` (devicectl, usbmux, team-detect, runner-build, runner-commands: the core) 3. `packages/tool-server/src/blueprints/ios-device-runner.ts` (service lifecycle, crash recovery) 4. Per-tool device arms and the capability gates (`ios-physical-device-classification.test.ts` is the ratchet) 5. Docs page, the two new skills, the Swift runner sources ## Requirements 1. macOS host with full Xcode (iOS platform installed); uses `xcrun devicectl` and `xcodebuild`. 2. An Apple ID signed into Xcode; automatic signing mints the runner's provisioning profile. 3. USB cable only. Wi-Fi is deliberately unsupported: the transport is usbmux, and a Wi-Fi-only device lists as `paired`, not `connected`. 4. iPhone paired with and trusting the Mac, Developer Mode on, A12/2018 or newer (modern 8+16-hex UDID, the CoreDevice floor). Legacy 40-hex UDIDs are not classified as iOS. 5. Signing resolves as: `ARGENT_IOS_TEAM_ID` in the tool-server's environment (explicit override, CI) -> keychain detection (newest identity first, memoized per server run, never persisted). One certificate: nothing to do. Several teams: newest wins; the first tool result names the pick, the runner-ups, and the override. No certificate: the error walks through Xcode > Settings > Accounts and Manage Certificates > + > Apple Development, then retry; it never names the env var, since a team id alone cannot sign. Bundle ids derive per team (`com.argent.runner.t<teamid>`). `ARGENT_IOS_RUNNER_PROJECT` overrides the project path for unusual layouts. Build artifacts cache under `~/.argent/ios-device-runner`, keyed by runner sources, Xcode/SDK version, and signing. A team with no registered devices self-heals: the failed generic build retries against the connected phone, which registers it. ## How it works 1. `xcrun devicectl` (CoreDevice) is the control plane: discovery, readiness, app lifecycle. Readiness gates on the wired transport, so an unplugged phone is refused before any build starts. 2. A signed XCUITest runner is built lazily with the resolved team, started via `xcodebuild test-without-building`, and parks as an HTTP command server for 24h. On an npm install the Xcode project is the copy shipped inside the package. 3. The listener binds to the device's loopback interface; the host reaches it over usbmuxd with a dependency-free `node:net` client. 4. Mutating commands are journaled by `commandId`; a lost reply is resolved by asking `status` for the journaled fate, never by replay (at-most-once delivery). 5. Routing hangs off `resolveDevice` UDID-shape classification plus a per-tool capability allowlist. CLI and MCP layers are untouched; outside tool-server, docs, and the runner package the diff is failure-code constants (registry), two agent skills plus rule updates (skills), the pack step (packages/argent), and the e2e smoke script. ## Supported tools \| Tool \| Status \| Notes \| \| ----------------- \| --------- \| ------------------------------------------------------------------------------------------------------------------------------------------------------------ \| \| launch-app \| Supported \| Plain `devicectl` launch. System UI (SpringBoard/Spotlight) registers a session only; no launch attempt. Carries a note when signing is not resolvable yet. \| \| restart-app \| Supported \| System UI bundle ids rejected before any device contact. \| \| reinstall-app \| Supported \| Same system UI pre-flight. \| \| screenshot \| Supported \| Served through the runner; device-wide, needs no registered app. (No host-side devicectl screenshot exists on any toolchain; that dead route was removed.) \| \| describe \| Supported \| AX tree snapshot via the runner. Snapshots the session app's process, so another process's dialogs are not in the tree. A backgrounded session app fails with an actionable error instead of being silently re-fronted. Focus polling is off for this source; typing uses a fixed settle. \| \| keyboard \| Supported \| Types into the focused element; typing with nothing focused is refused with "tap the text field first". `enter` and `backspace` are the named keys (both exist as software-keyboard characters; there is no per-keycode HID surface on hardware). Whitespace-only keys rejected, not silently "typed". \| \| gesture-tap \| Supported \| `clickCount > 1` dispatches as one native multi-tap gesture, not separate taps. \| \| gesture-swipe \| Supported \| Single-finger drag between two points. \| \| gesture-custom \| Supported \| Two shapes only: press-hold and straight single-finger drag. Second finger or `Move` waypoints rejected with authoring guidance, before any runner starts. \| \| button \| Supported \| `home`, `volumeUp`, `volumeDown`, `actionButton` (volume buttons are simulator-unavailable in the SDK, not device-unavailable). `power`/`appSwitch` have no public XCUIDevice API and reject before device contact; a button the model lacks gets a typed refusal via `hasHardwareButton`. \| \| await-ui-element \| Supported \| Polls the same runner snapshot describe returns; waits and taps see identical frames. Degrades to `cause: "unreadable"` with a note on a backgrounded app, never a raw throw. \| \| await-screen-idle \| Supported \| Same describe path; resolves not-settled instead of throwing, so a follow-up screenshot still runs. \| \| run-sequence \| Supported \| Outer gate only; each step pre-flights its own capability. \| \| open-url \| Supported \| devicectl launches the receiving app with the URL as its launch payload. http(s) defaults to Safari; any other scheme needs the new optional `bundleId` param (hardware cannot resolve a scheme's handler system-wide). \| \| screenshot-diff \| Supported \| Live captures through the runner (full-resolution, device-wide, no registered app needed); the diff itself runs on the host. Baselines are per device model/resolution; `rotation` is ignored on hardware. \| Flows run through these tools; the device flow tree gets the same `subtreeText` hoisting as every other platform, so container-scoped `text` asserts work. ## Not supported (400 at the capability gate) 1. gesture-pinch, gesture-rotate: no two-finger coordinate API on hardware. Pinch/rotate flow directives also fail upfront with a clear error. 2. screen-recording-start/stop (video): simulator-only capture pipeline. 3. rotate, paste, shake, settings-permissions: simulator-only backends, not ported (paste also hits the iOS 16+ cross-app paste permission prompt). 4. native-devtools, debugger, profiler families: simulator-side instrumentation (DYLD injection does not exist on hardware). 5. Physical iPads: filtered at discovery for now. A test ratchet (`ios-physical-device-classification.test.ts`) keeps `apple.device` declarations in sync with the ported list. ## Core contracts 1. Observation never mutates the screen: a backgrounded session app makes `describe`/`await-` fail with an error naming the three ways out (screenshot, launch-app, or launch-app com.apple.springboard). Mutating commands do re-front, and their result carries `reactivated: true`. 2. The runner listener is unauthenticated; mitigation is the loopback-only bind. 3. `list-devices` states: `connected` (cabled now, usable) vs `paired` (unreachable, never auto-bound). Wired transport alone decides; a Wi-Fi-migrated CoreDevice tunnel does not count. 4. The multi-team signing pick rides the first affected tool result as a one-shot note (the pre-existing notes channel). The `list-devices` result shape is unchanged from main: devices and avds, nothing else. 5. Rejections are typed 400 validation errors end to end, so a batch flow run skips one failing flow instead of aborting. Also: the runner is a first-class device-owned service (`stop-simulator-server` and `stop-all-simulator-servers` reap it, detached xcodebuild child included; a crashed runner respawns on the next call). A passcode-locked phone refuses launches; the devicectl 10002 hint also covers a pending system prompt on an unlocked phone. Build failures surface xcodebuild's own `error:` lines with signing hints (no registered devices, free-team app-id cap, missing provisioning, the `errSecInternalComponent` partition-list fix). An app XCTest reports as `.unknown` (a swipe-killed foreign app) is refused with launch-app guidance. Status-bar pinning is skipped on hardware; pixel-settle masks the top band. ## Testing - Unit coverage, hardware-independent: UDID classification and capability routing; usbmux framing and transport; devicectl list/hints parsing incl. the silent iPhone-only filter; keychain detection against real certificate fixtures; runner build/cache self-heals (poisoned cache, concurrent checkout, fresh-team retry); crash/journal recovery; the backgrounded-observation and reactivated contracts; keyboard guards incl. the not-focused refusal; system-UI pre-flight; per-tool device arms. - Manual smoke on a real device: `scripts/e2e-ios-physical-device.mjs`. ## Docs and skills 1. `features/physical-ios-devices.mdx`: the user-facing walkthrough (zero-config signing with the sign-in fallback, prerequisites, app-scoped model, troubleshooting). 2. `fundamentals/supported-platforms.mdx`: physical iPhones in the platform matrix (iPads not supported yet). 3. `reference/tools.mdx` and `reference/configuration.mdx`: list-devices states and the `ARGENT_IOS_TEAM_ID` override. 4. Two skills ship with the package via `argent init`: `argent-ios-device-setup` (setup ladder plus a troubleshooting table keyed to real error texts) and `argent-ios-device-interact` (the app-scoped hardware contract, supported subset with alternatives, gesture recipes, pitfalls). The agent rule and sibling skills route to them. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit New Features * Added experimental USB-connected physical iPhone support. * Physical iPhones now support discovery, app lifecycle actions, URL opening, accessibility snapshots, screenshots, taps, swipes, custom gestures, keyboard input, waits, flows, and screenshot comparisons. * Added automatic signing-team detection with optional `ARGENT_IOS_TEAM_ID` configuration. * Added clear handling for paired or unavailable devices and unsupported hardware actions. * Documentation * Added setup, troubleshooting, capability, and physical-device usage guidance. * Clarified that physical iPads and several simulator-only tools and gestures remain unsupported. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-03 14:33:11 +02:00
mjq2020	23181baaa8	fix(network): report UTF-8 response byte size (#783 ) ## Summary - measure React Native response bodies from a cloned Blob instead of a decoded JavaScript string - report one consistent metric: decoded response entity bytes - avoid Content-Length because HEAD/304 and compressed responses give it different semantics - keep response text capture independent, and leave the size unknown if Blob conversion is unavailable ## Root cause body.length counts UTF-16 code units. Re-encoding the decoded text as UTF-8 fixes ordinary UTF-8 text, but it overcounts replacement characters from non-UTF-8 and binary responses. Content-Length is also not a safe fallback: it can describe an unsent GET body for HEAD/304 or the compressed wire representation while the response body is decompressed. Response.blob().size measures the bytes in the decoded entity supplied by the React Native response. The same measurement therefore applies to text, binary, empty, cached, and compressed responses. ## Tests - network interceptor regression: 23/23 passed - covers Unicode JSON, HEAD, 304, non-UTF-8 replacement characters, compression, binary bodies, and unavailable Blob conversion - npm run build - npm run typecheck:tests -w @argent/tool-server - targeted ESLint and Prettier checks on both changed files Fixes #662 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes - Improved reported network response sizes to reflect the actual decoded response data. - Corrected size calculations for binary content, compressed responses, and text containing non-ASCII characters. - Added reliable handling for `HEAD` and `304` responses, which now report zero size. - Prevented failed size calculations from incorrectly reporting a zero-byte response. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-02 10:32:58 +02:00
Ignacy Łątka	1409195138	test(run-sequence): pin the one observation a sequence returns (#934 ) ## What `run-sequence` tells agents: > One screenshot is captured automatically after the whole sequence (not per step) The MCP-side half of that promise is already pinned — argent-mcp's auto-screenshot tests name `run-sequence` explicitly, twice. The tool-server half is pinned by nothing: this adds the gate that pins it, plus the renderer-side pin in argent-mcp. No behavior change. ## Why it is unpinned The steps stay silent only because no tool `ALLOWED_TOOLS` admits returns an image or an artifact handle. Nothing checks: - which tools the allow-list admits, - that every admitted name still resolves to a registered tool (a stale name still reads as allowed and stays advertised in the description, but every step naming it dies on `ToolNotFoundError` and halts the sequence), - that none of them declares `outputHint: "image"`. Admitting a frame-producing tool would go unnoticed: a sequence result reaches `toMcpContent` whole, and its `materializeArtifacts` walk collects handles from anywhere inside it, `steps[].result` included, appending one image block per handle. That walk recurses into arrays with `Promise.all`, so those frames arrive in completion order, not step order — the agent could not even tell which one is the screen the sequence ended on. Both halves are verified against the code, not assumed: - Putting an artifact handle in one step's result makes `toMcpContent` emit an image block for it, confirming the walk reaches `steps[].result`. - Materializing three step handles whose fetches complete fastest-last yields `[s2, s1, s0]` — exactly reversed from step order. ## What is gated tool-server — `test/run-sequence-observation-gate.test.ts`, against the real registry (`createRegistry()`, no device). `ALLOWED_TOOLS` is exported for it: - the allow-list admits exactly the reviewed set of 14 step tools - every name still resolves to a registered tool - none of them declares `outputHint: "image"` argent-mcp — `test/content.test.ts`: - a multi-step sequence result renders with no frame of its own, against a `fetchImpl` that resolves and is asserted never to be called (without a resolving fetch the assertion would hold equally for a sequence whose step frames merely failed to download, since `materializeArtifacts` swallows a failed download and renders the handle as nothing) - the positive control that pin rests on: a handle on `steps[].result` does surface as an image block, so the walk ceasing to reach steps cannot turn the pin vacuous The membership of `run-sequence` in `AUTO_SCREENSHOT_TOOLS` was already pinned by existing assertions and needed no new test. ## Verification Every test was mutation-checked — each fails on the broken code, not just passes on the good: \| Mutation \| Fails \| \| --- \| --- \| \| add `screenshot` to `ALLOWED_TOOLS` \| reviewed-set + `outputHint` checks \| \| rename an allow-list entry the registry lost \| reviewed-set + registration checks \| \| give a step result an image artifact handle \| the frameless-sequence check, on both its fetch-call and its image-block assertion \| \| stop `materializeArtifacts` recursing into arrays \| the step-carried-handle check, and nothing else in the file \| Locally green: `prettier --check`, `tsc --build`, `typecheck:tests` for both touched packages, the full argent-mcp suite (82 tests), and the gate test (3 tests). ## Scope note The allow-list is spelled out rather than counted, so adding a step tool means answering the question the file exists to ask — does it put a frame of its own in front of the agent? The gate deliberately sits on the allow-list rather than on the renderer: a tool that would break the guarantee is caught before it can be dispatched as a step, rather than having its frames filtered afterwards. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>	2026-08-31 20:51:21 +02:00
filip131311	927bfc630f	chore: bump version to 0.23.0 (#987 ) Bumps all workspace packages from 0.22.1 to 0.23.0. - All 16 `packages//package.json` bumped via `npm version 0.23.0 --workspaces` - `package-lock.json` regenerated on Node 24 / npm 11 (`npm install --package-lock-only --ignore-scripts`); diff is version-only - `server.json` updated in both places (top-level `version` and `packages[0].version`) - `npm run check:versions` passes No docs update needed — version-only change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VGjTuE8XKfrmmhJEjchBFq <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Chores* - Updated the release version across all packages and the server manifest from 0.22.1 to 0.23.0. - Ensures package metadata and server configuration consistently reflect the new release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-08-31 19:42:32 +02:00
Ignacy Łątka	a2ed83e041	fix(tool-server): await the simulator-server command ack so a lost tap fails (#985 ) ## The defect `sendCommand` returns `void`. It writes the JSON command onto the WebSocket — or queues it behind `once("open")` — and returns without ever reading the reply: ```ts export function sendCommand(api, cmd): void { const ws = getOrCreateWs(api); const payload = JSON.stringify({ id: String(++cmdId), ...cmd }); if (ws.readyState === WebSocket.OPEN) ws.send(payload); else ws.once("open", () => ws.send(payload)); // queued, never awaited } ``` simulator-server does acknowledge every command; argent just never looked. Probing the shipped binary's `/ws` endpoint: \| sent \| reply \| \|---\|---\| \| `{"id":"1","cmd":"touch","type":"Down",…}` \| `{"id":"1","status":"ok"}` \| \| `{"id":"7","cmd":"button","button":"notARealButton"}` \| `{"status":"error","message":"parse error: unknown variant `notARealButton`…"}` \| Because the return type was `void` there was no ack to check and no error to surface, so `gesture-tap` ended in an unconditional `return { tapped: true, timestampMs }`. Repro: shut a simulator down underneath an open session. The server sends no reply at all and `gesture-tap` still reports `{ tapped: true }`. Every transport failure — that one, a dead socket, a wedged device — looks identical to success from the agent's side. This is argent's own bug, and it is what turns an Apple regression into a silent one: awaiting the reply would have made #932 an actionable failure on day one instead of a silent no-op. ## The fix `sendCommand` now returns a promise that resolves on `{"status":"ok"}` and rejects when: \| condition \| `error_code` \| \|---\|---\| \| server answers `status: "error"` \| `SIMULATOR_COMMAND_REJECTED` \| \| socket fails or closes with the command in flight \| `SIMULATOR_COMMAND_TRANSPORT_FAILED` \| \| reachable server never answers (5s) \| `SIMULATOR_COMMAND_ACK_TIMEOUT` \| Every WebSocket call site awaits it: `gesture-tap`, `gesture-swipe`, `gesture-custom`, `gesture-pinch`, `gesture-rotate`, `button`, `rotate`. Two details of the wire protocol shaped the implementation: - A success reply echoes the command id; an error reply carries no id at all. So a rejection can only be matched positionally, against the oldest outstanding command. Since every send is awaited, that is normally the only one in flight. - The abort paths in `gesture-swipe` / `gesture-rotate` lift the finger before unwinding, and that lift is now fallible. It is therefore best-effort: a refused lift rides along as the `AbortError`'s `cause` rather than replacing it — matching the existing pattern in `gesture-drag`, because `format-error.ts` and `cdp-session.ts` key on `err.name === "AbortError"`. ## Verified on a real simulator Before, after shutting the device down mid-session: `{"tapped": true}`. After: ``` result: {"tapped":true,"timestampMs":…} ← healthy tap, unchanged --- simulator shut down underneath the open session --- tap FAILED as it should: code: SIMULATOR_COMMAND_TRANSPORT_FAILED msg : simulator-server did not accept the 'touch' command: connect ECONNREFUSED 127.0.0.1:62818. The command was NOT delivered to the device. … ``` The failure signal is fully populated for telemetry (`error_kind: "network"`, `network_failure: "connection_reset"`, `failure_command: "simulator_server"`). ## Cost An ack is a localhost round-trip behind an already-open socket: 0.06ms p50 / 0.17ms max, measured over 200 sends against a booted iOS sim. Awaiting one per event adds a flat ~30ms to a swipe of any duration — against the 36–142ms of `setTimeout` skew the 16ms frame loop already carries: \| `durationMs` \| wall before \| wall after \| added \| \|---\|---\|---\|---\| \| 200 \| 236ms \| 266ms \| +30ms \| \| 500 \| 559ms \| 591ms \| +31ms \| \| 1000 \| 1142ms \| 1178ms \| +35ms \| Flagging it explicitly since the swipe cadence is deliberately tuned. If reviewers would rather keep the cadence byte-identical, the alternative is to pipeline within a gesture and await all acks at the end; that costs more code and defers failure to the end of the gesture, so I kept the simpler per-send await. ## Deliberately out of scope - The MoQ (`ios-remote`) transport is unchanged — it has no equivalent ack. - The stdin `pressKey` channel, used by `keyboard` and paste's ⌘V, is untouched: simulator-server never replies on stdin, so it has the same blind spot with no local remedy. The WebSocket protocol does expose an acked `key` command (`{"cmd":"key","direction":"Down","code":4}` → `{"status":"ok"}`), so moving those onto it would close the gap — a separate, riskier change to the text-entry path that deserves its own PR. ## Tests `packages/tool-server/test/simulator-command-ack.test.ts` — 8 tests against a fake `/ws` server that mirrors the real wire format, all 8 fail on `main`: ack resolves, `status: error` rejects, socket death rejects in flight, silent server times out, no phantom success, `gesture-tap` throws instead of claiming a rejected tap, and the aborted swipe still reports `AbortError`. Full suite: 369 files / 4840 tests pass, `npm run lint` clean, `tsc --build` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_011bEvNDV7o3CpKdFmzjVBYF <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * New Features * Simulator commands now confirm successful delivery before completing. * Added clear errors for rejected commands, connection failures, and acknowledgement timeouts. * Bug Fixes * Button, rotation, tap, swipe, pinch, and custom gestures now report failures instead of silently succeeding. * In-progress gestures handle connection interruptions and cleanup failures more reliably. * Tests * Added coverage for command acknowledgements, rejections, timeouts, transport failures, and gesture error handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: latekvo <f.kaminski2000@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-31 18:31:53 +02:00
filip131311	b835de2326	fix(packaging): ship one shared copy of the Android screen-sharing agent (#983 ) ## Summary Fixes the biggest chunk of #980: the published package shipped four byte-identical copies of the Android screen-sharing agent (`resources/android/`: jar + per-ABI `.so`, ~12.7 MiB each) — one next to each platform's simulator-server binary, because the binary resolves the agent relative to its working directory and the spawn set cwd to the per-platform dir. This ships one shared copy at `bin/resources/android/` and points the spawn cwd at the bin root instead. Package size (built from this branch, same artifact set as 0.22.1): \| \| before \| after \| \|---\|---\|---\| \| unpacked \| 115.5 MB \| 76.4 MB \| \| packed \| 37.6 MB \| 25.4 MB \| 76.4 MB is under npmmirror's 83886080-byte unpacked-size cap (the limit #980 trips — note it applies to `dist.unpackedSize`, not the tarball). Already-published oversized versions are immutable, so the mirror also needs the `cnpm/unpkg-white-list` entry to sync the backlog; this PR keeps future versions under the cap. ## Changes - `scripts/download-simulator-server.sh` extracts the agent tarball once, to `bin/resources/android/`, instead of next to each binary. - `bundle-tools.cjs` copies that single shared directory into the package. - `@argent/native-devtools-ios` replaces `simulatorServerBinaryDir()` with `simulatorServerRunDir()`: the bin root when the shared `resources/android/` exists there, falling back to the per-platform dir so an `ARGENT_SIMULATOR_SERVER_DIR` override pointing at a pre-dedup layout keeps working. - The blueprint spawns simulator-server with `cwd: RUN_DIR` (binary path stays absolute; the agent lookup is the only cwd-relative path in the simulator-server crate — audited: every other fs path there is a tempdir, `dirs::cache_dir()`, or caller-supplied). No symlinks, no postinstall scripts — plain files plus a different cwd, so nothing changes for npm/pnpm/yarn/bun or Windows. ## Testing - Unit: resolver tests updated + new shared-layout case; blueprint test asserts the spawn cwd. - Packaging: ran the modified download script + `npm run pack` end-to-end; tarball contains exactly one `bin/resources/android/` (94 files vs 112). - Live: branch tool-server with `ARGENT_SIMULATOR_SERVER_DIR` pointed at the new layout — iOS simulator (screenshot) and Android emulator (boot-device, screenshot, gesture-tap) green; `lsof` confirms both spawned simulator-servers run with cwd = bin root, where `resources/android/<jar>` and each per-ABI `.so` resolve (verified present at those exact relative paths). - Physical Android phone (Galaxy S24, the one path that actually pushes the agent): live-tested — the `android_device` controller resolved the agent from the shared layout, pushed it, and connected ("All agent connections established successfully"); screenshot and tap through the on-phone agent both green, spawn cwd confirmed as the shared bin root. No docs update needed: internal packaging/layout only — no MCP tool, CLI, config, or flow-file surface changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01UGAsAvYBJ3Jy9DKLaX9aoZ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Bug Fixes * Improved simulator-server resource handling by using one shared screen-sharing resource location across host platforms. * Added a warning when required shared resources are unavailable. * Ensured simulator-server processes start from the correct runtime directory. * Documentation * Clarified simulator-server runtime and shared resource behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-08-31 14:49:11 +02:00
Ignacy Łątka	2cc1f5281c	test(android-profiler): drop an assertion that cannot fail (#933 ) Audit of every assertion in the repo that pins shipped agent-facing text, looking for more of the `/restart/i` shape that reddened main in #871 — an assertion satisfiable only by making the product string restate itself. The class is essentially absent. One instance found. ## The one `tool-server/test/android-perfetto/hang-stacks-offcpu.test.ts` ```ts expect(out).toMatch(/No usable on-CPU stack samples/); // L156 // The thread was on-CPU/executing — this is genuine CPU work, not a wait. expect(out).toMatch(/on-CPU/); // L158 ← removed expect(out).toMatch(/executing/); ``` `/on-CPU/` is a substring of the literal asserted two lines above, so `L156 ∧ L158 ≡ L156`. It cannot fail while L156 passes. Mutation-checked rather than argued: changing the pipeline's sentence from `_No usable on-CPU stack samples were captured…` to `_No usable stack samples were captured…` still fails the test — ``` × explains an empty sample set as un-unwound CPU work when the thread was executing AssertionError: expected ... to match /No usable on-CPU stack samples/ ``` — so nothing this line was catching goes uncaught. `/executing/` is not implied and stays; the comment above it still reads correctly. ## What the audit covered, and what it found instead \| stage \| count \| \|---\|---\| \| assertions pinning shipped agent-facing text \| 707 \| \| …in the `/restart/i` shape (short generic literal beside stronger siblings on the same value) \| 44 \| \| …surviving hand-verification \| 1 \| The 43 rejected are doing real work. A sample, so the reasoning is auditable: - `"844x390"` / `"390x844"` — dimension-mismatch test; the point is argument order. - `"(2×)"` / `"(1×)"` — different dedup counts. - `/wait/` beside `/No on-CPU stack samples/` — the sibling literal contains no "wait"; it pins the wait-vs-CPU explanation. - `"localhost:8081"` / `"127.0.0.1:8081"` — two hosts the interceptor must filter. - metro `discovery.test.ts` `"Do not retry in a loop"` — a different test from the one asserting the full sentence. More importantly, the 707 is not a pile of redundancy. It is mostly the only way to test a function whose output is a string — `formatTraceFreshness(NOW - 560MIN, NOW)` can only be checked by asserting `/5 hours ago/` — plus assertions that are load-bearing in their own right, e.g. `expect(description).not.toContain("hunter2")` in `configuration-core/tests/secrets.test.ts`, the sole guarantee a secret value never reaches a source description. Stripping the category wholesale would empty 74 tests into unconditional passes. ## Two adjacent findings, not changed here - `tool-server/test/native-profiler-missing-trace.test.ts:90` — `expect(result.report).toMatch(/missing\|not found\|unreadable/i)` is meant (per its comment) to check the wording tells the user the file is missing rather than the export empty. It passes vacuously: the fixture is named `missing_cpu.xml`, so `/missing/i` matches the filename the assertion two lines up already pins. It enforces nothing. The fix is a discriminating regex or a renamed fixture, not deletion. - `tool-server/test/capability.test.ts:58-59` — `toContain("demo-tool")` and `toContain("android")` are both substrings of `"tools/demo-tool/platforms/android.ts"` asserted at L60, so they are provably implied. Left in place deliberately: the test is named "composes a uniform message with toolId, platform, and the file", and deleting them would make the name over-claim relative to what the body checks. ## Verification Branched off `366f17c61`. \| gate \| result \| \|---\|---\| \| `vitest run` (tool-server) \| 358 files, 4401 passed, 1 skipped, exit 0 \| \| `tsc --build` \| clean \| \| `typecheck:tests --workspaces` \| clean \| \| `eslint . --max-warnings 0` \| exit 0 (with `npm ci` in `packages/docs`, as CI does) \| \| `prettier --check` \| clean \|	2026-08-31 12:51:26 +02:00
Juliusz Wajgelt	bf38e9069b	fix(sim-remote): pass the udid before -b in simctl bootstatus (#982 ) ## What `simctlBootstatus` in `packages/tool-server/src/utils/sim-remote.ts` built the command as `simctl bootstatus -b <udid>`, placing the flag before the device. simctl's grammar puts the device first — `bootstatus <udid> -b`. ## Why Newer Argent Cloud deployments are more faithful to how the local `simctl` handles its arguments, so they now reject `-b` in that position as an unknown flag. The result is that every remote boot fails, on every device: ``` [tool: mcp__argent__boot-device] -> invalid option -- b ``` The local `xcrun` path already used the correct order (`boot-device.ts:461`), so this only affected sim-remote / Argent Cloud. ## Notes for the reviewer - I checked the other `sim-remote` wrappers for the same flag-before-udid shape. `injectDylib` is the only other one with an optional flag and it already appends after the udid, so `bootstatus` was the sole offender. - `packages/tool-server/test/boot-device.test.ts` passes (23/23), but it only exercises the local `xcrun` path — there is no existing test for the remote wrapper, so this fix is not covered by tests. It's worth a manual check against a Cloud device before merging. - `tsc --noEmit` reports pre-existing errors in this package from a stale `@argent/registry` build; `sim-remote.ts` itself is clean and none of them are related to this change. - No docs update needed: this is an internal argument-construction bug with no user-facing surface change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Bug Fixes * Fixed simulator boot status checks by correcting the order of command-line arguments. * Ensured the simulator identifier is processed correctly when the optional boot flag is used. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-08-31 11:52:20 +02:00
filip131311	23b9a07760	feat(mcp): auto-describe — element tree after interaction tools + relaxed tapping rule (#958 ) ## Why The rule's `<tapping_rule>` ("Before every tap, you MUST call a discovery tool") makes agents loop `tap → describe → tap → describe`: over 35 baseline runs, `describe` was called 90 times against 86 taps — one round-trip per tap, each a full model turn. The screen after an action is already captured by the auto-screenshot; the element tree is the only thing the agent still had to fetch. ## What - MCP (`packages/argent-mcp`): after the action settles, call `describe` and append `--- Elements after action (describe) ---` + the tree to the tool result. Follows the same tool allow-list as the auto-screenshot, skips `describe` itself, and sits inside the non-secret branch so a `{{secret:…}}` value never re-enters context as text. Spy-logged as `auto_describe` (duration, chars). MCP `instructions` reworded. - Flag `disable-auto-describe` (opt-out, mirrors `disable-auto-screenshot`). Auto-describe has its own tool list (`AUTO_DESCRIBE_TOOLS`) and its own branch, so the two features are independent; the secret-placeholder skip and the screen-idle wait cover both. - Rule + skill: `tapping_rule` now says take coordinates from the returned tree, call a discovery tool only when there is no fresh tree for the screen (first action on a screen, after a wait, target not listed); the two-failed-taps rule and the "never from pixels" prohibition stay. `argent-device-interact` best-practice #6 and the general-rules line updated to match. - Docs: `reference/configuration.mdx` flag row, `features/interacting-with-apps.mdx` discovery section. - Tests: `autoDescribeEnabled` (global / project scope) and `shouldAutoDescribe`. ## Evidence A/B with `claude -p --model sonnet`, one build (variant env-gated during the experiment), two sandbox projects differing only in rule/skill text and the toggle; 7 tasks × 2 arms × 5 reps = 70 runs on the iPhone 17 Pro simulator (iOS Settings ×3, Expo `my-app` ×2, Contacts create verified in sqlite) and the Electron testbed. \| \| main \| this PR \| Δ \| \|---\|---\|---\|---\| \| Task success \| 35/35 \| 35/35 \| = \| \| Turns / run \| 10.4 \| 8.2 \| −21% \| \| Wall / run \| 36.5 s \| 28.0 s \| −23% (new won 28/34 pairs) \| \| Cost / run \| $0.28 \| $0.23 \| −17% (27/34) \| \| `describe` calls / run \| 2.2 \| 0.24 \| −89% \| \| Cache-read tokens / run \| 622k \| 421k \| −32% \| (34 clean pairs; one pair excluded for external interference on the simulator — with it included: wall −5%, cost −10%, turns −16%.) Every iOS task moved 15–30% in favour of the new arm; Electron was neutral (both arms describe once after `boot-device`, then a single `run-sequence`). The appended tree costs ≈2.4k chars (~700 tokens) and a median 73 ms per action; per-turn latency was unchanged, the saving is the removed model round-trip. Full report: https://claude.ai/code/artifact/279528cf-ea92-4f52-9276-4f0475ba3e3c ## Caveats - Sonnet only, short tasks, small screens. On feed-heavy apps with 10–20k-char trees the per-action overhead grows; a follow-up could cap or filter the appended tree. - Flags are read once at MCP start (same as `disable-auto-screenshot`), so toggling needs an MCP restart. ## Checks `vitest` argent-mcp (85) + configuration-core (122) green; `eslint --max-warnings 0` on changed packages; `prettier --check .`; `npx docusaurus build` green. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Interaction tools now return an accessibility element tree alongside the latest screenshot. - Agents can use fresh element data for follow-up actions without an additional discovery step. - Configuration - Added the `disable-auto-describe` feature flag to turn off automatic element trees. - Documentation - Updated interaction guidance and configuration references to explain the new behavior and flag. - Bug Fixes - Secret-placeholder handling now correctly accounts for both screenshots and element trees. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-08-28 17:20:51 +02:00
filip131311	e7c6f3324c	fix(tool-server): frame device sockets on \n only — U+2028 in a label stalled describe for 15 s (#975 ) ## Problem `describe` returned an empty tree after ~15 s whenever the screen had an accessibility label containing U+2028 (LINE SEPARATOR) — e.g. any Element/Matrix reply preview, or rich text pasted from the web. Nothing was wrong on the device: ax-service answered in ~170 ms with a full, valid tree. The tool-server framed the ax-service socket with `readline.createInterface`, which splits on the four ECMAScript line terminators (`\n`, `\r`, U+2028, U+2029). JSON permits U+2028 unescaped inside a string and `NSJSONSerialization` emits it raw, so one valid frame became three fragments, none parsed, each was dropped by `catch { return }`, and the pending RPC waited out its 10 s timeout — then the native-devtools fallback waited out its 5 s. On top of that, the timeout was reported as "this simulator was not booted through argent … call boot-device with force=true", sending the agent to reboot a healthy simulator. Minimal repro (no device): `JSON.stringify({label:"a b"})+"\n"` through `readline` → 2 lines, 0 parsed. ## Fix - `utils/ndjson-socket.ts` (new): `attachNdjsonReader` frames on the `\n` byte only (`0x0A` can never occur inside a multi-byte UTF-8 sequence, so this is exact) and reports every unparseable frame to stderr instead of swallowing it. - Wired into the three device-facing JSON sockets: `ax-service.ts`, `native-devtools.ts`, `android-devtools-client.ts` (the Android helper's `org.json` and the iOS dylib's `NSJSONSerialization` both emit U+2028 raw, so all three were exposed). The two remaining `readline` uses read process stdout for readiness markers — no JSON, no user text — and are left alone. - `describe/platforms/ios/index.ts`: a throw from `axApi.describe()` (timeout, RPC error) now carries the error message in the hint and uses the resolved api's real `degraded` flag, instead of being reported as the boot caveat. Separate commit so it can be reverted independently. ## Verification - 6 new unit tests for the reader (U+2028/U+2029 in a string, chunk split mid-multibyte char, several frames per chunk, CRLF/empty lines, dropped-frame reporting, trailing frame on `end`) and 3 for the hint (`describe-tool.test.ts`). Full tool-server suite: 4816 passed. - Live A/B on iPhone 16 / iOS 18.5, Safari page with `<meta charset=utf-8>` and a U+2028 inside a `<p>`: - argent 0.22.1 (`main`): `describe` 12.6 s, `source: native-devtools`, web text missing, false "not booted through argent" hint. - This branch (`dist` against installed binaries): 50 ms, `source: ax-service`, label `"In reply to Dave Loving it so far"` present, no hint, 0 dropped frames logged. ## Docs No docs update needed — no tool, CLI, config or flow surface changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VwqqHRhfuHXbKr6LYskxhJ	2026-08-28 17:07:15 +02:00
Ignacy Łątka	43b7d79a2a	fix(boot-device): stop passing -crash-report-mode to emulators that reject it (#557 ) ## Problem `boot-device` cannot launch an Android emulator on builds that do not accept `-crash-report-mode`. The emulator rejects the unknown option and exits before the device ever comes up: ``` $ argent run boot-device --args '{"avdName":"Pixel_6_API_34"}' emulator binary exited with code 1 before the device booted ``` ## Cause `-crash-report-mode` is undocumented and missing from many emulator builds, so #306 added a feature-detect gate: ```ts const crashReportArgs = (await emulatorSupportsFlag("-crash-report-mode")) ? ["-crash-report-mode", "never"] : []; ``` That commit was a pure addition. It never removed the flag from the shared `LAUNCH_HARDENING_ARGS` constant, which still reads: ```ts const LAUNCH_HARDENING_ARGS = [ "-no-boot-anim", "-netfast", "-crash-report-mode", "never", "-no-metrics", ] as const; ``` So the gate is dead code. Every launch gets the flag unconditionally through the constant, and builds that do support it are handed it twice. ## Proof On Android emulator 36.1.9.0, whose `-help` does not list the flag: ``` $ emulator -version Android emulator version 36.1.9.0 (build_id 13823996) $ emulator -help \| grep -c -- -crash-report-mode 0 $ emulator -help \| grep -c -- -no-metrics 1 ``` `-list-avds` and an unknown `-avd` both short-circuit before option validation, so the rejection only surfaces once a real AVD resolves. Against one, with everything else held equal: ``` $ emulator -avd Pixel_6_API_34 -check-snapshot-loadable default_boot \ -crash-report-mode never INFO \| Found systemPath .../android-34/google_apis/arm64-v8a/ unknown option: -crash-report-mode please use -help for a list of valid options # exits, never boots $ emulator -avd Pixel_6_API_34 -check-snapshot-loadable default_boot INFO \| Ok: System requirements to run avd: `Pixel_6_API_34` are met ``` The launch dies at option parsing, after the AVD resolves and before any device work. `hardeningArgs` reaches the snapshot probe, the hot boot and the cold boot alike, so on such a build all three spawns abort. ## Fix - Drop `-crash-report-mode never` from `LAUNCH_HARDENING_ARGS`, leaving the `crashReportArgs` gate as the only source of the flag. - Spread `crashReportArgs` into the `-check-snapshot-loadable` probe's `extraArgs`. The probe is itself an emulator spawn, so it gets the same crash-dialog suppression as the two boots; once the flag left the constant it would otherwise have been the only site missing it. This one is not a snapshot-parity requirement - unlike `-noaudio` and `-netfast`, `-crash-report-mode` selects crash reporting rather than qemu devices, so a mismatch would not invalidate the saved snapshot. - Leave the gate where it is, after the already-running reuse fast path, so the `emulator -help` probe is still skipped when nothing will be spawned. - Fix the prose that had drifted: the comment above `LAUNCH_HARDENING_ARGS` still listed `-crash-report-mode` among its flags and claimed they all need no detection, and both it and the `emulatorSupportsFlag` docstring said the flag is "only present in newer emulator releases (~36.x and late 35.x)" - which 36.1.9.0 not having it contradicts. `bootAndroidImpl` is the only emulator launch path in the repo, and all three of its spread sites are covered. Making the gate load-bearing put weight on a probe that was only ever advisory, so `emulator -help` detection is hardened alongside it: - `emulatorSupportsFlag` memoized its verdict for the process lifetime even when the probe learned nothing - a transient spawn failure, a timeout, or a silent zero-exit run pinned "unsupported" until restart. An empty capture is now reported to stderr and left uncached, so the next boot retries. - A killed probe is no verdict at all: the flag may sit in the part of the listing the child never transmitted, so partial output from a timeout is discarded rather than read as absence. - The three emulator probe spawns (`-help`, `-list-avds`, `-check-snapshot-loadable`) pass `killSignal: "SIGKILL"`, matching `runAdb`. `execFile`'s default SIGTERM can be ignored by an emulator wedged in Qt/graphics init, which leaves the promise unsettled past every caller's deadline. ## Tests A new `-crash-report-mode feature gate` block in `packages/tool-server/test/boot-device-hotboot.test.ts` asserts the real spawned argv and the probe's `extraArgs`: - gate false: zero occurrences at the probe, hot-boot and cold-boot sites; - gate true: exactly one occurrence at each, always followed by `never`. The count matters - asserting mere presence passes on the double-pass code and would guard nothing; - plus a test that `-help` detection is skipped entirely on the already-running reuse fast path, pinning why the computation sits where it does. `emulatorSupportsFlag` is stubbed in that file's existing `vi.mock("../src/utils/adb")` factory, because the real one memoizes its `-help` result per binary for the process lifetime and both branches could not otherwise be exercised in one file. The two pre-existing launch-hardening assertions were updated to the four remaining flags. `packages/tool-server/test/emulator-supports-flag.test.ts` is new and covers the detection hardening: which outcomes are memoized (a verdict read from a non-zero exit's captured listing, and a normal zero-exit run) and which are not (spawn failure, timeout with empty streams, zero exit with no listing, timeout with partial output), the stderr warning, and `SIGKILL` on all three spawns. Discrimination check - both source changes reverted (flag back in `LAUNCH_HARDENING_ARGS`, `crashReportArgs` dropped from the probe's `extraArgs`), tests kept: ``` Tests 5 failed \| 26 passed (31) x omits the flag from the snapshot probe and the hot-boot spawn when the emulator rejects it x omits the flag from the cold-boot spawn when the emulator rejects it x passes the flag exactly once to the snapshot probe and the hot-boot spawn when supported x passes the flag exactly once to the cold-boot spawn when supported x keeps exactly one flag occurrence on the probe-reject cold fallback when supported AssertionError: expected 1 to be +0 (x2 - the flag reaches a site that must not see it) AssertionError: expected 2 to be 1 (x3 - the flag arrives twice) ``` Fix restored: `Tests 31 passed (31)`. ## Verification - `npm run build` (`tsc --build`): clean. - `npx vitest run` in `packages/tool-server`: clean. - `npm run typecheck:tests --workspaces --if-present`: clean. This is the CI gate that plain `tsc` + vitest does not cover. - `npx prettier --check .` and `npm run lint` (`eslint . --max-warnings 0`): clean. - End to end on a real emulator (36.1.9.0, the build that rejects the flag). The gate reads the binary correctly: ``` emulatorSupportsFlag(-crash-report-mode) = false emulatorSupportsFlag(-no-metrics) = true emulatorSupportsFlag(-no-boot-anim) = true ``` and `boot-device` against `Pixel_6_API_34` returns `{"platform":"android","serial":"emulator-5554","booted":true}` in 4.8 s. The live process argv shows the flag dropped and the four remaining hardening flags intact: ``` qemu-system-aarch64 -avd Pixel_6_API_34 -force-snapshot-load -no-snapshot-save \ -gpu auto -noaudio -no-boot-anim -netfast -no-metrics ``` `-force-snapshot-load` means the hot-boot path ran, so the snapshot probe accepted its flag-free argv too. ## Docs No docs update needed: `-crash-report-mode` is an internal launch argument that no reference page, config key or tool parameter exposes.	2026-08-28 12:49:10 +02:00
Jakub Piasecki	d4a712c401	refactor(gesture): rename the momentum-free flag to momentum (#732 ) `settle: true` collided with the runner's other `settle` — waiting for the tree to quiesce — which is the very thing a swipe step does after every dispatch, whichever way the flag is set. One sentence of the flow skill used both senses at once. This names the physics instead: `momentum: false` is the momentum-free gesture, and the natural fling is the default the prose already described. ### Renamed \| surface \| before \| after \| \| --- \| --- \| --- \| \| `swipe` flow directive \| `settle: true` \| `momentum: false` \| \| `gesture-swipe` param \| `settle` \| `momentum` \| \| `gesture-drag` param \| `settle` \| `momentum` \| The directive passes the flag straight through to the tools, so all three move together. `momentum: true` is the default and normalizes to absent, mirroring what `settle: false` did — parse/serialize stay exact inverses. ### Also - Recording summary label `(settle)` → `(momentum-free)` - Internal constants → `MOMENTUM_FREE_EASE_EXPONENT`, `MOMENTUM_FREE_MIN_STEPS` - `flow-run` tool description and `argent-create-flow` skill updated ### Testing `tsc -b`, eslint (`--max-warnings 0`) and prettier clean; `test/flows` + `test/tools` pass 671/671, including a new case covering the legacy-spelling rejection.	2026-08-28 09:55:47 +02:00
Jakub Piasecki	e314b3ef4a	feat(flow): swipe directive (#520 ) Add one semantic finger flick for dismissing cards, paging carousels, opening drawers, and other gesture-driven actions distinct from goal-seeking scroll-to. Support bare directions plus from with exactly one of direction, to, or by, optional settle, and positive finite duration. Resolve selector or normalized-point anchors through the shared target path. Direction uses the Maestro geometry presets and now rejects anchored gestures that would collapse or travel opposite to the requested direction. Relative by deltas saturate supplied axes to [0, 1] and reject any effective zero or sign reversal. Resolved starts must remain on-screen, while explicit endpoints stay author-controlled. Keep parse and serialization invariants aligned for programmatic steps, improve unknown-key hints and selector diagnostics, surface swipe targets in run reports and recording summaries, share positive-millisecond validation, and reuse the common flow test harness. Dispatch gesture-swipe on touch platforms and gesture-drag on Chromium, reject touch directives on Vega, and document when to prefer scroll-to or a raw system-edge gesture.	2026-08-28 09:55:47 +02:00
Hubert Gancarczyk	2740a3ec38	chore(knip): clear the parked dead-code backlog and drop the ceiling to 0 (#770 ) > Built on top of #769, so this PR carries that commit too — the workflows only run for PRs targeting `main`, which is why it is based there rather than on the branch. #769 on its own is the minimal, low-risk fix for the currently-red job; if it merges first this shrinks to its own commit, and if this one merges first it closes #769 as well. ## What this does #572 landed the gate with 215 findings parked under `--max-issues`. That bounded the backlog but left it in place, and the first thing to touch it (#663) pushed the total to 218 and turned the job red. This clears the backlog and sets the ceiling to 0, so the next unused export fails the job instead of consuming headroom nobody knew was there. \| \| before \| after \| \| --- \| ---: \| ---: \| \| Unused exports \| 47 \| 0 \| \| Unused exported types \| 156 \| 0 \| \| Unused exported class members \| 12 \| 0 \| \| `--max-issues` \| 215 \| 0 \| ## Scope check first Every workspace holding a finding is `private: true`. `@swmansion/argent` — the one published package — had zero, so nothing here moves a public API. ## Two shapes of fix Still referenced inside its own file → drop `export` (146 types, 36 exports). The declaration stays put; it just leaves the module's public surface. `tsc` keeps emitting these into the `.d.ts` as local declarations that the exported signatures reference. For the 146 types that is the whole story: they are erased, so the emitted `.js` is byte-identical. For the 36 values it is not. Dropping `export` from a `const`/`function`/`class` removes its `exports.X` binding from the emitted `dist/.js` — 36 declarations across 26 files here. `packages/tool-server/dist/utils/device-info.js` no longer carries `exports.REMOTE_PREFIX` or `exports.VEGA_SERIAL_PREFIX`; `telemetry/dist/posthog.js` loses `exports.POSTHOG_PROJECT_TOKEN`; in the ESM-emitting `argent-installer`, `isTempRunnerPath` / `resolveLocalArgentDir` / `PACKAGE_ROOT` leave the module namespace object entirely. Nothing in the repo or in the `packages/argent-private` submodule at the pinned revision reads any of them by path, so nothing breaks. Flagging it because a `require()` of a built `dist/.js` loses a dropped `export` exactly the way it lost the deleted `__resetAndroidDevtoolsInstallCache` this branch had to restore — so this half of the diff does deserve runtime scrutiny, not a skim. Referenced nowhere → delete. Orphaned test seams (`__test`, `_testValidators`), stale compatibility re-exports, and a few real orphans. The ones worth a reviewer's eye: - `localSimctl` — the `SimctlBackend` strategy is only ever constructed with `remoteSimctl`. What keeps the local iOS impl off it is the handler signature: `buildIosLaunchHandler` / `buildIosRestartHandler` read `services.nativeDevtools`, and `ios-remote` is the only platform that declares that service, so the local impl — which resolves native-devtools per device inside its own handler — has nothing to hand them and shells out to `xcrun` itself. - `sendCharInsert` — removing the `__sendCharInsert` test alias exposed the underlying helper as unreferenced. It is superseded, not unwired: the Chromium keyboard impl dispatches its own `type: "char"` event inline (`keyboard/platforms/chromium.ts:58`), which is exactly what this helper did. Chromium typing is unaffected. - Error metadata fields — `ServiceNotFoundError.serviceId`, `ToolNotFoundError.toolId` and their two siblings were never read. The `e.serviceId` in `registry-error-events.test.ts` is a field on the event object the handler pushes, not on the error class; the `err.toolId` in `http.ts` is `NotImplementedOnPlatformError`, a different class that keeps its fields. Flagging these in case they are wanted as deliberate diagnostics. Deleting `SourceMapsRegistry.toGeneratedPosition` / `findMatchingSource` cascaded into their only helper (`buildSourceCandidates`) and then into `projectRoot`, which had no remaining reader — so the constructor parameter goes too. `StubSourceMapsRegistry` existed only to pass `super("")`, so it goes with it. ## The three cross-workspace members `ArtifactStore.register`, `ArtifactStore.list` and `TypedEventEmitter.off` are all called from other workspaces — the artifact route and screenshot tools in `tool-server`, and `registry-listener` in `telemetry`. They are reported unused anyway, and the reason is the unbuilt tree the gate runs against: every workspace resolves `main`/`types` to a `dist/` that does not exist, so `@argent/registry` resolves to nothing and the cross-workspace edge never forms. Build the tree and all three findings disappear on their own; a control member that really is dead is still reported in the built run, so the pass is live either way. Nothing about this is specific to `classMembers` — every cross-workspace reference is invisible the same way. Each is exempted at the declaration with a `@public` JSDoc tag rather than by name in `knip.jsonc`, for one reason: scope. `ignoreMembers` matches a name across the whole workspace, so it would also hide a future dead `register`/`list`/`off` on any other class in `packages/registry`; `@public` binds to the member it is written on. What `@public` does not buy is a staleness check, and neither does `ignoreMembers`. Measured on a clean unbuilt worktree: a bogus `ignoreMembers` name and a redundant `@public` each leave the run at exit 0 with no hint, while a bogus `ignoreDependencies` name exits 1 with `Remove from ignoreDependencies`. `treatConfigHintsAsErrors` is live and simply has no producer for either exemption style — so when one of the three stops being earned, nothing says so, and the JSDoc at each member is the only record a re-audit has. Each one names its callers, and `off` records how to re-derive them (rename the member and read the `Property 'off' does not exist` errors; grepping `.off(` over-reports, since 14 of the 38 call-site hits are Node `EventEmitter`s). One correction to the commit message on that change: it says 35 hits where the exact count is 38 — 14 Node `EventEmitter`, 24 `TypedEventEmitter` (21 production, 3 in a test). The 14 and the 21 are right. `knip.jsonc`, `CONTRIBUTING.md` step 5 and the workflow's failure-explanation step all described a parked backlog and a ceiling to stay under. All three are rewritten for a gate that must simply come back empty, and all three now name the `@public` escape hatch — the remedy for a symbol whose only caller is out of knip's reach. ## Verification Run against a clean worktree with no build output, the way CI counts: - `npm run knip` — exit 0, prints nothing. Both passes empty, no config hints. - `npx tsc --build` — clean. - `npm run lint` (`eslint . --max-warnings 0`) — clean. - `npx prettier --check .` — clean. - `npm test --workspaces` — tool-server 329 files / 3857 passed; registry, telemetry, update-core, tools-client, mcp, cli green. - `npm run typecheck:tests --workspaces` — 13 workspaces define the script and all 13 are clean, but the command itself exits 1: npm errors `Missing script: "typecheck:tests"` for the three that do not define it rather than skipping them. Worth knowing before re-running this bullet and reading the exit code as a failure. End-to-end on a tool-server built from this branch, driven over its HTTP API: - Chromium (Electron smoke app) — `describe`, `screenshot`, `gesture-tap` (counter advanced 0 → 1), `POST /api/clipboard/text` read back out of the renderer, the WS `clipboardSync` command, and `debugger-connect` / `debugger-status` / `debugger-evaluate` (`sourceMapReady: true` with `StubSourceMapsRegistry` gone). - iOS — `boot-device`, `launch-app`, `describe` via ax-service, `screenshot`. - tvOS — `boot-device`, `launch-app` on an Apple TV udid, focus-driven `describe`. The registry snapshot then carries `NativeDevtools:<tvOS udid>`, confirming injection is resolved on tvOS through the local impl. - Android — `boot-device` on a dedicated AVD, `describe` via android-devtools, `gesture-tap`. - Metro source maps — the registry driven against a real 12.5 MB Metro `.map` over loopback, the way `Debugger.scriptParsed` drives it: allowlist passes and `waitForPending()` resolves on both, while retained heap after gc drops from 17.3 MB to 3.6 MB. ## Review follow-ups Five commits on top, one per finding. The base is back on `main`, so the workflows run again. - `docs(launch-app)` — the services comment explained the empty shape through `LaunchAppAndroidServices`, deleted here, and skipped `LaunchAppVegaServices`, which is still there. Same edit in `restart-app/types.ts`. - `docs(registry)` — the `@public` tag on `ArtifactStore.register` named 2 of 7 callers. Derived the full set the way the sibling tag on `off` prescribes: renamed the member, read the 16 `TS2339` errors `tsc --build` reports across flow-visual (5), screenshot (4), native-profiler-stop (2), screenshot-diff (2), native-profiler-analyze, react-profiler-analyze and screen-recording-stop. The re-derivation recipe is recorded too — `.register(` greps as badly as `.off(`. - `docs(knip)` — "the build, the tests and this gate all stay green on a wrong delete" held for argent-private and not for the cross-workspace case that justifies three of the four `@public` tags. Deleting `ArtifactStore.register` gives 16 `TS2339` errors and `9 failed \| 2 passed` in `test/artifacts.test.ts`; only the gate stays green. Split in the workflow comment, the failure annotation and `CONTRIBUTING.md` step 5. - `perf(debugger)` — `doRegister`'s `data:` branch still base64-decoded and `JSON.parse`d a payload it dropped, inside a `catch` that swallows. Both arms returned the same `void`, so the branch collapses to an early return; `scriptUrl` / `scriptId` go with it, since their last reader was the deleted `this.maps.push(...)` and `doRegister` is private. - `test(debugger)` — the allowlist check moved to a top-level early return with nothing pinning it: delete the line and both SSRF files still pass 17/17. Two tests now assert `doRegister` consults it — four rejected URLs reach no `fetch`, and the loopback `.map` Metro emits still does. Re-verified after: `npm run knip` exit 0 and empty on a `tsc --build --clean` tree, `tsc --build`, `eslint . --max-warnings 0`, `prettier --check .` all clean, and `npm test --workspaces` with tool-server at 329 files / 3854 passed. The one red test, `argent-installer`'s `globalPath returns ~/.config/opencode/opencode.json`, fails identically with these commits stashed — it reads the real `~/.config/opencode`, which holds a `.jsonc` on this machine. End-to-end on a tool-server run from this branch over its HTTP API, on every platform: - Chromium* — `launch-app`, `describe`, `screenshot` (returned an artifact handle, `GET /artifacts/<id>` served the 137542-byte PNG back), `gesture-tap` (counter 0 → 1), `debugger-connect` / `debugger-status` (`sourceMapReady: true`) / `debugger-evaluate` (read `taps: 1` out of the page). - iOS — `launch-app`, `describe` via native-devtools, `restart-app`, `screenshot`. - tvOS — `launch-app`, focus-driven `describe` (12 focusables in TVSettings), `tv-remote`, `restart-app`. - Android — `launch-app`, `describe` via android-devtools, `restart-app`, `screenshot`. - Vega — `boot-device` on the `tv` VVD, `describe` via the automation toolkit. - Metro — the real `debugger-connect` / `debugger-status` / `debugger-evaluate` tools against a stand-in Metro (HTTP `/json/list` + CDP WebSocket) emitting three `Debugger.scriptParsed` events: a loopback `*.map` on a counting server, a 1.4 MB inline `data:` map, and `http://169.254.169.254/latest.map`. `sourceMapReady: true`, `loadedScripts: 3`, evaluate 42, the loopback map fetched exactly once and the metadata URL never. A malformed inline payload gives byte-identical output, which is the finding. ### Merge with `main` Putting the base back on `main` made CI resolve the merge, and the Unit Tests job went red on a conflict no side could see on its own: `#771` fixed the same red gate this branch's first commit did, the other way round — it made `component-names.test.ts` import `StrippedName`, `ComponentAnnotation` and `ComponentNameResolution` instead of dropping their `export`. Merged `main` in, reproduced it (`tsc --noEmit -p tsconfig.test.json` → three `TS2459`s) and took main's resolution: the three exports are back. The gate stays green because knip treats test files as entry points, so a type a test imports is not an unused export. All nine workflows are green on the head commit.	2026-08-25 17:27:12 +02:00
filip131311	efb87f1917	chore: bump version to 0.22.1 (#956 ) Bumps every workspace package, `package-lock.json`, and both version fields in `server.json` from 0.22.0 to 0.22.1. - `npm version 0.22.1 --workspaces --no-git-tag-version --workspaces-update=false` - lockfile regenerated on Node 24 / npm 11 (version-only diff, 16 lines each way) - `npm run check:versions` passes No docs update needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01EjrDqoeJa5X8Ry2ND1P714	2026-08-25 14:46:36 +02:00
Jakub Piasecki	2e182f7971	fix(flows): pin iOS tree reads to the launched app instead of auto-resolving every connection (#772 ) ## The problem Flow runs on CI iOS fail with `ViewInspector RPC timed out: Application.getState` at seemingly random steps - mid-flow after typing, on the first read after a fresh launch, in a probe flow 25 s after the original failure - while the app under test is provably healthy: spindumps taken at failure time show its main thread idle in the runloop, display links firing, and the injected devtools queue empty. The same flows pass locally. The chain: argent's iOS injection is simulator-wide (`launchctl setenv DYLD_INSERT_LIBRARIES` inside the simulator's launchd), so every process the simulator spawns afterwards loads the devtools dylib, not just the app under test. The dylib's constructor connects unconditionally (no UIKit or `com.apple.` guard) and the tool-server registers whatever bundle id shakes hands, so Spotlight workers (`mdworker_shared`), background-launched system apps doing first-boot work (`MobileCal`), and bare daemons (nil bundle id, registered as `"unknown"`) all become connections indistinguishable from the app under test. Flow tree reads auto-resolved the target on every read: `queryFullHierarchyTree` passed `undefined` to `resolveNativeTargetApp`, which `Promise.all`s `Application.getState` over every connection. The dylib serves `getState` with a `dispatch_sync` onto the main thread, and a background-launched or suspended process never services its main queue, so its `getState` parks until the 5 s RPC timeout and the `Promise.all` rejects. One poisoned sibling connection fails every tree read in the run; the app under test is never even asked. That one mechanism explains everything the failures did. CI spindumps caught the poisoner red-handed (`MobileCal` parked in `HandleViewInspectorRequest -> CurrentApplicationState`) next to a healthy app. "Permanent wedge" vs "recovery" was just poisoner lifetime: a Spotlight worker exits and its connection drops with it, so reads recover; Calendar lingers and the whole job stays poisoned, fresh app launches and probe flows included. And it never reproduced locally because a warm simulator did its first-boot indexing and migrations long ago, while CI boots a pristine device and does exactly that work during the test window - which also made the failures correlate with typing and tapping purely by timing. ## The fix Pin the target instead of resolving it per read. `runLaunch` records the launched app on the run state once the tree-source gate passes, as `ActionEnv.treeTarget` - a `FlowTreeTarget` carrying the `bundleId`, whether the runner still vouches for it (`pinned`), and whether its probe has ever answered in this run (`probeAnswered`). `fetchFlowTree` threads that target down to `queryFullHierarchyTree`, the only platform source that consumes it. A pinned read talks to the pinned app and nobody else: it never calls `resolveNativeTargetApp` at all, and runs four gates of its own instead (`flow-ios-tree.ts`). A `com.apple.` id is refused first, terminally and before anything is spent - a system app is never the app under test. Then `isConnected`, a map lookup, so a dead pin reports the dropped connection instead of driving env repair from 300 ms read polls, which could otherwise latch the device's process-wide give-up. Then a probe of the pinned app's own `getAppState`, and only that app's: a timeout before this pin has ever answered is ridden out as the cold start it usually is, one after an earlier answer is refused as an app that stopped servicing its main queue. Finally a foreground check on the answered state, as lenient over one app as auto-resolve is, so a flow that left the app fails with the crisp diagnosis instead of describing a screen that is not on screen. A poisoned sibling connection is never touched. The pin is only trusted while the runner can vouch for what is on screen, and it comes off in two tiers. The five `FOREGROUND_CHANGING_TOOLS` (`launch-app`, `restart-app`, `reinstall-app`, `open-url`, `button`) drop the target outright, because the launched app may no longer be on screen at all; `launch-app` and `restart-app` then re-set it from their own `bundleId` arg, unpinned, since they name the app they switched to - which is how a recorded flow switches apps, the recorder keeping them as raw `tool:` steps. Every other raw `tool:` step demotes the pin to an unpinned hint, its effect on the foreground being opaque to the runner; a launch attempt drops the target outright until it succeeds. Either tier also retires a proven tree-outage memo: a verdict proven against the pinned branch's four gates says nothing about the path the next read runs. An unpinned read is the pre-pin behavior with one arbiter added. `resolveNativeTargetApp` fans out over every connection, and only when that fan-out times out - the CI condition this change exists for - does the hint take the read, and only after passing the same terminal `com.apple.` policy gate, the connections check, and a foreground-checked probe of its own. A resolution that answers always wins, including its deliberate "single app but backgrounded" refusal, so the arbiter never overrides a guard that fired: it only rides out a probe a stalled sibling made unanswerable. Nested `run:` fragments share the run state, so they inherit the pin, its demotion and its clearing. E2e flows begin with a `launch` by definition, so every selector read in a CI run happens pinned. Only iOS consumes the target; the other platforms' tree sources are per-device and never auto-resolve. The review rounds also surfaced that the `isInjectableBundleId` "library validation, the dylib can never load" rationale was disproven by this PR's own spindump evidence, so that text - and every other home of the same claim, `unreadableHierarchyReason` and `treeSourceGate` included - is rewritten as the policy it actually is (system apps are never the app under test; their observed connections are background processes whose reads hang or lie) with the gates' behavior unchanged. ## What this doesn't cover Auto-resolve's own fan-out is still all-or-nothing, so every read that reaches it without a hint the gates accept stays poisonable: top-level fragment runs, `describe` without an explicit bundle id, and the stretch after a raw `tool:` step whose hint is refused or cannot answer. Recording (`captureTapSelector`) now passes the session's last `launch` id as an unpinned hint, so it arbitrates the same timed-out fan-out, but it never pins - the recorder cannot vouch for the foreground app the way a `launch:` step can. Closing the rest means hardening auto-resolve itself (exclude connections that don't answer instead of failing the fan-out) and refusing the connections at the source (dylib constructor bail for non-UIKit and `com.apple.` processes, plus a server-side handshake filter to cover already-shipped dylibs). Both are follow-ups. ## Tests Three files, and every behavior below is mutation-proven: disabling the seam, dropping the target from any single call site, removing an invalidation, or removing any one of the four gates each fails at least one dedicated test. The reviewers' repros were turned into tests rather than argued away. `flow-ios-tree-pinned-target.test.ts` works at the tree source. It reproduces the failure shape - a healthy app plus a poisoned `com.apple.mobilecal` sibling whose `getState` only times out - and proves a pinned read probes only the pinned app while an unpinned read still fails. It then covers the states auto-resolve could never reach: the terminal `com.apple.` refusal, including its position ahead of the connections gate so a disconnected system app is not sent into a restart loop; a dead pin, where the dropped connection is reported and no env repair runs; both sides of the `probeAnswered` split; and the foreground guard over the states a real device produces (`inactive` under a permission dialog, `inactive` with no scenes, `background` still holding a foreground-inactive scene, and `background` with none). A second block covers the unpinned arbiter: the hint read on a timeout, refused when its own probe answers from the background, and rethrowing the fan-out timeout when the hint is disconnected, cannot answer, or when the resolution failed for any other reason. `flow-launch-pins-tree-target.test.ts` proves the runner wiring through the real `fetchFlowTree` seam: every read after a `launch` carries the pin across all five read paths (assert, tap, type, rotate, idle), retry polls included; fragments inherit it; a foreground-neutral `tool:` step demotes it and a foreground-changing one that names no app drops it outright; a later launch re-pins on a fresh target with the ride-out re-armed; and a launch-free run keeps the fallback. `flow-tool-step-tree-target-e2e.test.ts` drives `createRunFlowTool` with only `resolveService` stubbed, so the real `fetchFlowTree` and `queryFullHierarchyTree` run. It pins what the two tiers cost against a wedged sibling: the launched app still arbitrating across a foreground-neutral `tool:` step, the outage memo being retired when that step demotes the pin, the `com.apple.` and foreground refusals staying in force through the demote, and a `hidden:` assert failing after the demote instead of going green against a backgrounded app.	2026-08-25 13:26:46 +02:00
Hubert Gancarczyk	50c9580e30	fix(tools): make a schema failure name the parameter that was sent (#718 ) > Stacked on #717. ## The problem An invalid tool call returned Zod's raw issue array. Someone who wrote `flow_name` instead of `name` got back, depending on the tool: ``` Invalid params for tool "flow-start-recording": [{"expected":"string","code":"invalid_type","path":["name"]}] Invalid params for tool "flow-execute": [{"code":"custom","path":["flow_path"],"message":"Pass exactly one flow source: name or flow_path."}] ``` The first names the parameter the tool wanted and never the one the caller sent. The second — flow-execute's, whose `name` is optional so `flow_path` can stand in — names neither: it is anchored on the one source field the caller had no reason to send. Either way the mistake is invisible, and working it out costs a whole turn. ## What changes 1. `describeParamIssues` — one sentence per bad parameter. Missing fields read as `` `name` is required (string) and was not provided`` rather than as a type error about `undefined`. Unknown keys are named and path-qualified, so a key nested in `selector` is not reported as a bare name that contradicts the top-level list printed beside it. The message closes with the caller's own keys — `You sent: \`flow_name\`, \`project_root\`.` — which is what makes the mistake self-evident. Key names only, never values: this string reaches logs, telemetry and the agent transcript, and a params object can carry a secret. Wired into both entry points: the registry's `invokeTool` and the HTTP boundary. 2. `flow-execute` accepts `flow_name` as an alias for `name`. The tool is called `flow-execute`, so "the flow's name" spells itself that way; this is a mistake worth absorbing rather than rejecting. The name is resolved in code rather than by the schema — a Zod `required` failure would once again name only the field it wanted — and the error text names the spellings that are silently discarded (`flowName`, `flow`, `flow_file`), because by then Zod has already stripped them and the caller has no other way to learn their key was dropped. 3. A flow DIRECTIVE passed as `command` points at the tool that records it. `command: "echo"` used to come back as a bare "Tool not found". It now names `flow-add-echo` — and says to call it directly, since routing it through the recorder would write the echo and a `tool: flow-add-echo` step that fails on every replay. The hint distinguishes directives the recorder rewrites from those it stores raw for the polish pass, because promising a rewrite that does not happen sends the author looking for a directive that is not in the file. `wait` and `long-press` get their own answers: neither has a recording tool at all. Only a genuine not-found is rewritten — a tool that ran and failed still reports its own error. 4. A recorder tool as `command` is refused. `flow-add-echo`, `flow-add-step`, `flow-start-recording`, `flow-finish-recording` each mutate the recording itself, and nesting one reported success either way. Each is refused for its own reason: `flow-add-echo` would write the echo and a replay-breaking `tool:` step, `flow-start-recording` would erase this flow at replay, `flow-finish-recording` would end the take it is a step in, and `flow-add-step` cannot record itself. Now nothing is executed and nothing is written. --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-08-25 10:48:49 +02:00
filip131311	efae6a8fb6	feat(tools): opt-in emulator sound on boot-device + `boot-sound` default flag (#813 ) Field report ([Slack thread](https://swmansion.slack.com)): when a session needs to test audio, it is very non-obvious that argent boots the Android emulator muted — the agent (and the user) burned significant time discovering it. The mute default is intentional (several agent-driven emulators all playing sound on the host would be worse), so this PR keeps it and instead makes it discoverable and overridable: ## 1. `sound` argument on `boot-device` (default `false`) - `sound: true` boots the emulator with audio output (drops the mute launch flag). - Applied consistently to the snapshot-loadability probe, hot boot, and cold boot — the qemu-device-topology parity invariant is preserved. A `default_boot` snapshot saved in the other audio mode fails the probe and falls through to a cold boot that re-saves it, so toggling costs one slower boot and nothing else. - iOS/Vega/Electron are unaffected (argent never mutes them); the argument documents itself as Android-only. ## 2. Agent-facing information: argument description only Per the thread, the info lives in the tool argument's description (no skill changes): it states the muted default, why (multiple agent-driven devices sharing the host's speakers), the `force: true` hint for an already-running muted emulator, the cold-boot cost of toggling, and the `boot-sound` flag. The underlying emulator flag is described behaviorally ("muted"), not by name, per our agent-docs convention. Validated that the description alone is sufficient by driving three models through the Cursor `agent` CLI with the schema excerpt and the exact scenario from the field report ("testing notification sound, hear nothing, emulator already booted with defaults"). All three — composer-2.5, gemini-3.7-flash, gpt-5.2 — correctly diagnosed the muted default and produced the exact fix `boot-device({avdName, sound: true, force: true})`, including the weakest model. Transcript excerpts in the PR discussion below if wanted. ## 3. `boot-sound` feature flag `argent enable boot-sound` defaults the `sound` argument to `true`. It only moves the argument's default, read live per call: an explicit `sound: false` on a call still boots muted, and without the flag the default remains muted. Registered in `FLAG_REGISTRY` so `argent enable`/`argent flags` pick it up with no further wiring. ## Tests - New `boot-device-sound.test.ts`: mute-by-default, `sound: true` dropping the mute flag from both probe and hot-boot spawn (parity), cold-boot spawn, flag-driven default, and explicit-`sound:false`-beats-flag. - The three existing Android boot test files now pin `isFlagEnabled` to `false` so a developer's real `flags.json` can't flip the args they assert on. - `tool-server` boot suites 62/62 green, `argent-cli` flags 33/33, `configuration-core` 122/122; lint, `typecheck:tests`, and knip all pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-08-24 14:51:16 +02:00
filip131311	b232114422	docs: audit every comment in src against the code it describes (#931 ) Audits every comment in `src/` and `scripts/` against the code it describes. 462 files, 57 commits, net −8,194 lines. Comment text only — the whole branch is code-identical to `main`. ## Method One subagent per file, strictly sequential. Scope was `src/` + `scripts/` (459 files); three more files were added at the end because they carried dead references of the same kind — two test headers citing design docs that do not exist, and `publish-npm.yml` citing a retired workflow. Each agent verified every comment — line, block, JSDoc, file header, trailing — against the surrounding code and the rest of the repo, following identifiers, paths, tool ids, config keys, env vars and issue links to see whether they still exist and still behave as described. Rules: - A false or misleading comment is deleted, not reworded. If a claim could not be confirmed by reading the source, it went. That is why the deletion count is so much larger than the rewrite count. - Survivors are cut to the shortest form carrying something the code does not already say. Restatement, preamble, hedging, changelog prose and ASCII banners are gone; the non-obvious why stays. - Preserved byte-identical: license headers, pragmas and directives (`@ts-`, `eslint-disable`, shebangs, `/// <reference>`), JSDoc tag tokens, everything inside a string or template literal, and the sole comment inside an otherwise empty block (ESLint `no-empty` counts a comment-bearing block as non-empty). ## Verification Every file passed two independent gates before being recorded as done: 1. `comments-only` — the required check. 2. A second comment-stripping comparator with a proper mode stack, written for this pass because `comments-only`'s flat scanner desyncs on nested template literals and quote-bearing regex literals and then reports comment lines as code changes. Two files hit that false FAIL (`utils/android-profiler/pipeline/index.ts`, `scripts/extract-tools.mjs`); in both the "changed code" it printed was literally `//` lines, and the second checker confirmed the code was byte-identical. After the last file, all 462 changed files were re-checked against `main` with the same comparator, rather than trusting any agent's self-report. 459 code-identical; 2 are non-code (`.svg`, `.md`); 1 intentional.* The intentional one is `packages/argent/scripts/bundle-tools.cjs`: the changed template literal is the comment header of the file it generates, `packages/native-devtools-android/src/bundled-meta.ts`. Fixing only the generated file would have been reverted by the next build, so the generator changed too — and it has been verified to reproduce the committed generated file byte-for-byte. ## Representative false claims removed Not wording nits — statements a reader would have acted on: - Reversed directions. `proxyStart`'s JSDoc had the tunnel backwards (it is a reverse tunnel: the host binds first and the simulator dials in). A `paste()` doc had the pasteboard copy direction reversed. - Contradicted by the code below it. A timeout budget multiplied by three where the probes run concurrently — the same comment said so six lines later. A "warn once" that warns on every call. A "binary search" that is a linear scan. - Named things that do not exist. A `vega-fast-cli` binary, a `finish-recording.ts`, a `publish-next.yml` workflow, two `profiler-react19-.md` design docs, a `DebuggerTarget.ts`, a commit hash git does not know, two tool ids, an `ensureEnv` cycle. - Wrong by construction.* "Welford accumulators" across four files where the code keeps naive `n`/`sum`/`sumSq`; `sum`/`sumSq` documented over `actualDuration` when reduce sums `selfDuration`; a strict-mode halving written `n/2` where the code ceils; field docs listing enum values the producers never emit. - Guarantees the code does not make. A validation matrix claiming to cover "EVERY tool" that skips flagless ones; a Pareto cutoff that `slice(0, 20)` makes inert; an idempotence claim where the real rule is at-or-ahead; a capability note describing a clean 400 the shape-based device resolver can never produce. - Unverifiable assertions about prebuilt binaries, external CLIs and the cloud SDK — deleted rather than kept as folklore, since nothing in the repo can confirm them. - Stale numbers: invented Android tool versions, hard-coded tool counts and description lengths that had drifted. ## Review A Fable agent reviewed both halves adversarially for over-deletion, misread code, `no-empty` hazards and byte-identity violations. Second-half verdict: SHIP, with two one-line restores, both applied in the final commit — the `npm view ""` rationale behind a blank-token guard, and the note that `argent-mcp` keeps a copy of `SECRET_PLACEHOLDER_MARKER` it cannot import. ## Code issues surfaced but deliberately not fixed This pass changes comments only. Eight genuine findings are logged for a follow-up: 1. `telemetry/src/consent.ts` — a non-ENOENT read error returns null and falls through to the default-on path, so file errors can silently flip telemetry on. 2. `chromium-server/navigation.ts` — `navigate()` is reachable from `POST /api/navigate` with only a `typeof === "string"` check; open-url's schema is a bare `z.string()`, so the "already validated by zod" premise never held. 3. `http.ts` — `constantTimeEqual` returns early on a length mismatch, so the auth token's length is observable. 4. `describe/index.ts:~114` — the ios-remote branch passes `{ isTvOs: false }` unconditionally, so a remote tvOS simulator takes the iOS ax-service path, though `isRemoteTvOsSimulator` exists and shake/paste do use it. 5. `devices/boot-device.ts` — `-crash-report-mode never` is passed unconditionally and appended again by the feature-detecting path, so every emulator spawn passes it twice. 6. `react-profiler/pipeline/04-rank.ts` — `PARETO_THRESHOLD_PCT` is dead: `slice(0, 20)` always wins. 7. `reaped-sessions.ts` — a user-facing hint string tells the agent that `react-profiler-start { force: true }` disposes the debugger and profiler session; it does not. Left byte-identical because it is a string literal, not a comment. 8. `utils/simctl-backend.ts` — `localSimctl` is exported with no importers anywhere. ## Docs No documentation change is needed: this pass touches only source comments, and no user-facing capability, tool, CLI flag, config key or flow-file behaviour changed. --------- Co-authored-by: filip131311 <f.kaminski2000@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-24 12:16:20 +02:00
filip131311	5d76ed4d66	feat: tag artifacts with a semantic kind at registration (#564 ) Fresh, from-current-main take on the artifact-kind idea from #398 / #466. Knowing what an artifact represents (not just its MIME type) is what a UI needs to group and name artifacts (e.g. in EAS); this PR delivers exactly that — the tagged handle — without the declared-output-map machinery. ## Summary - Adds a closed `ArtifactKind` union to the artifact wire contract; `kind` is carried on `ArtifactHandle`, `ArtifactEntry`, and the `/artifacts` inventory items. - `ArtifactStore.register` now takes a single `{ hostPath, kind, ... }` options object, so the compiler forces every producer — current and future — to declare what it is emitting. - Tags all producers on main today, including the ones #398/#466 predate: screenshot (all four capture paths), screenshot-diff, flow snapshot baselines/diffs (`screenshot` / `screenshot-diff-context`), screen recordings (new `screen-recording` kind), and the native/React profiler exports and reports. - Types the native-profiler export keys at their source (`IosExportKey` from the `EXPORTS` table, `AndroidExportKey`) and maps them through a kind table that is total over both unions — a newly added export key fails to compile until classified, replacing the previous stringly-keyed loop (and the `?? "native-profile-trace"` silent-default this design would otherwise need). - Client-side (`@argent/tools-client`), the mirrored handle type keeps `kind` optional and widened (`ArtifactKind \| (string & {})`): an older, already-running tool-server sends no kind, and a newer server may send kinds this client build predates. Consumers treat it as advisory metadata; unknown kinds are opaque, never an error. ## Differences from #398 / #466 (deliberate) - No `@argent/artifacts` package, no third `ToolDefinition` generic, no per-tool output-name registrar, no runtime throw for “undeclared output” — the kind is plain data at the registration call site, checked at compile time where it can be, and a wrong label can never fail a tool at runtime. If a pre-invocation `artifacts` declaration in tool-listing metadata is ever needed (an actual EAS consumer), it can be layered on later without changing this wire contract. - The two `ArtifactKind` definitions (registry / tools-client) stay deliberately separate — same as the existing `ARTIFACT_MARKER` must-match convention — because the client-side type is intentionally looser (optional + open) than the server-side one for version skew. - Extending the union stays backward-compatible on the wire. ## 2026-08-24 update Merged current main (0.22.0) into the branch. The flow-visual rewrite that landed since (cropOn crops, per-app snapshot-key dedupe, diff scratch-dir hygiene) is preserved verbatim; its five registration sites now carry kinds (`screenshot` for baseline/current, `screenshot-diff-context` for the annotated diff — the cropped `current` included). No producer added since the PR was opened needed a new kind: `paste`, `shake`, and the other new tools emit no file artifacts, and every register call site in the tree now goes through the typed options object. No docs update is needed: the docs site (added since this PR) does not document the artifact handle wire contract; `kind` is advisory wire metadata with no user-facing surface. ## Validation At the merge with main (0.22.0), 2026-08-24: - `npm run build` (full `tsc --build`) — clean - `npm run typecheck:tests` in registry, tools-client, tool-server, mcp, cli — clean - Full suites green: registry 90, tools-client 191, tool-server 4495 (+1 skipped), mcp 80, cli 472 - `prettier --check .`, `eslint . --max-warnings 0`, and the knip gate — all pass 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-08-24 12:09:51 +02:00
Hubert Gancarczyk	f21811d069	fix(flow): re-probe a recorded wait against the tree the runner reads (#717 ) > Stacked on #715. ## The problem `await-ui-element` evaluates against the tree it reads — the accessibility hierarchy on iOS and Android, the CDP DOM on Chromium, the toolkit page source on Vega. The `await:` / `assert:` directive that the polish pass converts a recorded wait into is evaluated against the runner's tree instead. They overlap, but outside Vega neither contains the other — an id present in one can be absent from the other, and on iOS even the role vocabularies are disjoint. Both directions have been hit in practice: an id visible to the runner but not the recorder, and text-field values visible to the recorder but not the runner. The consequence is that a check can pass live and fail once converted. That makes the core promise — each step is executed live so you verify it works before it's recorded — untrue at exactly the point it is relied on. ## What changes After a recorded `await-ui-element` succeeds, the recorder re-runs the same condition against the tree the runner resolves directives against, and reports the result on the step it just wrote. It warns; it does not refuse. What gets written is a raw `tool: await-ui-element` step, and at replay that tool reads the same tree it just passed against — so "this would fail every run" was never true of the form actually recorded. What the probe genuinely reports is whether the polish-time conversion is safe, which is why the warning says to act on it then rather than re-record now (re-recording would duplicate the step). Because polish begins after `flow-finish-recording`, each verdict is also carried there: it is repeated on its own step's `summary` line, and `message` counts how many steps carry one. A warning raised at step 7 of a 40-step recording is otherwise long out of reach by the time it is actionable. ## Three outcomes, kept distinct \| Runner tree says \| Reported as \| \| --- \| --- \| \| condition holds \| nothing — step recorded clean \| \| condition does not hold \| recorded + warning naming the causes to rule out before converting \| \| tree unreadable, or too slow to answer \| recorded + warning that the conversion is UNKNOWN, not known-bad \| The middle row is deliberately not "the conversion will fail". The identical verdict comes back when the screen simply moved on between the live wait and the re-probe, and on `text` checks when the selector matches several elements and the two sides elect different ones — so the consequence stays conditional on the cause, and the message names the causes in the order worth ruling out. The third row matters: nothing was compared, so calling it a divergence would send the author off to rewrite a selector that may be perfectly good. It is also the injection-free case the flow skills explicitly sanction, so refusing there would block a supported way of working. On iOS the quoted reason ends "provide bundleId explicitly" — advice aimed at the native tools. The warning corrects it, because no directive carries a bundleId and forwarding one would make the probe answer a question the runner never asks. ## Platform-specific explanation The why differs per platform, and stating the iOS story everywhere makes the message wrong where someone is trying to act on it. iOS gets AX-vs-native-hierarchy; Chromium gets "the flow tree keeps only addressable nodes"; Android gets the importance trim; Vega gets "the trees agree, so the screen changed". The named tool for reading the runner's side follows the same rule — pointing an Android author at `native-find-views` points at a tool they cannot call. The remedy is per condition as well as per platform: on `hidden` a determinate verdict means the runner's tree still has the element, so "retarget at an id the full hierarchy carries" is the opposite of what helps. ## A failed live wait is reported by cause `await-ui-element` returns `{ success: false }` for three different things: a condition read and found false, a tree source that never answered, and a cancelled wait. Only the first judges the condition, so only the first is narrated as "the wait never held" and prescribes re-recording or deleting the step. The other two observed nothing and are reported as UNKNOWN — the same unknown-vs-known-bad distinction the probe draws for the runner's tree. A wait nested inside a recorded `run-sequence` gets neither warning: that tool reports its own shape, which carries no `success` key. The skill says so, and says to read `toolResult` there. --------- Co-authored-by: Hubert Gancarczyk <claude-hubert.gancarczyk@swmansion.com>	2026-08-23 22:18:26 +02:00
filip131311	366f17c619	chore: bump version to 0.22.0 (#891 ) Bumps every workspace package, `package-lock.json`, and both `server.json` version fields from 0.21.0 to 0.22.0 (18 files). Lockfile regenerated on Node 24 / npm 11 so the diff is version-only; `npm run check:versions` passes. Covers everything on main since `v0.21.0`: `paste` tool (#879), rotation-aware Android capture (#693), 0.25 default screenshot scale (#878), no-reboot describe on externally booted sims (#583/#877), NDJSON flow progress (#855), OTLP telemetry (#570), native-devtools restart_required derivation (#560), Android profiler empty-CPU explanation (#669), plus test/CI fixes. Full-surface live QA of this main (tgz vs npm 0.21.0) on iOS, Android, tvOS, Electron, CLI and Linux/remote found zero regressions. Two release notes: the published artifact must be built with `ARGENT_OTEL_INGEST_TOKEN` set (a local tgz without it compiles `readIngestToken()` to an empty string, making telemetry a silent no-op), and remember to push the `v0.22.0` git tag so online `init` skills installs resolve. No docs update needed: version-only change.	2026-08-21 19:33:57 +02:00
filip131311	172e4d3a64	fix(android): capture the screen the way it is actually oriented (#693 ) Part of #609. ## What was wrong On a rotated Android device `screenshot` returned a portrait-framed image with the content lying sideways. `describe` frames and gesture coordinates were already upright — uiautomator measures against the rotated display — so the capture was the one surface out of step. An agent reading that image saw geometry that disagreed with every coordinate it was given. Measured on a landscape Pixel_9 (API 36, `mCurrentOrientation=1`): \| \| space \| \|---\|---\| \| `describe` \| upright — `"Search Settings"` centre `(0.530, 0.234)` \| \| taps \| upright — tapping describe's `"Display & touch"` centre opens it \| \| `screenshot` (default) \| portrait-native, sideways \| ## The fix simulator-server already applies `rotation` correctly; nothing was telling it which one. The rotation is now queried from the device per capture, so `screenshot`, `screenshot-diff`'s live captures and the auto-attached "Screen after action" screenshot all land in `describe`/tap space. Nothing is tracked. A cached orientation could only be staler than the ~40 ms probe, and serving a stale one is exactly the failure being fixed. When the rotation cannot be read the request is left byte-identical to before rather than guessing — a wrong orientation is a confidently wrong image, which is worse than a sideways one. iOS is deliberately untouched. There the whole surface is consistently portrait-native, so rotating only the capture would break an agreement rather than restore one. The iOS half of #609 needs a different change and is not in this PR. ### Also fixed: the legacy uiautomator describe path Same root cause, worse symptom. `wm size` reports the unrotated size — on the landscape Pixel_9 it still answered `Physical size: 1080x2424`. Dividing rotated bounds by that does not merely squash frames: `isVisibleRect` prunes nodes whose left edge is past the screen width, so the right-hand half of a landscape screen vanished from the tree. The dump carries `<hierarchy rotation="N">`, so the divisor is oriented from bytes already in hand — no extra round-trip on the path that runs precisely when the helper could not be reached. ## On the rotation-name table Pinned by measurement, never by what the names mean — the reporter of #609 found the same names are 180° inverted on iOS. `adb exec-out screencap` is rotation-aware, so it is ground truth; each candidate was scored as mean absolute difference against it both as-is and rotated 180°, so an inverted mapping could not pass: ``` rotation 1 LandscapeLeft 2.19 (vs 6.54 for its 180° twin) rotation 3 LandscapeRight 2.19 (vs 6.34) rotation 2 PortraitUpsideDown 2.67 (vs 13.13) rotation 0 Portrait identity ``` This table is the inverse of simulator-server's own `90° → LandscapeRight` convention, because it compensates for a rotation that layer applies while decoding the scrcpy stream. That makes it a property of a particular simulator-server build rather than of Android — which is why a table test alone is not enough, and the delivered PNG's aspect is checked against the rotation requested, falling back to an unrotated capture if they disagree. ## Verified live Branch tool-server against a real emulator: - landscape, default capture → 2424×1080 upright, MAD 2.19 vs ground truth (6.54 vs its 180° twin) - cropping `describe`'s frame for the toolbar out of that default capture shows exactly the "Display & touch" toolbar — the two spaces agree - rotation 0 → 1080×2424, MAD 2.82, unchanged - rotated iPad → still 2064×2752 sideways, i.e. iOS provably untouched Plus 27 new unit tests, including one that asserts the legacy path drops the right-hand node before the fix and keeps it after. Full suite 3125 passing; lint, prettier, `typecheck:tests` and the tool-description gate all clean. ## Not in scope - The iOS half of #609. - `rotate` on Android silently no-ops when auto-rotate is enabled (it returned `{orientation:"LandscapeLeft"}` while `dumpsys` still read `cur=1080x2424`). Filing separately. - `screen-recording` still records sideways — same root cause, different pipeline. Worth noting a session's screenshots are now upright while its video is not. - Existing visual baselines captured on a rotated Android device will stop matching, since the baseline key embeds capture dimensions and those now swap. This fails loudly as a missing baseline rather than silently comparing the wrong thing — and those baselines were sideways captures of an upright UI. --- > #673 and #658 are both stacked on this branch — each conflicts with this one (`screenshot/index.ts` and `screenshot-diff/index.ts` respectively) but not with each other, so both were rebased onto this branch as siblings rather than chained. Merge this first; GitHub then retargets both to `main`. --------- Co-authored-by: Filip131311 <f.kaminski2000@gmail.com>	2026-08-21 17:34:59 +02:00
filip131311	d0b3356ebf	feat(tool-server): `paste` tool (device clipboard + paste shortcut) (#879 ) ## Summary Registers a real `paste` tool. The `tools/paste/index.ts` from #506 was never registered and sent a `{cmd:"paste"}` WebSocket command simulator-server doesn't implement — dead on every platform. - iOS simulator: simulator-server `POST /api/clipboard/text` fills a private pasteboard and pushes it into the sim synchronously (host clipboard untouched); the tool then sends the ⌘V chord over the existing `key` command (the same `simulatorSendKeyEvent` path the keyboard tool uses). - Android emulator: same route over the emulator gRPC bridge, then `KEYCODE_PASTE` via `adb shell input`. - ios-remote: the MoQ transport's existing paste primitive, awaited. - Clean 400 for tvOS / Android TV (runtime probe), physical devices, Chromium (a renderer paste would clobber the host clipboard), Vega. - `{{secret:<NAME>}}` placeholders as in `keyboard`; allowed inside `run-sequence`; device-interact SKILL updated. - Description/skill tell the agent `paste` is not a `keyboard` substitute — only where a real user would paste (OTP/2FA code, long link/token, testing paste handling). ## Blocked on a simulator-server built with the `clipboard` feature: software-mansion/radon#185 adds it to the `argent` alias (no sim-server API change). Until a release + bin bump, the tool reports `PASTE_CLIPBOARD_UNSUPPORTED` (with an update hint) against the shipped binary. ## Test plan - 13 new unit tests (`test/paste.test.ts`) + secret-placeholder tests; full tool-server suite, lint, knip green (the 4 telemetry/bind test files that fail locally fail identically on `main`). - Live against a locally built simulator-server: iOS 26.5 Settings search (12 back-to-back pastes in order), Messages composer (multi-line/tab/emoji/diacritics preserved), `{{secret:EMAIL}}` never echoed or logged; Pixel_9 Settings search (ASCII + `Zażółć🔑`), Expensify RN login field; `run-sequence` paste steps; live 400s for tvOS sim / phone serial / chromium id.	2026-08-21 16:51:51 +02:00
Ignacy Łątka	08f91fffd1	perf(screenshot): lower the default capture scale to 0.25 (#878 ) Screenshots are billed to an agent's context by pixel area, not by file size — so PNG level, JPEG quality and codec choice buy nothing here, and the default scale is the only real lever. `0.3` had never been measured against what a model can actually read, so I measured it. ## What was run Bluesky (`bsky.app`) built from source and driven on an iOS 18.6 simulator, signed in against the repo's own seeded dev PDS (`yarn e2e:mock-server` + `e2eSignInAlice`) — a live feed, profile, notifications, Explore and Settings, not marketing screens. Every image came out of argent's own `screenshot` tool over the tool-server HTTP API, so the downscaler under test is the shipped one. 160 API calls: 5 screens × 5 scales × 2 models × 3 reps, plus a blind control per screen per model. Questions were the things an agent actually needs off a screen — an `@handle` two rows down, a relative timestamp, a follower count, a badge number, which tab carries the underline, the label of the row above another row. Two controls decide whether the numbers mean anything: - Blind pass. Each model answered every question with no image at all. The 6 of 30 questions it still got right from priors alone (the screen title, the tab selected by default) were struck from scoring. 24 survived. - Counting split out. "How many rows between X and Y" was unreliable at every scale including full resolution — it measures arithmetic, not legibility — so it is excluded from the headline and reported separately. ## Result Text-reading accuracy, 66 graded answers per cell. Token counts are the API's own reported `input_tokens`, image column net of the ~345-token prompt. \| scale \| image tokens \| Haiku 4.5 \| Opus 5 \| \|---\|---\|---\|---\| \| 1.0 \| 2692 \| 100.0% (66/66) \| 100.0% (66/66) \| \| 0.3 (old default) \| 451 \| 98.5% (65/66) \| 100.0% (66/66) \| \| 0.25 (new default) \| 298 \| 100.0% (66/66) \| 100.0% (66/66) \| \| 0.2 \| 196 \| 98.5% (65/66) \| 100.0% (66/66) \| \| 0.15 \| 114 \| 81.8% (54/66) \| 97.0% (64/66) \| 0.25 is the only scale that scored perfectly for both models on every repetition, and it costs 34% less context than 0.3. ## The cliff, and why the margin Every wrong answer at 0.15: \| question \| truth \| answered \| model \| \| \|---\|---\|---\|---\|---\| \| 2nd author's handle \| `@bob.test` \| `@carla` \| Haiku ×3 \| read the row above \| \| selected feed tab \| `Following` \| `Feeds` \| Haiku ×3 \| couldn't resolve the underline \| \| 4th profile tab \| `Videos` \| `Likes` \| Haiku ×3 \| drifted two positions \| \| 3rd settings row \| `Moderation and content filters` \| `Privacy and security` \| Haiku ×3 \| off by one row \| \| bell badge number \| `1` \| `UNREADABLE` \| Opus ×2 \| declined rather than guess \| Opus fails safe; Haiku fails silent. Opus refused the unreadable badge. Haiku returned a confidently wrong neighbouring row — the same wrong answer three times out of three. For an agent that turns a read into a tap, a confident off-by-one row is worse than a refusal, because nothing downstream catches it. ## Why 0.25 and not 0.2 0.2 measured no worse than the old default and would have saved 57% instead of 34%. I didn't take it: the evidence is five screens of one app at one device size, and Bluesky sets fairly generous type. A denser interface — a table, a settings screen with subtitles, a smaller phone — reaches the floor sooner. 0.25 keeps two steps of margin above the cliff instead of one. `ARGENT_SCREENSHOT_SCALE=0.2` stays available for anyone who wants to spend that margin deliberately. ## Scope - Chromium is untouched. Its default is 1.0 and its `scale` is silently dropped without the optional `sharp` dep — that's #673's territory and I'm not duplicating it. Worth noting the two interact: once Chromium can downscale (via the in-repo `resizeDecodedPng`, as #673 suggests as a follow-up), it should pick up this default too, and it is currently by far the most expensive platform per capture. - `ARGENT_SCREENSHOT_SCALE` is documented as `0.3` in `argent-private/docs/environment-variables.md` — a submodule, so it needs its own PR to stay in sync. - Stale prose fixed in passing: `getScreenshotScale` carried a `// default: halve the resolution` comment next to a `0.3`, and `vega-screen.ts` documented the fallback as `0.3`. ## Checks `npm run build` clean, `tsc --noEmit -p packages/tool-server/tsconfig.test.json` clean, eslint `--max-warnings 0` clean, prettier clean. Shard 1/4 of the tool-server suite: 88 files / 1261 tests pass; the four files touching screenshot scale pass individually (22 tests). The default had no test coverage at all — the constant could be edited to any value and the suite stayed green. This adds one. Mutation-checked: reverting the constant to `0.3` fails 6 of the 8 new cases. ### On the red Unit tests check Inherited, not introduced. This branch's Unit tests job fails on exactly one test — `describe-tool.test.ts > states the limitation without ordering a reboot when the degraded read returned a tree` — which is the same single test failing on `main` itself (main has been red since #583, this branch's base). Already tracked as #871 and fixed by #877. `flow-composition.test.ts` passes in CI on both. Everything else is green: ESLint, Prettier, Knip, Static checks, SpiderShield, lockfile sync, Linux + Chromium E2E, macOS + Chromium E2E, macOS + iOS E2E.	2026-08-21 14:47:00 +02:00
Ignacy Łątka	d43938e4fe	fix(describe): drop the redundant restart assertion that reddened main (#877 ) Fixes #871. Supersedes #872, which fixed this the wrong way round. ## The assertion was demanding a restatement `describe-tool.test.ts:793` asserted: ```ts // It still names the remedy and what the remedy costs. expect(result.hint).toMatch(/boot-device/); expect(result.hint).toMatch(/restart/i); ``` against `DEGRADED_STANDING_HINT`, which reads: > …If something you expect is missing, boot-device with force=true reboots the simulator with the full accessibility settings "reboots the simulator" is already there. So `/restart/i` could only ever be satisfied by a second clause restating the reboot — and that is precisely what #583 carried: ``` … with the full accessibility settings — at the cost of restarting it and everything running on it. ``` A review suggestion applied through the GitHub web editor trimmed that clause (`dd99a2625`, `07f39cea8`) at 07:41:49Z and 07:41:57Z. The squash-merge fired at 07:42:23Z — 34 seconds later, so no CI ran on that head. The trim was right: it removed eight words that said the same thing twice. The test was what was wrong. #872 restored the redundant clause to turn CI green. This deletes the assertion instead, and leaves the shipped hint byte-for-byte as it is on main today. ## No coverage leaves with it The four remaining assertions are the ones that carry the test's purpose — #583 existed to stop the caveat ordering a reboot that kills the developer's Metro session: ```ts expect(result.hint).not.toMatch(/MUST/); expect(result.hint).not.toMatch(/\bnow\b/i); expect(result.hint).not.toMatch(/before continuing/i); expect(result.hint).toMatch(/boot-device/); ``` Mutation-checked. Restoring the pre-#583 wording — `"You MUST call boot-device with force=true now to restart the simulator … before continuing."` — fails the test on the first assertion: ``` × states the limitation without ordering a reboot when the degraded read returned a tree AssertionError: expected 'This simulator was not booted through…' not to match /MUST/ Test Files 1 failed (1) \| Tests 1 failed \| 36 passed (37) ``` So the regression the test guards is still guarded. The deleted line was pinning a redundancy, not a behaviour. ## Verification Branched off `dd5d82383` (current main). Reproduced the failure there first — `1 failed \| 36 passed` on the untouched tree. \| gate \| result \| \|---\|---\| \| `vitest run` (tool-server) \| 351 files, 4313 passed, 1 skipped, exit 0 \| \| `tsc --build` \| clean \| \| `typecheck:tests --workspaces` \| clean \| \| `eslint . --max-warnings 0` \| clean \| \| `prettier --check` \| clean \| Diff is one test file, one assertion and half a comment: ```diff - // It still names the remedy and what the remedy costs. + // It still names the remedy. expect(result.hint).toMatch(/boot-device/); - expect(result.hint).toMatch(/restart/i); ``` The comment loses "and what the remedy costs" because that half described the assertion being removed.	2026-08-21 14:46:49 +02:00
Ignacy Łątka	da61d4f2eb	test(tool-server): pin the ARGENT_* suite guard and correct what it documents (#859 ) Post-merge review of #707. Three findings, all from that change's own surface; no behaviour outside the test suite moves. ## 1. Nothing pins the guard `test/setup/clear-argent-env.ts` strips the developer's `ARGENT_` overrides so unit tests assert argent's shipped defaults. On a machine with nothing exported its loop body never runs, and CI is exactly such a machine, so no suite failure can report the guard being weakened. Measured at `9dc7c7cfc`, each mutation applied alone to the shipped file: \| mutation \| full tool-server suite \| \|---\|---\| \| body replaced with `export {}` \| 350 files / 4300 passed, exit 0* \| \| `startsWith("ARGENT_")` -> `startsWith("ARGENT")` \| 350 files / 4300 passed, exit 0 \| \| `delete process.env[name]` -> `process.env[name] = ""` \| `bind-failure-telemetry.test.ts` 3 passed, exit 0 \| \| entry removed from `setupFiles` \| suite green \| All four survive every gate: `vitest`, `eslint`, `prettier`, `tsc`, `typecheck:tests`, `knip`. `test/clear-argent-env.test.ts` re-imports the module against sentinels planted at call time, so the loop runs on demand rather than only when the developer happens to have an override exported. Each mutation now kills exactly one test, and the mapping is distinct: ``` body removed -> removes an ARGENT_ key outright instead of blanking it prefix underscore -> keeps a name that shares the prefix without the underscore delete -> blank -> removes an ARGENT_ key outright instead of blanking it setupFiles entry -> is registered as a setup file, so it runs before any test module ``` `ARGENTINA_REGION` is the look-alike: it pins that the underscore is load-bearing. The `in` check rather than a truthiness check is deliberate -- `= ""` reads as cleared to `if (process.env.X)` while still reaching a spawned child as a set key. ## 2. The header named the wrong file, and argued from the smallest cases > `ARGENT_PORT` / `ARGENT_HOST` retarget the bind that the startup-telemetry tests inspect. `startup-telemetry.test.ts` exists in that directory, mocks `createHttpApp`, and asserts nothing about host or port. `bind-failure-telemetry.test.ts` is the file that inspects the bind, and it is the one #707's own table named. A reader following the comment opens the sibling that literally bears that name and finds no bind assertion. The example list also covered only the four variables worth 9 failures, while the widest one was absent. Guard removed, one variable, full suite: ``` ARGENT_AUTH_TOKEN=devtoken -> Test Files 10 failed \| 340 passed (350) Tests 80 failed \| 4220 passed \| 1 skipped (4301) ``` It puts every HTTP route behind a bearer check (`src/http.ts:59`), so ten files fail on status code alone. That is the strongest argument for scrubbing the whole prefix instead of a hand-maintained list, and the paragraph defending exactly that choice did not have it. ## 3. A comment #707 silently falsified `malloc-stack-logging.test.ts:89-95` promised to "restore the ambient value afterwards", capturing `process.env.ARGENT_IOS_CAPTURE` at module scope. The guard clears that variable before the module is imported, so the capture is always `undefined` and the `else` arm of the `afterEach` restore is unreachable. Clearing in `afterEach` is what the file actually needs and all it now claims. `export {}` is appended to the setup file so `tsconfig.test.json` -- which includes `test/*/` -- treats it as a module rather than a global script; without it `typecheck:tests` fails with TS2306. ## Verification Run at `d0cb84de5`, rebased on `9dc7c7cfc`. - Full `packages/tool-server`: 351 files, 4306 passed, 1 skipped, 0 failed on a clean environment, and identical with `ARGENT_EMULATOR_GPU_MODE=host ARGENT_EMULATOR_NO_WINDOW=1 ARGENT_SIMULATOR_NO_WINDOW=1 ARGENT_PORT=9999 ARGENT_HOST=0.0.0.0 ARGENT_AUTH_TOKEN=devtoken ARGENT_IOS_CAPTURE=all-processes` exported. - Mutation matrix above re-run against the new test: 4/4 killed, intended test each time, control green. - `prettier --check .` 0, `eslint . --max-warnings 0` 0, `tsc --build` 0, `typecheck:tests` 0, `knip` 0 (206 issues against the 215 ceiling, unchanged -- the new file is not flagged). ## Checked and left alone - The four `it.each(PLATFORMS)` retitles from #707 render correctly (`on linux ... -gpu swiftshader`); all four sites were fixed and a sweep of all 77 `.each(` sites in the repo found no other transposition. - The guard is genuinely unnecessary in the other ten packages: the union of all 53 `ARGENT_` names read anywhere in `packages//src`, exported in two opposing polarities, leaves every other suite green. `argent-tools-client` already has its own copy from #714. - No CI workflow exports an `ARGENT_*` variable into a vitest process, so nothing in CI is destroyed by the scrub. `wayland-e2e.yml:46` sets `ARGENT_EMULATOR_NO_WINDOW` at job level but runs no vitest. - The other vestigial ambient save/restore pairs (`window-shake`, `http-auth`, `http-upload`, `http-flow-path-boundary`, `update-argent-tool`, `boot-device`, `boot-device-hotboot`) carry no false prose and are harmless defence-in-depth, so they stay.	2026-08-21 12:01:10 +02:00
Ignacy Łątka	13f43bd0cd	test(chromium): pin the ports-file guard and the readiness failure code (#862 ) Follow-up to #709, which merged while this was still under review. #709 landed the port fix; these are the two gaps a review pass over it turned up, both in the same two files. ## The ports-file guard was pinned by nothing #714 added `vi.mock("../src/utils/chromium-discovery", …)` to `boot-electron-spawn-error.test.ts` so a completed boot stops persisting its port into the developer's real `~/.argent/chromium-cdp-ports.json`. Nothing asserted it still worked. Measured, on the pre-change file: deleting the mock leaves the suite green at 9/9 while `$HOME/.argent/chromium-cdp-ports.json` is created containing a live ephemeral port. Replacing it with a call-through variant — the shape `list-devices.test.ts:76-81` already uses on this same module — does the same: ```ts vi.mock("../src/utils/chromium-discovery", async () => { const actual = await vi.importActual<...>("../src/utils/chromium-discovery"); return { ...actual, trackChromiumPort: vi.fn(actual.trackChromiumPort), untrackChromiumPort: vi.fn(actual.untrackChromiumPort) }; }); ``` → `Tests 9 passed (9)`, and the ports file contains `[55770]`. That is how the file accumulated entries before #714, so the guard rotting back is not hypothetical. Each file now scopes `HOME` with #714's own `scopeTempHome()` helper and asserts, after a completed boot, that no ports file exists. Both mutations now fail. All three files that carry this mock are fixed, not just the one. `boot-electron-kill.test.ts` and `boot-electron-ready-race.test.ts` hold the byte-identical mock — under a comment naming each other — and each completes a real boot. Measured before this change: deleting the mock left `kill` green at 15/15 while eleven live ports, one per boot, were written; the call-through variant did the same to `ready-race` while still satisfying its own `expect(trackChromiumPort).toHaveBeenCalledWith(port)` at line 175, which is precisely why the absence assertion is the right shape. Asserting the absence rather than `expect(trackChromiumPort).toHaveBeenCalledWith(port)` is deliberate: the call is already pinned by `boot-electron-ready-race.test.ts:175`, and a spy assertion only proves the export is a spy — the call-through mutation satisfies it while still writing. ## The readiness timeout was pinned by its wording, not its code The catch-path test matched `/CDP never became reachable/`. `FAILURE_CODES.CHROMIUM_ELECTRON_CDP_TIMEOUT` had no assertion anywhere in the repo — `grep -rn CHROMIUM_ELECTRON_CDP_TIMEOUT packages/ --include=.ts` outside `dist/` hits only the throw site and the enum. Swapping the throw site to `CHROMIUM_ELECTRON_SPAWN_FAILED` left `boot-electron-spawn-error` + `-kill` + `-ready-race` + `failure-classification` + `chromium-discovery` at 60/60 passed. So the half agents and telemetry bucket on was free to change, while a harmless reword would have broken the test. Now pinned by code via `getFailureSignal`, matching how `boot-electron-ready-race.test.ts:137` pins its own failure path. It still discriminates the vacuity the message match was added for: a nonexistent `appPath` yields `CHROMIUM_ELECTRON_APP_PATH_INVALID` and fails. `failure_stage` is pinned alongside it, as `ready-race` already does for its own path — `electron_cdp_ready` likewise had no assertion anywhere. ## Two rationales corrected - The catch-path comment blamed "no HTTP server on the port" for the probe failing. `fetch` declines port 1 as a WHATWG bad port before it dials* — verified with `net.connect` hooked: 0 sockets opened for port 1, 2 sockets for ports 2, 1023 and 19222. No listener is involved either way, and the file's own constant JSDoc says so. - The `chromium-discovery.test.ts` constant pointed at "a `listen(0)` fake CDP server in a sibling test file", when the server that makes the point is `startFakeCdpServer` in this same file, above the constant. - It also credited "anything in the unprivileged range" with being bindable by `listen(0)`. `listen(0)` draws from the kernel's ephemeral range — `net.inet.ip.portrange` 49152-65535 on macOS, `ip_local_port_range` 32768-60999 on the Ubuntu runner. That narrower range is what put the old `43211` at risk; calling it "unprivileged" also revives the privilege reasoning the sibling constant explicitly disowns. ## `chromium-discovery.test.ts` gets a second layer too It is the one file here that drives the real `persistPorts`, and its only guard was a single `beforeAll` line setting `ARGENT_CHROMIUM_PORTS_FILE`. That line is pinned — deleting it fails 3 tests — but the write lands before the detection, and `persistPorts` merges rather than replaces, so on a real machine the test ports would be appended for `list-devices` to probe afterwards. It now scopes `HOME` as well, so a lapsed redirect falls back somewhere disposable. Measured with a pre-seeded `$HOME/.argent/chromium-cdp-ports.json` = `[9222]`: deleting the redirect still fails 3 tests, and the seeded file comes back unchanged. ## Verification \| mutation \| before \| after \| \|---\|---\|---\| \| delete the module mock \| 9/9 pass, ports file written \| fails \| \| call-through `vi.fn(actual.trackChromiumPort)` \| 9/9 pass, ports file written \| fails \| \| swap `CHROMIUM_ELECTRON_CDP_TIMEOUT` at the throw site \| 60/60 pass \| fails \| \| nonexistent `appPath` in the catch-path test \| — \| fails (`APP_PATH_INVALID`) \| \| swap `failure_stage` at the throw site \| — \| fails \| \| delete / call-through mock in `boot-electron-kill` \| 15/15 pass, 11 ports written \| 11 failed \\| 4 passed \| \| call-through mock in `boot-electron-ready-race` \| 2/2 pass \| 1 failed \\| 1 passed \| Full `packages/tool-server` in 6 shards: 4303 passed, 1 skipped, every shard exit 0, and no `.argent` directory created under a redirected `HOME`. `eslint` 0, `prettier --check` 0, `tsc --build` 0, `tsc -p tsconfig.test.json` 0, `knip` 0 — exit codes read directly.	2026-08-21 12:00:50 +02:00
Ignacy Łątka	6d31aed752	fix(describe): stop ordering a simulator reboot on every externally-booted read (#583 ) Fixes #521. ## The report holds, and is worse than described Reproduced against an unmodified `origin/main` tool-server driving a simulator booted the way a developer boots one (plain `xcrun simctl boot`, no argent involvement), iOS 18.6: ``` ===== describe call #1 ===== ===== #2 ===== ===== #3 ===== hint: "This simulator was not booted through argent — system dialogs and native modals may not appear. You MUST call boot-device with force=true now to restart the simulator and apply full accessibility settings before continuing." ``` Identical on every call, as reported. The part that makes it more than a wording nit is what came back alongside it — a complete, working tree: ``` ROOT AXGroup (0.000, 0.000, 1.000, 1.000) AXStaticText "19:51" (0.139, 0.023, 0.102, 0.023) AXGroup "Maps" value="Widget" (0.082, 0.096, 0.395, 0.206) ... ``` And the caveat's own claim does not hold either. Triggering a real system permission alert on that same externally-booted sim (`simctl privacy reset location com.apple.Maps` + launch): ``` AXGroup "Allow “Maps” to use your location?" (0.193, 0.367, 0.614, 0.296) AXButton "Allow Once" (0.193, 0.524, 0.614, 0.046) AXButton "Allow While Using App" (0.193, 0.570, 0.614, 0.046) AXButton "Don’t Allow" (0.193, 0.617, 0.614, 0.046) ``` The system dialog reads fine, buttons and all. So an agent obeying "You MUST ... now" restarts the simulator — killing the Metro session, dev client and staged app state — to fix nothing. ## Root cause `AXServiceApi.degraded` is `!entitlementBypassActive`, read from `IgnoreAXServerEntitlements`. That pref is only writable pre-boot, so `boot-device` is the only thing that ever sets it and any externally-booted sim reads as degraded for its whole life. The pref bypasses the AX-server entitlement check introduced in iOS 26.5 — on earlier runtimes it is irrelevant to whether the read works. The flag describes how the sim was booted, never whether describe succeeded, and the hint conflated the two. ## Change Resolve the caveat against what the accessibility read produced: - Blind read — the reboot has something to gain, so the actionable guidance stays, on every call, and now names what the reboot costs. - Populated read — the read is its own proof no reboot is needed. States the limitation instead of ordering anything, once per device per server lifetime. The per-device gate is applied at the `describe` tool boundary only. `await-ui-element`, the flow runner and `await-screen-idle` poll `describeIos` directly, treat a present `hint` on an empty tree as "this emptiness is untrustworthy" (`isBlindRead`), and fold it into one terminal timeout note — they keep every copy, so a `hidden` wait still cannot resolve off a blind read. The native-devtools fallback can fill a tree the accessibility read left empty, and that split has to key on the accessibility read rather than on the tree that comes back. Its elements are the injected app's own view hierarchy: no SpringBoard chrome, no system dialog, and no evidence the accessibility subsystem works. Found doing exactly that on a fresh iOS 18.6 sim. Describing within seconds of `simctl boot` leaves the accessibility read blind for the rest of that boot (3/3; it survives a tool-server restart, and only `boot-device force=true` recovers it). With the Maps location prompt on screen in that state, the fallback returned the app's own 20 elements with the dialog absent — and resolving against that tree picked the standing note, which the per-device gate then deleted, so from the second call the agent had a tree silently missing a modal blocking the screen and no caveat at all. The note it briefly carried says "everything else reads normally", which nothing there did. Keying on the accessibility read makes both wordings true of the state that produces them. I skipped the suppression env var / flag the issue floated as a "maybe" — once the caveat is a single non-directive line per device, there is little left to suppress, and it seemed better not to add config surface for it. Happy to add it if you'd rather have the switch. ## Verification E2E against the branch build, same externally-booted sim: \| \| elements \| hint \| \|---\|---\|---\| \| describe #1 \| 14 \| standing caveat, no MUST/now \| \| describe #2-4 \| 14 \| (none) \| \| after `simctl shutdown` (blind) ×2 \| 0 \| blind caveat, both times \| \| after `boot-device force=true` ×2 \| 28 \| (none) — `IgnoreAXServerEntitlements` = 1 \| Unit: 6 new tests in `describe-tool.test.ts`. Each was mutation-checked — restoring the MUST wording, removing the dedup, collapsing the blind/standing split, keying the gate globally instead of per device, and re-resolving the caveat against the native-devtools tree each fail exactly one test and nothing else. Full gate: `vitest run` 4715 passed / 1 skipped, `tsc --build`, `typecheck:tests`, `eslint --max-warnings 0`, `prettier --check`, `test:scripts` 59 passed.	2026-08-21 09:42:22 +02:00
Ignacy Łątka	9dc7c7cfcb	test(chromium): stop electron boot tests depending on host CDP ports (#709 ) ## Problem `boot-electron-spawn-error.test.ts` drove `bootElectronApp` against hardcoded CDP ports 19222-19226 and relied on nothing answering there. ## It is not load, and it is not the timeout I chased the obvious hypothesis first — vitest's 5000 ms default colliding with a ~3.5 s built-in wait — and it is wrong. Measured at `origin/main` before any edit: \| condition \| result \| \|---\|---\| \| 15 isolated runs \| 15/15 pass \| \| 10 concurrent copies \| 10/10 pass \| \| 3 full-suite runs (load avg 16) \| 3/3 pass \| \| 10 runs under sustained full-suite load (load avg 19-20) \| 10/10 pass \| 38/38, a 0% spontaneous failure rate. And the one test whose `readyTimeoutMs` is 5000 against vitest's 5000 ms default completes in 15-54 ms at load ~20 — nowhere near the boundary. ## The real trigger, and it is deterministic Anything on the host answering the hardcoded CDP ports the test used. Squatting one with an HTTP server that serves `/json/version` fails the file 100% of the time. Reproduced against `origin/main`: ``` × rejects with a clear, actionable message when spawn emits ENOENT AssertionError: promise resolved "{ platform: 'chromium', …(5) }" instead of rejecting + "id": "chromium-cdp-19223", Tests 1 failed \| 8 passed (9) ``` Mechanism. When a real endpoint answers, `waitForCdpReady` wins `bootElectronApp`'s `Promise.race` in ~1 ms, long before the test's 10 ms `setTimeout` fires its synthetic error. The boot then completes and resolves, so the rejection the test asserts never arrives. The source is correct throughout. The test bound a global shared resource it did not own. `chromium-cdp-19222` is argent's canonical device id (`src/utils/chromium-discovery.ts:9`) and `e2e-device-smoke.yml:139,173` boots a real Electron on exactly that port — a developer with an Electron app open, or a second agent on the same machine, is enough. ## Fix One `UNREACHABLE_CDP_PORT` constant pinned to port 1, which `ensureCdpReachable` can never reach: it dials through Node's `fetch`, which refuses port 1 as a WHATWG "bad port" before it opens a socket. Measured: \| port \| result \| \|---\|---\| \| 1 \| `fetch failed` → cause `bad port`, no socket opened \| \| 2 \| `connect ECONNREFUSED 127.0.0.1:2` \| \| 22 \| `bad port` \| \| 1023 \| `connect ECONNREFUSED 127.0.0.1:1023` \| Privilege is not the mechanism, and would not be sufficient: 2 and 1023 are equally privileged yet dial normally, and Windows has no privileged range at all. The JSDoc says so, so the constant cannot be "tidied" below 1024 and quietly reinstate the flake. Not a timeout widening. ## Second defect, same file This file was the only one of the three Electron boot tests that did not isolate port persistence, so its success-path boot wrote a real ephemeral port into the developer's actual `~/.argent/chromium-cdp-ports.json` on every run. That file currently holds 107 entries, and I watched a concurrent session on this machine append one mid-review. Both siblings — `boot-electron-kill.test.ts:27` and `boot-electron-ready-race.test.ts:30` — already guard this with `vi.mock("../src/utils/chromium-discovery", …)`. This file now uses the same mock, and asserts `trackChromiumPort` was called with the booted port, so deleting the guard fails the suite instead of silently resuming the writes. ## Sibling fixed `chromium-discovery.test.ts` asserted port `43211` was unreachable, but that sits inside Linux's ephemeral range (`ip_local_port_range` → `32768 60999`), so a `listen(0)` fake CDP server in a parallel test file can legitimately be handed it. It now shares one named `UNREACHABLE_PORT`, replacing that literal and a second bare `1` that was already there. Left alone, verified safe: `boot-electron-kill.test.ts`, `flows/flow-chromium-boot.test.ts`, `list-devices.test.ts` — each owns its port via `listen(0)` or fully mocks the launcher. The fixed ports `43210`, `54321`, `54322` (all in `chromium-discovery.test.ts`) are only ever tracked or untracked, never network-probed. ## Verification - Deterministic repro above, reproduced at `origin/main` and re-run after the change with all five of 19222-19226 squatted: 20/20 pass. - Mutations, each of which passed 9/9 before the guard was added: deleting the module mock → `TypeError: [Function trackChromiumPort] is not a spy`; pointing the catch-path test at a nonexistent `appPath` → `expected … /CDP never became reachable/ but got 'path does not exist'`. - Ports file: full suite under a redirected `HOME` writes no `chromium-cdp-ports.json` at all; the real file's md5 is unchanged across runs. - `eslint` 0, `prettier --check` 0, `tsc --build` 0, `tsc -p tsconfig.test.json` 0 — exit codes read directly. - Full `packages/tool-server` in 6 shards: 4065 passed, 1 skipped, every shard exit 0.	2026-08-20 16:32:31 +02:00
Ignacy Łątka	f79e53a6e3	test(tool-server): scope vega-cli-timeout sentinels to the running process (#711 ) ## Problem `vega-cli-timeout.test.ts` fails intermittently, always the same two tests, always the same way: ``` rejects on its own deadline when the CLI never returns → expected { stdout: '', stderr: '' } to be an instance of Error reaps the ENTIRE worker tree on timeout → promise resolved "{ stdout: '', stderr: '' }" instead of rejecting ``` Both at ~60-130 ms against a 400 ms deadline — `runVega` resolved rather than timing out. It never fails in isolation. ## Not a tight budget, and not a reaping race \| condition \| failures \| \|---\|---\| \| isolated, 20 runs \| 0/20 \| \| under a parallel full-suite run, 15 runs \| 2/15 \| \| pinned to one CPU (`taskset -c 0`), 8 runs \| 0/8 \| The single-CPU pin is the discriminator: if the 400 ms budget were too tight, or the reap raced under contention, starving it to one core would make it worse. It passes 8/8. `packages/tool-server/src/utils/vega-cli.ts` is not implicated and is unchanged here. ## Cause: the sentinel namespace is machine-global The test's worker sentinels were fixed constants (`sleep 69101` … `sleep 69107`), and cleanup matches them with `pkill -f` / `pgrep -f` — which are global to the machine, not scoped to the process. Two concurrent runs of this file therefore share one namespace, and each run's `afterEach` `pkill`s the other run's live workers. The fake `vega` launcher's blocking `spawnSync("sleep", …)` returns early, the launcher exits 0, and `runVega` resolves cleanly well inside its deadline — exactly the observed signature. The existing comment shows the pattern was already narrowed once ("Match only this suite's sentinels (6910[0-9]) rather than a loose `sleep 691`"). That tightened it against other suites but not against a second copy of this suite, which is the case that actually occurs: CI matrices, `vitest --no-file-parallelism` reruns, and any machine running more than one checkout. ## Deterministic reproduction A neighbour issuing this file's own sweep (`pkill -f '^sleep 6910[0-9]$'`) on a 20 Hz loop reproduces it every time: ``` before → Tests 2 failed \| 7 passed (9) (3/3 rounds, both named tests) after → Tests 12 passed (12) (3/3 rounds, same neighbour live) ``` ## Fix Derive the sentinels from a per-process tag (pid plus three random digits) and scope every `pgrep`/`pkill` to it. Two live processes cannot share a pid, so two concurrent runs never collide; the random digits cut the leftover case — a worker leaked by an earlier run whose pid has since been recycled — to 1/1000. A concurrent run computes a different tag and its sweeps no longer reach these workers. The tag rides in the fractional part of the sleep duration, which also shortens a leaked worker's life from ~19 h to 10 min. That matters more than it looks: previously a later run of this file would clean up an abandoned worker as a side effect of the shared namespace, and scoping the namespace deliberately removes that accident. The same investigation turned up three more problems in the file, and the rest of the diff is those: - Regressions reported a bare timeout. Several tests here outrun vitest's 5s default - the delayed-close test's own deadline is 10s - so a real failure printed `Test timed out in 5000ms` and named nothing. The file now sets its own 20s budget; `beforeAll` carries an explicit one because a hook resolves its budget when it is registered. - Four assertions could pass with nothing to assert against. `waitForClear` reads 0 just as readily for a worker that never spawned, so both reap tests observe their workers alive first, and `strayCount` returns -1 rather than 0 when it cannot look at all. The two output-cap reaps have no such window (#841). - The reap test's premise was unasserted. It is named for a worker that escaped the launcher's process group, but nothing checked that it had: with the observation below removed, dropping `detached: true` from the `hang` fake leaves every test green, and with that fixture the descendant sweep could be deleted from `reapVegaGroup` with everything still green. One polled sample - a single `ps`, so the count and the groups cannot disagree about the instant they describe - now returns both, so dropping the flag fails with `{ workers: 2, groups: 1 }` — naming which half broke — while deleting the sweep still fails with `expected 1 to be +0`. Smaller, each covered below: the near-deadline `timeoutMs` 600 -> 900ms, now sharing one const with the bound that pins it; the reap deadline 400ms -> 4s, to give the launcher room to spawn both workers before the reap ends the observation window; a `beforeAll` warm-up that absorbs macOS's first-exec penalty; and three added tests (sweep positive control, random-block width, longer-pid decoy). Three comments about production behaviour were corrected too - the header's account of which mechanism reaps which worker, the reap test's on what the descendant sweep covers by itself, and the fake's on what reaps the launcher's own sleep. ## Verification - Deterministic repro above, before and after, same neighbour process. - Teeth check — a sweeper matching the post-fix shape but ignoring the tag (`^sleep 600\.[0-9]+$`) still kills the fixed code 4/4. The workers remain real and killable; only the tag protects them, so the fix is not passing by making them unmatchable. - Vacuity check — mutating the source (`collectDescendantPids` → `[]`) makes `strayCount` report a live stray (`expected 1 to be +0`), proving the new escaped `pgrep` pattern genuinely matches. - Mutation — pinning `RUN_TAG` to a constant, with a neighbour using the pattern a concurrent run of the mutated file computes: 4/4 fail. It now takes down four tests, not the two originally named — the two observations this PR adds, the near-deadline elapsed bound and the drain count, are killed by the same neighbour. - Positive control — every reap assertion here reads `waitForClear(...) === 0`, which is also what `strayCount` returns when the pattern matches nothing, so the suite passed with a pattern that matched nothing. Two of the three `sentinel bookkeeping` tests require a count of 1 first, and the drain reap counts its worker before the reap; the timeout reap counts through `ps` instead, so it is the bookkeeping tests that stand behind it here. Neutering `cmdlinePattern` fails those three (`expected +0 to be 1`), neutering `sweep()` fails one (`expected 1 to be +0`); the rest stay green in both cases. - The workers under test are the ones observed — giving only the fake launcher's `hang` workers a sentinel `strayCount` cannot see leaves the whole suite green without the counts, and fails `{ workers: +0, groups: +0 }` against `{ workers: 2, groups: 2 }` with them. Same for the drain path. The two overflow tests keep no count, and are therefore not pinned: breaking only the `flood` branch's sentinel leaves all 12 green. Their reap fires within ~20-100ms of the spawn, so an observation taken after the call never catches it (0/8), and one taken concurrently loses the race about a third of the time under load. Filed as #841, which records a non-racy alternative. - Each observation is budgeted off the call it watches — a poll shorter than that call turns a slow launcher start into a failed observation (`expected +0 to be 1` on the drain path, `{ workers: +0, groups: +0 }` on the reap) rather than into the reap it guards. The drain observation gets its call's whole 10s; the timeout reap's window is ended by the reap itself, so there the deadline is the budget, and it and the poll are both 4s. The reap observation's worst was 1485ms at 50-way concurrency against that 4s (2.7x); the drain observation's own worst was 1836ms against its 10s (5.5x). Stalling the `linger-grouped` branch 3.5s fails the drain observation on the 3s default poll and passes on the call's 10s; stalling `hang` 2.5s fails the reap observation on a 2s deadline, which kills the launcher mid-stall, and passes on 4s. The reap and drain observations are both new here, so the reap deadline is what the added runtime buys: that test goes from ~450ms on main to 4.13s, about 3.7s of the file's 4.4s increase. Measured against the branch's own intermediate 2s deadline the step is +1.95s idle (7.18 -> 9.13s) and +1.99s under load (10.52 -> 12.51s), and it takes that observation from 1.35x to 2.7x. The file's thinnest margin is the near-deadline row above, which no deadline change can widen. The drain-side change is free, since the poll returns as soon as the count matches. An interleaved A/B soak, 40 runs per arm at load 90-240, produced no failure of either observation in either arm, so it is neutral rather than confirming: the case rests on the injected stall and on the measured margins, not on a reproduced natural rate. - Slot width is pinned — `SentinelSlot` makes a 2-digit slot a compile error (`TS2345: Argument of type '10' is not assignable to parameter of type 'SentinelSlot'`). A 2-digit slot would both escape the sweep's `[0-9]` and break the equal-width argument the isolation rests on. - The launcher is warmed in `beforeAll` — the drain-grace test gives the launcher 900ms to start, and no larger value is available: it also has to stay under `VEGA_EXIT_DRAIN_GRACE_MS` (1000) to force the race at all. macOS charges the FIRST exec of a newly written file and nothing after it, and that cold exec does not fit — measured p90 1099ms / max 1684ms at 20-way concurrency under 92 hog processes, which made that test fail 38 of 60 (63%) when run on its own. It is not `node` starting: a fresh `#!/bin/sh` script shows the same jump, and warming `node` elsewhere leaves the solo rate at 55/60 - worse than not warming at all. Without the warm-up a whole-file run never saw it, because whichever tests run first pay the exec incidentally. Paying that cost once in `beforeAll` makes it invocation-independent: startup at that call site measured 136-148ms without the warm-up and 32-37ms with it. - Margins, worst observed against budget — across ~950 runs at up to 80-way concurrency under 92-132 hog processes (load avg 218-275): \| wait \| budget \| worst observed \| margin \| \|---\|---\|---\|---\| \| near-deadline launcher start \| 900ms \| 700ms (80-way) \| 1.29x \| \| `elapsed < 5_000` (delayed close) \| 5000ms \| 3017ms \| 1.66x \| \| worst `waitForClear` \| 3000ms \| 1648ms \| 1.82x \| \| reap observation \| 4000ms \| 1485ms \| 2.69x \| \| `elapsed < 2_500` (deadline) \| 2500ms \| 742ms \| 3.37x \| \| slowest test vs `testTimeout` \| 20_000ms \| 4130ms (idle) \| 4.84x \| \| drain observation \| 10_000ms \| 1836ms \| 5.45x \| The slowest-test row is an idle measurement, and a floor rather than a worst: the reap test's launcher never exits, so that test cannot settle before its own 4s deadline. The thinnest margin is structural, not tunable: that test's `timeoutMs` must clear the launcher's startup and stay under `VEGA_EXIT_DRAIN_GRACE_MS` (1000) for the race it forces to exist at all. Both bounds sit in that one sub-second band, so buying margin here spends it on the `toBeGreaterThan` bound below; the `beforeAll` warm-up is the only lever that does not. It still discriminates — dropping the exit path's `clearTimeout` fails it with `promise rejected "FailureError: vega linger ..." instead of resolving`. The `toBeGreaterThan(900)` bound on that same test sits at 1.11x but is safe by construction: the drain cannot fire before `grace` ms and scheduler delay only pushes it later, so load can never move it toward the bound. - Considered and not taken: sweeping the eight exact sentinels — replacing the `[0-9]` slot glob with an ERE alternation of the literal sentinels looks like it would drop the equal-width invariant and the two tests that pin it. It would not. `pkill -f` still matches every process on the machine, so isolation still rests on equal width: pid `1234` / rand `567` / a two-digit slot `11` spells `600.123456711`, byte-identical to pid `12345` / rand `671` / slot `1`, so the second run's alternation lists the first run's worker verbatim and kills it. The unpadded random block collides the same way (`1234`+`56`+`7` = `123`+`456`+`7`). `SentinelSlot`, the padding test and the `$` anchor are load-bearing under either form; only part of `cmdlinePattern`'s docblock would go. - Deliberate idiom deviation, flag if you disagree — `vi.setConfig` and `testTimeout` appear nowhere else in the repo; the convention here is a positional `it(..., 10_000)` (`boot-device-hardening.test.ts:214`) or `{ timeout: N }` (14 uses). Several tests in this file can exceed the 5s default, and two rounds of per-test budgeting each missed some, so the budget is set once for the file instead of enumerated per test. The wall-clock assertions (`elapsed < 2_500`, `< 5_000`) are unaffected — they assert speed directly rather than relying on the test budget. - The wrapper self-match is real, but GNU-only — `pgrep`'s own `sh -c "pgrep -f '<pattern>' \|\| true"` wrapper carries the pattern in its argv. Verified in debian against procps-ng 4.0.2: a bare sentinel with no anchors returns 2 matches (the second being the wrapper), and `^`, `\.` or `$` alone each cut it to 1. On macOS it cannot be reproduced at all — BSD `pgrep` excludes its own ancestors — so the docblock says which platform the hazard belongs to. - The `$` anchor is pinned — a decoy shaped like a longer-pid run's sentinel must survive `sweep()`. Dropping `$` kills it (`expected +0 to be 1`). `^` is deliberately left unpinned: without it the pattern reaches only command lines ending in this run's unique tag, which nothing can produce. - The slow-failing regressions stay legible — tests here outrun vitest's 5s default on a runVega deadline of up to 10s, a 3s clearance poll, or both. Past 5s vitest reports `Test timed out in 5000ms` and names nothing, so the file raises its own budget with `vi.setConfig({ testTimeout: 20_000 })`. Measured, each with the matching production path disabled: the delayed-close test `Test timed out in 5000ms` @5029ms → `promise rejected "FailureError: vega linger …" instead of resolving` @10031ms; the flood test `Test timed out in 5000ms` @5059ms → `expected 'timeout' to be 'subprocess'` @5030ms; the reap test → `expected 1 to be +0` @7254ms. No regression in this file reports a bare vitest timeout any more. - Two simultaneous runs, 4 rounds: 8/8 green; 15 sequential and 15 concurrent-triple runs green, no strays. - `eslint` 0, `prettier --check` 0, `tsc -p tsconfig.test.json --noEmit` 0 — exit codes read directly. ## Sibling audit No other test identifies its workers by matching a pattern against the machine-wide process table — the only other `pkill`/`pgrep` invocation under `packages/` or `scripts/` is a research CLI (`argent-private/research/physical-ios/cli/argent-ios.ts:441`), which is not a test and owns no sentinel workers. Every candidate checked: `adb-terminal-error-format` (mocks `child_process`), `await-ui-element` / `await-screen-idle` / `mjpeg-stream` (in-process timers, no subprocess), `artifacts` (real `tar`, no deadline assertion), `list-devices` (`ps` mocked), `vega-process` (real `ps`, read-only, owns no workers), `launcher-helpers` (TCP, no subprocess). That is the narrow claim. The broader one — "the only test that spawns real workers into a global namespace under a deadline" — does not hold: `launcher-spawn`, `launcher-duplicate-spawn`, `launcher-version-gate` and `launcher-sweep` all spawn real long-lived servers (the first three through `spawnToolsServer`, so detached; `launcher-sweep` a plain `spawn`), and `launcher-spawn` asserts on elapsed time. They are safe for a reason worth stating, because it is the same principle as this fix: each redirects `HOME` to a `mkdtemp` dir, so their state directory is per-run. Several unrelated tests do share the defect and are filed separately, since none is in this diff: #835, #836, #837. The mutation sweep run over this suite also surfaced `runVega` behaviours its assertions do not pin: the signal-death `subprocess` classification (#845), `settle()` clearing the exit timer (#847), the output cap's UTF-8-byte semantics (#849), and both `destroy()` pairs (#850) each survive as a mutant through a fully green run, as does `VEGA_KILL_SIGNAL` softened to `SIGTERM` (#848). The process-group SIGKILL (#846) is caught, but incidentally and flakily - by the stderr-cap test, in 1 of 3 consecutive runs. All of these predate this PR: the nine tests that exercise `runVega` are the same nine on `main`, the three added here covering the sentinel helpers themselves.	2026-08-20 16:32:22 +02:00
Ignacy Łątka	c0a63875df	test(debugger): pin the source-map wait debugger-status promises (#706 ) Found while reviewing #610 and set aside as out of scope: the code and the prose it comes from are byte-identical at that PR's merge base, and the same mutant survives there too. ## The gap `debugger-status` returns `sourceMapReady: true` as a hardcoded literal (`debugger-status.ts:57`). The only thing that makes the literal true is the line above it: ```ts await api.sourceMaps.waitForPending(); ``` and the description sells exactly that: "sourceMapReady (always true — waits for pending source maps before returning; no-op on Chromium)". Nothing pins it. Deleting the await: \| \| existing suite (`test/debugger test/metro test/flows test/utils`) \| \|---\|---\| \| unmutated \| 856 passed \| \| await deleted \| 856 passed \| So a status that reports readiness it never established would ship green. ## Scope, stated honestly This is a test-only change; the shipped behaviour is correct. The impact of a regression is narrow but real: `js-runtime-debugger.ts:214` already drains the registry once while the service is being created, so the await in the tool is what covers maps registered after that point - a Fast Refresh, or a lazily loaded bundle chunk. In that window a non-awaiting status would return `sourceMapReady: true` while `SourceMapsRegistry.pendingRegistrations` is non-empty, and a `debugger-inspect-element` issued straight after can miss the file:line it would otherwise resolve. I did not manufacture a user-visible failure end to end, and I am not claiming one - the finding is that the single promised behaviour of this field has no test. ## The tests Both assert the ordering, not the literal - a test that only checked `sourceMapReady === true` would pass on the mutant, since the literal is unconditional. 1. `does not resolve until the pending source-map registration settles` - hands the tool a `waitForPending` that returns a deferred promise, drains several macrotask turns, asserts the tool has not returned, then releases it. 2. `reports sourceMapReady only after the wait` - records `["maps-settled", "status-returned"]` and asserts that exact order. \| \| new tests \| \|---\|---\| \| on `main` as shipped \| 2 passed \| \| with the await deleted \| 2 failed \| eslint `--max-warnings 0`, `tsc --noEmit -p tsconfig.test.json` and prettier all clean.	2026-08-20 15:24:28 +02:00
Ignacy Łątka	5d8fc10799	test: stop tests binding machine-wide resources they do not own (#714 ) ## Why Three flaky or failing tests were diagnosed separately this week, and all three turned out to be one defect wearing different clothes: a test binding a machine-wide resource it does not own, so a second concurrent run — another checkout, a CI matrix cell, a developer's own app — perturbs it. Those three are fixed in #707, #709 and #711. Each of those was found by chasing a symptom, and each audit covered only its own resource type. This is the systematic sweep: all 448 test files across 13 packages, plus the 4 `node --test` scripts. (The sweep itself ran over the 400 files that existed when it started; merging `main` brought the count to 448, and every category below was re-run over the full set — 25 files assign `env.HOME` and none omit `USERPROFILE`, one test file calls `pkill`/`pgrep` (`vega-cli-timeout.test.ts`, scoped out to #711 below), and no `listen()` names a fixed port.) ## Findings \| Category \| Files checked \| Real findings \| \|---\|---\|---\| \| Network endpoints (bind / probe / unix socket) \| 448 \| 4 \| \| Filesystem — shared temp namespace (`os.tmpdir()` and literal `/tmp`) \| 448 \| 8 \| \| Filesystem — real `$HOME` / `~/.argent` \| 448 \| 4 (28 files) \| \| Ambient environment \| 448 + all `src` readers \| 12 \| \| Device / emulator / SDK state \| 448 \| 2 \| \| Process table (`pgrep` / `pkill` / fixed-pattern sweeps) \| 448 \| 0 \| \| In-process globals leaking across files \| 13 packages × 13 modes \| 0 \| Every finding was reproduced with a concrete instrument before being touched, and the instrument re-run after. Representative: - `stop-tools.test.ts` ran the real `stop-metro` against port 59999, which `lsof`s and SIGTERMs whatever it finds. With a squatter there: `16 passed` → `1 failed`, and the squatter logged `received SIGTERM #1`. Now port 1. - `bind-failure-telemetry` / `lens-relay-telemetry` were gated on the real `~/.argent/flags.json`. Enabling the documented `tool-server-event-log` flag: `4 passed` → `4 failed`, and records were appended to the developer's real event log. - LogFileWriter (12 files) — its constructor `mkdir -p`s `os.homedir()/.argent/tmp`. Instrument: run each of the 16 test files that reference a writer, or a blueprint that builds one, under a scratch `HOME`/`USERPROFILE`, then test `$HOME/.argent/tmp` for existence. 4 created it; the 12 that actually construct a writer now all call `scopeTempHome`, and none creates it. - `screen-recording.test.ts` — the device ids are fixtures and `vi.useFakeTimers()` seeds its clock from the real time, so only the millisecond a run starts in separates two runs' `.mp4` paths, and each test deletes the path it derived. Self-concurrent: 2 of 3 rounds failed → 0 of 4. Scoping `TMPDIR` removes the window rather than narrowing it. - `DO_NOT_TRACK=1` → telemetry `282 passed` → `13 failed`. `npm_config_user_agent=pnpm\|yarn\|bun` → installer `535 passed` → 4/5/3 failed. 19 tests required a real `adb` — PATH shadow + empty HOME: `30 passed` → `19 failed`. - A fixed `/tmp/argent-nd-<udid8>.sock` destroyed a concurrent holder's live socket (proven by inode) in 3 files that run the native-devtools factory. Two of them also fail outright: `native-devtools-app-state` lost 3 of 3 rounds of two concurrent runs on `expected 'unregistered' to be 'connected'`. All three now derive the UDID from `process.pid`. - `flow-visual`, `react-profiler/dump`, `file-inputs`, `http-upload` — shared `/tmp` paths and prefix scans, each reproduced with a squatter. In `http-upload` the collision surfaced as a timeout, inside `vi.waitFor`. (`flow-chromium-boot` was fixed on this branch too, but main's #585 rewrote that file afterwards and the merge kept main's copy, so it is byte-identical to `origin/main` here and carries no fix.) ## Clean, with the evidence - Process table — an `LD_PRELOAD` exec interposer over a full run (507 execs) plus all 12 other suites. Excluding `vega-cli-timeout.test.ts`, which is #711's and does sweep the table (`pkill -f '^sleep 6910[0-9]$'` at line 134): no `pgrep`/`pkill`/`killall`/fixed-pattern sweep anywhere, and no test signals a pid it did not spawn. - Real device state — zero exec attempts of real `adb` / `simctl` / `emulator` / `avdmanager` / `vega`; no test asserts a device list. That list was too narrow: `workspace-reader-integration` spawned the real `yarn` / `pnpm` / `bun` / `pod` / `eas` / `expo`, which is the second finding in this row. - Cross-file in-process leakage — structurally impossible: vitest 4.1.9 resolves `pool: "forks"` / `isolate: true`, confirmed by a scratch experiment (distinct pids, no env leak, module `Map` = 1,1,1). 120 default-mode runs across 12 packages produced 1 failure. - `scripts/` — 4 files, 86 tests: `mkdtemp` throughout, no ports, no sweeps, writes nothing. ## Verification Each fix was mutated back and killed by a named test under its instrument — e.g. `× reads a plain absolute path`, `× returns null when only the extensionless binary exists`, `× pins both names os.homedir() consults, at the same directory`. Restores verified with `git diff` / `git status`, never a substring grep. Pinned by an instrument only: the socket fix for `ios-only-blueprint-gate`, the `~/.argent/tmp` and `~/.argent/flags.json` fixes, the `/tmp` fixture cleanup, the `stop-metro` port change (the test asserts `stopped: false` / `pids: []`, which hold for either port whenever nothing is listening) and the `workspace-reader` `PATH` pin (only `node` is asserted; the other seven probes are not) are not pinned by any suite assertion — reverting them still passes, and only the external squatter / scratch-`HOME` instruments catch them. They are pollution fixes. The `native-devtools-app-state` socket fix is the exception in the other direction: reverting it turns two concurrent runs red. Gates, unpiped and read by exit code: `eslint .` 0, `prettier --check .` 0, `tsc --build` 0, `typecheck:tests` 0. Suite, run as six shards: tool-server 4287 passed, 1 skipped, 0 failed; argent-cli 462; telemetry 306; argent-tools-client 191; configuration-core 122; argent-mcp 80; argent-installer 555. All other packages and `test:scripts` green, identical to baseline. `lens-tools-platform-gate.test.ts` times out at the 5s default under full-suite load and passes at `--testTimeout=30000` (24.7s, almost all of it transform). It predates this branch and is untouched by it. ## Scope This branch is test-only — no `src/` file is touched. Both production defects the sweep turned up are separated out, so each can be reviewed as the user-facing fix it is: `ps` without `-ww` orphaning live tool-servers is #713, and #852 is `ensureToolsServer` respawning on every call when `ARGENT_HOST` is exported — `buildToolsServerEnv` passes the inherited value to the child while the state records the literal `127.0.0.1` the health check then probes. The second surfaced here as eight stranded `fake-tool-server` processes, and pinning the environment in this branch removes that signal, which is why it is filed rather than merely worked around. A third, outside this branch entirely, is #853: ten `tool-server` test servers omit the host on `listen(0)`, binding every interface where twenty-two sibling sites pass `127.0.0.1`. It is hardening, not a live exposure — `createHttpApp`'s Host allow-list answers a LAN-addressed request with 403, and only a raw client forging `Host: 127.0.0.1` reaches the stub registry behind it, inside the ~100 ms one `it()` holds the socket. It is also not a collision risk: ephemeral ports are kernel-assigned and never reissued while in use, which is why the fixed-port audit above is unaffected. All ten predate this branch. Left untouched on purpose: `boot-electron-spawn-error.test.ts`, `chromium-discovery.test.ts` and `vega-cli-timeout.test.ts` — owned by the still-open #709 and #711, and all three still carrying their defects on this base. (#707 has since merged into this base, so `boot-device-hotboot.test.ts` and `vitest.config.ts` carry its fix rather than a defect; `test/setup/clear-argent-env.ts` is its work, and this branch relies on it.) Not fixed, deliberately — an adjacent class (test-order dependence, not machine-global resources), reproducible if wanted: `link-config.test.ts` (deterministic: `-t "returns null when required field"` → 4 failed), `network/network-integration.test.ts` and `android-perfetto/dispatch.test.ts` (fail under `--sequence.shuffle` seeds). Also observed, not addressed: one green tool-server run leaves 56 entries behind in a scratch `TMPDIR` — 30 in the `argent-screenshot-diff` family, 4 `argent-events-`, 3 `argent-flow-crop-`, 3 `argent-tar-upload-`, 2 `argent-flow-diff-`, 2 `argent-file-input-`, and 12 assorted singletons (`ast-index-`, `component-source-`, `argent-profiler-cwd`, `argent-chromium-media`, …). Measured per run against a scoped temp dir, so it ranks the contributors rather than counting one machine's accumulated history — screenshot-diff, not flow-crop, is the dominant one. A separate leak. That instrument is blind to a literal path, which is how it missed the two `/tmp/native-profiler--report.md` writers the fourth pass found. Re-run across all 13 packages, `argent-mcp` is the only one outside `tool-server` that leaves anything — two entries per run, filed as #854. ## Second pass A review of this branch re-ran every category above with its own instrument rather than reading the diff, and found the sweep had pinned four resources in one file while leaving a sibling that does the same thing live. Fixed in `4289e3d8f` and `8164955b4`; the table and bullets above are the corrected totals. - `tool-server-event-log`* was mocked off in 2 of the 4 files that call the real `start()`. With the flag on, `startup-telemetry` and `tool-fail-invalid-params-emission` failed 3 tests in `attachRegistryEventLogger` and truncated the real `~/.argent/tool-server-events.jsonl` to 0 bytes first. `ARGENT_EVENT_LOG` cannot redirect that: `test/setup/clear-argent-env.ts` strips every `ARGENT_` var before the module graph loads. - `scopeTempHome`* reached 8 files; 4 more built a real `LogFileWriter` through the JS-runtime-debugger blueprint. - `os.homedir()` on Windows reads `USERPROFILE`, not `HOME`. `android-binary-windows` and `adb-resolve-avd-path` pinned only `HOME`, and both run on a real windows-latest host. Proven by running the resolver under a `node:os` mock that reproduces the Windows lookup: with `HOME` alone the stock Studio SDK under the profile resolves; with both pinned it misses. `update-checker` had the same gap. - `getConsentState` honours a falsy `ARGENT_TELEMETRY` at the same precedence as `DO_NOT_TRACK`, ahead of the config override the suite writes. Only `DO_NOT_TRACK` was pinned: `ARGENT_TELEMETRY=0` gave `17 failed \| 6 passed`. - `workspace-reader-integration` let the version probe spawn the real package managers off `PATH`; their shims self-install, so the file pulled ~44 MB into `~/.cache/node/corepack` and `~/Library/Caches/eas-cli`. `PATH` is now a directory holding only `node`, the one version the test asserts — 44 MB → 4 KB. - `argent-mcp` snapshotted `ARGENT_AUTO_SCREENSHOT_DELAY_MS` without clearing it, and that package has no `ARGENT_`-stripping setup file, so 3 tests asserted the developer's exported value. Smaller items in the same commits: the `/tmp` socket fixtures now remove the directories they create per run; `http-upload`'s pre-scan snapshot is gone, since scoping `TMPDIR` to a fresh directory made it permanently empty and its filter inert, as its two siblings already did; and two `detectProjectPackageManager` assertions that enumerated the entire return union are pinned to a single value. The confirmation pass also caught a defect the first round of fixes introduced: pinning `HOME`/`USERPROFILE` to the fixture root in `android-binary-windows` let `<home>/Android/Sdk` shadow the `%LOCALAPPDATA%` root the two positive tests are named after, so deleting `roots.push(join(localAppData, …))` from the resolver left all three green. Home has its own directory in `6aee3ee7a`, and that mutation now fails both tests. A third pass, running the nearest-twin move repo-wide, found the `os.homedir()`-on-Windows gap in one more package: eight `argent-tools-client` test files redirect only `HOME`, and four of them recursively delete `STATE_DIR` — two before each test, two after — which on Windows is the developer's real `%USERPROFILE%\.argent`. Fixed in `139447d4a`, together with the same gap in `avd-snapshot`. Nothing runs that package on Windows CI today, so no current run fires it. ## Fourth pass A fourth pass audited the three fix commits and re-ran the sweep with two instruments the earlier ones did not have. Both holes were in the measurement, not the diff — which is why three passes missed what they found. Fixed in `a67c43f8d`. - `os.tmpdir()` does not read the same variable on every platform.* POSIX takes `TMPDIR`, `TMP`, `TEMP` in that order; Windows takes `TEMP`, then `TMP`, and never looks at `TMPDIR`. The five pins this branch added set `TMPDIR` alone, so on Windows production still materialized into the machine-wide `%TEMP%` while the test scanned a scratch directory nothing ever wrote to — and `file-inputs`, `http-upload` and `flow-visual` each end in a leak assertion over that listing, which then passes whatever the code does. Against `origin/main`, which diffed a before/after listing of the real tmpdir, that is a regression rather than a wash. `redirectTmpdir` (`test/helpers/tmpdir-env.ts`) sets all three and returns the restore; the 7 files that scope a tmpdir use it, including the two `android-perfetto` ones that predate this branch. `test/tmpdir-env.test.ts` pins both lookup chains — reverting the helper to `TMPDIR` alone fails two of its three tests. All three leak assertions were re-confirmed live on POSIX by mutating the production cleanup each one covers. - A hardcoded absolute path is invisible to a `TMPDIR`-scoped instrument. `session-correctness` and `ios-instruments/analyze-freshness` passed a literal `/tmp/native-profiler-{x,ios}.trace` as the session's trace file, and the renderer writes its report beside it (`deriveReportPath` = `dirname(traceFile)/<base>-report.md`). Every run left `native-profiler-x-report.md` and `native-profiler-ios-report.md` in the shared `/tmp`; nothing removed them, and two concurrent runs write the same names. Reproduced with `TMPDIR` pointed at a scratch dir — the files still landed in `/tmp`, which is exactly why the `os.tmpdir()` row and the leftover census below both read clean here. Both tests now own the directory. - A prefix-scanned env var is invisible to a grep for variable names. `secretSources` enumerates `process.env` by the `ARGENT_SECRET_` prefix and ranks it above every file the test writes, so `secrets-command.test.ts` — which already pins `HOME`, `USERPROFILE` and cwd — still read the developer's shell. `ARGENT_SECRET_FOO=bar`, argent's own documented way to supply a secret: `6 passed` → `3 failed \| 3 passed`, deterministic. `argent-cli` has no `ARGENT_`-stripping setup file, so the prefix is cleared per test. Its sibling `configuration-core/tests/secrets.test.ts` was already immune — it threads an explicit `env: NO_ENV` into every call. - `android-binary` and `vega-cli-fallback`* were the last two files redirecting `HOME` without `USERPROFILE` — the former the direct twin of `android-binary-windows`, in the same directory, driving the same resolver. Under a `node:os` mock reproducing the Windows lookup with an SDK planted in the profile, it loses 5 of 11 tests. No test file in the repo redirects `HOME` alone any more. (`6fbb66ce1` later skipped both of this file's describes on win32 — its fixtures are extensionless and the win32 resolver only takes `.exe` — so its own `USERPROFILE` pin is now symmetry with the sibling files that do run there, not a live guard.) Two smaller items in the same commit. `native-devtools-socket-bind`'s ENOENT fixture still rooted at `os.tmpdir()` while the comment three screens above explains why the others do not: with a 142-character ambient `$TMPDIR` that path exceeds `sun_path` and `listen` rejects it for that reason, which the assertions cannot tell from the missing-directory one they were written for. And `<home>\AppData\Local\Android\Sdk` — the resolver's fallback for a GUI-spawned server that never inherited `%LOCALAPPDATA%` — could be deleted with the whole suite green; it now has a test, and deleting either win32 root fails exactly the tests named for it. ## Fifth pass A fifth pass took the platform-branch shape the fourth pass exposed and ran it as its own move: for every function that branches on `process.platform`, does the test's guard cover every arm, or only the one the author's machine takes? Fixed in `e08b466b6`. - `init-stale-config.test.ts` deletes the developer's real VS Code MCP config on Windows. The file mocks `os.homedir()` and says it does so "so hidden-scope probes (`~/.claude.json`, VS Code user profile) never touch the real home directory". `vscodeUserDirs()` branches `darwin → homedir()/Library/Application Support`, `win32 → %APPDATA%`, `else → homedir()/.config` — the win32 arm never calls `homedir()`, so the mock is inert there, and the test's own `userMcpJsonPath()` mirrors the same branch. Both user-profile tests then write a one-entry fixture over `%APPDATA%\Code\User\mcp.json` with a whole-file `writeFileSync`, and assert that `remove()` deleted it. Reproduced by forcing `process.platform` to `win32` with a two-server `mcp.json` planted at a probe `%APPDATA%`: `3 passed`, and the file and its directory were gone. With `APPDATA` pinned, the whole file passes under the same instrument — 31 tests — and the planted config is byte-identical. - *`argent-tools-client` hands its ambient `ARGENT_` to a real spawned child. The launcher spawns an actual tool-server, and `buildToolsServerEnv` overrides only the keys it is given — everything else rides in on `...baseEnv`. `ARGENT_HOST` is the one that bites: the fixture binds the host it names while the launcher records and health-checks `127.0.0.1`, so reuse misses and a second server is spawned. With six overrides exported: `10 failed \| 181 passed`, and eight stray `fake-tool-server` processes still holding TCP listen sockets after the run.** With the prefix stripped: `191 passed`, none surviving. The package now carries the same setup file `tool-server` has, for the same reason — and it is the second of the two packages whose missing `ARGENT_` strip this review found. ## Later passes A sixth pass audited the fix commits themselves and bounded the SDK probe (`6fbb66ce1`); a seventh ran the moves against the diff one more time and turned up the two below, plus the `listen(0)` defect filed as #853. - The `PATH` pin made `workspace-reader-integration` POSIX-only without the guard its siblings got.* The shim it puts on `PATH` is an extensionless `node`, and `symlink()` to a file needs an elevated process or Developer Mode on Windows, so `beforeAll` throws `EPERM` there before any assertion runs; a copy in place of the symlink would not rescue it either, since a bare command name resolves through `PATHEXT`. This sweep already skips `android-binary` and `native-devtools-socket-bind` on win32 for exactly this class, and the Windows job runs a curated list of seven `tool-server` files that excludes this one, so nothing was catching it. Fixed in `095d75bd8` — verified POSIX `2 passed`, and under a forced win32 platform `2 skipped` with the top-level `beforeAll` never entered. - A rationale that rested on a case the file excludes. `android-binary`'s header justified pinning `USERPROFILE` by what `os.homedir()` reads on Windows, two sentences before noting that both describes skip on win32. Fixed in `f37b3605e`; the variable stays in the snapshot list, the justification no longer claims a platform this file never reaches. A seventh pass also re-ran the sweep against a hostile machine: every feature flag on in a scratch `~/.argent/flags.json`, no `adb` on `PATH`, and a `TMPDIR` long enough to overrun the 104-byte `sun_path` limit. It found one resource this branch had not closed. - A red `launcher-version-gate` run strands a real tool-server. `ensureToolsServer` spawns a fixture server, but its pid only reached the reaper after three or four assertions on the state it returned; any of those throwing left the pid unrecorded, and the same `afterEach` then deleted `STATE_DIR`, the only other record of what to kill. Eight such processes were alive on the development machine, spread from 14:33 to 18:23, every one from an `argent-version-gate-test-` fixture and none from any other family — each still holding a TCP listen socket with its fixture directory long gone. Forcing one assertion to fail took the `tool-server.cjs start` count from 10 to 11. The nearest twin, `launcher-duplicate-spawn`, guards this twice: it records the pid straight after the null check and sets `FAKE_TTL_MS` so an escaped server self-exits. Fixed in `26c2c60ec` with both, the TTL also covering the window between the spawn and the state read where no pid exists yet — the same forced failure now leaves zero* strays. An eighth pass audited the seventh's own fixes, and found the version-gate fix had been applied to one file when the defect was a property of four. - Three more files strand a real tool-server on a red run. `launcher-spawn` and `launcher-exit` set `FAKE_TTL_MS` nowhere; `launcher-duplicate-spawn` — the file `26c2c60ec` cites as the model — armed it inside one test's `try/finally`, so eight of its nine spawns ran without it and the `finally` disarmed it for the remainder of that test. Reproduced per file by forcing a failure before the pid is recorded: a detached node process survives, reparented to init, holding a listening socket, with its `TEST_HOME` removed and its state record already cleared. Fixed in `3bb69ded6` by moving the net to a file-wide `beforeEach` in all three. Verified end to end — the child carries `FAKE_TTL_MS=60000`, and a stranded server spawned at 21:28:36 exited by itself at 21:29:36 with nothing signalling it; three consecutive green runs stay at 191 passed. - A 29th file writes the real `~/.argent`. `boot-electron-spawn-error`'s success-path test completes a boot, and a completed boot calls `trackChromiumPort`, persisting to `os.homedir()/.argent/chromium-cdp-ports.json`. The real file on the development machine had accumulated 106 dead ephemeral ports. Its two siblings in the same directory already mock `chromium-discovery` for exactly this reason; this was the third, with no mock. Fixed in `3306b6f40`. - `scopeTempHome` was itself unpinned. Deleting its `USERPROFILE` line — the Windows half, the same property the sibling helper's test pins — left all twelve consumers green at 48 passed. `3306b6f40` adds the missing test; it kills that mutation plus dropping the `HOME` pin and replacing the per-test `mkdtemp` with one shared directory. Two findings from that pass were refuted on verification and are recorded rather than acted on: the tool-server tmpdir leftovers (mechanism real, but they are the separate leak this body already scopes out) and a claim that `26c2c60ec`'s message overstated its twin. Four comments were also corrected against their code: `deriveReportPath` joins `<base>-report.md` inside the trace's directory rather than appending to the directory's name, which is what makes the `rm(traceDir)` added in `a67c43f8d` cover it; two `android-perfetto` comments still named `TMPDIR` as the mechanism after the switch to `redirectTmpdir`; and `android-binary`'s header claimed the home pin lets its two positive tests resolve "on either platform", though its fixtures are extensionless and the win32 resolver only accepts `.exe`.	2026-08-19 23:06:48 +02:00

1 2 3 4 5 ...

426 Commits