mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
v0.20.8
1383 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michał Pierzchała
|
19cea66c8b |
chore(deps): resolve dependabot alerts (#1372)
Pin transitive dependencies past their vulnerable ranges via pnpm overrides, scoped to the affected major so unrelated majors elsewhere in the tree stay untouched: - undici 7.24.7 -> 7.28.0 (root/website): @limrun/api pins undici to an exact version even in its latest release (0.44.0), so bumping the direct dependency can't fix this; override the transitive resolution instead. - shell-quote 1.8.4 -> 1.10.0 (examples/test-app) - js-yaml 4.1.1 -> 4.3.0 (examples/test-app) - brace-expansion 5.0.6 -> 5.0.7 (examples/test-app) - @babel/core 7.29.0 -> 7.29.7 (examples/test-app) - ws 7.5.10 -> 7.5.13 (examples/test-app) Resolves all 13 open Dependabot alerts (7 high, 3 moderate, 3 low). |
||
|
Michał Pierzchała
|
d9b583b950 |
test: serialize client-metro and harden the unit suite against ambient daemon env (#1365)
* test: serialize client-metro subprocess-stub test to end contention flakes src/__tests__/client-metro.test.ts stubs npx and the package managers on PATH and spawns a real Metro dev server per case, so each case waits real subprocess time (~570-910ms measured). Run in the unit-core project at ~7x file parallelism it contends for CPU with every other stub-spawning file; a starved spawn pushes production down a generic failure path that returns a different error than the assertion expects. This is the same contention flake #1362 serialized runtime-hints.test.ts and apple/core/index.test.ts for, but this file was not included in that batch (observed failing a full test:unit run while passing 20/20 in isolation and green on a re-run). Add it to SUBPROCESS_STUB_TESTS so it joins the fileParallelism:false, maxWorkers:1 subprocess-stub project, so at most one real-stub-spawning file spawns at a time. Injecting the spawn budget is not the lever here: the fake dev server becomes ready fast (no waited-out timeout to inject), the per-case cost is a genuine subprocess spawn, and each case is already under the 2.5s slow-test budget so the gate never flags it. Removing the cost would mean mocking the very spawn/args/package-manager-detection path the file exists to verify, and an exec-options DI seam is forbidden by the CI gate (AGENTS.md). Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * test: make the vitest suite hermetic against ambient daemon env A machine actually running agent-device — including this repo's own remote dev containers — exports AGENT_DEVICE_DAEMON_BASE_URL and AGENT_DEVICE_DAEMON_AUTH_TOKEN pointing at a live daemon. Production flag-default resolution folds those into every command's input and connection config (resolveConfigBackedFlagDefaults -> readEnvFlagDefaults, plus the daemon client's own env fallbacks in daemon-client-lifecycle), so a configured host silently diverges from CI, which runs with them unset: - command-tools and cloud-connect-profile assert exact command input / profile shapes and gain phantom daemonBaseUrl/daemonAuthToken keys. - daemon-client and daemon-client-lifecycle take the remote-daemon path ("Remote daemon is unavailable") instead of the local one they exercise. 28 tests across those four files fail deterministically on such a host — fast, in isolation, not contention — while passing on CI. Add a shared setup file that deletes the two ambient daemon-connection vars before each test file, wired into every vitest project alongside the existing process-memo reset, so a configured host matches CI. Tests that need these set assign their own value or pass an explicit env object, which runs after this setup module loads and is therefore unaffected. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * test: add a regression guard for the hermetic-env setup CI runs with AGENT_DEVICE_DAEMON_* unset, so it can never exercise the scrub that hermetic-env-setup.ts performs — deleting the setup file or forgetting to wire a project would leave the whole suite green. Add a guard that closes that gap two ways: - A static check that every configured vitest project lists the setup file in its setupFiles (catches an unwired project). - A real vitest child, launched with both daemon vars set, running a probe fixture that proves a wired project sees them scrubbed — plus a negative control (same vars, setup disabled) proving the probe actually detects the leak, so a green result is the setup working, not a no-op. A control var the setup must not touch proves the child inherited the injected environment. The two children run concurrently to stay within the unit wall-clock budget, and the file joins the serialized subprocess-stub project since it spawns real vitest processes. Ignore the fixture config's default export in fallow — vitest loads it by path rather than importing it, the same class as the other tool-config default exports already listed there. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * fix(test): route the hermetic-env guard's vitest child through runCmd AGENTS.md hard rule: TypeScript process execution must go through src/utils/exec.ts, never raw spawn/spawnSync. The guard's runProbe spawned the vitest child with node:child_process spawn directly, bypassing the shared timeout, process-tree cleanup, normalized failure, and diagnostics behavior. Replace it with runCmd invoking the repo-local vitest CLI (node node_modules/vitest/vitest.mjs) with allowFailure: true and a bounded timeoutMs, asserting the returned exitCode. allowFailure surfaces the negative control's non-zero exit as data instead of throwing, so that assertion keeps working; the two probes still run concurrently to stay under the unit budget. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * fix(test): make the hermetic-env guard's probe cleanup timely and complete Follow-up to review. Two process-tree cleanup gaps in the guard's runProbe: - It gave the nested vitest a 60s timeout while the enclosing test used vitest's default (5s) timeout, so a hung probe would time the parent test out first and leave the child running (orphaned). - runCmd only process-group-kills when detached:true; without it a timeout kills only the direct child, so vitest worker descendants could survive. Run the probe detached so a timeout kills the whole process group (the vitest child and its workers), and set the child timeout (20s) strictly below an explicit enclosing test timeout (40s) so a hang is reaped by runCmd before vitest abandons the parent test. The repo-local node/vitest invocation, allowFailure, exit-code assertions, and the concurrent positive/negative probes are unchanged. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * test: replace the child-process hermetic-env guard with an in-process test The previous guard spawned a real vitest child to exercise the setup through vitest's setupFiles machinery, which dragged in process-group cleanup, timeout ordering, a fixture config + probe, and a fallow suppression — a pile of scaffolding around a scrub that is two delete statements. Prove the same contract in-process instead: - Behavior: set the daemon vars, vi.resetModules(), re-import hermetic-env-setup, and assert they are gone. This exercises the real import-time scrub on any host (CI included, where the vars are otherwise absent) and fails if it regresses. - Wiring: assert every configured vitest project lists the setup in setupFiles. Runs in ~5ms in unit-core with no subprocess, so it also drops out of SUBPROCESS_STUB_TESTS and removes the fixtures and the fallow ignore entry. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
d237bc555d |
chore: remove verified dead code and migration scaffolding (~700 LOC) (#1367)
* chore: remove verified dead code and migration scaffolding Multi-agent audit of accumulated waste, every finding adversarially verified against call sites, git history, and the published surface before removal. Net -710 lines. - delete src/core/platform-descriptor/ (superseded ADR-0009 migration scaffold; parity tests now assert an inline table) - remove test-only seams: registry introspection exports, CommandFacet.extraDaemonWriters, MaestroEngineOptions.timing - remove dead flexibility: backend capability allow-list, screenshot-diff maxRegions, CloudWebDriverSupportLevel 'partial', clearFirst on the TS+Swift runner wire contract - remove dead deprecated surface: --session-locked / --session-lock-conflicts aliases (hard migration error now points at --session-lock), replay export --format single-value enum, unused Lease*Payload contract types, runtime-layer rotate duplicate - collapse pass-throughs/duplication: withRetry adapter, default-cloud-artifact-provider, connect-profile client-id hashing (3x sha256 impls -> one helper, byte-identical output), shared scripts walker, cloneValue -> structuredClone, fill-diagnostics moved into android/ BREAKING CHANGE: --session-locked and --session-lock-conflicts now fail with a migration error pointing at --session-lock; replay export --format is removed (Maestro was the only value); Lease*Payload types are dropped from the ./contracts subpath. * chore: satisfy fallow gates tightened by #1363/#1364 after rebase - drop the consumer-less AndroidFillVerificationNode re-export - reuse requireSnapshotSession in resolveSnapshotForRef instead of inlining the same authorized-frame resolution (fallow clone group); the helper's return type now guarantees the session it already throws for * chore: address review — keep cloud-webdriver partial capability metadata The partial/supported/unsupported levels and their notes are part of the lease-response capability contract for genuinely limited operations (Appium page-source snapshots, upload-then-install), not dead scaffolding. Restore them and the asserting tests unchanged from main. Also add the missing CHANGELOG entry for the Lease*Payload type removal from agent-device/contracts. |
||
|
Michał Pierzchała
|
317469f35a |
chore: remove dead type re-exports, prune stale suppressions, gate unused-types (#1364)
* chore: remove dead type re-exports and gate unused-types Clears the 72 `unused-types` findings left by #1363 and flips `rules.unused-types` from `warn` to `error` so new ones cannot creep in. Every fallow detector is now at zero. 71 of the findings were re-export lines (`export type { X } from './y.ts'` with no importer of X through that path); one was a direct declaration (`HelpTopicName`, zero references anywhere). Removing them cascaded, which is the point: imports that existed only to feed a re-export became unused (8 of them, caught by `noUnusedLocals`), and clearing those exposed a further chain through `command-projection.ts` -> `batch/index.ts` -> `batch/projection.ts`. Also updated a comment in `batch-policy.ts` that described the `command-surface.ts` re-export path this removes, and dropped two `export type {}` husks left where every name in a block was dead. One finding was NOT what it looked like: fallow flagged `DebugSymbolsOptions` and `DebugSymbolsResult` in `apple/core/debug-symbols/types.ts`, but removing them broke `apple/core/debug-symbols.ts`, which re-exported them from there. Both ends were dead — every real consumer imports from `contracts/debug-symbols.ts` — so the fix was removing the intermediate re-export too, not restoring the leaf. Safety: the published type surface is unchanged. All 11 entry points in package.json#exports export exactly the same names before and after. Two chunk files differ only because `AndroidSnapshotBackendMetadata` relocated between internal code-splitting chunks, which is not a consumer-visible boundary. * chore: prune fallow suppressions that no longer suppress anything The whole point of this arc was that fallow looked clean mainly because of its own ignore list, so the list itself deserved an audit. Emptying `ignoreExports` and re-running shows most entries no longer match a real finding: 20 blocks collapse to 5, with no change in what either fallow invocation reports. Removed 8 fully-stale blocks. They went stale for three different reasons: - code moved (`assertSafeDerivedCleanup` is exported from runner-cache.ts, not the runner-xctestrun.ts the suppression named), - the exports became genuinely used (apps.ts, runner-contract.ts, runner-session.ts, cloud-webdriver.ts, the test-utils fixtures), - and `installAndroidInstallablePath` was suppressed in two places at once. `app-lifecycle.ts` needed a code fix rather than a suppression: it re-exported three `parseAndroid*` helpers from app-parsers.ts that nothing imports through it, so the re-export is dropped and the block goes away. The two type re-exports on that statement ARE consumed and stay. The seven `src/daemon/handlers/*` blocks are KEPT, collapsed into one documented glob. They are not stale: those handlers are reached only through the dynamic `import()` table in request-handler-chain.ts, which the --production analysis behind `check:production-exports` cannot follow. A first pass removed them because the staleness probe only exercised the default config; the repo runs fallow twice, and `check:production-exports` caught it. |
||
|
Michał Pierzchała
|
c0fc822e80 |
chore: remove dead code and tune fallow's dead-code rules (#1363)
Evaluated knip (webpro-nl/knip) against the fallow setup already in the
repo, cleaned up everything it surfaced, then removed knip again: measured
head-to-head on the same tree, fallow is a strict superset once two
switches it already supports are flipped.
Dead code removed:
- `daemon/artifact-materialization.ts` (224 lines) had no production
caller, only its own test. Removing it exposed that
`downloadArtifactToTempDir` and the whole URL-fetch-with-redirects path
in `artifact-download.ts` were reachable only through it — the live
upload paths use the incoming-request helpers instead. That file goes
348 -> 123 lines. `readZipEntries` then fell out of `artifact-archive.ts`.
- Dead test-helper exports: 12 unused re-exports and 6 needlessly-exported
mocks in `session-test-harness.ts`, dead barrel entries in
`__tests__/test-utils/index.ts`, plus `withMockedXcrun`, `matchesSchema`,
`IOS_FRAME`, `IOS_TAB_FRAME`, `snapshotWithOffscreenContent`.
- `androidSnapshotHelperOutput` was duplicated byte-for-byte in
`provider-scenarios/android-world.ts`; it now imports the shared copy.
- 8 unreferenced type aliases, and 17 redundant type re-export lines in
`client/client-types.ts`. The published `.d.ts` is byte-identical before
and after all 19 files: those types already reach consumers through
`contracts/*` via `CommandResult<...>`, so this is not an API change.
Tooling:
- `.fallowrc.json` gains `includeEntryExports`,
`ignoreExportsUsedInFile: {type, interface}` and `unused-types: warn`.
That combination is what made the findings above visible; the previous
config was quiet mainly because of its own suppression list.
- Dropped 2 now-obsolete `ignoreExports` suppressions, added 3 documented
ones (published `sdk/*` surface, tool-config `default` exports, and the
`AssertTrue<...>` totality guards that exist only to satisfy
`noUnusedLocals`).
`unused-types` stays at `warn`: 72 pre-existing type re-export lines across
27 files remain, tracked separately. Every other fallow detector is at zero.
|
||
|
Michał Pierzchała
|
074ca140a2 |
test: serialize PATH-stub daemon/apple tests to end contention flakes (#1362)
`src/daemon/__tests__/runtime-hints.test.ts` and `src/platforms/apple/core/__tests__/index.test.ts` inject stub `adb`/`xcrun` binaries by mutating `process.env.PATH` and then spawn them, so each case waits real subprocess time. Run in the `unit-core` project at ~7x file parallelism they contend for CPU with every other stub-spawning file; a starved stub spawn pushes production down a generic failure path that returns a different error than the assertion expects. The failures present as assertion errors (not timeouts) and the failing subset shifts between runs — a pre-existing contention flake, reproducible on a clean tree. The repo already serializes the android scripted-adb tests into their own `fileParallelism: false, maxWorkers: 1` project for exactly this reason. This generalizes that project (renamed `android-adb` -> `subprocess-stub`) and adds the two files, so at most one real-stub-spawning file runs at a time across the whole suite. Assertions no longer let host wall-clock decide which error path runs. Option of injecting the spawn budget instead was not viable: runtime-hints passes no timeout to inject, and adding an exec-options seam purely for the test would be a test-only DI seam the CI gate forbids (AGENTS.md). Claude-Session: https://claude.ai/code/session_01NFepuH3Rh6ciRkvYJSCCtU Co-authored-by: Claude <noreply@anthropic.com> |
||
|
devin-ai-integration[bot]
|
611c7ed03d |
fix(apple): parse parenthesized xctrace physical device format (#1360)
* fix(apple): parse parenthesized xctrace physical device format Accept the new 'Device Name (OS Version) (device-id)' output from xctrace list devices while preserving the legacy bracket format. Relates to #1355 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(apple): split xctrace parser to reduce cyclomatic complexity Extract device line parsing and DeviceInfo construction into focused helpers so the discovery loop stays under fallow thresholds. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(apple): avoid interpreting bracket-format device names as having OS version Only treat 'Name (version) (id)' as the new parenthesized xctrace format; preserve the full name for legacy 'Name [id]' lines, including names that contain parentheses. Addresses review feedback in #1360. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(apple): add route-specific xctrace fallback test When devicectl reports no devices, listAppleDevices must source the physical device from the parenthesized xctrace output. Addresses review feedback in #1360. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
c84b449ecc |
fix(android): stamp status-bar chrome during the walk instead of reconstructing it downstream (#1319) (#1359)
* test(settle): pin that an expanded quick-settings shade stays visible to --settle (#1319)
#1319 asked whether the systemui run-condemnation rule leaves `--settle` blind
to a fully expanded quick-settings shade, the way it blinded the replay
divergence in #1318. It does not, and this pins the reason.
The settle loop captures `interactiveOnly: true` (`stable-capture.ts`). That
walk drops the structural systemui window spine (`legacy_window_root`,
`notification_panel`, `qs_frame`, the quick-settings ComposeView chain) — and
those are exactly the nodes that merge the shade into ONE contiguous run in the
`--raw` / non-raw shape #1318 measured. Under settle's shape the same capture
arrives as five runs, only `split_shade_status_bar` carries a marker, and the
29 quick-settings nodes survive into both diff sides.
So settle and divergence do not read one tree differently, as #1318 framed it:
they consume different capture shapes, and settle already gets the outcome that
layer wants — shade content diffs, status-bar churn does not.
Separately, the quiet-detection loop digests the UNFILTERED capture
(`digestSnapshotNodes` in `stable-capture.ts`), so the shade would reset the
quiet window even in the hypothetical where chrome stripping had emptied the
diff. Both halves of the question come out clean.
No product change. The behavior is correct but rested on an untested structural
coincidence: retaining the systemui spine in the interactive walk would re-merge
the runs and make `--settle` report a full-cover shade as bare removals with no
added content and no hint. The test fails if that happens (verified by flipping
the helper to `interactiveOnly: false`).
Verified live on emulator-5554 (Pixel 9 Pro XL API 37, deskclock), both
directions #1319 asked about and both halves the test asserts:
- shade OPENING mid-settle: settled after 6917ms: +28 -25
(added: brightness seekbar, Wi-Fi/Bluetooth/Mobile data/Quick Share/
Modes/Wallet tiles)
- shade CLOSING mid-settle: settled after 6919ms: +25 -28 (mirror image)
- the shade's own status bar ("Tue, Jul 21, Wifi signal full., T-Mobile") is
absent from both settle diffs while `snapshot -i` of the identical screen
still lists it — the run rule filters the churn rather than sitting inert.
The archived #1318 capture run through the real interactive-only walk reproduces
the live tree node-for-node (35 nodes, 29 kept, 6 stripped).
* docs(android): correct the chrome-classifier TODO — window-type keying is ruled out on device
Follow-up to the #1319 investigation, acting on review feedback that a
paragraph justifying fragile behavior means the code is wrong.
Two candidate fixes for the capture-shape-dependent chrome classification were
tested, and BOTH are dead. Recording that here so the next person does not
re-walk them:
1. The window-type approach this file's own TODO proposed. Measured against the
live helper XML (emulator-5554, Pixel 9 Pro XL API 37): systemui reports
`window-type=3` (TYPE_SYSTEM) both collapsed and expanded, `TYPE_STATUS_BAR`
(2000) never appears, the helper stamps window metadata on window ROOTS only
(1 of 169 nodes in an expanded-shade capture), and an expanded shade is ONE
window hosting the status icons AND the quick-settings tiles. No window-level
signal separates them. The TODO promised a fix the device data rules out.
2. Replacing run-condemnation with "condemn each marked node's subtree plus
fully-condemned ancestors". Shape-independent as intended, and it keeps the
tiles in both walks — but on the COLLAPSED status bar it leaks the unmarked
chrome the walk re-parents next to the markers ("Battery 100 percent.", the
notification-icon summary, neither carrying any resource-id). That is the
ticking-clock regression #1319 explicitly warned against.
So run-condemnation is not a lazy workaround: it reconstructs identity the walk
already discarded when it drops the `status_bar*` wrappers and re-parents chrome
leaves next to content. That upstream information loss is the actual root cause,
and a provenance-preserving walk is the real fix — larger than this PR, and it
would let #1318's divergence fallback be revisited.
Also trims the #1319 test's doc comment from a long justification of the current
behavior down to the finding, the defect, and what the test holds in place.
* fix(android): stamp status-bar chrome during the walk instead of reconstructing it downstream
Chrome classification gave opposite answers about the same screen depending on
capture shape: an expanded quick-settings shade was 100% chrome under the
`--raw`/non-raw walk (#1318 — every tile condemned, needing a divergence-local
fallback) and ~17% chrome under the interactive-only walk (#1319). Two layers
were papering over one broken classifier.
Root cause: `shouldIncludeStructuralAndroidNode` drops the `status_bar*` /
`navigation_bar*` containers — the only nodes that identify the region — and
re-parents their leaves next to real content. Everything downstream was
reconstructing identity the walk had already discarded, and reconstruction is
what depended on shape.
Fix: record it while it still exists. `walkUiHierarchyNode` threads
`ancestorSystemChrome` exactly like the `ancestorHittable` it already carries,
and stamps `systemChrome` on the emitted node; `androidUiNodes` tracks the same
subtree for the streaming content-recovery pass. Classification is then per node
and intrinsic, so `--raw` and non-raw agree by construction.
This deletes what was compensating for the loss:
- the 18 hand-picked marker leaf ids and their justification comment (what
enumerates them is "descendant of a status/nav-bar container" — now stamped);
- `collectAndroidSystemChromeRunIndexes`, the run-condemnation rule that let one
`clock` node condemn 95 unrelated ones;
- the leaf-id/prefix split, replaced by one container predicate that matches
`status_bar`/`navigation_bar` as an id SEGMENT so the shade's own
`split_shade_status_bar` counts.
Net −53 lines across 8 files, almost all of it classifier and prose.
Two alternatives were measured and rejected before this one (both recorded in
#1319 so they are not re-attempted):
- keying off AOSP window types, which this file's own TODO proposed:
impossible. On a live device systemui reports `window-type=3` (TYPE_SYSTEM)
collapsed AND expanded, `TYPE_STATUS_BAR` never appears, metadata is stamped
on window roots only (1 of 169 nodes), and an expanded shade is ONE window
holding the status icons and the tiles.
- condemning marked subtrees plus fully-condemned ancestors: leaks the unmarked
chrome the walk re-parents beside the markers ("Battery 100 percent.", the
notification-icon summary) — the ticking-clock regression #1319 warned about.
Test fixtures that modelled the old mechanism now model the device instead: the
synthetic status bar gets the `status_bar_launch_animation_container` root the
real capture has, and the content-recovery XML nests its chrome leaves inside
their container. Assertions were not weakened — settle.test.ts stamps via the
production predicate, and the #1319 test now asserts both walks classify
IDENTICALLY, which is the property that was missing.
Verified live on emulator-5554 (Pixel 9 Pro XL API 37, deskclock):
- shade opening mid-settle: settled after 7655ms: +28 -25, tiles present,
zero status-bar chrome (unchanged from before — settle was already right)
- ordinary action, no shade: +1 -1, no chrome leak
- real captures: collapsed status bar 9/9 chrome; expanded shade keeps every
tile under BOTH walks, which is the behavior that changed
* fix(android): keep systemChrome provenance out of published nodes; simplify androidUiNodes
Review blockers on
|
||
|
Michał Pierzchała
|
a55a1a2600 | 0.20.0 v0.20.0 | ||
|
Michał Pierzchała
|
b1b26f3b6a |
feat: publish scripts from active sessions (#1357)
* feat: publish scripts from active sessions * test: cover save-script force retargeting * fix: address active publication review findings |
||
|
Michał Pierzchała
|
f087a5938e |
fix: align Android Maestro gesture dispatch (#1356)
* fix: align Android Maestro gesture dispatch * fix: preserve Android gesture guarantees |
||
|
Michał Pierzchała
|
32ba4b67f4 |
chore: add FreeRange range-analysis check (#1354)
* chore: add freerange check * ci: install bun for freerange check * fix: preserve numeric range contracts * fix: guard diff overlay geometry * refactor: isolate diff overlay bounds |
||
|
Michał Pierzchała
|
9efe445398 |
fix: target Maestro scrollUntilVisible container (#1338)
* fix: target Maestro scrollUntilVisible container * fix: align Maestro scroll viewport selection * fix: ignore hidden Maestro scroll containers * fix: derive Android Maestro scroll viewport |
||
|
Michał Pierzchała
|
c6a8e78768 |
fix(tvos): skip SpringBoard system-modal probe on tvOS (#1351) (#1353)
tvOS has no SpringBoard (it uses PineBoard/HeadBoard), so probing
com.apple.springboard for blocking system modals raises an XCTest failure
("Application com.apple.springboard is not running") that safely() cannot
trap, terminating the whole runner test. Every snapshot and alert
resolution then failed, and the CLI misreported it as the screen
"overwhelming the accessibility capture".
resolveBlockingSystemModal — the single chokepoint every snapshot/alert
path funnels through — assumed a SpringBoard host always exists. Model that
assumption explicitly with `hasSpringBoardSystemModalHost` and bail to
`.absent` when no host exists, so app-owned alert queries still run. The
fourth probe site (shouldRouteToSpringboardBlockingSystemModal) is already
`#if os(iOS)` and needs no change.
Add tvOS regression tests covering the snapshot and alert-resolution paths.
|
||
|
Michał Pierzchała
|
d9f16de26f | docs: retire Maestro compatibility tracker (#1350) | ||
|
Michał Pierzchała
|
52e306082f |
docs: simplify README language (#1352)
* docs: simplify README language * docs: restore README technical details |
||
|
Michał Pierzchała
|
b3b9ff59b0 |
docs: propose active-session script publication (#1347)
* docs: propose active-session replay publication * docs: address active-session replay review * docs: align active-session publication on save-script * docs: qualify destination guard identity * docs: close active-session publication boundaries |
||
|
devin-ai-integration[bot]
|
1f042c3d8d |
refactor(mcp): extract reference-pin state into tool-ref-pins module (#1344) (#1345)
* refactor(mcp): extract ref-pin state into focused tool-ref-pins module Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mcp): add success-path ref-pin wiring test Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): narrow tool-ref-pins result types and replace as-cast with type guard Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): remove unnecessary as-casts in tool-ref-pins and command-tools Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): align ref-pin result handling with #1343 typed result projection Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): upstream types for ref-pin module (#1345) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): introduce honest public targetKind-discriminated interaction response contracts Replace the public AgentDeviceClient return types for press/click/fill/longpress/find with serialized-payload-shaped response data (targetKind, flat ref/selector/x/y, per-command extras) instead of internal runtime result types. The internal PressCommandResult/FillCommandResult/LongPressCommandResult keep their kind/target shapes for the daemon runtime; the public CommandResultMap now points to the new response contracts. - Add PressCommandResponseData/FillCommandResponseData/LongPressCommandResponseData/FindCommandResponseData in src/contracts/interaction.ts. - Update CommandResultMap and command-result tests. - Add client-facing shape tests asserting the public response data discriminates on targetKind and exposes flat identity fields. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(contracts): include cost and iOS Maestro fallback fields in public response contracts Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
devin-ai-integration[bot]
|
5a33f8bfc7 |
feat(maestro): numeric option fields accept ${VAR} lookup interpolation (#1293) (#1342)
* feat(maestro): numeric option fields accept ${VAR} lookup interpolation (#1293)
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(maestro): validate resolved numeric strings before coercion
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(maestro): address review feedback on numeric resolution
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(maestro-conformance): preserve unresolved ${VAR} numeric tokens in canonical model
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
||
|
Michał Pierzchała
|
a924f0bb3e |
fix(mcp): restore result and config parity (#1343)
* fix(mcp): restore result and config parity * refactor(mcp): narrow parsed input types * refactor(mcp): simplify parity boundaries * refactor: narrow validated MCP tool inputs * refactor: preserve command result types in MCP * refactor: remove impossible MCP result fallbacks |
||
|
devin-ai-integration[bot]
|
ab804340c8 |
feat(maestro): support optional on scrollUntilVisible and extendedWaitUntil (#1291) (#1339)
* feat(maestro): support optional on scrollUntilVisible and extendedWaitUntil Add optional support at command level and element level for scrollUntilVisible and extendedWaitUntil. The parser now accepts optional in both positions and propagates it to the command so the existing optional-command execution boundary downgrades a timed-out lookup to a warning and continues the flow. Update the upstream/076_optional_assertion divergence entry so only assertTrue remains unsupported, and keep the docs/support matrix in sync. Closes #1291 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(maestro): reject bare optional selectors, ORed visible/notVisible optionality, and add device differential scenario - parseMaestroSelectorMapEntries now rejects selectors that contain only optional and no real matching criteria, with rejection tests for scrollUntilVisible.element, extendedWaitUntil.visible, and .notVisible. - extendedWaitUntil now rejects simultaneous visible and notVisible conditions and derives optionality only from the single condition that will execute. - Add layer-3 differential flow/scenario optional-warned-scroll-and-wait that exercises both command-level and element-level optional on a missing target and verifies the flow continues to the final assertion. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(maestro): split parseExtendedWaitUntil to satisfy fallow complexity gate Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
devin-ai-integration[bot]
|
18b950407c |
fix(maestro): consume deferred stability before waitForAnimationToEnd (#1326) (#1337)
- Mark waitForAnimationToEnd as requiring a settled predecessor so it consumes the stability requirement parked by a preceding tap, swipe, or other mutation. Without this, the engine generation advances past the deferred requirement and the next tap's settlePending throws a generation mismatch. - Add a unit test that exercises tapOn -> waitForAnimationToEnd -> tapOn through the full daemon replay engine. - Add a layer-3 differential scenario for the same idiom. Closes #1326 Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
e7c02a9f4c |
feat: add advisory device claims (#1329)
* feat: add advisory device claims * fix: preserve advisory claim ownership * fix: retain claims after incomplete cleanup * fix: retain claims across pre-open effects |
||
|
Michał Pierzchała
|
f4df878a9d |
docs: improve README onboarding (#1333)
* docs: improve README onboarding * docs: restore README discovery details * docs: link README usage proof * docs: expand README usage proof * docs: refine README tagline |
||
|
devin-ai-integration[bot]
|
0f253f311c |
Maestro compat: support childOf on assertVisible/assertNotVisible (#1294) (#1334)
* Maestro compat: support childOf on assertVisible/assertNotVisible (#1294) - Accept childOf at command level in the Maestro IR and parser. - Thread childOf through the observation condition to the snapshot target resolver, reusing the existing ancestor-scoping path. - Project childOf into the conformance canonical selector so upstream/114_child_of_selector matches. - Remove the stale divergence declaration for 114 and update docs. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: regression-cover assertVisible/assertNotVisible childOf forwarding Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
ef118b9d11 |
ci(test-app): fingerprint-keyed build cache — disk locally, Release artifacts in CI (#1321)
Splits the test app's build caching by context instead of running one remote cache for both. Locally, `expo run:*` caches the native build on disk via the expo-build-disk-cache provider, keyed by the Expo fingerprint. A second run with no native change reuses the first build; a screen edit never rebuilds, because Metro serves JS. This is the original ask — "next time we don't build unless native changes" — and needs no token, no network, and no custom provider. In CI, test-app-build-cache.yml builds a Release binary per platform when the fingerprint has no artifact yet, and publishes it as a GitHub Actions artifact named `fingerprint.<hash>.<platform>`. Release, not dev-client, so the JS bundle is embedded and a consuming job needs no Metro. setup-fixture-app installs it by downloading the artifact and refreshing the JS with @expo/repack-app, so keying on the native-only fingerprint stays correct — a JS-only change reuses the same native binary in seconds. It falls back to an inline build when no artifact exists yet, so a caller is never left without an app. Release removes the sharp edges the dev-client cache needed. Its simulator .app is universal (x86_64+arm64) rather than the active-arch-only slice a debug build emits, so no architecture tag. It links against the SDK but loading is gated by the deployment target, which the fingerprint already covers, so no toolchain tag. And the CLI only narrows *debug* builds to the device ABI, so a Release APK spans every ABI without the undocumented --all-arch flag. The artifact name collapses to fingerprint plus platform. This deletes build-cache-provider.js entirely — with it goes the custom Expo provider that had to reach GitHub from inside @expo/cli, and every workaround that forced: the fetch-nodeshim User-Agent shim, the arch/Xcode identity, the upload-intent handoff. CI now talks to the artifacts API with plain `gh api` outside the patched fetch, and locally the disk cache never hits the network. The fingerprint comes from @expo/fingerprint's own `fingerprint:generate` (no --platform, matching what @expo/cli hashes). Gitignoring /ios and /android is what makes it machine-independent: the library asks the VCS whether the platform markers are ignored and, concluding CNG, skips hashing them — so a developer's prebuild output and a fresh CI checkout agree. conformance-differential consumes setup-fixture-app, so it gains `permissions: actions: read` for the artifact lookup. The artifact lookup is non-fatal: a query outage leaves the id empty and falls through to an inline build like a miss does, rather than exiting the composite under set -e and turning a cache blip into a caller failure. test/scripts/setup-fixture-app-fallback-smoke.sh drives that step's real shell against a failing gh and asserts source=build; ci.yml runs it. |
||
|
Michał Pierzchała
|
d0227998d4 |
feat: add daemon stop lifecycle (#1323)
* feat: add daemon stop lifecycle * fix: harden daemon stop cleanup * fix: fail closed daemon stop cleanup * fix: bound daemon shutdown lease releases * fix: await active shutdown lease release * fix: release provider leases independently on shutdown |
||
|
Adam Trzciński
|
12500ddc75 |
fix(record): finalize active recording on session/daemon teardown (#1325)
* fix(record): finalize active recording on session/daemon teardown
A session torn down while a video recording is still active leaked its
recorder. Neither the session-close teardown (stopBestEffortSessionResources)
nor the daemon-shutdown teardown (teardownSessionResources) stopped an active
recording — only the explicit `record stop` / `test --record-video` finalize
paths did. So when a session ends without a successful explicit stop (e.g. the
daemon is signalled/reaped or replaced mid-suite), the recorder is orphaned.
On the iOS simulator this leaves the detached `simctl io <udid> recordVideo`
child reparented to launchd (PPID 1); because simctl only finalizes the mp4 on
SIGINT, recording.mp4 stays 0 bytes and the single host recording slot stays
held, so later attempts fail with "Host recording is already in progress" and
the runner lease can wedge ("already owned by another agent-device daemon").
Add a best-effort teardown step that routes any still-active recording through
the normal stopActiveRecording path (SIGINT + awaited finalization, all
platforms), wired into both teardown paths. Runner-retention semantics are
preserved by capturing the retain decision before the recording is finalized.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(record): budget shutdown for recorder-stop escalation and surface teardown stop failures
Review follow-ups for the teardown recording-finalization fix:
1. Daemon shutdown could still orphan a slow recorder: the per-session
teardown race gave every session 5s, while the iOS simulator recorder stop
alone needs up to 11s (5s direct-handle SIGINT wait plus three 2s PID-based
SIGINT/SIGTERM/SIGKILL retries), so shutdown advanced toward process exit
exactly when fallback cleanup began. Export the recorder-stop escalation
budget (IOS_SIMULATOR_RECORDING_STOP_ESCALATION_BUDGET_MS) and extend the
per-session shutdown budget by it when the session has an active recording
(resolveDaemonSessionTeardownTimeoutMs, resolved before cleanup detaches
session.recording). The daemon-shutdown session teardown is extracted as
teardownDaemonSessionForShutdown so the slow direct-handle path is testable.
2. stopSessionRecordingForTeardown previously discarded the typed stop
failure from stopActiveRecording, so session-close aggregation and daemon
teardown reported clean cleanup even when the recorder was not finalized.
It now rethrows the failure as an AppError, which both teardown paths'
isolated cleanup channels collect as a `recording` cleanup failure while
later cleanup steps still run.
3. Pin both production routes with regression tests: ordinary `close` and
daemon session teardown finalize an active iOS simulator recording (SIGINT)
and surface stop failures, and daemon shutdown lets a dead-direct-handle
recorder run its full stop escalation instead of timing out at the base
budget. Removing either wiring call or the budget extension turns them red.
Verified live from source on an iOS simulator: SIGTERM of the daemon
mid-recording finalizes a playable mp4 with no orphaned recordVideo process on
both the fast path and a simulated dead-direct-handle slow path (daemon waits
~11s for the PID-based fallback instead of exiting at 5s).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* style: format daemon-runtime-recording-teardown.test.ts with oxfmt
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Michał Pierzchała
|
a84caa8182 |
test: give the tap-retry differential a fixture control that forces the retry (#1327)
* test: give the tap-retry differential a fixture control that forces the retry tap-retry-if-no-change was parked in #1289 for being a coin flip: tapRetries measured 0 in run 29504440599 and 1 in 29510020718 with no change to the flow or commit. This re-adds it with a control that holds still, so the retry fires every run. The original diagnosis (a dynamic cart badge in the tapped title's subtree) had the right shape but the wrong scope. maestroSnapshotSignature hashes EVERY node on screen, not the tapped subtree, so no "static region" of the home screen could have worked. The actual coin flip is the gesture lab's remote image (reactnative.dev/img/logo-share.png): whether it lands before or after the tap decides whether the engine sees "changed" and skips the retry. So the fixture gets a dedicated inert surface — no state, effects, timers, images, or pressables — presented as a full-screen modal so iOS detaches the presenting screen and the tab bar's live badges leave the hierarchy too. On a real run it is 9 nodes against Settings' 55. Reached by a launcher on Settings via the Settings TAB, which is a deliberate choice twice over. A deep link would have kept the launcher out of every other screen's snapshot, but simctl openurl raises a SpringBoard "Open in app?" confirmation on iOS 26 even cold, and Maestro's openLink goes through the same path. And home's "Open settings" button sits below the fold, so reaching it needs scrollUntilVisible — the engine bug already waived under #1299. The flow carries no waitForAnimationToEnd: a navigating tap defers a stability requirement that the next tap settles before resolving its target, so the baseline signature is already captured on a settled screen. Adding one fails the flow outright, which is a real engine divergence filed as #1326. Verified on device (iPhone 17 Pro Max, iOS 26.2, Maestro 2.5.1): 10/10 consecutive differential runs ok, tapRetries [0,0,1] every run — the two navigating taps correctly do not retry, the inert tap retries exactly once. A single green run proves nothing here, which is the trap #1300 fell into. The parking guard in invariants.test.ts is replaced by three guards: the scenario stays active, carries its tapRetries invariant, and is never waived by a knownDivergence — a flaky scenario must be fixed, not declared. Fixes #1300 * chore: gitignore expo prebuild output in the test app Building the fixture app locally (what .github/actions/setup-fixture-app does in CI) runs expo prebuild and generates examples/test-app/ios/. It is generated and untracked but not ignored, so it shows up in git status and a `git commit -a` would sweep the whole native project in. Same for android/ when building there. Scoped to examples/test-app, so the repo-root android/ — which holds real tracked sources like android/ime-helper — is unaffected. Nothing is tracked under either path today, and the CI fixture-app cache key hashes src/**, app/**, modules/** and the config/lockfiles, so it does not reference these and is unaffected. |
||
|
Michał Pierzchała
|
b2b7ddd0d0 |
chore: gitignore .env files (#1322)
The repository is public and the root `.env` was untracked but matched no ignore rule, so it showed up in `git status` and any `git add .` or `git add -A` would have staged it — publishing whatever it holds to the repo and every fork. It has never been committed to any ref, so this is prevention rather than a leak; nothing needs rotating. `.env.*` subsumes the old `examples/test-app/.env.local` entry, which was the only dotenv rule here and covered a single path. The rules match basenames at any depth, so every variant is now covered wherever it appears. `!.env.example` keeps templates trackable; none exist today, but the negation is what makes the broad `.env.*` safe to add. |
||
|
devin-ai-integration[bot]
|
6d99914f49 |
feat!: remove deprecated gesture duration and rotate velocity inputs (#1218, #1216) (#1315)
* feat!: remove deprecated gesture duration and rotate velocity inputs (#1218, #1216) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: address CI failures - remove dead export, dedupe positional validation, migrate linux-desktop swipe test to pan Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fixup! preserve Maestro swipe endpoint-hold execution profile via internal seam Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs(adr): describe Maestro endpoint-hold internal seam in ADR 0013/0015 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * feat: surface Maestro swipe executionProfile in replay trace and assert endpoint-hold in differential Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
3fed8acda5 |
fix(remote): preserve tenant scope for proxy artifact downloads (#1317)
* fix(remote): preserve tenant scope for proxy artifact downloads * docs(remote): clarify auxiliary tenant precedence * refactor(remote): carry artifact request scope together * fix(remote): bump daemon RPC protocol for tenant scope |
||
|
devin-ai-integration[bot]
|
10dab65bbf |
fix(mcp): advertise --no-record on every recordable command's tool schema (#1313)
* fix(mcp): advertise --no-record on every recordable command's tool schema Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(command-descriptor): make recordsSessionAction explicit and exhaustive - Require recordsSessionAction on every RawCommandDescriptor so new commands must decide their recording behavior at compile time. - Set the field explicitly on all 73 raw descriptors (26 true, 47 false) and remove replayScopedAction from daemon trait literals. - Derive daemon replayScopedAction and MCP noRecord schema projection from the single recordsSessionAction classification. - Keep parity test guard asserting raw descriptors declare the field and that derived replay policy stays in sync. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(command-descriptor): classify recordsSessionAction from actual recording seams and cover every MCP-exposed recordable command Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
a6711a46cb |
fix(replay): publish a full-cover system overlay's targets in divergence screen.refs (#1318)
* fix(replay): publish a full-cover system overlay's targets in divergence screen.refs A fully expanded quick-settings shade left the divergence `screen` available but empty: 95 captured systemui nodes, 0 refs, no suggestions. `selectDivergenceScreenRefNodes` drops `collectSettleChromeRefs` nodes before `isForeignOverlayDismissTarget` can rank them, and the run-level chrome rule condemns a whole contiguous same-package run when any node in it carries a status/nav marker. An expanded shade is a SINGLE systemui run: the 4 status-bar icons it hosts (clock, mobile_combo, mobile_signal, wifi_signal) condemn all 95 nodes, the 23 hittable qs_tile targets included. The run rule assumes the status bar is its own window — true collapsed, false expanded. Since #1301 a plain `snapshot` of that same surface returns the tiles (the `systemSurfaceOnly` carve-out), so the divergence had become strictly NARROWER than `snapshot` — the invariant `captureDivergenceObservation` documents and the ADR 0012 decision 4 amendment forbids: the chrome filter must stay a FILTER, never a narrower scoping. Fixed in the divergence layer, not in the shared run rule: settle and divergence genuinely disagree about these nodes and both are right. Settle must keep stripping the shade run — the markers are literally clock/signal/wifi, the churn settle exists to ignore, and sparing runs that hold actionable content would let a ticking clock hold settle awake. The divergence layer owns the violated contract. Chrome exclusion now falls back to hittable chrome nodes when it would otherwise empty the pool, mirroring the existing `covered` fallback. The gate is hittability, not label presence, and both real captures back that: the collapsed status-bar clock is labeled "7:03" but not hittable (so a chrome-only screen still publishes nothing), while the expanded shade's clock is hittable. Live-verified on emulator-5556 (Pixel 9 Pro XL API 37): same repro 0 -> 20 refs (cap, truncated) — the brightness slider plus 19 hittable tiles, matching the fixture-driven test exactly. `ANDROID_QS_SHADE_CAPTURE_RAW_NODES` is a real 138-node --raw capture, not hand-authored ids. * docs(replay): state the chrome-fallback boundary (PR #1318 review) The fallback fires only when chrome exclusion would empty the pool, mirroring the `covered` fallback right below it. A marker-bearing overlay over PARTIALLY visible app content keeps its tiles condemned — the pool is non-empty, so no fallback. That is deliberate (the full-cover case is the one that strands an agent), but it was implicit; stating it saves the next investigator a live session. * refactor(test): move Android capture fixtures into .json files The fixture module had grown to 1080 lines, of which ~990 were two inlined device-capture literals — the walkers and their docs were buried under the data they operate on, and any future capture would bury them further. Capture fixtures are archived device trees: DATA to be regenerated from a device, not code to be hand-edited. They now live in `.json` beside the module and load via `fs` + `import.meta.url` — the existing `test/output-economy` baseline pattern, which needs no `resolveJsonModule` or import-attributes support to typecheck, and keeps the trees out of the bundle. Mechanically extracted from the current exports and verified byte-identical (`JSON.stringify` before/after match for both fixtures), so this is a pure move: no fixture data changed. Module drops 1080 -> 109 lines. The leg-E regression test remains revert-sensitive. * refactor(test): typecheck the capture fixtures via resolveJsonModule Reading the fixtures with `fs` + `JSON.parse(...) as RawSnapshotNode[]` bought nothing over a static import and gave up the one thing that matters for a typed data fixture: the cast asserts the shape away unchecked, so a fixture that drifts from `RawSnapshotNode` compiles fine and only surfaces as a confusing test failure — or silently passes. `resolveJsonModule` + `import ... with { type: 'json' }` structurally checks each tree against `RawSnapshotNode[]` at typecheck time instead. Verified: corrupting a fixture (`hittable: "yes-please"`, a stray field) now fails `tsc` at the export, where the `fs` version accepted it. Also drops the reader helper and the runtime read. The `test/output-economy` precedent I first copied reads baselines through `fs` for a different reason — those are runtime-compared artifacts, not statically typed fixtures — so it did not apply here. tsconfig gains one option; `noEmit` is already set, so it has no output-layout effect. Fixture data unchanged (byte-identical), leg-E test still revert-sensitive, full tooling + 1931 tests green. * refactor(replay): inline the chrome fallback instead of explaining it `hittableChromeCandidates(nodes)` did not filter chrome — it returned every hittable node, and was only "chrome candidates" by virtue of its one call site. The name lied, and a chunk of the 15-line comment above it existed to cover the gap between the name and the behaviour: exactly the case where the comment is propping up the code. Inlined into the same `x.length > 0 ? x : y` idiom the `covered` fallback two lines below already uses, so the two narrowings now read in parallel. The boundary that comment spelled out — the fallback fires ONLY when exclusion empties the pool, so a partial-cover overlay keeps its tiles condemned — is now the ternary itself rather than a paragraph asserting it. What survives is the part the code genuinely cannot show: WHY a chrome-only screen must still publish (the never-narrower-than-`snapshot` contract) and why hittable is the discriminator. 6 lines, against the 8-line `covered` comment beside it. No behaviour change: same node set, fixtures untouched, leg-E test still revert-sensitive, 1931 tests + full tooling green. |
||
|
Michał Pierzchała
|
a68fcddc42 |
fix(android): drop the dumpsys scroll-hint probe instead of capping it (#1270) (#1314)
The snapshot helper is the only capture backend and emits `can-scroll-forward`/ `can-scroll-backward` for exactly the nodes Android reports as scrollable. Their absence therefore means nothing on screen scrolls, not that scroll state is unknown — so the `dumpsys activity top` probe only ever ran when there was no scrollable content for it to describe. #1288 bounded that probe at 1.5s rather than removing it, leaving every capture of a screen with a scrollable-typed but non-scrollable node (a list short enough to fit) paying the cap for hints that cannot exist. `dumpsys activity top` serializes a view dump of every top activity through each app's main thread, so one busy app stalls the call until Android's own 10s service-dump timeout — the mechanism behind the 37ms-to-5.9s spread in the issue's evidence. Hints for genuinely scrollable nodes are unaffected: they come from the parsed helper attributes, which is where they already came from on every screen that reports a scroll action. |
||
|
Michał Pierzchała
|
8eeed1dded |
fix: sleep past the settle quiet deadline instead of onto it (#1306) (#1308)
* fix: sleep past the settle quiet deadline instead of onto it (#1306) runStableCaptureLoop derived its whole cadence from pollMs = min(300, max(25, quietMs)), so pollMs === quietMs for every quietMs in [25, 300]. The loop settles once two identical captures span quietMs, and the gap it measures is exactly one sleep(pollMs) plus capture time — so across that entire range "settle at capture 2" rode a 0ms margin. Node decides that margin, not the UI: setTimeout(n) advances Date.now() by only n-1 in 0.13% of calls idle and 0.63% under load, because libuv times the sleep on the monotonic loop clock while now() reads the wall clock. On an undershoot the loop spends a wasted extra capture and poll before settling. The sleep is now deadline-aware: while the quiet deadline is further away than one poll the cadence is unchanged, so changes are still noticed promptly; once it is within reach, the loop sleeps to just past it. The capture that decides settled always spans the window. Effects: --settle-quiet in [25,300] now settles at capture 2 rather than 2-or-3 by coin flip, and the default 500ms window settles at ~502ms instead of ~600ms with the same 3 captures. No behaviour change beyond timing; the quiet-window semantics are identical. * fix: bound the quiet-deadline sleep by the loop's own budget (P2 review) The review is right: the epsilon could spend the very capture it exists to land. With quietMs=300 and timeoutMs=301, capture 1 asked for a 302ms sleep, woke past the 301ms deadline, and the loop exited with one capture — where the old 300ms cadence settled. waitedMs could exceed timeoutMs too, and the recovery-reset branch shared the same helper. stableCaptureDelayMs now takes the deadline and never wakes past it: the loop only runs again while now < deadline, so where the budget is the tighter constraint it wakes just inside it and takes the capture the plain cadence would have taken. The epsilon still applies whenever the budget has room. The motivating regression test is also strengthened per review, from asserting the requested delay is > 300 to modelling the defect itself: an injected clock whose sleep advances now by ms - 1, asserting the observable two-capture settle. Each test now catches exactly one defect: vs main ( |
||
|
Michał Pierzchała
|
9b5f333a25 |
fix(cli): deliver --no-record to the daemon (supersedes #1305) (#1311)
#1305 claimed to forward --no-record from "every recordable command reader". Measured through the real argv -> reader -> client -> daemon chain on its own merge commit, the flag reached the daemon for ONE command (`open`). It is now 5 of 33 on current main -- `open` plus get/is/find/snapshot, the latter four only incidentally, because #1303 declared `noRecord` in their metadata. #1305's fix was inert because it fixed a layer that is not load-bearing. Its test asserted on `readInputFromCli` output -- an intermediate object two later layers rebuild from scratch: 1. `defineExecutableCommand.invoke` runs `metadata.readInput(input)` -> `readFieldInput`, which keeps ONLY declared metadata fields plus `readCommonInput`'s output. `noRecord` was neither, so it was filtered. (`open` survived solely because its metadata declares the field.) 2. Each `to*Options` projection rebuilds the client options from `commonToClientOptions` plus its own named fields; that helper did not carry `noRecord` either. So `--no-record` parsed, was accepted on every command, and was silently dropped before dispatch -- including on press/click/fill and, per the maintainer's review, gesture/back/home. Fixed at the seams the flag must survive, not per reader: - `commonInputFromFlags` and `selectionOptionsFromFlags` (the reader layer has TWO parallel common helpers -- reader-input shape vs client-options shape -- so both must carry it; `settings` used only the latter, which is why it was the last gap); - `readCommonInput` (stop `readFieldInput` filtering it); - `commonToClientOptions` (stop `to*Options` dropping it). Measured after: 33/33 deliver the flag, with zero hand-listed commands. #1305's `noRecordInputFromFlags` helper and all 13 hand-added call sites are deleted: they are redundant against the seams, and leaving both would be two sources of truth for one behavior -- exactly how the next gap breeds. Preserves the --record asymmetry (ADR 0012 decision 6 amendment): --no-record is common and rides the common seam; --record stays scoped to snapshot/get/is plus a dynamically-validated find, on its own narrow helper. Also fixes `get --record`, dead through the CLI since #1303 for the same re-projection reason (`toGetOptions` rebuilds its options object), which that PR's daemon-level scenario could not see. Coverage is asserted where it is observable, not at the intermediate object: - `cli-record-flag-delivery.test.ts` drives real argv and asserts on the DAEMON REQUEST for all 32 recordable routes; it fails on reverting either seam ("press accepted --no-record but never delivered it to the daemon"). - `no-record-recorder-routes.test.ts` is a healed-script regression: gesture/ back/home with --no-record must not land in a written .ad. Reverted, it fails with the leaked `gesture "fling" "up" 100 200` line in the script. A derived `recordsSessionAction` classification + completeness gate follows in a separate PR: this fixes the 32, that makes a 33rd impossible. |
||
|
Michał Pierzchała
|
11a4212f45 |
fix(android): return meaningful occluding system surfaces instead of failing the snapshot (#1301)
* fix(android): return meaningful occluding system surfaces instead of failing the snapshot (#1253) The notification shade and quick settings legitimately own the whole screen: the helper faithfully captures the active system window, but the content classifier treated the missing application window as a helper failure. Add a carve-out: an active/focused non-application window carrying meaningful content is returned as the snapshot, flagged systemSurfaceOnly, with an agent-facing warning explaining how to reach app content. Sparse or inactive system windows keep failing with the structured retriable error. * fix(android): thread system-surface disclosure through selector routes; gate the carve-out on non-chrome content (PR #1301 review) P1a — selector/find/wait routes no longer lose the disclosure. SnapshotState gains systemSurfaceOnly, stamped at the one seam where snapshot state and capture annotations meet (captureSnapshotAttempt), so every consumer — including session-stored snapshots — inherits it. The disclosure message moves to a shared module (snapshot/system-surface-disclosure.ts) used by the capture-runtime warning and a daemon response helper (handlers/system-surface-disclosure.ts) that appends it to ok-response warnings and error-response hints. Applied on both found and not-found outcomes across the public daemon selector routes: read-only find (exists/wait/get_text/get_attrs), mutating find (matched, unmatched, and ambiguous), wait (text/selector/ref/stable; pure sleep is exempt), get, and is. P1b — the >=3 meaningful-node floor for the system-surface carve-out now counts only NON-CHROME nodes. The status/nav-bar marker ids from the settle-chrome classifier (#1198/#1251) move to a shared contracts/android-system-chrome.ts (core/snapshot-chrome.ts keeps byte-identical behavior via the shared resource-id predicate), and classifyAndroidHelperContent excludes chrome-classified resource-ids from activeSystemSurfaceMeaningfulNodeCount: an active nav bar (Back + Home + Recents) or status chrome (clock/battery/wifi) is missing-app-content residue, not a usable shade. Shade/QS fixtures are unaffected (tile/notification ids are not chrome markers). P2 — regressions prove the production wiring, not just the classifier: snapshotAndroid stamps androidSnapshot.systemSurfaceOnly for a helper-backed shade capture; the capture runtime renders the disclosure warning from the annotation; daemon regressions pin the disclosure on mutating find (found), read-only find exists, and wait timeout against a shade capture; classifier fixtures pin nav-bar and status-chrome windows as unusable. Live-verified on emulator-5556 with the shade expanded: snapshot -i returns systemSurfaceOnly true plus the warning; find exists returns found:true plus the disclosure warning; wait timeout carries the disclosure in its hint. * fix(daemon): disclose system surfaces on sessionless selector routes; pin disclosure composition (PR #1301 review) Sessionless find/wait never store the consumed capture on a session record, so the disclosure read from the session store returned nothing. The selector capture runtime now reports every consumed snapshot through a shared slot on the runtime params, and disclosure reads prefer it; the session-store read remains only as a fallback for pre-captured snapshots. Regressions pin the sessionless route end-to-end and that the disclosure appends after existing success warnings and failure hints instead of replacing them. * fix(daemon): initialize the consumed-snapshot slot on the wait route (PR #1301 review) dispatchWaitViaRuntime builds its selector runtime directly rather than via createSelectorRuntime, so sessionless waits had no slot for the capture runtime to report the consumed snapshot into and lost the system-surface disclosure. Regressions pin sessionless wait success and timeout, both asserting no session record exists and the disclosure is present. |
||
|
Michał Pierzchała
|
d851658421 | test: stabilize iOS snapshot timeout regression (#1309) | ||
|
Michał Pierzchała
|
dd153a6233 |
fix(replay): default-exclude observation-only reads from repair heals, add --record opt-in (#1271 stage 2) (#1303)
* fix(replay): default-exclude observation-only reads from repair heals, add --record opt-in (#1271 stage 2) Amends ADR 0012 decision 6: snapshot/get/is/a read-only find are excluded from a repair-armed heal by default (session.saveScriptBoundary set), never from ordinary open --save-script authoring recording. wait keeps recording (flow timing, not observation). The corrective-read trap (wave-3 E3: the diverged step was itself a get) means blanket read-exclusion is unsafe, so a new --record flag forces one action through when the correction is itself a read. --record/--no-record are mutually exclusive (INVALID_ARGS if both are set) and are plumbed identically across CLI, the Node client, and MCP. The exclusion lives at the single daemon-side choke point (recordActionEntry/isExcludedRepairSegmentObservation), so an excluded read never grows session.actions.length -- the same counter the existing record-and-heal resume watermark (describeUnperformedRecordAndHeal) already checks, so the empty-segment fail-loud guard falls out for free (message updated to mention --record). Also fixes a latent bug found along the way: the get/is/find/snapshot CLI readers never forwarded --no-record/--record into the built request (only `open` did), so stage 1's "use --no-record" guidance was silently inert via the CLI. * test(integration): cover --record with a provider-backed repair-segment scenario (#1271 stage 2) The progress ratchet (test:integration:progress:check) flagged `record` as an unclassified public CLI flag. Classifying alone would only trade that failure for "missing Provider-backed integration workflow flag coverage" -- and the exclusions bucket is for config/output/transport flags, not behavior flags, so using it would dodge the ratchet rather than satisfy it. Adds a focused provider-backed scenario instead, next to the `--no-record` precedent in android-lifecycle.test.ts. It drives the real request router, session store, replay runtime, and script writer (only the ADB provider is faked), and proves the flag's actual purpose end-to-end: inside a repair-armed `replay --save-script` segment that diverged, the SAME `get text <selector>` runs twice differing only in `--record`; exactly one line lands in the committed healed .ad. Also asserts `--record` + `--no-record` is INVALID_ARGS. Verified the scenario reproduces the bug: with the exclusion neutered it fails on "a diagnostic read inside a repair segment must not be recorded". * fix(replay): key the repair-segment exclusion on provenance, scope --record (#1271 review) Addresses the maintainer review on #1303. P1 — the exclusion dropped PLANNED reads from the heal. It discriminated by command class, but the real discriminator is provenance. Replayed plan steps dispatch through the ordinary request path, so an authored get/is/find step hit the same recordIfSession -> exclusion path as an interactive read and never reached session.actions -- and the heal IS session.actions.slice(boundary). A repaired flow therefore replayed its authored `is visible` assertion and then silently dropped it from its own healed script: the heal quietly stops checking what it used to check, which for a 10x-QA-replay suite is the worst failure mode. Fix: an explicit provenance marker, not a heuristic. `internal.replayPlanStep` is stamped by invokeResolvedReplayAction -- the single point every plan step is dispatched, so it covers annotated and unannotated steps alike. `internal` is daemon-only (toDaemonRequest never copies it off the wire), so authored provenance cannot be spoofed; same channel as replayTargetGuard. The rule now lives once in isInteractiveObservation and both recording call sites consume it, so the mock fixture uses the production classifier instead of mirroring it. Planned observations survive automatically -- users never annotate their own .ad steps. --record is no longer a common flag: removed from COMMON_COMMAND_SUPPORTED_FLAG_KEYS, statically scoped via allowedFlags to snapshot/get/is, and validated dynamically for find (read-only allows; a mutating find click|fill|focus|type is INVALID_ARGS before any device work, sharing one isReadOnlyFindAction predicate with the read-only routing so the two cannot disagree). --no-record stays shared -- it applies to every recordable command. Removed from `open`, which is never observation-only. Rebased onto #1304 and dropped the four hand-rolled reader blocks. Split its helper rather than broadening it: noRecordInputFromFlags (all 13 readers) + observationRecordInputFromFlags (snapshot/get/is/find only). Two named helpers over one `allowRecord` policy arg -- the capability is then the helper's NAME, so a mutating reader physically cannot forward --record, whereas a policy arg would let a future mutating reader opt in by flipping a literal with no schema change. ADR-0012 decision 6 now states the provenance rule, not a command-class rule. The scenario gates the P1: its authored step is a distinguishable `is visible`, and it fails without the provenance check ("the authored 'is visible' step must survive the heal"). * test(daemon): pin that wire-supplied `internal` never reaches a daemon request #1271 stage 2 made `DaemonRequest.internal` semantics-affecting: `internal.replayPlanStep` decides whether an observation-only command is an authored plan step (kept in a repair heal) or an out-of-band diagnostic (excluded). That makes "internal means internally-stamped" worth pinning rather than leaving to convention. The invariant already holds, structurally and twice over: the boundary's `commandRpcParamsSchema` is an allowlist projection emitting only its eight named fields, and `toDaemonRequest` then builds the request field by field. Neither can carry `internal` off the wire. This posts a real JSON-RPC request carrying `internal: { replayPlanStep: true }` through a loopback server and asserts the dispatched request has no `internal`. Verified it fails ("a wire-supplied `internal` must never reach the daemon request") when both allowlists are regressed, so it guards the composite contract instead of restating one layer. |
||
|
Michał Pierzchała
|
856d5d4900 |
test: replace the hand-typed Maestro fixture with a generated conformance oracle (#1289)
* test: replace the hand-typed Maestro fixture with a generated conformance oracle Closes #1274. The old harness (scripts/maestro-conformance*) compared 5 hand-authored flows against a hand-typed transcription of Maestro 2.5.1's command model. It proved parser self-consistency, not conformance: all four bug classes that cost #1217 days of live debugging slipped past it by construction, and it verified no upstream SHAs despite parsing them. Every expected value here is generated from the pinned upstream artifacts. dev.mobile:maestro-orchestra:2.5.1 is published on Maven Central, so the harness runs the real parser and reads the real bytecode — no full Maestro source build. Layer 1 (parser): a Gradle/Kotlin harness drives the pinned YamlCommandReader over a corpus of 42 vendored maestro-test flows (sha256-recorded) plus authored bug-class, coverage, and invalid flows, capturing each parse. The verifier parses each flow with the live engine and classifies it identical / both-reject / we-reject / mismatch / we-are-lenient. Every non-identical outcome must be a declared divergence, so the 17 we-reject entries in expected-divergence.ts are the mechanical parity backlog (assertTrue, clipboard, travel, killApp, and option-level gaps) rather than silent drift. Layer 2 (semantics): ASM reads static-final constants straight from the pinned bytecode without initializing driver classes (MAX_RETRIES_ALLOWED=3, SCREENSHOT_DIFF_THRESHOLD=0.005, ANIMATION_TIMEOUT_MS=15000, erase cap, and the iOS pre-tap gate we intentionally omit), plus the parser-observed 400ms swipe default. Each is cross-checked against MAESTRO_COMPATIBILITY_PRESETS. Layer 3 (differential): scheduled device scenarios. Cross-engine comparison is outcome parity only and says so; finer behavior is asserted engine-side via invariants over replay-timing.ndjson. Bug class 4's detector — a tap must not consume the whole settle budget, since a full-budget tap means the stability loop never latched while the flow still passes — is pure and unit-tested against synthetic traces; only the device run is scheduled-only. regenerate.mjs verifies the pinned jar SHA-256s before trusting output and is byte-deterministic across runs. Layers 1-2 verify in normal CI via node --test with no Java (the job installs deps: unlike the layering guard it copies, the verifier parses with the live engine, which imports the `yaml` package). Acceptance: the four bug classes each have a fixture; every command in SUPPORTED_MAESTRO_COMMAND_NAMES (the parser's own dispatch table, now exported as the single source of truth) is corpus-covered or listed unverified; the five documented deviations are expected-divergence entries. * fix: address review findings on the conformance oracle P1 — layer-3 scenarios could never run. They pointed at layer-1 corpus flows, which exist only to be PARSED: they name a fictional com.example.app and elements that exist on no device. A device run would have failed before exercising any runtime behavior, making bug class 4's detector silently vacuous. Layer 3 now has its own flows under differential/flows/ driving the real fixture app (examples/test-app, com.callstack.agentdevicelab); the workflow builds and installs it and hard-fails if it is missing. A test enforces the separation so a scenario can never point back at the parse corpus. Nothing else in this repo builds or installs the Expo fixture app, so those steps are new and unproven. The workflow is therefore dispatch-only: the cron is removed until a supervised first run proves the path. A nightly job that fails at 05:00 every day teaches nothing. P2 — layer 3 installed whatever version the online installer served. It now pins MAESTRO_VERSION from pinned-upstream.json, so layer 3 cannot drift from the version layers 1-2 claim, and asserts `maestro --version` matches. P2 — fixture content was not bound to regeneration. CI compared only the embedded upstream metadata, so a hand edit to a captured command or constant passed: the transcription failure mode this oracle exists to remove. Two-layer fix, because per-PR CI must stay Java-free and cannot re-derive: - Each fixture now carries a contentHash seal that the verifier recomputes, so editing a capture breaks the build. Tamper-evident, and tested by actually tampering rather than assuming a hash comparison works. - New scheduled conformance-regenerate job re-runs the harness against the pinned jars and fails on any byte difference. Forgery cannot survive a real re-derivation. This is what makes "generated from upstream" enforced. P3 — boot-ios-test-simulator requires runtime-version; now passed alongside preferred-device-name, as the other iOS workflows do. * tmp: trigger layer-3 differential on this branch to prove the device path workflow_dispatch cannot run pre-merge (it registers from the default branch), so this temporary push trigger exists only to execute the never-run device path on the PR head and capture evidence. Removed before merge. * fix(ci): install the fixture app unfrozen for the layer-3 device run First live run of the device path failed at the very first step: ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. CI implies --frozen-lockfile and the fixture app's lockfile is out of sync with its package.json overrides. No CI job has ever built examples/test-app, so that drift was never surfaced. * fix: drop --ignore-workspace from test-app:install (defeats #649 security overrides) The first live run of the layer-3 device path failed at ERR_PNPM_LOCKFILE_CONFIG_MISMATCH, and the cause is a real latent bug rather than a stale lockfile. #649 moved the fixture app's `overrides` into examples/test-app/pnpm-workspace.yaml precisely because pnpm only honors overrides from a workspace root — they pin transitive deps (ws, brace-expansion, xmldom, postcss, uuid, shell-quote) to versions that clear Dependabot alerts. But `test-app:install` passes --ignore-workspace, which ignores that very file, so the overrides are dropped and no longer match the lockfile that has them baked in. It goes unnoticed locally because interactive installs are not frozen, and no CI job has ever installed this app. Dropping --ignore-workspace makes examples/test-app resolve as its own workspace root (it has its own pnpm-workspace.yaml and is not a member of the repo-root workspace), so the overrides apply and a frozen install succeeds. Verified both directions locally: with the flag + --frozen-lockfile reproduces the CI failure; without it, a frozen install completes and the lockfile's overrides stay intact. Note the workaround this replaces would have been actively harmful: installing with --no-frozen-lockfile resolves the mismatch by regenerating the lockfile WITHOUT the overrides, silently reverting the app to the vulnerable transitive versions #649 pinned away. * fix: make layer-3 scenarios prove what they claim, and parse the Maestro version Run 3 (29497919702) got the whole device path working: Expo build (30m), app installed, simctl check, pinned Maestro CLI install. Only the version ASSERTION failed — `maestro --version` prints an analytics banner before the version, and `tr -d '[:space:]'` mashed banner+version into one string. The CLI was correctly 2.5.1. Match the semver line instead, and set MAESTRO_CLI_NO_ANALYTICS (CI should not phone home). Verified the parse against the exact CI output: banner and clean forms both yield 2.5.1, wrong/empty still fail. tap-retry-if-no-change was vacuous: it tapped a navigating control, so the first tap always succeeded and retryIfNoChange never ran — it passed while proving nothing. It now taps the app's non-interactive title so the screen cannot change and the retry path is forced, and asserts tapRetries >= 1 from the trace (MaestroRuntimeMetrics already records it per step). A new metricAtLeast invariant kind carries the assertion; a test reproduces the old vacuity. percent-swipe no longer claims bug class 1. Truncation vs rounding is a <=1px delta that no app-observable device outcome can distinguish, so pass/pass could never back that claim up. The runtime half is instead pinned exactly by a pure unit test of resolveMaestroCoordinate (it short-circuits on a known viewport, so no device is needed) — verified to catch the regression by flipping trunc->round, which turns 3 of 6 tests red. Truncation had no test coverage at all before this. A test now forbids any device scenario from re-claiming bug class 1. * fix(ci): pass --maestro and match the fixture app's real UI in layer-3 flows Run 4 (29500262301) reached the differential itself — build, install, simctl check and the pinned Maestro 2.5.1 verification all passed — and surfaced two real bugs, both mine: 1. The runner invoked `agent-device test <flow>` without --maestro, so every scenario failed with "test does not support this file type". The repo's own scripts/run-test-app-maestro-suite.mjs passes it; the flag is what routes a .yaml through the Maestro compat engine. 2. settle-after-tap and percent-swipe assumed home-open-form is on screen at launch. It is not: real Maestro reported "Element not found: home-open-form", and the app's own helper flow scrolls it into view first. settle-after-tap now scrolls before tapping, mirroring that helper; percent-swipe no longer navigates at all and swipes the scrollable home screen, so it tests the conversion and nothing else. The remaining two flows already reported maestro=pass, so only the agent-device invocation was wrong for those. Note the settle invariant correctly reported "no-data: no completed tapOn steps" and FAILED rather than passing — a detector that cannot run is a failure, as intended. * feat: declare layer-3 divergences and schedule the differential Layer 3 ran both engines for the first time (29504440599) and immediately found a real engine bug. Blocking the measurement instrument on repairing what it just measured inverts the dependency, so layer 3 now gets the contract layer 1 already had: every divergence is a decision on the record. Adds `knownDivergence: { reason, tracking }` to the scenario type — the layer-3 twin of FLOW_DIVERGENCES. A declared divergence keeps the run green; only UNDECLARED ones fail. Two rules stop that from rotting, both enforced mechanically rather than by prose discipline: - `tracking` is required and must be a real issue URL (run.test.ts), because a declaration with nothing behind it is how "temporarily expected" becomes permanent without anyone deciding to. - a stale declaration FAILS: if a declared-divergent scenario starts passing, the run goes red until the declaration is removed. The fix PR must delete it, and the differential then enforces the gap stays closed — the oracle is the acceptance test for its own findings. Declared: - settle-after-tap -> #1299. Our scrollUntilVisible times out finding home-open-form where Maestro 2.5.1 scrolls to it and passes. Real engine correctness bug in an advertised command, found by this differential. Blocks bug class 4's device detector until fixed. - tap-retry-if-no-change -> #1300. The invariant caught the scenario being vacuous: both engines pass but tapRetries was 0, so retryIfNoChange never ran. Needs an inert fixture control; a scenario defect, not an engine one. Proven green on both engines and enforced now: percent-swipe, optional-warned-not-failed — the latter is real device-verified warned-vs-failed parity. With declarations in place the differential is green, so the schedule goes in (cron 05:00) per #1274. A green run still prints what it is not proving. * fix: park the flaky retry scenario instead of declaring it a divergence Run 29510020718 fired the stale-declaration guard on its first outing and caught my own mistake. tap-retry-if-no-change measured tapRetries=0 in run 29504440599 and tapRetries=1 in 29510020718 — same flow, same commit. So it is not vacuous as #1300 originally claimed: it is NON-DETERMINISTIC. The tap sometimes holds the hierarchy signature still and sometimes does not, because the fixture home screen carries live content. That exposes a real limit of the mechanism added in the previous commit: knownDivergence assumes the divergence REPRODUCES. A declared-but-flaky scenario flips between known-divergence (green) and stale-declaration (red) at random — a coin-flip scheduled job, which is worse than no scenario because it teaches people to ignore the differential. So the scenario is parked, not declared. The flow and the tapRetries invariant stay implemented and unit-tested, so the fix PR only re-adds the scenario once the fixture has an inert control. retryIfNoChange therefore has NO device coverage right now — tracked in #1300 and stated plainly rather than disguised by a green run. A test keeps it out of the active set until then. #1300 updated with the corrected diagnosis and both runs' evidence. Active differential: settle-after-tap (declared divergence, #1299), percent-swipe and optional-warned-not-failed (both enforced, pass/pass on real devices). * fix: make a knownDivergence waiver cover exactly one failure, not any failure P1 from re-review, and a real flaw: the code did not do what its own comment claimed. runScenario() collapsed every unexpected outcome and every invariant failure into `misbehaved`, then turned ANY of them green if the scenario carried a declaration. So while the #1299 scrollUntilVisible waiver is open, upstream Maestro could start failing too — or a different invariant could break — and the scheduled job would still report known-divergence and pass. A waiver for one bug was silently amnesty for the next. That is the exact failure this oracle exists to prevent, committed one commit after building the guard against it. knownDivergence now requires an `expected` signature: both engines' outcomes plus each declared invariant's status. The runner matches it exactly — - matches -> known-divergence (green, tracked) - misbehaves differently -> failed (red): not the failure the waiver covers - stops misbehaving -> stale-declaration (red): remove the declaration #1299's signature pins what runs 29504440599/29510020718 actually observed: maestro=pass, agent-device=fail, settle invariant no-data. Tests prove unrelated failures stay red under an open waiver: upstream also failing, our engine unexpectedly passing, a different invariant status, and a new invariant appearing are each NOT covered. A signature where both engines pass is rejected outright as describing no divergence. Also retains replay-timing.ndjson as a run artifact (review evidence note): the invariants are computed from that trace, so a report saying "tapRetries was 0" cannot be audited once the runner is gone without it. * perf(ci): cache the fixture app build for the layer-3 differential The differential job took ~30 minutes, of which 1331s (22 min, 79%) was building the Expo fixture app and only 347s was the differential itself — rebuilt from scratch on every run for an app that changes almost never. Cache the built .app, keyed on everything that can change the binary: the app's sources, native config, dependency graph, the build step itself, the iOS runtime, and the Xcode version. Mirrors the existing setup-apple-replay prebuilt-runner cache (same action pin, same Xcode-key + source-hash shape). On a hit the build is skipped entirely and the bundle is installed straight onto the booted simulator (~seconds), taking the job to roughly 8 minutes. On a miss it falls back to exactly the previous behaviour and repopulates, so the worst case is unchanged. The existing simctl verification still gates both paths, so a bad cache cannot produce a vacuous green: if the app is not installed, the job fails loudly rather than running scenarios against nothing. Note the first run after this lands is necessarily a miss. * refactor(ci): extract setup-fixture-app so any job can use the cached app The fixture-app build + cache was inline in the differential workflow, so nothing else could reach it. Extracted to a composite action mirroring setup-apple-replay, because the capability is what #320 has been missing: it wants replay coverage moved off Apple system apps onto a controlled fixture with stable ids, and that fixture (examples/test-app) already exists — CI just had no way to build and install it. The cache is genuinely shared. GitHub caches are per-repository and readable across workflows, and a run restores from its own branch or the default branch, so once a run on main populates it every workflow gets the hit and only the first one pays the ~22 minutes. The key is computed inside the action from a fixed input list and deliberately contains nothing caller-specific — folding a caller's workflow path into it would silently unshare the cache. Also removes a duplication risk: the action reads the bundle id from the built app's Info.plist rather than hardcoding it, so it cannot drift from what was actually built, and it fails loudly if the app is not installed. The conformance workflow keeps its own narrower assertion — that the installed id is the one its scenarios target — since that is its concern, not the action's. Usage: - uses: ./.github/actions/setup-fixture-app with: runtime-version: ${{ env.IOS_RUNTIME_VERSION }} # outputs: app-path, app-id, cache-hit * chore(ci): remove the temporary branch push trigger Run 29519848340 on this head executed both engines against the real fixture app and came back green, so the trigger that existed only to prove the never-run device path has done its job. Merged config is now cron (05:00) + workflow_dispatch, as required by #1274. known-divergence settle-after-tap maestro=pass agent-device=fail (#1299) ok percent-swipe maestro=pass agent-device=pass ok optional-warned-not-failed maestro=pass agent-device=pass This commit will not itself trigger a run: GitHub evaluates triggers at the pushed commit, and the push trigger is gone in it. |
||
|
Michał Pierzchała
|
c245906b70 |
fix(cli): forward --no-record from every recordable command reader (#1304) (#1305)
`--no-record` is accepted on every command (its key is in
COMMON_COMMAND_SUPPORTED_FLAG_KEYS, which seeds every command schema's
supportedFlags) and is documented as "Do not record this action", but no
interaction/capture reader forwarded it into the options object the daemon
request is built from. The flag parsed, then vanished.
The rest of the chain was already wired: command-flags.ts maps
options.noRecord onto the request, and recordActionEntry reads
entry.flags?.noRecord to skip the action. Only the reader step was missing,
so the documented behaviour never happened for any of press, click, fill,
longpress, swipe, focus, type, scroll, get, is, find, snapshot, or wait.
`app` was the sole command that forwarded it.
Measured through the real argv path (parseArgs -> readInputFromCli), before
this change: 13/13 commands accept `--no-record`, 0/13 reach the options
object. After: 13/13.
Fixed at the shared seam rather than per reader. noRecord is a common flag,
so it gets a recordControlInputFromFlags() helper next to the existing
settleInputFromFlags/repeatedInputFromFlags group helpers, and each
recordable reader spreads it. A future reader picks it up by spreading one
helper instead of re-deriving a flag it never names.
The regression test covers all 13 recordable commands and fails without the
fix ("press dropped --no-record").
|
||
|
Michał Pierzchała
|
a6789d086b |
docs: clarify keyboard dismiss fallbacks (#1302)
* docs: guide iOS keyboard blur fallback * docs: simplify iOS keyboard blur guidance * docs: clarify keyboard dismiss fallbacks * fix: make keyboard fallback skillgym cases decisive |
||
|
Michał Pierzchała
|
95f838f514 |
test: stop pinning settle capture counts against a wall-clock loop (#1307)
* test: model the UI in settle-observation fixtures instead of a capture count settle-observation's "contention flakes" were a zero-margin comparison meeting a 1ms clock skew, not generic load. runStableCaptureLoop derives pollMs = min(300, max(25, quietMs)), so the test's settleQuietMs: 25 made pollMs === quietMs, and the settle check (one sleep(25) plus capture time, against >= 25) a 0ms margin. Node's setTimeout(25) advances Date.now() by only 24ms in 0.13% of calls idle and 0.63% under load, because libuv's timers and Date.now() read different clocks. On a 24 the loop takes a third capture that the transcript never scripted, and settle's best-effort catch reports the resulting throw as settled: false. The fixtures now model the surface rather than the runner's speed: a quiet UI serves the settled tree to every capture, a busy one a fresh tree per capture, via new transcript `repeat` entries and result factories. The snapshot-floor economy guard survives as a bound (2-3 captures), and the follow-up's "no fresh capture" cost — previously implied by the exact transcript — is now asserted directly. Production is untouched: the same zero margin only costs a wasted extra capture and poll there, filed separately as #1306. * test: stop pinning the settle capture count in the iOS contract scenario too A sweep for the same bug class found direct-ios-selector's settleObservation scenario carrying the identical 0ms margin: settleQuietMs: 25 (so pollMs === quietMs) against a consume-once transcript scripting exactly two settle captures, with no injected clock. It has not lost the coin flip in CI yet, but it fails the same way when it does — a third capture finds no entry and settle's best-effort catch reports settled: false. Same fix: the fixture models a quiet UI (every settle capture sees the same tree) instead of scripting how many captures fit in a wall-clock window. The rest of the sweep was clean. The other contract scenarios and the interaction runtime tests already use clamped mocks that repeat the last snapshot, so any capture count is tolerated; the fake-clock tests are correct to pin exact counts. * style: oxfmt quietRunnerSnapshotEntry signature * test: make one-shot-outranks-repeat a real transcript rule (P2 review) The review is right: `one-shot entries still outrank a repeat entry` asserted a guarantee the lookup did not provide. It passed only because the one-shot happened to be declared first — unordered lookup took the first match, so a repeat declared ahead of a matching one-shot shadowed it forever and left it permanently unconsumed. Both failure modes reproduce; the reverse-order test added here fails on the previous implementation. Unordered lookup now searches matching one-shots before repeats, so outranking holds whatever the declaration order. A repeat is documented as its command's fallback. Ordered transcripts now reject repeats at construction: ordered lookup only ever reads the head, so a repeat there never advances and strands every entry behind it. Refusing the combination beats failing later as a confusing "Provider command mismatch". Coverage added for both: reverse declaration order, and ordered + repeat. |
||
|
Michał Pierzchała
|
117f78107e |
feat: add direct Limrun provider runtime (#1278)
* feat: add direct Limrun cloud runtime * refactor: reuse Android provider runtime for Limrun * refactor: pass runner context to provider runtimes * fix: remove Android gesture swipe fallback * fix: reconcile Limrun direct runtime with main * refactor: compose Android provider interactors in core * fix: satisfy packaged Limrun runtime checks * perf: load Limrun provider runtime on demand * docs: document Limrun device cloud flow * refactor: reuse Android reverse provider for Limrun * fix: isolate provider-owned iOS sessions * fix: preserve provider runtime boundaries * refactor: split close repair lifecycle * fix: reject unavailable provider leases * fix: reconcile provider runtime review feedback * test: stabilize alert deadline smoke assertion * fix: recover expired provider leases * fix: limit Limrun to remote simulators * fix: make Limrun provider cleanup durable * test: cover Limrun connect through CLI * fix: make provider expiry recovery durable * refactor: remove Limrun compatibility cleanup * fix: release live provider leases on expiry |
||
|
Michał Pierzchała
|
1fdbf80c32 |
fix(replay): retarget identity-empty press containers to their labeled descendant (#1280) (#1286)
* refactor(replay): share the id-demotion predicate via target-identity-node Extract session-target-evidence.ts's demoteNonUniqueId into a shared demoteNonUniqueLocalIdentity (target-identity-node.ts), and export build.ts's normalizeSelectorText. Both become shared building blocks a third call site (#1280's press-retarget identity-empty check) reuses instead of re-deriving the id-demotion rule and value/text normalization a third way. No behavior change. * fix(replay): retarget identity-empty press containers to their labeled descendant (#1280) Android list-row presses target a clickable container (role="linearlayout") with no id, no label, no value/text — its title lives on a labeled descendant (the android:id/title TextView, whose own id #1272 already demotes for being non-unique). The container's identity is role-only and shared by every row, so replay disambiguates positionally and mis-binds under reorder (measured matchCount 12, 20/20 identity-mismatch). Retarget at record time: when a press/click/fill resolves to an identity-empty container (rule 1), substitute its first labeled descendant in document order (rule 2), but only when the container's subtree has no other interactive/hittable node (rule 3, fail-closed — a trailing Switch/Checkbox must not retarget, since a tap at the descendant's center vs the container's could land on different controls). Guard-blocked or label-less subtrees record exactly as today. Implemented once at the single recording choke point (describeResolvedInteractionNode, resolution.ts): the returned node feeds BOTH buildSelectorChainForNode's chain and (downstream, via recordedTargetCapture) computeTargetEvidence, so the two writers can never half-retarget. Recording-time only — resolveSelectorChain and live press/fill dispatch are unchanged; the tap point is already fixed against the original container before this substitution runs. Adds an ADR 0012 decision 3 amendment (mirroring #1269's), a press-retarget unit/guard/cross-invariant suite (including an RN FlatList iOS parity fixture), and a reorder+insert e2e proving the retargeted recording rebinds by role+label where the un-retargeted container recording refuses. * fix(replay): keep response hittability on the dispatched container, not the retargeted descendant Review blocker on #1286 (flag 1 adjudicated): describeResolvedInteractionNode was computing describeNonHittableTarget from the retargeted descendant, so every retargeted press on a non-hittable title TextView would emit a false `targetHittable: false` + misleading hint on the exact happy path the fix serves — a live-response regression violating the design's recording-time-only rule. Split the fields by what they are FOR: recording-coupled fields (node as evidence source, selectorChain, refLabel — they become the .ad step) keep following the retargeted descendant; the response-semantic describeNonHittableTarget (targetHittable + hint) reverts to the original node, describing what was actually dispatched. Documented in the function comment and the ADR amendment; new load-bearing test (fails against the pre-fix line): a hittable container with a non-hittable labeled child presses with no targetHittable/hint while chain/evidence/refLabel belong to the descendant. * fix(replay): carry the press retarget on a recording-only side channel; harden the guard (#1280 re-review) Maintainer re-review corrections, four findings: P1a (side channel): the runtime response is now entirely container-based — node, selectorChain, refLabel, point, resolution disclosure, hittability all describe the dispatched container, restoring the response-identity contract. The retarget travels as an optional recordingTarget {node, selectorChain, refLabel} on the runtime result (contracts/interaction.ts), consumed only at the recording boundary (interaction-touch-response.ts): the recorded action entry — the .ad writer's result.selectorChain source — takes the descendant chain/ref-label and recordedTargetCapture feeds the descendant node to computeTargetEvidence, while both wire payloads keep container materials. Daemon-route regression proves response container-based + recorded entry, target-v1 evidence, and the physically written .ad line descendant-based. P1b (fill): removed from retarget scope — a fill chain carries editable=true constraints a label descendant can never satisfy, saving an unreplayable script. click/press only; replay test proves the recorded fill chain on an identity-empty editable container still resolves uniquely. P2a (duplicate container ids): the identity-empty predicate now evaluates from the DEMOTED identity view — dropped the extractNodeText probe whose raw-identifier fallback resurrected an id that had been demoted for non-uniqueness, which made duplicated-container-id rows skip the retarget they need most. Fixture proves retarget fires; unique-id contrast unchanged. P2b (guard): replaced the private role-fragment list with the canonical interactive classification — isSemanticTouchTarget (exported from core/interaction-targeting.ts, the same policy hittable-ancestor promotion uses) plus the hittable flag; the module moves to src/core/press-retarget.ts since selectors -> core would be a layering back-edge. Added the geometric containment condition: the selected descendant's rect center must lie inside the container's rect (missing rects fail closed) — the replay tap point must be provably within the original activation region. Tests: nested Cell (role the old list missed) blocks; out-of-bounds descendant blocks; rect-less container blocks. ADR 0012 decision-3 amendment rewritten to the side-channel design, click/press-only scope, demoted-view rule, and both guard halves. The daemon regression runs on the iOS runtime path (direct-iOS is recording-gated) so the unit lane spends no real wall-clock on Android adb dialog probes. |
||
|
Michał Pierzchała
|
991e723e8b |
fix(android): actionable error when the snapshot helper is unavailable (#1284) (#1285)
* fix(android): actionable error when the snapshot helper is unavailable (#1284) #1284 kept the hard-fail from #1217 (a silent stock-UIAutomator fallback produced a materially different, app-window-only capture) but asked for actionable hints on both failure modes: - Artifact missing on disk: hint now names the exact `pnpm build:android` command, the full dist file set it produces, and notes packaged installs ship it via prepack. - Artifact present but the device rejects the install (adb/OEM policy): the underlying adb error already surfaces in the message; the hint now explicitly frames this as a device-side failure distinct from a missing build artifact, tagged via a new androidSnapshotHelperInstallFailure detail set at the adb install call site. * fix(android): correct helper-missing hint to the two runtime-required files Review correction on #1285: resolveAndroidSnapshotHelperArtifact only fs.access'es the versioned .manifest.json and the .apk it references — the sha256 is a manifest field, and *.idsig is excluded from the npm package by design. The hint no longer claims the full sidecar set is required, and the test now rejects any future idsig claim. * fix(android): cover install rejections and preserve diagnostic identity (#1285 review) P1: the device-side install marker now covers the whole install phase. ensureAndroidSnapshotHelper previously tagged only a resolved nonzero install result; an AndroidAdbProvider.install rejection (enriched INSTALL_FAILED_* AppError from the provider funnel) bypassed the marker and fell back to the generic retry/doctor hint. Both paths now flow through markAndroidSnapshotHelperInstallFailure, which mutates details in place so the original code, message, hint, details, and cause all survive. Regression covers the public daemon snapshot route with a real request handler and an injected provider whose install rejects. P2: androidSnapshotHelperCaptureError rewrapped through normalizeError, which lifts diagnosticId/logPath out of details — the rewrap dropped them (ADR 0010 violation). They are now reinstated into the rewrapped error's details. Hint selection moved to a helper to keep the function under the complexity gate. * fix(android): restore all lifted wire fields through the capture rewrap (#1285 review) normalizeError hoists hint, diagnosticId, logPath, retriable, and supportedOn out of details (ADR 0010); the capture rewrap restored only diagnosticId/logPath, so a transient-classified install rejection (e.g. connection_dropped) lost its structured retriable signal on the public daemon error. liftedDiagnosticIdentity is generalized to liftedWireFields covering the complete hoisted set (hint stays owned by the capture hint selector). Public route regression: a retriable provider install rejection keeps error.retriable === true on the daemon snapshot response. |
||
|
Michał Pierzchała
|
bd62502126 |
fix(android): bound the scroll-hint dumpsys probe and skip it in wait polling (#1270) (#1288)
* fix(android): bound the scroll-hint dumpsys probe and skip it in wait polling (#1270) deriveScrollableContentHintsIfNeeded's dumpsys activity top probe ran with an 8s timeout equal to a whole wait/get budget, and measured latency ranges from 37ms to ~5.9s under contention. Cap it at 1.5s so one pathological call can't starve a caller's timeout, and skip hint derivation entirely during find ... wait polling, since a presence check never consumes scroll hints. The wait polling loop actually lives in commands/interaction/runtime/selector-read.ts's waitForFindMatch (daemon/handlers/find.ts's own handleFindWait is unreachable for wait today — dispatchFindReadOnlyViaRuntime always intercepts read-only find actions first), so the skip-hints option threads through that capture path down to snapshotAndroid via the existing flags -> contextFromFlags -> dispatchCommand -> interactor.snapshot channel. * fix(android): skip scroll-hint derivation in standalone wait polling too (#1270) The issue's motivating repro — wait 'label="Battery"' 8000 — polls waitForSelector, not the find-wait loop, and text/ref waits poll waitForText (backend.findText -> captureWaitSnapshot on the daemon, or the snapshotContainsText fallback). All three presence-only polling captures now disable hidden-content-hint derivation, matching the Android alert-wait capture which already did. Stable wait keeps full snapshot semantics. Adds daemon-route regressions for the exact repro shape on both the selector-wait and text-wait routes, asserting every per-poll snapshot dispatch carries snapshotIncludeHiddenContentHints: false. |
||
|
Michał Pierzchała
|
1a1ef7c419 |
feat(android): one persistent automation helper owning snapshot + viewport + canonical injection (#1281)
* feat(android): consolidate touch injection and gesture viewport into the persistent snapshot helper (#1275)
One Android automation helper now owns snapshot capture, gesture viewport
resolution, and canonical one-/two-pointer plan injection. A live persistent
helper session executes gesture/viewport commands over its socket protocol;
without a session the same APK runs one-shot via am instrument. The separate
one-shot multitouch helper APK is deleted (atomic replacement, no fallback).
Touch scheduling/injection is extracted into focused Java classes
(TouchPlan, TouchPlanInjector, PointerEventSchedule, GestureViewportReader)
instead of growing SnapshotInstrumentation. ADR 0013 amended.
* fix(android): stop a structurally-failed helper session before the one-shot viewport retry
A structured ok=false viewport response leaves the session process alive, and
Android permits only one instrumentation owner of UiAutomation - running the
one-shot fallback against a still-live helper contends with it and masks the
original structured failure. Stop the session first; regression pins that the
one-shot retry only executes once the session is gone.
* refactor(android): extract helper touch dispatch into focused classes; split session tests; document helper API v2 (PR #1281 review)
Addresses findings 2 and 3 from PR #1281 review (finding 1, viewport
session-stop ordering, was already fixed in
|
||
|
Michał Pierzchała
|
b10c8cfb3a |
fix: warn agents that armed-repair diagnostic reads are recorded too (#1271 stage 1) (#1287)
During an armed `--save-script` record-and-heal repair, read-only diagnostics an agent runs to locate the corrected target (snapshot -i, get attrs, find, is) are recorded into the healed script by default, alongside the corrective press. The wave-3 E3 repair-economics experiment measured 0/4 trials producing a clean healed script hands-off, and one recorded `get attrs` caused a second, self-inflicted identity-mismatch divergence on fresh replay. This is stage 1 of the maintainer's two-stage triage on #1271: safe interim guidance only, no recording-behavior change. Stage 2 (defaulting read-only commands out of the repair transaction) stays gated on an ADR-0012 amendment. - divergence.ts: buildRepairHintGuidance appends a diagnostics --no-record clause to every repairHint's text guidance, gated on resume.repairSessionHeld === true (decision 6, R7 C1's armed-repair signal) so it never renders on a plain, non-repair divergence. - cli-help.ts: the "Agent-supervised repair (heal-by-doing)" section in `help workflow` now says the same thing. - Unit coverage: divergence.test.ts asserts the clause is present iff repairSessionHeld is true, across record-and-heal/state-repair/caution/ manual. - SkillGym regression: agent-device-smoke-suite.ts adds record-and-heal-diagnostics-no-record, verified 3/3 against claude-haiku and codex-mini live runners. |