Commit Graph

839 Commits

Author SHA1 Message Date
Michał Pierzchała 56b72c5cf7 refactor(boundaries): put shared contracts below their consumers, gate the result (#1405)
* refactor(boundaries): move shared contracts below their consumers

Acts on the depgraph findings: type-only edges are invisible to R5, so
vocabulary that everything depends on had drifted above the zones that use it.

- contracts/: the four platform-plugin facet tags (LogBackend,
  RecordingBackendTag, PerfMetricsSamplerTag, PlatformGatedProviderResolverKey)
  now live beside the plugin contract itself, which also moves out of core/;
  NetworkEntry moves next to the command surface that renders it; and the
  click-button, recording-export-quality, interactor-types and
  runner-lease-context vocabularies move down out of core/.
- (root) drops from 29 files to 13: the internal *-contract/output/annotation
  modules move into contracts/, kernel/ (daemon-error, observability-redaction
  beside kernel/redaction), core/ (batch-policy, an ADR 0008 projection),
  commands/ (cli-command-aliases) and remote/ (upload-progress, upload-stream).
  What remains is entrypoints and the composition roots that R2 requires to
  sit outside the spine.
- utils/ joins the ranked spine at rank 1 after its only two upward files move
  to the zones they were reaching for (cli/resolve-cli-options,
  cli-schema/cli-config), putting ~336 value edges under the gate.
- Internal imports that routed types through the client-types re-export hub now
  name their real source.

Type-only spine inversions drop from 61 to 35; the remainder is two clusters
(client/client-types.ts and the ADR 0003 daemon facet). No behaviour change:
4470 unit tests and the layering gate pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* style: merge the duplicate contract imports the tag moves created

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* refactor(imports): name the declaring module, share find's argument rules

Two follow-ups from re-measuring the graph after the boundary moves.

1. 89 type imports across 79 files routed through a re-export hub in another
   zone: `CliFlags` reached through commands/cli-grammar/flag-types.ts (52) when
   it is declared in contracts/cli-flags.ts, the replay suite result types
   reached through daemon/types.ts when they are declared in contracts/replay.ts,
   the doctor types through a daemon handler module, and so on. Each hop invented
   a cross-zone edge the architecture never asked for — including every apparent
   replay -> daemon and utils -> commands dependency. They now name the module
   that declares them. Within-zone hops are left alone; those are a local style
   choice, not a boundary claim.

2. `find`'s three positional/flag checks existed in both daemon entry points with
   hand-repeated messages, and the copy in dispatchFindReadOnlyViaRuntime was
   unreachable — its only caller validates first. Both now call checkFindArgs in
   selectors/find.ts, beside parseFindArgs and isReadOnlyFindAction, for the
   reason that module's own comment already gives: so the two paths cannot
   disagree. The refusal is returned rather than thrown, because the two
   mechanisms are not observationally identical in the session event log.

Type-only spine inversions: 61 -> 35. 4470 unit tests and every gate pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* feat(layering): ratchet type-only spine inversions (R6)

R5 ignores type-only edges by design — they cost nothing at runtime and do not
affect cold start — so nothing was watching the direction they point. Ranking
them the same way found 61 inversions, including contracts/ and utils/ declared
in terms of rank-4 zones. 26 are fixed by the preceding commits; R6 pins the
rest per zone pair so they can only shrink, and a new pair fails outright rather
than being added to the baseline.

The two remaining clusters each need their own change, and the baseline says so:
the per-command Options/Result vocabulary declared inside the public Node-client
surface, and the ADR 0003 daemon facet shape that core's descriptor registry
composes.

Both ratchet directions are covered: growth fails, and shrinking without
lowering the number fails too, so the baseline cannot quietly stop describing
the tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* docs: record the import-graph findings behind this refactor

A dated snapshot, not a normative document: when it disagrees with
scripts/layering/, the gate wins. The graph tool that produced it lives on the
claude/depgraph-viewer branch, deliberately out of this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* refactor(selectors): state the shared selector argument rules once

R2 (commands-floor) forbids the daemon from importing commands/, and that is the
right call: commands/ is the client-side surface — its only consumers are cli/,
cli-schema/, mcp/, client/ and the composition roots — while the daemon is the
executor on the other side of the wire. ADR 0008 protects exactly that seam.
Relaxing R2 would let the executor depend on a client projection and pull CLI
grammar and output formatting into the daemon's bundle.

But the rule does force duplication: the daemon must validate independently
because it accepts requests from any client, so 10 refusal messages existed in
both zones. The only place a shared rule can live is below both, and selectors/
already held the parsers (splitIsSelectorArgs, splitSelectorFromArgs,
isSupportedPredicate) and even the `is` predicate message — just not the checks
that use them.

Three drifts had already appeared in the `is` predicate rule alone:

- commands/interaction/selectors.ts re-implemented the predicate list as an
  inlined seven-way `!==` chain while importing the message and hint from
  selectors/predicates.ts, so adding a predicate to the shared list would not
  have reached the CLI grammar.
- That inlined chain compared the raw token, so the CLI rejected `is TEXT ...`
  while the daemon it hands the command to accepts it. The CLI now matches the
  executor; this is an intentional alignment, not an accident.
- isCommand raised the same refusal without IS_PREDICATE_USAGE_HINT, so whether
  an agent got recovery guidance depended on which layer noticed first — the
  failure mode ADR 0010's audit calls out.

checkIsPredicate, checkIsArgs, checkGetFormat, checkElementTargetArgs and
checkWaitText now hold those rules, each beside the parser it wraps, and report
a refusal rather than choosing how to raise it: the daemon returns a response,
the command surface throws. Those mechanisms are not interchangeable — they
write different session events — so the shared check stays out of that decision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* feat(daemon): give ADR 0014's ref frame one transition, pin SessionState owners

`SessionStore.get()` returns the live record out of a private Map and `set()`
re-puts the same reference, so every `session.<field> = …` in the daemon is a
durable write to store-owned state: 57 of them across 17 files, against 26
`set()` calls that are therefore ceremonial. Nothing at the store boundary can
check what those writes are supposed to keep true.

Measuring which module writes which field showed the problem is narrower than
the raw count suggests — 16 of 27 fields already have exactly one writer. The
sharp case is ADR 0014's ref frame: `refFrameState`, `refFrameScope`,
`refFrameTree` and `refFrameGeneration` must move together or the frame is
incoherent (an `active` state with a stale tree resolves refs against a
namespace nobody authorized), yet complete issuance wrote them in ref-frame.ts
and partial issuance wrote the same four in session-snapshot.ts. ref-frame.ts's
own header claims to be "the single owner of the frame's transitions", and
session-snapshot.ts documented itself as the exception. Both forms now go
through `activateRefFrame`; they differ only in scope.

`recordSession` deliberately moves alone in two paths (recording without arming
a publication), so the save-script cluster gets no invented abstraction — it
gets ownership instead. R7 records every field's owner and stops the set from
growing quietly: a new SessionState field must declare one, a foreign write
fails naming the owner to call, and an owner that stops writing must be removed
so the table cannot drift into fiction. Field names are read out of the
`SessionState` declaration, so a daemon module with an unrelated local named
`session` — a provider or runner session — cannot trip it.

4475 unit tests and every gate pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* docs: record the reference semantics and refresh the findings

SessionStore.get/set now document that the record is handed out live, since that
is the fact behind R7. The findings snapshot picks up the resolved R2 question,
the ref-frame consolidation and the two new gate scopes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* refactor(boundaries): rank every satellite zone, extract the provider port

Second-order effect of the earlier rounds. With `utils` on the spine and
`(root)` emptied of shared contracts, the eleven zones that were unranked
"because ranking them would invent an order the architecture had not committed
to" turned out to have a consistent rank already — the order was there,
unasserted. Solving the constraint system showed one blocker: `utils/remote-config.ts`
projected a remote-config profile into `CliFlags` while reaching up into
`remote/`, and its only three consumers were in `cli/`. It moves there as
`cli/remote-config-flags.ts`, and every satellite zone joins the spine.

Ranked coverage goes from 730/895 files to 882/895. Only `(root)` stays out, and
now for one stated reason: R2 forbids `daemon/` from importing `commands/`, so
the files that wire them compose the spine from above.

Ranking them exposed 22 type-only inversions R6 had never been able to see, and
they were concentrated rather than scattered:

- The device-provider port. `providers/` and `cloud-webdriver/` implement what
  the daemon calls, so both sides name `DeviceLease`, `LeaseLifecycleProvider`,
  `LeaseLifecycleContext` and `DeviceInventoryProvider` — now declared in
  contracts/device-provider.ts, below both. The adapters also imported the
  daemon's NARROWED `DaemonRequest` while only ever reading `req.flags`; they now
  name the public one from kernel/contracts.
- `MetroPrepareKind` and the remote-config profile field groups move to
  contracts/ for the same reason: the command surface validates them and
  contracts/cli-flags.ts is composed from them.

Two clusters remain, ratcheted with their reasons in TYPE_INVERSION_BASELINE:
the client-types vocabulary, and `SessionAction`, which needs `CommandFlags` and
`DaemonBatchStep` to move with it.

Also fixes two things CI caught: the eight type re-exports my earlier import
redirection orphaned (none published through any src/sdk/* entrypoint, so no
public surface changes) and `isSupportedPredicate`, now module-private since
`checkIsPredicate` is the admission API. `fallow-baselines/health.json` is keyed
by path, so the moved cli-config entry moves with the file rather than being
regenerated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* fix(selectors): use the admitted predicate, not the raw option

Review finding. `isCommand` called `checkIsPredicate` and then kept reading
`options.predicate` for the capture policy, the `exists` branch,
`evaluateIsPredicate`, the failure message and the returned result. Admission
normalizes case, so an upper-case predicate was let past the gate and then
evaluated against lower-case branches: `EXISTS` skipped its own branch and fell
through to the generic path, and the result echoed the raw token. I widened
admission at that surface without threading the normalized value through it —
the CLI-grammar surface in the same change does use the admitted value.

Every decision after admission now reads it.

Two tests, both verified to fail without the fix:

- a production-route regression driving `device.selectors.is` with
  `EXISTS`/`TEXT`, plus one pinning that an unknown predicate is still refused
  WITH the ADR 0010 usage hint;
- a surface parity gate (selectors/__tests__/is-argument-surface-parity.test.ts)
  in the repo's existing parity style, asserting the daemon and CLI-grammar
  surfaces reach the same verdict and hand the same normalized predicate
  downstream across an input table. A helper-only test cannot catch a surface
  that admits correctly and then discards the result, which is what happened
  here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* docs: name the pre-push gate, and the formatter's path allowlist

Both misses in this PR's review were process, not judgement, and the docs
pointed the wrong way for both.

AGENTS.md said "prefer the aggregate package.json scripts" without naming which
aggregate, and CONTRIBUTING listed `pnpm test` and the targeted checks but never
`pnpm check`. `check:tooling` looks like the gate and is a subset of it: it stops
before the Fallow audit, so the dead exports this PR introduced passed a clean
`check:tooling` and failed CI. Both files now name `pnpm check`, say what it
covers, and say what it cannot (the device matrix).

The same gap produced a second mistake twice: `oxfmt <path>` reformats whatever
you point it at, while the repo's `format` script is an allowlist that excludes
`scripts/` and every `.md`. One run reformatted 50 unrelated script files into a
commit; the next nearly did it to AGENTS.md. AGENTS.md now says to run
`pnpm format`, never `oxfmt <path>`.

It also records the rule that cost a CI cycle: Fallow's baselines are keyed by
path, so a renamed file needs its baseline entry moved, not the baselines
regenerated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* revert: undo stray formatter output across docs and scripts

Three separate `oxfmt <path>` runs in this branch reformatted files the repo's
`format` script deliberately excludes: 55 files under scripts/maestro-conformance
plus scripts/perf, sync-mcp-metadata and the slow-test reporter, and 12 markdown
files including six ADRs and docs/agents/. All of it was whitespace, quote style
and markdown table padding — no content — but it inflated the diff a reviewer has
to read and would have rewritten prose ownership across files this change has no
business touching.

All 70 are back to their origin/main content, so the diff outside src/ is now
exactly this change's scope: three docs, scripts/layering, the Fallow baseline,
and five provider integration tests.

The rule this violated is now in AGENTS.md: run `pnpm format`, never
`oxfmt <path>`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* style: reformat two provider tests with the repo's pinned oxfmt

`pnpm format:check` failed in CI on the two files whose imports I merged by hand.
The repo pins oxfmt 0.42.0 as a devDependency and both `format` scripts invoke
`./node_modules/oxfmt/bin/oxfmt`; I had reformatted with `npx oxfmt`, which
resolved 0.60.0, and the two versions disagree about wrapping a 100-column import.

This is the rule AGENTS.md already states — run `pnpm format`, never oxfmt
directly — so there is nothing to add to the docs, only to do.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* fix(ci): install deps for the layering guard, and gate the zero-dep contract

The Layering Guard job failed with ERR_MODULE_NOT_FOUND on `oxc-parser`. The job
ran with `install-deps: false` — no `pnpm install`, so no `node_modules` — and R7
had started parsing the daemon with oxc-parser instead of matching assignment
operators with a regex. `pnpm check:layering` passed on every local run, because
locally `node_modules` is always there.

The job now installs dependencies. The alternative was to put R7 back on a regex,
which cannot see `??=` or a computed `session[key] =` write, so it would trade a
correct rule for a fast job.

That leaves the interesting part: the zero-dep contract is real for the jobs that
keep it, and it is invisible to every local run, which is the worst combination a
constraint can have. R8 makes it checkable. It reads the zero-dep job list out of
`.github/workflows/` rather than restating it — declaring a job zero-dep is what
puts it under the rule — walks each job's entry scripts and their whole
relative-import closure, and requires every specifier to be a Node builtin or
another repo file. A zero-dep job whose entry scripts the scan cannot identify
fails too, so the rule cannot be escaped by changing how the job invokes them.

Specifiers come from oxc-parser's module record, not a line scan. The closures
include `--test` files, and a test about imports naturally embeds import syntax in
a fixture string; the line scanner reported two such phantom violations in
model.test.ts before the switch, which is how a gate stops being trusted.

Verified by re-running the real gate against three injected regressions: the
layering job back on `install-deps: false` (reproduces the exact CI failure,
pointing at session-state.ts:24), a package import added to the still-zero-dep
affected-selector closure, and a zero-dep job whose run step names no script.

Also corrects the CONTEXT.md spine paragraph, which still described the satellite
zones as deliberately unranked after they had all joined the ranked spine.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* fix(layering): make R7 exhaustive, and follow session records through aliases

Review finding: `SESSION_STATE_FIELD_OWNERS` covered 27 of `SessionState`'s 42
fields and nothing asserted parity, so a new field could be added and pass the
gate by being invisible to it. R7's advertised claim — "every SessionState write
is inside its declared owner" — was broader than what it checked.

Investigating that turned up a second, larger gap the finding did not name: the
scan only recognized a binding literally named `session`. The daemon names these
records by role, so `nextSession`, `provisionalSession`, `completedSession`,
`preRunSession` and `preEntrySession` were all invisible — and three of those
writes were genuine violations R7 existed to catch:

  src/daemon/snapshot-runtime.ts:256  nextSession.snapshotScopeSource
  src/daemon/snapshot-runtime.ts:265  nextSession.snapshotGeneration
  src/daemon/handlers/session-replay-runtime.ts:707
                                      preEntrySession.pendingRecordAndHeal

The first two are the #1076 versioned-ref invariant: the generation advances
exactly when the stored tree is replaced. That rule lived in `setSessionSnapshot`
and had acquired a second statement of itself in snapshot-runtime.ts, whose own
comment admitted the bypass. It now goes through `setSnapshotLineage` in the
owning module. The third clears a watermark stamped by session-replay-resume.ts;
`clearPendingRecordAndHealWatermark` puts the clear beside the stamp.

Gate changes:
- Binding detection accepts aliases, paired with the existing declared-field
  filter so an unrelated `…Session` local only registers if it also writes a
  field SessionState owns — where the remedy is the same anyway.
- `fieldClassificationDrift` asserts parity in all three directions:
  unclassified, in-both, and naming a field SessionState no longer declares.
- `STORE_OWNED_SESSION_STATE_FIELDS` classifies the 11 fields the store
  establishes at construction. It is a positive claim, so a direct write to one
  fails and names both remedies.
- Four fields the widened scan made visible (`lease`, `deviceClaim`, `appName`,
  `saveScriptComplete`) got real owners.

`nextSnapshotGeneration` is now module-private: replacing its only external call
site orphaned the export, which `pnpm check` caught via Fallow.

Verified against three injected regressions: a new SessionState field with no
direct write (the reviewer's exact scenario), a foreign write through an alias
binding, and a direct write to a store-established field. All three rejected.
`pnpm check` green, 4486 unit tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* docs(daemon): correct the snapshot-lineage claim, and pin the real contract

Device verification of the snapshot-lineage route found that a ref pinned before
a `diff` keeps resolving with no pinned-ref warning. That is the designed ADR
0014 behaviour, not a regression — the comment describing it was wrong, and I
propagated it.

`main`'s comment in snapshot-runtime.ts said a diff "leaves client refs pinned to
the previous generation, which is exactly what the pinned warning diagnoses". The
counter and the authorization epoch are different clocks:

  - `diff` passes `issuesRefsToClient: false`, so it never reactivates the frame;
  - `resolveRefStalenessWarning` compares a pin against the frame EPOCH, not the
    observation counter, and its own comment says why — a capture that bumped the
    counter must not make a valid pin from the issuing frame look stale.

So advancing the counter is not the same as invalidating client refs, and the
observable the comment promised does not exist. I carried the sentence into
`setSnapshotLineage`'s doc when the transition moved, and then into a hardware
verification request, which cost a reviewer a device run against a false claim.

`setSnapshotLineage` itself is unchanged and was a pure move: same expressions,
same inputs as the inline assignments it replaced, so this route behaves exactly
as it does on main.

A comment that contradicts the code should be an assertion instead, so the
contract is now pinned in session-snapshot.test.ts: the diff advances the counter,
preserves the epoch, leaves the pre-diff pin resolving without a warning, and
still warns for a pin from a different frame. Verified to fail when the epoch
comparison is swapped for the counter. A second test covers the keep-current
branch, which had no coverage.

`pnpm check` green, 4488 unit tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-27 08:08:34 +02:00
Michał Pierzchała 287cc18c29 fix(gestures): fail pre-removal gesture forms at .ad parse time (#1216) (#1393)
* fix(gestures): fail pre-removal gesture forms at .ad parse time (#1216)

#1315 removed the timed forms of `swipe`, `gesture fling`, and `gesture swipe`
and `gesture rotate`'s `velocity`, but shipped without the migration guide, the
repository sweep, or the parse-time error that issue #1216's own checklist
gates a removal on. The sweep finds what that left behind: both
`06-swipe-gestures.ad` integration fixtures still carried the 5-argument swipe
and would fail at replay, two tests still asserted the removed shapes, and two
branches still read the retired positional.

Argument arity for every public gesture syntax now lives in one table keyed off
the canonical `GESTURE_KINDS`, so a new kind cannot skip it and a form removed
from the CLI is removed from `.ad` in the same edit. Both callers read it: the
CLI argv parse, and a new `.ad` preflight. A stale script now fails when it is
parsed — before the replay executes any device action — naming the line and
computing its rewrite, instead of running up to that step and failing as a
repairable divergence. The preflight checks arity only: `${VAR}` tokens resolve
after planning, and interpolation never splits a token, so the count is
decidable while the values are not.

Deleting the dead duration read in `readSwipeGeometry` would have left
`replay export` emitting no duration, handing Maestro's 400ms default to a
gesture the script runs at 100ms, so the export now states `duration: 100`.

`.ad` positional gesture parsing is NOT removed. Its only remaining callers are
the CLI argv parse and the `.ad` line parse, both the current public syntax
rather than a bridge to an older one, so there is nothing to migrate off. ADR
0013 records that and drops the "compatibility" framing that made it read as
debt.

Both migrated fixtures verified on real devices with the repo's own CLI: iOS
simulator 34.9s, Android emulator 45.9s.

* fix(gestures): reject removed swipe input at the Node/MCP boundary

Review findings on d88c6ed8.

P1: `interactionDaemonWriters.swipe` hand-projects five fields, so a JavaScript
caller's `durationMs` was dropped before the daemon's `readSwipeInput` could
reject it and a default-duration fling ran instead — the exact silent
reinterpretation the guide promises does not happen. `gesture` was already safe
because its writer runs `readGestureInput` -> `readGesturePayload`, which
rejects the removed keys; `swipe` was the one surface with no reader of its own.
The rejection now lives in contracts and is shared by the client writer and the
daemon handler, so there is one rule and one message. The SDK regression covers
all four removed keys and asserts the transport is never reached; reverting the
writer call fails it on `swipe durationMs`.

P2: the preflight's retired-slot test required a numeric token, so
`swipe 197 650 197 300 ${DURATION}` fell back to bare usage text. An unresolved
`${VAR}` now counts as the retired slot and is carried into the pan rewrite,
while a stray flag or word stays a plain usage error.

P2: the removal shipped in 0.20.0, not 0.21 — removal commit 6d99914f4 is
contained in tag v0.20.0. The guide said 0.21 because the CHANGELOG still files
it under `Unreleased`; the tag is the truth (headings lag several releases
repo-wide, so that is pre-existing and left alone). The `.ad` grep recipe now
matches variable-backed durations too.

* docs(gestures): make the migration sweep and MCP claim accurate

Re-review findings on 328bad8e (both migration-guide accuracy).

The saved-script sweep matched only the five-argument `swipe` form; it missed
timed `gesture fling`, timed `gesture swipe`, and `gesture rotate ... velocity`,
so the repository-cleanliness step was incomplete. Provide one grep per retired
form (number-or-`${VAR}` token), each verified to flag the removed shape and
skip the valid one.

The MCP section claimed the structured rejection carries the CLI's concrete
replacement command. It does not: `readGesturePayload` and
`assertNoRemovedSwipeInput` return a message that names the removed key and the
replacement command (e.g. `gesture fling does not accept durationMs; use gesture
pan for timed movement`) but not the fully-substituted coordinate rewrite,
because the structured request carries no positional string to rewrite. Describe
what the structured path actually returns.

* docs(gestures): make the .ad sweep parser-aligned

Re-review finding on 4fd5d39: the documented sweep required literal single
spaces and unquoted numeric tokens, but the `.ad` tokenizer separates on any
whitespace (`/\s/`, so tabs too) and accepts double-quoted tokens, so
tab-separated or quoted-duration stale lines the parser rejects were missed.

Rewrite the patterns to follow the tokenizer: `[[:space:]]+` between tokens and
a numeric slot that accepts a bare or double-quoted number (optionally signed)
or `${VAR}`. Requiring a digit in the numeric slot keeps a trailing flag like
`--count` from being read as the retired positional. Verified against a fixture
of tab/space/quoted/quoted-var/negative encodings that the four greps flag
exactly the lines `parseReplayScriptDetailed` rejects and none it accepts, and
that the live repo sweeps clean.

Also state plainly what a regex cannot promise: the parser is the authoritative
gate — every retired form is rejected at parse time before any device action, so
running the suite finds every stale line by construction and a missed grep can
never reach execution. The grep stays a bulk pre-flight, and the "every affected
line" claim is scoped to that.
2026-07-26 11:41:02 +02:00
Michał Pierzchała 256887f194 feat(apple): injectable Apple runner transport seam for provider interactors (#1389)
* feat(apple): injectable runner transport seam for provider interactors (#1297)

createAppleInteractor now accepts an optional AppleRunnerProvider (or bare
command executor). When injected, every runner-command method runs inside
withAppleRunnerProvider scope, so the shared selector/tap/fill/scroll/
snapshot stack rides the provider transport instead of local XCTest —
mirroring createAndroidInteractor's AndroidAdbProvider parameter.

Methods backed by local Apple tooling (simctl/devicectl: open, openDevice,
close, screenshot, clipboard, setSetting) fail fast with
UNSUPPORTED_OPERATION in provider mode instead of silently running local
tooling against a remote device; provider sessions compose their own
implementations on top.

Local behavior is unchanged: without the new parameter the factory returns
the same interactor as before, and daemon-owned sessions keep resolving the
local XCTest runtime.

* fix(daemon): request-boundary provider runner scope + per-request interactor context

Review findings on #1389 (P1/P2):

P1 — daemon routes that issue Apple runner commands outside interactor
methods (keyboard, native alert, point read, iOS sequence chunks) escaped
to the local XCTest runtime for provider devices. ProviderDeviceRuntime
now exposes getAppleRunnerProvider; createProviderDeviceRuntimeRequestProviders
composes it into an appleRunnerProvider request resolver and the daemon
runtime wires it, so the existing request-boundary scope covers those
routes with the provider transport.

P2 — per-request RunnerContext was discarded for provider devices:
getInteractor threads it through getProviderDeviceInteractor into
ProviderDeviceRuntime.getInteractor, so runtimes composing the shared
Apple interactor keep requestId (cancellation/accounting), appBundleId,
and log paths per request.

Lease-route commands (lease_allocate/heartbeat/release, artifacts) now
skip sessionless provider-device resolution: they manage lease lifecycle,
not a device session, and resolving a default device there spuriously
triggered local device discovery before any lease existed.

Integration coverage: keyboardDismiss reaches the provider transport via
the request scope; every runner call in a request carries that request's
id.

* test(daemon): assert direct-route runner calls keep the request id through the provider scope

Re-review follow-up on #1389: the keyboard-dismiss provider-scope test now
sends a requestId and asserts the recorded runner call carries it, proving
the request-boundary appleRunnerProvider scope preserves per-request
context for direct daemon routes (not just deviceId matching).

* fix(daemon): revert-sensitive transport tests, route-derived lease skip, guarded provider-scope resolve

Review round 3 on #1389:

1. The integration acceptance test passed with the interactor transport
   param removed — the request-boundary scope was routing for it. Tests now
   run in two worlds: the shared-stack and per-request-id tests use a world
   WITHOUT getAppleRunnerProvider (the interactor param is the only seam;
   verified failing when the param is dropped), while the direct-route test
   keeps the request scope it pins.

2. skipSessionlessProviderDevice for lease-route commands is now derived
   from daemon.route === 'lease' in shouldSkipSessionlessProviderDevice
   instead of hand-spread across four descriptors, with a registry-driven
   invariant test enumerating the route.

3. resolveScopedProviderDevice catches resolveTargetDevice failures and
   returns undefined: provider-scope plumbing failing to find a device
   means 'no provider scope', never a failed request.

Also collapses the duplicated provider-scope Proxy from android.ts and
apple/interactor.ts into core/interactor-scope.ts, and restates the
transport param's role (local-tooling partition + out-of-daemon scoping)
in its doc comment.

* fix(core): move the provider-scope proxy below the ranked spine

Layering Guard (R5 zero-back-edges) rejected platforms/apple/interactor.ts
value-importing core/interactor-scope.ts: platforms (rank 1) may not import
core (rank 2). The helper needs nothing from core — generic withMethodScope
in utils (unranked, already imported by both zones) replaces it.

The prior local layering pass was a false green: the guard enumerates
tracked files and the new helper was untracked when the gate ran.
2026-07-25 15:22:42 +02:00
Michał Pierzchała 75b5bc5d6d feat: add first-class Vega VVD TV support (#1396)
* feat: add first-class Vega OS TV support

* fix: scope Vega support to VVD

* fix: tighten Vega platform boundaries
2026-07-25 14:02:25 +02:00
Andrew Barnes 14be01b781 fix(replay): preserve cwd scope for opened sessions (#1401)
Co-authored-by: Bortlesboat <169967362+Bortlesboat@users.noreply.github.com>
2026-07-25 12:17:54 +02:00
Michał Pierzchała 877e68fe30 fix(cli): compact stale device status (#1388)
* fix(cli): compact stale device status

* fix(cli): quote stale status selectors
2026-07-25 11:44:22 +02:00
Michał Pierzchała 11e0a1f187 feat: add WebView accessibility lab (#1397)
* feat: add WebView accessibility lab

* refactor: tighten iOS snapshot presentation rules

* fix: preserve semantic WebView containers
2026-07-25 11:40:27 +02:00
Michał Pierzchała 5507a08b9c feat: parameterize sensitive recorded inputs (#1369)
* feat: parameterize recorded inputs

* fix: harden parameterized replay recording

* fix: sanitize parameterized fill echoes

* fix: scrub embedded parameterized fill echoes

* fix: make recorded fill scrubbing idempotent

* fix: replay parameterized coordinate fills

* test: align parameterized publication landmark
2026-07-24 20:37:29 +02:00
Michał Pierzchała 5a50cfb892 fix(daemon): keep an active replay session's daemon alive over the CLI path (#1390)
* fix(daemon): keep an active replay session's daemon alive over the CLI path

A `replay <script>.ad` with no terminal `close` reports its session as
still active per ADR 0016's consumption contract, but the real CLI
client tears down the daemon that ran it (and its owned ephemeral
state dir) regardless — the request's own success response gets
overwritten seconds later by an empty `session list`. This happens
whenever the client started the daemon itself, independent of whether
the state dir was randomly generated or passed explicitly via
--state-dir/AGENT_DEVICE_STATE_DIR, matching #1384's live repro.

Add `sessionActive` to `ReplayCommandResult`, computed from whether the
session survives in the daemon's own store (never by re-parsing the
script), and gate the client's one-shot teardown on it — mirroring the
existing ADR-0012 repair-divergence keep-alive. A kept-alive owned
daemon now also attaches a --state-dir address hint to the response so
the caller can reach it. `test` is unaffected: its own per-file runner
already closes each session before the suite summary is built.

Fixes #1384

* fix(daemon): fix CI formatting, address #1390 review feedback

- oxfmt --check flagged the new test file; reformatted (CI fix).
- Address hint now names --session <name> too, using the response's
  session verbatim (already the fully-qualified cwd-scoped store key) —
  a bare --session default only resolves by coincidence from the same
  cwd, per resolveEffectiveSessionName's explicit-flag bypass.
- Add a test closing the loop: a follow-up sendToDaemon using the
  hinted --state-dir/--session reaches the same kept-alive daemon
  without spawning a new one.
- Pin that a completed (non-diverging) --save-script repair also keeps
  its daemon alive via the same guard, since its terminal source close
  is always skipped (ADR 0012 Fix 3) — document the resulting deferred
  heal-commit timing in ADR 0012.

* test(daemon): reduce complexity of new active-session tests for CI gate

Fallow's audit gate (new-only findings) flagged the two new active-
session tests for exceeding the CRAP threshold. Extract the shared
fixture wiring into replayLeavingSessionActive/parseAddressHint
helpers (also cutting duplication between the two tests), and drop
repeated optional chaining on response.data in favor of a single
narrowing assert.ok(data) — same assertions, lower branch count.

* fix(daemon): add sessionActive to MCP schema, real-producer tests, ADR fix

Addresses the second review pass on #1390:

- MCP replay output schema (src/mcp/command-output-schemas.ts) omitted
  the new required sessionActive field entirely; add it.
- ADR 0016 still claimed "the absence of close changes ... nor the
  success response shape", contradicting the new required field this
  PR adds. Amend it to document the sessionActive contract and why
  the real CLI/IPC client needs it (issue #1384).
- All prior lifecycle tests exercised sessionActive only through a
  fake HTTP response in the client-layer tests, so deleting either
  real producer line (session-replay-runtime.ts, session-replay-
  maestro-response.ts) would not have failed anything. Add tests
  against the real runReplayScriptFile producer (native .ad close-less
  -> true, terminal close -> false, Maestro close-less -> true) and
  strengthen the provider-scenario (real daemon route) test with the
  same assertion. Verified each new test fails when its corresponding
  producer line is reverted, then restored.

Live-validated the fix on real backends (booted iOS 16 simulator and
a running Android Pixel 9 Pro XL emulator), replaying issue #1384's
exact repro end to end: the owning daemon and its session both survive
a close-less replay and remain fully addressable via the hinted
--state-dir/--session on both platforms. That validation surfaced a
separate, pre-existing bug -- `session list` (no explicit --session)
omits cwd-scoped sessions opened via a replay's internal `open`
dispatch, because session-open.ts's resolveImplicitSessionScope(req)
sees a different req than the top-level replay request and leaves
session.sessionScope unset -- filed as #1394, out of scope here since
it is a sessionScope-propagation gap unrelated to the client-side
teardown this PR fixes; the session itself is never actually lost.

* fix(daemon): hint --session at explicit state dirs too, shell-quote the hint

Addresses the third review pass on #1390 (P2 x2):

- withActiveSessionAddressHint (renamed from ...IfOwned) no longer
  suppresses the active-session hint entirely for an explicit
  --state-dir/AGENT_DEVICE_STATE_DIR caller. The session name is
  cwd-qualified and, per #1394, `session list` can't rediscover it
  either, so --session is now hinted regardless of ownedStateDir;
  --state-dir is only included when the state dir is the client's own
  randomly-generated one the caller has no other way to learn.
- attachActiveSessionAddressHint now shell-quotes (shellQuoteIfNeeded,
  the same helper session-recovery-hints.ts/request-lock-policy.ts
  already use) both the state dir and session name, so the hint stays
  literally copy-pasteable even if either contains spaces or shell
  metacharacters. Added tests pinning both the unsafe-value quoting
  and that quoting was actually exercised (not just coincidentally
  unchanged) -- verified they fail against a raw-interpolation
  reversion, then restored the fix.
2026-07-24 20:09:35 +02:00
Michał Pierzchała 9b610fbd1e feat(replay): recorded landmark identity for wait, is coverage — read-only step identity (#1349) (#1381)
* refactor(replay): extract shared target-evidence tree helpers into src/replay

Move buildIndexMap/buildAncestryChain/filterIdentitySet out of the daemon's
session-target-evidence into the shared replay zone so the commands runtime
(wait's polling loop, #1349) can consume them without importing the daemon;
press-retarget drops its private buildIndexMap duplicate.

* feat(replay): recorded landmark identity verification for wait, get-pattern coverage for is (#1349)

- New CommandDescriptor trait targetIdentityVerification pins the
  evidence-carrying command set and routes wait to a post-resolution
  phase so an annotated wait never enters the generic pre-dispatch
  verification (an absent landmark is its expected starting condition).
- wait <selector> records landmark-mode target-v1 evidence (existence
  self-check; identity-empty matches record no annotation) and, on
  replay, keeps polling until a selector match carries the recorded
  identity; a deadline with only impostor matches fails closed as an
  identity-mismatch REPLAY_DIVERGENCE, a recorded-unverifiable
  annotation refuses before polling, and a plain timeout stays an
  action-failure divergence.
- is (except exists) joins the get pattern: evidence at record time,
  generic pre-dispatch verification, and the post-resolution guard
  threaded through dispatch; direct-iOS fast paths for wait/is are
  gated during recording and guarded replays.
- Read-only find stays intentionally unannotated (fuzzy-locator
  resolution has no selector-chain identity token), proven by test.

* feat(publication): destination guard requires verified recorded landmark identity (#1349)

A qualifying ADR 0016 guard is now a selector wait whose target-v1
annotation is verified; identity-less or unverifiable guards are refused
with a recovery hint. Adds the reshuffled-screen false-pass regression:
record -> publish -> replay against a same-label/different-ancestry tree
diverges as identity-mismatch (matchCount >= 1 proving the selector
alone would have false-passed).

* refactor(replay): dedupe post-dispatch identity-mismatch shaping, trim evidence-writer complexity

Shared buildPostDispatchIdentityMismatchResponse behind the guard and
wait-landmark conversions; extracted payload-ceiling helpers from
computeTargetEvidence; identity-refusal conversion split out of
resolveReplayStepResponse. Docs: ADR 0012 decision 3 amendment (#1349),
ADR 0016 guard strengthening, help workflow/save-script text.

* refactor(replay): make landmark evidence's record-time verification explicit, trim ADR-restating docs

The landmark-mode self-check was provably a tautology (the winner is a
member of its own identity set whenever the parent walk is intact), so a
membership scan defended only by a comment is replaced with the explicit
decision: broken walk fails closed, landmark is verified by construction,
action mode keeps decision 3's step-5 self-check. Doc comments that
re-argued the ADR amendment now state behavior and point to it.

* chore: untrack multitouch-helper build artifacts, ignore its build/dist dirs

Generated Android helper output swept into the earlier refactor commit by
accident; analogous snapshot-helper/ime-helper build dirs were already
ignored.

* fix(interaction): wait polls ride out content-unreadable captures (live-validated on Android)

Live ADR 0016 validation on a Pixel emulator showed a destination-guard
wait replayed immediately after a navigation press deterministically
dies: the first poll's capture lands mid-transition and the Android
helper's 'insufficient foreground app content' verdict threw out of the
polling loop. iOS already yields the same state as a sparse verdict
with no matches, so the loop kept polling there — this makes wait
semantics platform-consistent. A content-verdict capture failure
(isUnreadableCaptureContentError) now counts as a no-match poll for
selector and text waits; a wait whose screen never became readable
rethrows the last capture verdict at the deadline, so persistent
breakage keeps its diagnosis. Other capture failures still throw
immediately.

* fix(snapshot): narrow unreadable-capture classification to enumerated content verdicts

Android stamps androidSnapshotHelperFailureReason on mechanism failures
too (helper timeouts, adb failures, missing helper artifact — free-form
reason strings), so matching any string made waits poll those to their
deadline instead of failing immediately. The predicate now matches only
the enumerated content-recovery reasons, and
AndroidHelperContentRecoveryDecision derives its reason union from the
same list so a new content verdict cannot miss the predicate. Adds the
realistic wrapped mechanism-error regression the synthetic test missed.

* test(interaction): make the wait mechanism-failure regressions revert-sensitive

Assert exactly one capture attempt: the broad any-string classifier
would poll the repeated fixture error to the fake-clock deadline and
rethrow the same message, passing the message-only assertion. Verified
the mechanism test fails against the broadened classifier and passes
against the narrowed one.
2026-07-24 20:05:09 +02:00
jibin jose 968db8b26f fix(daemon): explicit abort message on uncommitted repair close (#1383)
* fix(daemon): explicit abort message on uncommitted repair close

Fixes #1380

* style: run pnpm format

* fix: loud abort for close --save-script on uncommitted repair

* fix: address static check failures from review
2026-07-24 12:54:52 +02:00
Michał Pierzchała a0ab735ffd fix(interaction): direction-named, selector-first off-screen recovery hints (#1366) (#1374)
* fix(interaction): direction-named, selector-first off-screen recovery hints (#1366)

An agent that hits a scrollable form section gets correctly rejected for
targeting an off-screen element, but the recovery hint didn't name a
concrete next move. Its "obvious" retry — re-issue the same @ref after
scrolling — is exactly what both live guards reject: the off-screen guard
still sees it off-screen, and the scroll expired the ref frame (ADR 0014),
so the @ref is refused as stale. Two bootstrap-bench runs burned all 60
turns on a single checkout-form screen this way (arm-independent).

Make the rejection self-sufficient for recovery instead of relaxing either
guard:

- Off-screen selector/ref rejection now names the exact scroll direction
  (computed from the target rect vs its effective viewport) and steers the
  retry to a *selector*, which re-resolves against a fresh snapshot and
  bypasses the ref-frame admission guard entirely. The direction is also
  surfaced as a machine-readable `scrollDirection` detail.
- `scroll`'s unknown-direction error now carries a grammar hint, since the
  transcripts show agents mis-shaping it as `scroll @ref down` — scroll
  takes a direction and no target.

New geometry helper `classifyOffscreenScrollDirection` maps a rect +
viewport to the reveal direction (largest-overshoot axis), following the
same convention the CLI off-screen summary already uses.

Both guards keep their exact rejection semantics and messages; only the
hints and one added detail field change.

* fix(interaction): derive scroll direction from the boundary that rejected (#1366 review)

Address P1: the direction classifier used a rect-vs-single-viewport test, so it
only fired when the whole rect was fully past an effective-viewport edge. But
`isNodeVisibleOnScreen` rejects on two boundaries — and the second one (tap-point
CENTER outside the ROOT viewport while the rect still overlaps its container) is
exactly the off-screen-drawer / edge-straddling case #1366 is about. Those got
the generic hint and no scrollDirection, leaving the loop unresolved.

`classifyOffscreenScrollDirection` now takes (node, nodes) and mirrors both
rejection boundaries: full separation from the effective viewport, then a
center pushed outside the root viewport. Direction is taken from whichever
boundary failed, using the same unrounded center as the tap-point rule, so a
rejected target always yields a direction.

Regressions: partial clip whose center is past the viewport edge (both the pure
classifier and the live selector-press rejection path), and a child inside an
off-screen scrollable ancestor (closed drawer).

* style: oxfmt the windowRoot test helper (#1366 CI)

* fix(interaction): bounded off-screen recovery hint — a large scroll overshoots (#1366 review)

Live evidence exposed that the hint over-promised: it advertised `scroll
<direction>` -> retry-selector as the terminating recovery, but on the
motivating checkout form a plain `scroll` (iOS fling momentum) repeatedly
overshoots the narrow pressable band and the loop never terminates — even
bounded `scroll --pixels 200` oscillates up/down. A momentum-free `gesture pan`
in small steps lands the target (verified: 3–4 attempts -> Tapped "Cash").

Revise the hint to prescribe BOUNDED movement in the named direction, retrying
the same selector after each step, and to name the overshoot failure mode and
the reliable `gesture pan` fallback. Direction + selector-first steering are
unchanged. `scrollRevealClause` drops the "to bring it on-screen" over-promise.

Tests assert the bounded-recovery guidance (small steps / gesture pan) on both a
selector and a ref off-screen path so it can't silently regress.
2026-07-24 08:17:57 +02:00
Szymon Dziedzic 3faeb97855 feat(events): enrich session event details (#1379)
* feat(events): enrich session event details

* fix(events): harden session event projections

* fix(events): sanitize provider-derived metadata

* fix(events): close remaining projection gaps
2026-07-24 08:17:23 +02:00
Michał Pierzchała 67d5d21cf8 fix(android): preserve fixed siblings after snapshot filtering (#1378) 2026-07-23 16:08:02 +02:00
Michał Pierzchała 5cea83d994 fix: select iOS simulator by installed app (#1376)
* fix: select iOS simulator by installed app

* fix: preserve app-aware open selection
2026-07-23 16:03:33 +02:00
Michał Pierzchała 00ef28734f refactor(commands): derive pass-through command bindings with a generic BoundOf helper (#1368)
The five fully pass-through families (capture, system, admin, recording,
observability) hand-wrote 'key: (options) => x.key(runtime, options)' per
command plus a Bound* type mirroring RuntimeCommand -> BoundRuntimeCommand
field-by-field. One mapped type + one generic binder now derives both.
BoundOf keeps the optional-parameter ergonomics for commands whose options
include undefined, so call sites like commands.back() are unchanged.

bindAppCommands stays as a spread + manual 'list' override (filter
normalization); selector/interaction families stay hand-written since most
of their entries reshape signatures (prepended target/text positionals).
2026-07-23 10:06:32 +02:00
Michał Pierzchała 0b0deb3b9e refactor(interaction): resolve the CapturedSnapshot name collision (#1371)
resolution.ts and selector-read-shared.ts (both in
src/commands/interaction/runtime/) each exported an unrelated
CapturedSnapshot type with a different shape. resolution.ts's export
is never imported anywhere else, so rename it to InteractionSnapshot
rather than force a merge with the richer, session-carrying shape
that selector-read.ts/settle.ts/stable-capture.ts actually depend on.
2026-07-23 09:57:44 +02:00
Michał Pierzchała 9a61ac71a1 refactor(cli): decompose runCli into explicit phase functions (#1373)
runCli inlined parse/help short-circuits, binding resolution, remote
auth + materialization, four special-cased command kinds, dispatch, and
a catch block coupled to ~10 closure-mutated let bindings across ~390
lines. Each phase is now a named function over an explicit CliRunContext:
parseCliInputOrExit, resolveRunContextOrExit, runReactDevtoolsCli,
resolveRemoteContext, buildClientConfig, maybeStartDaemonLogTail,
createReplayReporterForTest, dispatchCliCommand, handleRunCliFailure.

The context is mutated in place by resolveRemoteContext so the failure
handler observes exactly the state the throwing phase saw — matching the
previous closure semantics, including the close-with-no-daemon success
path and daemon-log-tail-on-error. Behavior is unchanged; runCli itself
is now ~75 lines of orchestration.
2026-07-23 09:57:02 +02:00
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>
2026-07-23 09:55:57 +02:00
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.
2026-07-23 07:36:52 +02:00
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.
2026-07-22 18:17:59 +02:00
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.
2026-07-22 17:16:02 +02:00
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>
2026-07-22 17:13:37 +02:00
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 8615771c6:

1. `systemChrome` reached CLI/SDK/MCP payloads: `serializeSnapshotResult`
   passed `result.nodes` straight through, so an internal classification became
   a de-facto public contract and per-node bytes. Added `publicSnapshotNodes`
   next to the field it strips, mirroring the existing
   `publicSnapshotCaptureAnnotations` projection, and applied it at that
   boundary (the single serializer both the CLI and capture output route
   through). The regression test asserts the published nodes deep-equal their
   input minus the field AND that the serialized JSON never contains the string;
   it fails if the projection is removed.

2. Fallow complexity on `androidUiNodes` (10 cyclomatic / 19 cognitive):
   extracted `createAndroidChromeSubtreeTracker`, which owns the depth and
   chrome-subtree state, leaving the generator a token loop. Not baselined.
   Note the tracker keeps open/close as distinct operations on purpose: a
   self-closing tag leaves at the depth it entered, a container descends and is
   left by its `</node>`, so collapsing them silently breaks nesting.

* refactor(android): let the chrome classifier read the tree, deleting the test-side stamping helper

Review: the remaining long comments read as workarounds. Three were, and each
pointed at code worth changing rather than explaining.

1. `collectAndroidSystemChromeIndexes` required the walk-stamped `systemChrome`
   even when the tree still held the container that defines it. Hand-built trees
   (every settle unit test) therefore classified as "no chrome at all", which is
   why they needed `withAndroidSystemChrome` — a test helper re-implementing the
   production descent, with a comment explaining why it had to exist.

   The classifier now reads the container from whichever source still has it:
   the parent chain when the capture kept it, the stamped flag when the walk
   dropped it. The helper, its type and its comment are deleted; no test
   re-states production logic, and any caller holding an intact tree is now
   classified correctly regardless of how it was produced.

2. `createAndroidChromeSubtreeTracker` tracked depth with arithmetic subtle
   enough to need a comment about self-closing tags — and subtle enough that an
   earlier revision of it was wrong. Replaced with a stack of open elements:
   push on descent, pop on `</node>`, nothing to explain.

3. `walkInteractiveOnlyAndroidFixture`'s 16-line comment still described the
   run-classification this branch deleted — stale, not merely long. Cut to what
   the helper does.

Also trimmed the contract, kernel, classifier and serialization-test comments to
the fact each states. Comment lines in the branch drop from 116 to 51.

No behavior change beyond (1), which strictly widens correct classification.
4318/4318 unit tests, lint, typecheck, build and smoke pass.

* fix(android): keep chrome provenance internal

* refactor(android): tighten chrome provenance seam

* fix(android): close chrome provenance response leaks

* test(provider): enforce chrome invariant across clients
2026-07-22 15:50:16 +02:00
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
2026-07-21 20:24:26 +02:00
Michał Pierzchała f087a5938e fix: align Android Maestro gesture dispatch (#1356)
* fix: align Android Maestro gesture dispatch

* fix: preserve Android gesture guarantees
2026-07-21 18:44:08 +02:00
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
2026-07-21 11:49:33 +02:00
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
2026-07-21 10:54:48 +02:00
Michał Pierzchała d9f16de26f docs: retire Maestro compatibility tracker (#1350) 2026-07-20 21:09:11 +02:00
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>
2026-07-20 19:59:35 +02:00
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>
2026-07-20 18:06:04 +02:00
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
2026-07-20 16:21:06 +02:00
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>
2026-07-20 14:00:57 +02:00
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>
2026-07-20 12:44:51 +02:00
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
2026-07-20 11:29:59 +02:00
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>
2026-07-20 09:46:05 +02:00
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
2026-07-17 18:21:06 +02:00
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>
2026-07-17 18:06:44 +02:00
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>
2026-07-17 12:29:41 +02:00
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
2026-07-17 11:36:53 +02:00
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>
2026-07-17 11:36:33 +02:00
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.
2026-07-17 11:35:37 +02:00
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.
2026-07-17 09:49:33 +02:00
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 (bd6250212):  undershoot FAILS, boundary passes
  vs addbcc3ae:         undershoot passes, boundary FAILS
  vs this commit:       both pass

Boundary cases added on both sides of the edge: timeoutMs one millisecond past
the quiet window (settles at capture 2, waitedMs within budget), and equal to
it (cannot settle — the window has to elapse inside the budget).

* fix: bound the no-useful-wake sleep too, honouring the deadline contract (P2)

The review is right, and it caught my own contract being violated one branch
below where I wrote it. stableCaptureDelayMs claimed it "never wakes past the
deadline", then exempted the no-useful-wake case: with quietMs=300 and
timeoutMs=300, capture 2 cannot settle, the helper returned a full 25ms
cadence and the loop exited at waitedMs=324 where the old code exited at 300.

That branch now sleeps out only what remains of the budget. The loop is about
to exit either way; overrunning just reports a wait longer than the caller
asked for.

The equal-boundary test asserted rejection only, so it could not see the
overrun — it now asserts the injected clock's elapsed time against the budget
(the timeout error carries captures and nodeCount, but not waitedMs, so the
clock is the honest observable). It fails on 59c936353 with "elapsed 324ms",
exactly as the review predicted.

Each test still catches exactly one defect:

  vs main (bd6250212):  undershoot FAILS
  vs addbcc3ae:         301-budget boundary FAILS
  vs 59c936353:         equal-boundary overrun FAILS
  vs this commit:       all pass

* fix: guarantee the settle poll always buys time, and stop when it cannot (P2)

The review found the real hazard behind the boundary work: not an overrun but
a hot loop. Combine the 1ms-undershooting clock with quietMs=300 and
timeoutMs=301 and capture 2 lands at 299ms; lastUsefulWakeMs is then 1, the
helper asks for 1ms, and the skew eats all of it. The clock never advances,
the deadline never arrives, and the loop captures forever — hammering the
device at exactly the skew this PR exists to absorb.

A delay that buys no time is the bug. Every delay stableCaptureDelayMs returns
is now at least QUIET_DEADLINE_EPSILON_MS, which is what makes the loop
terminate under the modelled undershoot. Where the budget cannot afford a
delay that both progresses and stays inside it, there is no capture left to
place: it returns 0 and the loop stops instead of spinning.

That also subsumes the previous no-useful-wake branch, so the equal-boundary
case now stops at 299ms rather than sleeping out the remainder.

The undershooting test clock now yields to the event loop like a real sleep.
Without that the spin starves the runner on microtasks and the regression
hangs the whole job instead of failing; with it, the reviewed head fails the
new test cleanly with "Test timed out in 6000ms".

Rebased onto 9b5f333a2.

  vs main:         undershoot FAILS
  vs b6261da05:    301-budget boundary FAILS
  vs 8c963fc23:    equal-boundary overrun FAILS
  vs 4f6beb87e:    combined undershoot+301 spin FAILS (timeout)
  vs this commit:  all pass

* fix: use final settle budget under clock skew
2026-07-16 22:16:56 +02:00
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.
2026-07-16 21:12:33 +02:00
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.
2026-07-16 21:08:35 +02:00
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.
2026-07-16 20:31:05 +02:00
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.
2026-07-16 20:22:28 +02:00
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").
2026-07-16 20:16:21 +02:00
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
2026-07-16 20:15:47 +02:00