92 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	152894cce2	docs(adr): rules-first ADR restructure + ADR 0017 proposal (unified event journal) (#1399) ... * docs(adr): rules-first restructure of 0012/0014/0016, drop completed migration logs ADR 0012 alone was 42% of the ADR corpus by bytes; consulting it cost ~28k tokens of mostly process history. Restructure per the new shape convention (added to the ADR README): Status + a normative 'Rules at a glance' first so a reader can stop after ~50 lines, rationale and refuted alternatives kept below the fold, and completed migration plans/landing tables deleted — git history is the archive. - 0012: delete migration plan/progress; fix the Status section that still claimed #1235 unimplemented against its own landing table; demote the 2026-07-10 evidence audit to the end (still cited by the decisions). - 0014: same; the accepted Android blocking-dialog-recovery evidence gap and its covering fixture tests move into Status so the waiver survives. - 0016: verified implemented; rules summary added (nothing was history). No rule's meaning changed; edits are reorganization plus stale-status fixes. * docs(adr): propose ADR 0017 — unified request event journal Apply ADR 0008's registry thesis to events. Inventory (2026-07-24) found four parallel event vocabularies — ~155 stringly-typed diagnostics phases, the session events.ndjson, the progress wire stream, and the replay timing trace (one of its two writers unredacted) — with consumers coupled to emit sites by string: agent-cost counts runner round-trips by matching two phase names. Proposal: an EVENT_CATALOG in contracts making every kind a typed, trait-carrying declaration; the diagnostics scope becomes the single journal append point; every consumer becomes an explicitly registered sink; all existing file/wire formats stay byte-compatible behind golden fixtures. Explicitly rejects pub-sub and event sourcing. Status: Proposed — not indexed in the ADR README until accepted. * docs(adr): revise ADR 0017 per architecture review Address all five review findings and adopt both requested judgments: - P1 out-of-request events: finalizeRepairTeardown records a synthesized close during idle-reap/daemon-shutdown with no live request; a request-scoped-only journal would silently drop it. Added an explicit session-scoped teardown scope model (fatal-scope precedent) and rejected the ambient-fallback alternative. - P1 redaction vs byte-compat: progress stays unredacted on its own channel; the replay-trace unredacted->redacted change is now a declared, intentional compatibility change with its own fixture update, not smuggled under a byte-compat claim. - P1 per-attempt trace routing: sinks with dynamic destinations read scope-bound routing context (logPath-rebind precedent); drop-when-unbound semantics; sink ordering/isolation/flush contract made normative. - P2 progress typing: progress streaming removed from the journal entirely - it is a transport-owned output port (ordering, disconnect-as-cancellation, closed typed union); mirror emits noted as the future opt-in shape. - P2 completeness check: orphan detection is now a static source scan in the layering-lint style; runtime unit-suite observation explicitly rejected. Also per review: catalog keys are internal identities; sinks map to legacy wire discriminators, which are never automatically canonical. Migration plan reduced to 4 steps. * docs(adr): ADR 0017 — fork, never rebind, for per-attempt trace routing Review found a blocking concurrency flaw in the revised routing design: sharded test attempts run concurrently (Promise.allSettled in runReplayTestShards) under one inherited AsyncLocalStorage request scope, so mutable scope rebinding would let one attempt overwrite or clear another's replay-timing destination after an await — cross-writing or dropping events. Replace rebinding with a journal fork primitive: journal.fork(bindings, fn) runs fn in a new ALS scope object sharing the parent's buffer/phaseCounts/ envelope/sinks but carrying frozen routing bindings. Each attempt wraps its work (including nested replay dispatch) in a fork binding its own trace path; the binding dies with the fork, so no clearing step exists to race. Existing updateDiagnosticsScope rebinds stay confined to sequential request setup, pre-fan-out. Validation gains a concurrent-shard regression proving each replay-timing.ndjson contains only its own attempt's events. * docs(adr): ADR 0017 — scope identity on the envelope for future exporter sinks Reserve the one shape decision an OTel-style exporter would otherwise force a retrofit for: every scope (request, teardown, fork) carries scopeId, forks record parentScopeId, both ride the event envelope. Forks already form a tree, so an exporter sink can emit parent-child spans from envelope fields alone. Cross-process correlation stays requestId; a traceparent-style meta field is additive under ADR 0006 and deferred. No exporter in this ADR. * docs(adr): address re-review — renumber to 0018, full fork isolation, 0016 record-as 1. Renumber the proposal 0017 -> 0018: main now carries accepted ADR 0017 (parameterized recorded inputs, #1369); branch rebased onto it. 2. Fork contract strengthened: forks clone EVERY mutable scope field (envelope, logPath, routing bindings) and own their event buffer; only the sink list and the request-global phaseCounts tally are shared. Verified in code: nested dispatch creates child execution scopes (request-router.ts:257) whose updateDiagnosticsScope rebinds session/logPath mid-flight, so a shared mutable envelope would cross-route debug/session-log events between concurrent shards even with frozen trace bindings. updateDiagnosticsScope now specified as mutating only the innermost scope. The regression now covers all three routed outputs (replay-timing, per-request diagnostics ndjson, events.ndjson). 3. 0016 rules summary updated for shipped #1348: sensitive fills use fill --record-as <VAR> (ADR 0017); unparameterized fill/type stays literal (body sections already updated by #1369's merge). * docs(adr): ADR 0018 — name the usage sink as first consumer, privacy by construction The motivating consumer is opt-in usage analytics over agent behavior: command frequencies, typed failure codes, and outcome sequences that trip agents (consecutive snapshots, screenshot-after-snapshot). Decision 4 pins its discipline now, before any exporter exists: an allowlist-by-construction UsageRecord schema whose every field draws from a registry-enumerated vocabulary (command names, ADR 0010 error codes, flag names, durations, hashed session + sequence number) — positionals, selectors, labels, fill text, and error messages are unrepresentable by type, not redacted. Anti-pattern detection is downstream analysis over the stream, never emission-side logic; the sink itself is a follow-up after migration step 3. Adds the matching invariant and a schema gate to validation.	2026-07-25 14:03:03 +02:00
Michał Pierzchała	1a76344685	docs: restructure AGENTS.md and CONTEXT.md for progressive disclosure (#1402) ... * docs: restructure AGENTS.md and CONTEXT.md for progressive disclosure Apply the Claude 5 context-engineering guidance to the repo's agent docs: keep the always-loaded file to gotchas and invariants, and move situational guidance one hop away behind a routing table. AGENTS.md 315 -> 229 lines. Cut generic agent-behavior boilerplate, three-way duplication (Common Mistakes restated Hard Rules; Finding Source Owners restated the registry section), and facts visible from the repo itself. Kept verbatim: the expensive-lessons principles, enforcement gates, Hard Rules, and environment traps. Split out docs/agents/{cli-flags,pull-requests,device-verification}.md and folded the Testing Matrix into docs/agents/testing.md, reframed around pnpm check:affected so the prose stops duplicating the selector. CONTEXT.md keeps all 50 terms, now grouped under a section index so a task loads one section instead of the whole glossary. * fix(check-affected): move the selector-owning sentinel to the Testing Matrix The Testing Matrix moved from AGENTS.md to docs/agents/testing.md, but the affected-check selector still treated only AGENTS.md as selector-owning. A later matrix edit would have been classified as inert docs and skipped the fail-open, so the selector could keep deriving gates from a spec that had changed underneath it. Move the sentinel with the prose, as a named SELECTOR_OWNING_DOCS set so the next move is one line, and fix the two in-code comments plus the testing.md paragraph that still pointed at the AGENTS.md matrix. * docs: restore two rules dropped by the AGENTS.md split Review caught two repo-specific rules that did not survive the move. Both are prose without any backticked identifier, so the identifier-diff used to verify the split could not see them. - "Test through public interfaces; do not add unrelated production exports solely to enable tests" returns next to the behavioral-tests rule in docs/agents/testing.md, with the reason it exists. - The guidance-ownership rule (decide whether new guidance/schema/metadata belongs to the command surface, CLI grammar, CLI help, MCP projection, or daemon runtime) returns to the always-loaded Docs & skills section, since it governs all command-surface work and not just the flag case. Also point the ADR routing row at docs/adr/README.md, which is already the "read when you touch…" index, rather than at the bare directory.	2026-07-25 12:13:09 +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
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	b3b9ff59b0	docs: propose active-session script publication (#1347) ... * docs: propose active-session replay publication * docs: address active-session replay review * docs: align active-session publication on save-script * docs: qualify destination guard identity * docs: close active-session publication boundaries	2026-07-20 19:59:52 +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	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	1fdbf80c32	fix(replay): retarget identity-empty press containers to their labeled descendant (#1280) (#1286) ... * refactor(replay): share the id-demotion predicate via target-identity-node Extract session-target-evidence.ts's demoteNonUniqueId into a shared demoteNonUniqueLocalIdentity (target-identity-node.ts), and export build.ts's normalizeSelectorText. Both become shared building blocks a third call site (#1280's press-retarget identity-empty check) reuses instead of re-deriving the id-demotion rule and value/text normalization a third way. No behavior change. * fix(replay): retarget identity-empty press containers to their labeled descendant (#1280) Android list-row presses target a clickable container (role="linearlayout") with no id, no label, no value/text — its title lives on a labeled descendant (the android:id/title TextView, whose own id #1272 already demotes for being non-unique). The container's identity is role-only and shared by every row, so replay disambiguates positionally and mis-binds under reorder (measured matchCount 12, 20/20 identity-mismatch). Retarget at record time: when a press/click/fill resolves to an identity-empty container (rule 1), substitute its first labeled descendant in document order (rule 2), but only when the container's subtree has no other interactive/hittable node (rule 3, fail-closed — a trailing Switch/Checkbox must not retarget, since a tap at the descendant's center vs the container's could land on different controls). Guard-blocked or label-less subtrees record exactly as today. Implemented once at the single recording choke point (describeResolvedInteractionNode, resolution.ts): the returned node feeds BOTH buildSelectorChainForNode's chain and (downstream, via recordedTargetCapture) computeTargetEvidence, so the two writers can never half-retarget. Recording-time only — resolveSelectorChain and live press/fill dispatch are unchanged; the tap point is already fixed against the original container before this substitution runs. Adds an ADR 0012 decision 3 amendment (mirroring #1269's), a press-retarget unit/guard/cross-invariant suite (including an RN FlatList iOS parity fixture), and a reorder+insert e2e proving the retargeted recording rebinds by role+label where the un-retargeted container recording refuses. * fix(replay): keep response hittability on the dispatched container, not the retargeted descendant Review blocker on #1286 (flag 1 adjudicated): describeResolvedInteractionNode was computing describeNonHittableTarget from the retargeted descendant, so every retargeted press on a non-hittable title TextView would emit a false `targetHittable: false` + misleading hint on the exact happy path the fix serves — a live-response regression violating the design's recording-time-only rule. Split the fields by what they are FOR: recording-coupled fields (node as evidence source, selectorChain, refLabel — they become the .ad step) keep following the retargeted descendant; the response-semantic describeNonHittableTarget (targetHittable + hint) reverts to the original node, describing what was actually dispatched. Documented in the function comment and the ADR amendment; new load-bearing test (fails against the pre-fix line): a hittable container with a non-hittable labeled child presses with no targetHittable/hint while chain/evidence/refLabel belong to the descendant. * fix(replay): carry the press retarget on a recording-only side channel; harden the guard (#1280 re-review) Maintainer re-review corrections, four findings: P1a (side channel): the runtime response is now entirely container-based — node, selectorChain, refLabel, point, resolution disclosure, hittability all describe the dispatched container, restoring the response-identity contract. The retarget travels as an optional recordingTarget {node, selectorChain, refLabel} on the runtime result (contracts/interaction.ts), consumed only at the recording boundary (interaction-touch-response.ts): the recorded action entry — the .ad writer's result.selectorChain source — takes the descendant chain/ref-label and recordedTargetCapture feeds the descendant node to computeTargetEvidence, while both wire payloads keep container materials. Daemon-route regression proves response container-based + recorded entry, target-v1 evidence, and the physically written .ad line descendant-based. P1b (fill): removed from retarget scope — a fill chain carries editable=true constraints a label descendant can never satisfy, saving an unreplayable script. click/press only; replay test proves the recorded fill chain on an identity-empty editable container still resolves uniquely. P2a (duplicate container ids): the identity-empty predicate now evaluates from the DEMOTED identity view — dropped the extractNodeText probe whose raw-identifier fallback resurrected an id that had been demoted for non-uniqueness, which made duplicated-container-id rows skip the retarget they need most. Fixture proves retarget fires; unique-id contrast unchanged. P2b (guard): replaced the private role-fragment list with the canonical interactive classification — isSemanticTouchTarget (exported from core/interaction-targeting.ts, the same policy hittable-ancestor promotion uses) plus the hittable flag; the module moves to src/core/press-retarget.ts since selectors -> core would be a layering back-edge. Added the geometric containment condition: the selected descendant's rect center must lie inside the container's rect (missing rects fail closed) — the replay tap point must be provably within the original activation region. Tests: nested Cell (role the old list missed) blocks; out-of-bounds descendant blocks; rect-less container blocks. ADR 0012 decision-3 amendment rewritten to the side-channel design, click/press-only scope, demoted-view rule, and both guard halves. The daemon regression runs on the iOS runtime path (direct-iOS is recording-gated) so the unit lane spends no real wall-clock on Android adb dialog probes.	2026-07-16 19:17:38 +02:00
Michał Pierzchała	1a1ef7c419	feat(android): one persistent automation helper owning snapshot + viewport + canonical injection (#1281) ... * feat(android): consolidate touch injection and gesture viewport into the persistent snapshot helper (#1275) One Android automation helper now owns snapshot capture, gesture viewport resolution, and canonical one-/two-pointer plan injection. A live persistent helper session executes gesture/viewport commands over its socket protocol; without a session the same APK runs one-shot via am instrument. The separate one-shot multitouch helper APK is deleted (atomic replacement, no fallback). Touch scheduling/injection is extracted into focused Java classes (TouchPlan, TouchPlanInjector, PointerEventSchedule, GestureViewportReader) instead of growing SnapshotInstrumentation. ADR 0013 amended. * fix(android): stop a structurally-failed helper session before the one-shot viewport retry A structured ok=false viewport response leaves the session process alive, and Android permits only one instrumentation owner of UiAutomation - running the one-shot fallback against a still-live helper contends with it and masks the original structured failure. Stop the session first; regression pins that the one-shot retry only executes once the session is gone. * refactor(android): extract helper touch dispatch into focused classes; split session tests; document helper API v2 (PR #1281 review) Addresses findings 2 and 3 from PR #1281 review (finding 1, viewport session-stop ordering, was already fixed in 5961b9247). - Extract SnapshotInstrumentation.java's one-shot/session touch dispatch into TouchCommandHandler.java (viewport/gesture population, UiAutomation- parameterized) and SessionResponseWriter.java (session response encoding), with shared PROTOCOL/HELPER_API_VERSION/OUTPUT_FORMAT constants moved to a tiny HelperProtocol.java. SnapshotInstrumentation.java shrinks from 908 to 803 lines; wire format (header keys/values, error shapes) is unchanged. - Split touch-helper.test.ts (~720 lines) into touch-helper.test.ts (normalize/parse/one-shot gesture+viewport+result envelope) and touch-helper-session.test.ts (persistent-session transport + fake-session harness), moving shared device/plan/install-probe fixtures used by both files into touch-helper.fixtures.ts. - Update android/snapshot-helper/README.md to document helper API v2: the one-shot viewport/gesture modes, the android-touch-plan-v1 payload shape, and the persistent session's socket command/response contract. * fix(android): invalidate helper session after APK replacement; recycle viewport windows; align ADR 0002 (PR #1281 re-review) - prepareAndroidTouchHelper now mirrors the snapshot path: when ensureAndroidSnapshotHelper replaces the APK (install.installed), the persistent session started against the previous binary is stopped before any touch command, so gestures run one-shot against the fresh install instead of a dead/stale session socket. Regression: 'an APK replacement stops the stale session and the gesture runs one-shot' drives a live fake session through an outdated-install probe (new outdatedVersionAdb fixture) and asserts the session socket receives no gesture, the one-shot instrumentation path executes, and the session is gone. - GestureViewportReader.read no longer leaks AccessibilityWindowInfo: a single pass copies the active/focused and first-application bounds into locals, every window is recycled in a finally, and the existing precedence (active/focused app bounds, root-in-active-window, fallback app bounds, IllegalStateException) is applied afterwards, unchanged. - ADR 0002's touch-synthesis paragraph is amended (2026-07, issue #1275) to the shared-helper model, consistent with ADR 0013: a live persistent helper session executes touch commands directly, one-shot otherwise; the old stop-before-gesture requirement is kept as historical context. * fix(android): resolve touch helper artifact from the ADB provider like snapshots do (PR #1281 re-review) - prepareAndroidTouchHelper now uses the same artifact precedence as snapshot capture: the scoped adbProvider's snapshotHelperArtifact when present, otherwise the bundled resolver (whose strict unavailable error is preserved). The provider artifact drives both the install decision and the instrumentationRunner used for one-shot commands, so an ADB-backed provider that supplies a helper artifact but no native touch override runs snapshots and gestures against the same single helper (issue #1275). Regression: 'a provider-supplied snapshotHelperArtifact overrides the bundled artifact for touch' pins the provider packageName on the install probe, the provider apkPath on the install call, the provider instrumentationRunner on the am instrument args, and that the bundled resolver is never invoked. - ADR 0002 now states explicitly that one-shot retry applies only to idempotent reads (viewport) after the failed session is stopped; non-idempotent gesture failures surface directly. - Helper README session transport corrected: a persistent process serving one short-lived socket connection per request (the server closes each accepted connection), not a single long-lived connection. * fix(android): guard touch session reuse on helper identity, stop mismatched sessions (PR #1281 re-review) Persistent helper sessions are keyed by device, so touch reuse must also prove the live session runs the helper binary the command selected. The session record now stores its helper identity (packageName, runner, helperVersion, helperVersionCode — the same values that feed the snapshot session identity), and runAndroidSnapshotHelperSessionTouchCommand takes the requesting helper identity: on mismatch (packageName/runner always; version/versionCode when both sides define them) it stops the session and returns undefined, so the touch command runs one-shot against the selected artifact — gestures never start sessions; the next snapshot restarts one with the right artifact. Matching identity reuses the session as before. Snapshot capture identity and behavior are unchanged. Regression: 'a provider artifact that mismatches the live session helper stops it and runs one-shot' — a live fake session from the bundled fixture artifact, then a gesture through an ADB provider supplying an already-current artifact with a distinct packageName/runner (no install): the old session socket receives zero gesture commands, the session is stopped, the one-shot am instrument args end with the provider runner, and helperTransport is 'instrumentation'. * fix(android): include artifact sha in helper session identity; evict stale install memo entries (PR #1281 re-review) Same-version binary replacement changes only the APK sha, so identity guards keyed on package/runner/version/versionCode could not detect a crossover between two artifacts that differ only in bytes: - The artifact sha256 now joins the helper identity end-to-end: AndroidSnapshotHelperCaptureOptions gains helperSha256 (snapshot.ts passes artifact.manifest.sha256 alongside version/versionCode), the session record stores it, createSessionIdentity includes it (making snapshot session reuse sha-aware, consistent with the install path's existing sha check), and the touch identity guard compares it via the same both-defined rule. - ensureAndroidSnapshotHelper's install memo now evicts every other cached decision for the same device+package when it records an install/current decision, so installing B invalidates A's stale 'current' memo and a later command selecting A re-inspects the device instead of skipping the sha check. Regressions: 'a same-version artifact with a different sha stops the live session and runs one-shot' (touch-helper-session.test.ts — B owns the live session, a gesture selecting same-version different-sha A sends zero commands to B's socket, stops it, and completes one-shot) and 'installing a same-version different-sha helper evicts the stale install memo' (snapshot-helper.test.ts — A:current cached, B installed, selecting A re-inspects and reinstalls instead of serving the stale memo). Both verified to fail without their fix.	2026-07-16 17:04:42 +02:00
Michał Pierzchała	13b3d4fc88	fix: demote non-unique ids from writer identity/selector chain (#1269) (#1272) ... * fix(replay): demote non-unique ids from writer identity/selector chain (#1269) Android list-row GET replays bind the wrong row because the recorder uses the non-unique framework resource id `android:id/title` (matchCount 11 on Settings root) as primary identity; positional drift then makes the identity verifier correctly refuse with `identity-mismatch`. Demote an id from identity whenever it matches more than one node in the record-time tree (capture-time uniqueness, not an `android:id/*` namespace check — a reused RN FlatList testID hits the same class on iOS). Applied in both places a recorded id feeds identity: - `computeTargetEvidence` (session-target-evidence.ts): the `target-v1` identity tuple falls back to role+label when the id's own capture-time match count exceeds one, reusing the existing `filterIdentitySet` domain machinery (an empty ancestry degrades it to a plain id scan). - `buildSelectorChainForNode` (selectors/build.ts): the recorded selector chain omits a non-unique id rather than leading with it. Every writer call site (get/press/fill recording, plus the divergence-suggestion path) now passes the record-time tree so the check has something to count against; omitting it preserves prior behavior for isolated-node callers (tests). Resolver-side `resolveSelectorChain` and live press/fill resolution are untouched per ADR 0012 (disclosed-not-changed disambiguation) — this is writer/replay-scoped only. Amends ADR 0012 decision 3: an id may serve as identity (and lead the selector chain) only when it uniquely denotes the target in the record-time tree. Adds fixtures: an Android duplicated-`android:id/title` list (the measured repro) and an iOS/RN duplicated-testID FlatList shape, both demoted and still verifying via the now-selective label; a regression case confirming an already-unique id is unaffected. Out of scope: the Android list-*press* class (matchCount 12, label-less `role="linearlayout"` container with no id at all to demote) needs a separate design decision — deriving identity from the labeled descendant. Tracked as a follow-up, not attempted here. * fix(replay): unify the id-uniqueness predicate across both writer sites (#1269 review) Address the maintainer review on #1272: 1. ONE shared uniqueness predicate. The two demotion sites were counting id matches with DIFFERENT semantics — `demoteNonUniqueId` via `filterIdentitySet` (NFC + 256-byte cap, and a broken-parent-walk exclusion), `selectableId` via a raw `normalizeSelectorText` scan (trim, no NFC/cap, no exclusion) — so the identity tuple and the selector chain could disagree and half-demote (id gone from one, kept in the other). Extract `idMatchCountInTree(nodes, id)` in target-identity-node.ts, counting over the canonical `readNodeLocalIdentity` id the replay verifier keys on, with no ancestry/parent-walk exclusion. Both `demoteNonUniqueId` and `selectableId` now call it. Corrects the inaccurate "vacuously-true / plain id scan" comment. Cross-invariant test (build.test.ts): for the same node+tree, evidence.id === undefined iff the built chain has no id= clause — across demoted, unique, and a non-NFC (decomposed vs precomposed) edge case. Verified it fails under the old raw-scan and passes under the unified predicate. 2. End-to-end reorder proof (session-replay-target-classification.test.ts): record against a tree whose rows share android:id/title, then classify against a DIFFERENT replay tree where the shared-id rows reorder — the demoted role+label identity rebinds the correct row (verified, matchCount 1) while `id="android:id/title"` resolves ambiguously (null). This pins the FDR 1.0 -> 0 mechanism, not just record-time demotion. 3. Removed the conflated "20/20 clean" live-number comment from the unit test; it now states the mechanism (role+label selectivity) instead. Behavior for the already-clean unique-id path is unchanged: for ordinary ascii ids the canonical count equals the old raw count. The only outcomes that change are the edge cases the old split mishandled (non-NFC, broken parent walk) — where demotion is the correct result. The kept clause still emits the chain's own normalizeSelectorText id string, so unique ids lead the chain exactly as before. * fix(replay): thread record-time tree through the extracted suggestion helper Rebase-conflict resolution against origin/main. #1217 (typed direct Maestro engine) extracted `buildReplayDivergenceSuggestionForNode` out of `resolveSuggestionCandidate` and added a second caller in `session-replay-maestro-failure.ts`. My #1269 change had added `nodes` to the `buildSelectorChainForNode` call that #1217 moved into the extracted helper, so after rebase the helper referenced an out-of-scope `nodes`. Thread the record-time tree as a required `nodes` param on the helper and pass it from BOTH callers (each already has it in scope). This keeps the non-unique-id demotion applied wherever a divergence/repair suggestion chain is built — now including the typed-Maestro suggestion path — with no behavior change for the already-unique-id case.	2026-07-16 08:19:56 +02:00
Michał Pierzchała	37895caf99	refactor: replace Maestro compat with typed direct engine (#1217) ... * test: add pinned Maestro conformance harness * feat: add typed Maestro program IR parser * docs: define direct Maestro engine architecture * test: compare Maestro oracle with typed IR * feat: add direct Maestro program engine * refactor: narrow Maestro execution context * refactor: tighten Maestro program parsing * fix: verify iOS Maestro visibility waits * refactor: isolate retained Maestro runtimes * refactor: type Maestro target resolution * refactor: harden typed Maestro execution * refactor: share in-page swipe planning * feat: add typed Maestro runtime port * refactor: parse Maestro suite metadata from typed IR * refactor: centralize Maestro include loading * feat: execute Maestro files through typed engine * refactor: share replay built-in variables * fix: make Maestro target intent explicit * fix: refresh Maestro targets before input * refactor: format Maestro progress from typed IR * feat: compile typed Maestro replay plans * feat: bind typed Maestro runtime to public commands * feat: route Maestro YAML through typed runtime * refactor: remove legacy Maestro runtime * refactor: remove obsolete replay control model * refactor: split typed Maestro plan modules * fix: harden typed Maestro runtime semantics * docs: update direct Maestro architecture * fix: reconcile Maestro runtime with merged contracts * fix: harden typed Maestro execution boundaries * fix: harden typed Maestro runtime evidence * perf: avoid eager Maestro device resolution * refactor: finalize typed Maestro execution * fix: reject Android system-only helper snapshots * fix: preserve Android system dialog snapshots * fix: make helper-backed CI deterministic * refactor: invalidate Maestro observations before dispatch * fix: make Maestro selector policy explicit * refactor: remove Maestro ranking sentinels * refactor: make Maestro own observation stabilization * refactor: source Maestro compatibility presets * refactor: keep Maestro failure reports typed * refactor: simplify Maestro runtime policy * fix: isolate Maestro engine failures * refactor: consolidate Maestro swipe presets * fix: align Maestro selector and observation semantics * fix: preserve atomic iOS Maestro taps * fix: require semantic uniqueness for Maestro taps * fix: preserve Maestro parse provenance * docs: pin Maestro compatibility presets * docs: reconcile Maestro gesture viewport contract * perf: resolve Maestro gesture viewport directly * test: align Maestro replay regressions * fix: order Android gesture lift after endpoint * fix: settle Maestro gestures before continuation * fixup! fix: order Android gesture lift after endpoint * refactor: normalize Maestro swipes once * refactor: fail impossible Maestro observations * refactor: normalize Maestro defaults alias * test: reconcile Android provider scenarios * fix(android): synchronize single-pointer move events * test: align repair digest parsing * refactor: type Maestro runtime operations * refactor: keep Maestro controls compact * refactor: name Maestro diagnostic limit * fix: align Maestro parser and settling semantics * fix: complete Maestro compatibility semantics * docs: define Maestro compatibility boundaries * fix: refresh iOS runner target after relaunch * fix: reset prewarmed iOS runner after URL open * fix: preserve iOS Maestro target and swipe intent * fix: harden direct Maestro runtime semantics * fix: preserve ranked Maestro replay suggestions * fix: align maestro tap runtime semantics * fix: stabilize maestro ci contracts * fix: tighten maestro runtime architecture * fix: reconcile maestro replay with latest main * perf: tighten Maestro iOS stabilization * fix: preserve Maestro app lifecycle sessions * fix: restore Maestro CI coverage * fix: address Maestro engine review findings * refactor: consolidate Maestro compatibility internals * fix: scope Maestro target evidence to childOf	2026-07-15 21:26:42 +02:00
Michał Pierzchała	22a3c4711c	refactor(daemon): remove the superseded coarse snapshotRefsStale marker (ADR 0014 step 8) (#1268) ... * refactor(daemon): remove the superseded coarse snapshotRefsStale marker (ADR 0014 step 8) The coarse `snapshotRefsStale` client-stale marker is fully superseded by the ref-frame model and is removed: - `setSessionSnapshot` and `buildNextSnapshotSession` no longer set/clear it — replacing the latest observation is a read that never touches the frame. - Read-only ref staleness now derives from frame state: a plain ref warns once the frame has EXPIRED (a device side effect changed the screen), and a read-only capture no longer marks refs stale because it does not expire the frame. Pinned-ref warnings keep comparing against the frozen frame epoch. - Deletes `markSessionSnapshotRefsIssued` (its only job was clearing the marker) and the `session.snapshotRefsStale` field. Migrates every test off the marker to the frame model (frame-expiry drives the read warning; complete/partial activation drives admission), and updates the ADR status + module docs to record step 8 as landed. Ships as follow-up to the merged #1257 since that PR closed before this step. Full unit-core + provider-integration green; tsc/lint/fallow/production-exports clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix(daemon): resolve @ref reads from the frame tree; scope find's internal warning Address three review blockers on the coarse-marker removal (ADR 0014 step 8): 1. @ref reads now bind against the authorized frame tree (`refFrameSnapshot ?? snapshot`) in `requireSnapshotSession`, so an internal read-only capture that replaced the observation cannot let a plain `@eN` resolve a different element by positional coincidence. Missing frame evidence fails instead of falling through to a newer observation. 2. A mutating find's internal leaf dispatch (`internal.findResolvedTarget`) no longer attaches a stale-ref warning in either the press or fill path — the caller never consumed a `@ref`, so the public find response must not claim it did. 3. `resolveRefStalenessWarning` checks frame expiry FIRST, matching the admission order: an expired frame is stale for any ref, even a pin that matches the epoch (a matching pin proves identity within the retained frame, not that the UI is current). Regressions: divergent observation-vs-frame trees resolve from the frame tree or fail when evidence is missing; a locator-based mutating find from an expired frame carries no stale-ref warning; the reordered resolver unit test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix: correct stale-ref warning comments and ADR-0014 present-tense marker refs The get/wait dispatch comments in selector-runtime.ts still described the superseded coarse snapshotRefsStale marker ("warn when that tree was replaced since the client last received refs") even though staleness is now derived from ref-frame expiry (ADR 0014 migration step 8). Reworded both to describe the frame-derived mechanism actually implemented by resolveRefStalenessWarning. session-snapshot.ts's early-return comment in markSessionPartialRefsIssued referenced "the coarse marker" as something still left untouched, but that field no longer exists — reworded to name the ref frame fields it actually preserves. ADR-0014's "Ref frames are separate from operational observations" section still described snapshotRefsStale as part of "the existing... implementation" in present tense, contradicting the Decision section's own note (line 39) that migration step 8 already removed it. Reworded to keep the historical mention while stating the removal. * fix: frame-lifetime wording for the stale-ref warning and read comments Address the follow-up review blocker plus the co-located terminology cleanup (ADR 0014 step 8): - STALE_SNAPSHOT_REFS_WARNING no longer claims "the session snapshot changed"; it now describes frame lifetime in terms valid for both read warnings and mutation rejection — the UI may have changed since the refs were issued, so take a new snapshot before relying on or interacting with them. The warning fires on frame expiry, including device side effects where no stored snapshot changed. - selector-runtime.ts: the get/wait @ref comments now say the read binds to the retained ref-frame evidence and its staleness is frame-derived, not a property of the stored snapshot or the live polling capture. - settle.ts: an unsettled stored capture replaces the observation without touching the ref frame; read staleness is driven by side-effect-seam expiry, not by storing a fresh observation. - interaction-settle.test.ts: renamed the settle test off the removed stale-marker language to "activates a partial ref frame" (what it asserts). Comments/test-name/warning-text only — no runtime behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix(daemon): name the ref-frame epoch in the pinned-stale-ref warning The pinned-ref warning is compared against refFrameEpoch(session) — the frozen frame epoch — not the latest observation generation, and after a read-only capture those two diverge. The message still said "the session tree is now sN", which is ambiguous once the observation counter has advanced past the frame epoch. Name the ref-frame epoch instead: Ref @e12 was minted from snapshot s3 but the session's ref frame is now s15 — re-run snapshot -i. Renames the builder param to `currentFrameEpoch` and corrects its doc comment to say the pin is compared against the frame epoch, not the stored tree generation. Regression: `resolveRefStalenessWarning` names the frozen frame epoch, not the bumped observation generation — a read-only `setSessionSnapshot` advances the observation counter (15 -> 16) while the frame epoch stays frozen at 15; a pin at s15 is clean and a pin at s12 names s15, never s16. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-15 20:55:15 +02:00
Michał Pierzchała	6efe54451b	fix: unify divergence screen capture with snapshot's full-window scope (#1265) ... * fix: unify divergence screen capture with snapshot's full-window scope Route captureDivergenceObservation through captureSnapshotData — the same function the snapshot command itself builds its capture with (Android's snapshot-helper full-window route with its graceful app-scoped fallback, iOS's bounded system-modal probe path, macOS/Linux surface-scoped branches) — instead of a parallel hand-rolled dispatchCommand call. The chrome filter and meaningful-target filter stay layered on top as filters over that full capture, never as a scoping. Amends ADR-0012 decision 4 to state the invariant: an agent must never see a healthier `screen` in a divergence report than a plain `snapshot` would show it, so a separate-window system overlay (volume dialog, quick-settings shade, permission dialog) must survive into `screen.refs` exactly as `snapshot` would present it. Also fixes the synthetic `volume_dialog_slider` id in snapshot-chrome-android-statusbar.test.ts to the real, live-verified `volume_new_ringer_active_icon_container` id and rewords the test comment to read as a filter-logic unit test rather than a live-capture-path claim, and adds unit coverage for the invariant itself. Fixes #1264 * fix: rank divergence screen.refs within the cap so overlays are not buried The #1264 root cause is cap burial, not capture scope: buildReplayDivergenceScreenRefs sliced candidates in document order, so a fully-captured separate-window overlay (volume dialog, QS shade, permission dialog) that enumerates after the app window's ~77 nodes lands past position 20 and is truncated away — the report shows a healthy-looking app under a covering overlay it cannot see (archived evidence: screen.truncated: true, zero volume refs). - Rank within the cap instead of document-order slicing: foreign-window (non-app-bundleId) hittable nodes — the dismiss targets for whatever covers the app — are promoted ahead of app content, otherwise stable (document order preserved within each tier; equal-priority app nodes never reshuffled). The 20-cap is a byte bound, not a first-20-in-tree-order policy. - Occlusion fallback: when a system overlay mass-covers the app (every app node annotated interactionBlocked: 'covered'), surface those covered nodes rather than emitting an empty screen.refs — a report whose capture holds meaningful nodes but whose refs is empty is broken by construction. - repairHint/suggestions consume the full captured node list, not the capped refs slice, so hint routing is unaffected; only screen.refs selection changes. Detection keys off node.bundleId (Android-only, from the a11y package); iOS/macOS leave per-node bundleId undefined, so ranking degrades to document order there (safe — those platforms surface modals via the probe path, not by cap-competing). Guarded on a known appBundleId so a sessionless capture never reorders. Tests: replaces the small-fixture #1264 test (which the overlay fit inside the cap regardless of order, so it did not prove the invariant) with a realistic full fixture (24 app controls + overlay dismiss-target captured LAST) that fails on document-order slicing and passes with ranking; plus occlusion tests (mass-covered app -> overlay surfaces, refs non-empty; bare-scrim fallback -> covered app nodes surfaced, refs non-empty). Both were verified to fail before the fix. ADR-0012 decision 4 amendment reworded to cover ref-selection ranking and the occlusion guarantee, not only capture scope. Refs #1264 * fix: route divergence capture through captureSnapshot wrapper + clean flags policy Completes the #1264 capture unification. The prior round routed captureDivergenceObservation through captureSnapshotData (the inner single-shot capture), but plain `snapshot`'s backend calls the HIGHER captureSnapshot wrapper, which owns Android freshness + post-action retry. A divergence could therefore consume the first stale/app-scoped dump while a plain `snapshot` retries to the fresh full-window tree — a divergence staler/narrower than `snapshot`, violating the invariant. - Route the divergence capture through the same `captureSnapshot` wrapper as plain snapshot, so it inherits freshness/post-action retry parity. No fork: the wrapper's params (device, session, flags, logPath) are all suppliable from the divergence path. - Build the divergence capture's flags from a clean, fixed policy (`divergenceCaptureFlags`: full-window, non-raw, default depth) instead of spreading the failed action's flags — so a failed `snapshot --raw`/scoped/`-d` action can no longer narrow the diagnostic tree. Only the interactive-only policy is carried (extracted as a helper so captureDivergenceObservation stays within complexity budget). Tests: a freshness-retry regression (session carries an active Android freshness marker; capture-1 is a stale near-empty dump that trips sharp-drop, capture-2 holds the overlay — asserts the divergence uses the retried fresh tree and dispatched twice), and a clean-flags regression (a failed raw/scoped/depth action — asserts the snapshot dispatch context drops snapshotRaw/scope/depth while still applying interactive-only). Both verified to fail on the pre-fix code. ADR-0012 decision 4 amendment updated to state the same-wrapper (freshness parity) and clean-flags guarantees. Live overlay acceptance remains a maintainer device step (env down): unit fixtures prove ref SELECTION after nodes are supplied, not that the Android helper returns the separate-window overlay at divergence time. Refs #1264 * test: stub the freshness-retry sleep so the capture-parity test doesn't real-wait The #1264 capture-parity regression exercised the real Android sharp-drop retry, which awaited the real ~250 ms `sleep` delay — repo guidance forbids real-time waits in unit tests. Mock `sleep` (the delay the retry path in snapshot-capture.ts awaits) to a no-op at the module level, so the retry BRANCH still executes (loop runs, retries, re-captures) without a wall-clock wait. The test still proves the branch: two on-device dispatches and use of the retried fresh tree (overlay present). Verified it still fails on the pre-fix single-shot path (1 dispatch) with the delay stubbed, so the stub does not make it vacuous. No production change; the delay stub needs no DI seam since `sleep` is a plain module export. Refs #1264 * fix: reconcile divergence ref selection with #1257 ADR-0014 partial ref frame Rebase reconciliation. #1257 (ADR-0014 session ref-frame lifetime) landed on main and changed captureDivergenceObservation to activate a PARTIAL ref frame (markSessionPartialRefsIssued) authorizing exactly the divergence screen's emitted refs — computing that "digestBodies" set with its own document-order, non-covered-only filter. My #1264 change made buildReplayDivergenceScreenRefs emit a DIFFERENT set (ranked, occlusion-fallback, meaningful-filtered), so the authorized frame would no longer match the shown screen: in the mass-covered fallback the screen surfaces covered refs that #1257's non-covered-only frame filter excluded, leaving the agent a ref the screen advertised but the frame rejects. Extract selectDivergenceScreenRefNodes as the single source of truth for which nodes screen.refs publishes and in what order. Both the rendered digest (buildReplayDivergenceScreenRefs) and the partial-frame authorization (captureDivergenceObservation -> markSessionPartialRefsIssued) derive from it, so the frame authorizes exactly the emitted set — preserving BOTH #1257's ADR-0014 intent and #1264's ranking/occlusion intent. Also refresh the captureDivergenceObservation doc to the partial-frame sequence. Test: assert the partial ref frame scope (session.refFrameScope) equals the emitted screen.refs set in the mass-covered fallback (covered refs included) — verified to fail on #1257's original non-covered-only digestBodies. Refs #1264 #1257	2026-07-14 21:46:56 +02:00
Michał Pierzchała	54977f3b87	feat(daemon): ADR 0014 session ref-frame lifetime — full implementation (#1257) ... * feat(daemon): classify ref-frame effect on every daemon command (ADR 0014 step 2) Add the ADR 0014 `refFrameEffect` trait to the daemon command descriptor facet: every command that reaches a session-owning daemon leaf declares how it relates to the session's authorized ref frame — `preserve`, `may-invalidate`, `delegated`, or a request-sensitive resolver for subaction-dependent commands (keyboard status vs dismiss, alert get/wait vs accept/dismiss). This is the honesty/completeness guard, not the transition site: a `may-invalidate` command still calls the (future) ref-frame module only when its mutating path runs. No runtime behavior changes here. - `RefFrameEffect` / `DaemonRefFrameEffect` types and a `resolveRefFrameEffect` accessor honoring the resolver form, mirroring the existing closure traits. - Classify all 58 daemon-faceted commands; `find` is the honest superset (`may-invalidate`) pending a read/mutate resolver during enforcement wiring. - Give `app-switcher` a daemon facet (route unchanged) so the generic-fallback escape hatch the ADR calls out is covered instead of silently unclassified; drop it from parity's UNROUTED set. - Completeness gate (`ref-frame-effect.test.ts`): every daemon-projected command classifies an effect, every public command is classified or in the explicit non-daemon allowlist (`install-from-source`, which projects via the `install_source` internal command), and the resolvers/app-switcher resolve as declared. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): introduce ref-frame module + admission matrix (ADR 0014 step 1) Introduce `src/daemon/ref-frame.ts` as the single owner of the ADR 0014 ref-frame model — the authorization namespace for mutation refs, kept distinct from the latest operational observation (`session.snapshot`). It defines the frame's issuance scope and lifecycle state and the pure mutation-admission matrix (`admitRefMutation`) with the ADR's typed, order-sensitive reasons: ref_frame_expired, ref_generation_mismatch, plain_ref_requires_complete_frame, ref_not_issued. The frame is introduced behind the existing `snapshotGeneration` (epoch) and `snapshotRefsStale` (coarse client-stale) fields, whose wire-visible names (`refsGeneration`, the `@e12~s42` pin grammar) are unchanged. New `refFrameState`/`refFrameScope` session fields default to active/all, so the matrix currently reduces to the generation-pin check the iOS path already did — no behavior change. Expiration at the side-effect seam and non-`all` scope land in later steps. The existing #1241 iOS stale-ref guard now routes its decision through `admitRefMutation` (plus the transitional coarse-stale check for plain refs), so the module is production-live; the external error contract is identical. Adds a unit test covering the full admission matrix and reason ordering. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): wire pre-side-effect frame expiration at the seams (ADR 0014 step 3) Route device mutations through the idempotent ref-frame transition. A leaf expires the current frame synchronously, immediately before awaiting the device operation, so success, timeout, cancellation, or connection loss all leave it expired — there is no success-only rollback. Seams wired: - interaction runtime backend closures (tap/click, fill, longPress, native web clickRef/fillRef, gesture, type) — post-resolution, pre-dispatch, so a resolution failure before the seam preserves the frame; - the generic daemon leaf (back/home/rotate/scroll/tv-remote/app-switcher/ viewport/focus, ...), gated by the daemon `refFrameEffect` classification via `resolveRefFrameEffect`, which is that resolver's first production consumer. Re-authorization: issuing a complete namespace re-activates the frame — `markSessionSnapshotRefsIssued` and the snapshot command's `buildNextSnapshotSession` — so a fresh capture between mutations restores usability. A diff or kept tree preserves the prior authorization state; internal read captures never re-authorize. Enforcement of the new expired-frame rejection is intentionally deferred to step 7, which the ADR gates on fresh live device evidence per platform. The iOS #1239 guard therefore stays armed-but-not-enforced here: it consults the admission matrix but still rejects only on the pre-existing conditions (pinned generation mismatch, coarse plain-ref stale marker), so behavior is unchanged. Tests prove the transition is wired (a press expires the frame; a re-issue re-activates it) alongside the idempotency and re-authorization unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix(daemon): address ADR 0014 review — partial issuance, keyboard, seam coverage Exact-head review found three blockers; all fixed with focused seam tests. 1. Partial issuance no longer restores complete authority. Every caller of `markSessionSnapshotRefsIssued` (find, settled diff, replay divergence) is a PARTIAL publication, but it re-activated a complete `all`-scope frame. It now only clears the coarse marker; complete re-authorization is reserved for the snapshot command (`activateCompleteRefFrame`, from `buildNextSnapshotSession`). 2. Keyboard resolver covers every mutating subaction. keyboard accepts status/get/dismiss/enter/return; only status/get read, so dismiss/enter/return (enter/return dispatch a real return key) are now `may-invalidate`. Alert reads are likewise a named set. Completeness test extended. 3. Remaining step-3 leaf seams wired: the direct iOS selector fused dispatch, the direct `find` focus/type dispatches (find click/fill already delegate through the interaction leaf), and Android blocking-dialog recovery (expire before the recovery tap). Focused seam tests for each prove the frame expires. Enforcement of the expired-frame rejection remains deferred to step 7 behind the ADR's per-platform live-evidence gate; behavior is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): cross the seam at every specialized mutating leaf (ADR 0014 step 3 complete) Wire expireRefFrame at the remaining may-invalidate leaves so EVERY mutating daemon leaf crosses the side-effect transition, not just the interaction/generic paths: - keyboard dismiss/enter/return, push, trigger-app-event (shared session leaf) — gated by resolveRefFrameEffect so keyboard status/get preserve the frame; - alert accept/dismiss (get/wait preserve, via the alert resolver); - settings mutations; - React Native overlay dismissal; - install / reinstall (deploy op); - open / relaunch — expires the reused session's frame before the launch; - close — expires for uniformity, though a successful close deletes the whole session (and its frame) anyway. Seam tests: keyboard dismiss expires while status preserves (proves the resolver-gated pattern), and RN overlay dismissal expires. Enforcement stays deferred; behavior unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): partial issuance scope + MCP pin retention + pinned CLI refs (ADR 0014 step 4) A find/settled-diff/divergence result publishes only the refs it returned, so it now activates a bounded PARTIAL frame authorizing exactly those ref bodies (`markSessionPartialRefsIssued`) instead of nothing — a plain ref then requires a complete frame and a pinned ref outside the set is rejected. An empty partial result leaves prior authority intact. - read-only find publishes its one ref; settled diff publishes its added lines + `refs` + `tail`; divergence publishes its capped, non-covered, non-chrome digest set. - MCP: a mutating `find` returns no `refsGeneration` and is explicitly non-issuing — it no longer hits the missing-generation branch that wiped the whole per-session pin scope (forwarding the old pin is how the daemon produces a precise stale rejection). - Human-CLI partial results render reusable refs in ready-to-copy `@eN~s<gen>` form (find + settled tail); JSON/Node keep plain bodies + one response-level generation, and MCP stays plain (it auto-pins). Output-economy waiver covers the +8-byte tail-pin increase with an ADR justification; the workflow oracle treats a pinned ref as surfacing its plain body. Enforcement of the frame's expiry and partial-scope rejections stays deferred to step 7 (behind the ADR's per-platform live-evidence gate), so this is behavior-preserving; the iOS guard now consumes the admission verdict for a typed `details.reason` on the rejections it already emitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): resolve refs against the authorized frame tree (ADR 0014 step 5) Retain the ref frame's immutable source tree (shared reference, no deep copy) and resolve a `@ref` against it rather than the latest operational observation. An Android freshness — or any read-only — capture advances `session.snapshot` without disturbing the frame tree, so the two intentionally diverge. At resolution, adopt the fresh observation's node (its current on-screen coordinates) ONLY when its local identity still matches the authorized node — the legitimate "element moved" case. If a different element now sits at that index, keep the authorized frame node so a positional coincidence cannot retarget the action. Expose the frame tree to the command runtime through `CommandSessionRecord.refFrameSnapshot`; pre-frame sessions fall back to `snapshot` and behave exactly as before. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): fail-closed ref-mutation enforcement across platforms (ADR 0014 step 7) Enforce the ref-frame admission matrix on every platform before dispatch: an expired frame, a superseded generation pin, a plain ref against a partial frame, or an unissued pinned ref is now rejected with a typed `details.reason` and an honest message that names the lifetime failure instead of claiming the ref was missing or lacked bounds. The prior iOS-only, coarse-marker guard is replaced. Freeze the frame epoch at issuance (`refFrameGeneration`) so a later read-only capture that advances the observation counter cannot falsely reject a correct pin from the issuing frame; staleness warnings compare against the same frame epoch. A mutating `find` re-resolves its target by locator against a fresh capture, so its internal leaf dispatch carries `internal.findResolvedTarget` and skips ref admission (it still crosses the seam and expires the frame). Update unit and provider-integration scenarios to the new contract: multi-mutation ref sequences re-observe between mutations, settled refs are consumed in pinned form, and rejections assert the typed reason. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * docs(adr-0014): promote ref-frame vocabulary and mark implementation status Flip ADR 0014 to Accepted, promote the ref-frame / frame-expiry-seam / mutation-admission vocabulary into CONTEXT.md, correct the `@ref` resolution note to the frame-tree model, record the migration status (steps 1–7 landed; coarse-marker removal follows live-evidence confirmation), update ADR 0012's divergence-ref amendment to accepted, and add a CHANGELOG entry for the fail-closed ref lifetime. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * test(daemon): lock ADR 0014 evidence #1 and refresh module docs Add a daemon-level sequence test proving the canonical contract: after an unobserved first ref mutation, a second mutation rejects both bare and pinned with ref_frame_expired, and a fresh snapshot re-authorizes. Refresh the ref-frame module header and seam-expiry test comment now that enforcement is live. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix(daemon): address ADR 0014 exact-head review — six lifetime blockers 1. Android dialog recovery aborts an outstanding ref action: a ref press/fill admitted against the pre-recovery frame now fails with ref_frame_expired when before-command recovery mutates the UI, instead of continuing against the recovered screen (selector/coordinate actions still re-resolve and continue). 2. open --relaunch expires the existing session's frame BEFORE the close dispatch, so a close timeout/failure that already tore the app down still leaves the old frame expired. 3. expireRefFrame clears scoped-snapshot lineage (snapshotScopeSource) at the seam, so snapshot -s @ref -> mutation -> snapshot -s @same-ref can no longer borrow stale lineage across a device side effect. 4. Missing authorized-frame evidence fails closed: resolveSnapshotForRef no longer recaptures and accepts the same ref body from a newer tree by positional coincidence. A mutating find's internal dispatch resolves against its own fresh capture (omitRefFrameSnapshot), not the frame. 5. Mutating find omits refsGeneration — its acted ref is diagnostic pre-action identity and must not be pinnable after the action. 6. An empty partial publication leaves all session state untouched (including the coarse marker), instead of clearing it before finding there were no refs to issue. Adds focused regressions (lineage-cleared sequence, empty-partial no-op, fail-closed on unusable bounds, in-frame label recovery, mutating-find non-issuance) and extracts the find action dispatch to keep complexity in budget. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix: preserve snapshot refsGeneration + shared recovery rejection (ADR 0014 re-review) P1: structured JSON/Node snapshot results now retain the response-level refsGeneration. It was declared on the daemon response but dropped by the public CaptureSnapshotResult type, the serializer, and the Node normalizer, so default `snapshot -i --json` emitted refs with no generation to pin against. Added to the type, serializer, normalizer, plus CLI/Node tests. P2: Android dialog-recovery abort now reuses the SHARED admission rejection (refMutationAdmissionResponse) instead of a bespoke error, so the failure carries the full typed context (reason, ref, currentGeneration, scope, mintedGeneration) identical to every other expired-frame rejection across platforms. Removes the now-unused AppError/refFrameState imports. Adds a regression proving recovery aborts the outstanding ref action before any press dispatch. Also adds the relaunch failure-boundary regression (existing-session close fails after dispatch → old frame stays expired), and corrects the ADR implementation-status note so Android blocking-dialog recovery and a real provider-backed interaction/lifecycle are recorded as unexercised release blockers rather than confirmed enablement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * docs(adr-0014): record provider seam as live-verified; Android recovery sole blocker The provider-backed interaction + lifecycle seam is now confirmed by fresh live evidence (AWS Device Farm, webdriver backend). Update the ADR implementation-status note so only Android blocking-dialog recovery remains an unexercised release blocker — and note it is blocked on a bootable free Android target plus a deterministic app-owned ANR trigger, not on any code gap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * docs(adr-0014): record Android ANR recovery as an accepted evidence gap Per the review decision: the Android blocking-dialog recovery seam has no deterministic app-owned ANR repro in the harness, so it was not live- exercised. The team accepted shipping without a live run for it — its transition/abort logic is covered by fixture regressions and it is enforced in code identically to the verified paths. Reword the status note from an open release blocker to a documented, accepted evidence gap, which unblocks step 8's coarse-marker removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-14 21:25:11 +02:00
Michał Pierzchała	0436793c25	fix(replay): extend resume.from record-and-heal shape to caution/manual (#1267) ... * fix(replay): extend resume.from record-and-heal shape to caution/manual (#1262) caution/manual divergences kept resume.from unshifted (correct — per resolution item 1, N stays unconditionally legal) but never offered a concrete N+1 continuation for their record-and-heal-shaped alternate repair, and a last-step caution/manual divergence repaired by a recorded action was a dead end: pendingRecordAndHeal was only ever stamped for record-and-heal, so the N+1 empty-tail resume was unauthorized (out of range) and close on the not-yet-COMPLETE transaction discarded the just-recorded corrective action — the same trap #1260 closed only for record-and-heal. - buildRepairHintGuidance (src/replay/divergence.ts) now renders BOTH concrete commands for caution/manual when resume.allowed: --from N for a --no-record state fix, --from N+1 for a recorded corrective action. - stampPendingRecordAndHealWatermark (session-replay-resume.ts) now also stamps for caution/manual, but only when the diverged step is the plan's LAST one and N+1 is independently preflight-safe — a mid-plan --from N+1 was already unconditionally legal and un-gated for these hints (unlike record-and-heal, they never mandate a corrective action), so that pre-existing pattern stays un-gated. * fix(replay): add resume.alternateFrom so caution/manual dual-path never advertises a --from the daemon refuses (#1262) The dual-path text guidance offered `--from N + 1` whenever resuming AT `N` was allowed, but `--from N + 1` needs its OWN preflight — which additionally requires the diverged step `N` to be skip-safe. When `N` is a runScript (outputEnv producer) or inside runtime control flow, preflight(N) passes while preflight(N+1) fails, so the text advertised a command the daemon then refused (the text-vs-structured disagreement #1260 blocker 2 banned). - Add optional `resume.alternateFrom` to the decision-4 wire shape (`ReplayDivergenceResume`). The daemon populates it (`N + 1`) for caution/manual ONLY when `evaluateReplayResumePreflight({ from: N + 1 })` passes — the same acceptance condition on both the mid-plan (un-gated, in range) and last-step (watermark-stamp) paths. Its checked range is a strict superset of `from`'s, so alternateFrom present implies allowed. - The text renderer gates the `N + 1` command on `alternateFrom`'s PRESENCE and renders its value verbatim, never re-deriving resumability — so text and the structured wire can never disagree on the advertised next command. - This also closes a parity gap: a JSON/MCP caller now gets both ordinals (previously the dual-path was text-only, structured callers saw only `from`). - ADR-0012 decision-4 documents alternateFrom as additive/optional; projection and trigger tests cover both positions (runScript/control-flow → no alternateFrom, no `--from N + 1`; skip-safe → alternateFrom present). * fix(replay): withhold empty-tail alternateFrom when no session can stamp the watermark (#1262) The empty-tail alternate (`alternateFrom > actions.length`) is accepted by the range check ONLY when it matches a stamped `pendingRecordAndHeal` watermark, and that watermark can only be stamped on a live session. For a last-step caution/manual failure with no active session — a one-step `open` failure, or a session closed mid-replay — the watermark can never be stamped, so the advertised `--from length+1` is then rejected as out of range, re-introducing the text/structured mismatch this arc fixed. - Thread `sessionExists` into `buildReplayDivergenceResume`; gate the one-past-the-end `alternateFrom` on it (mid-plan alternate stays in-range and session-independent). Both divergence sites pass `session !== undefined`. - At the last step this makes `computeReplayResumeAlternateFrom`'s emit condition exactly `computeRecordAndHealWatermark`'s stamp condition, so alternateFrom present ⟺ the watermark gets stamped in the same request. - ADR-0012 decision 4: reword the mechanical resume.from parity statement for the dual-path hints (from=N + optional alternateFrom=N+1); update the empty-tail paragraph to cover caution/manual last-step stamping and the no-session withholding. - Tests: unit (last-step no-session → no alternateFrom; mid-plan no-session → still present) + integration (single-step failure, no session → no alternateFrom on the wire, no --from length+1 in text).	2026-07-14 21:15:46 +02:00
Michał Pierzchała	392dc1cded	refactor: rename rotate command to orientation (rotate kept as deprecated alias) (#1252) ... * refactor: rename rotate command to orientation, keep rotate as a deprecated alias The top-level `rotate` command (device orientation: portrait/landscape) shared a name with the `gesture rotate` two-finger rotation gesture. Rename the orientation command to `orientation` and keep `rotate` working as a minimal, silent CLI alias (same mechanism as `tap`->`press`) for a few versions. The rename is applied across every layer: - command-descriptor registry `name`, daemon dispatch handler, and the typed system facet (metadata/cliReader/daemonWriter/schema/output formatter) - navigation projection + `CommandResultMap` (`OrientationCommandResult`, `action: 'orientation'`), client types (`OrientationCommandOptions`), and the runtime family (`device.system.orientation`) - interactor + backend methods -> `setOrientation` (matching the backend's `setKeyboard`/`setClipboard` verb convention); Android helper `rotateAndroid` -> `setAndroidOrientation` - Apple/cloud-webdriver capability keys and plugin gate - user-facing docs (commands.md, client-api.md) Client SDK method is `orientation` (client convention = camelCase of the command name, matching `back`/`home`/`appSwitcher`); execution layers use the imperative `setOrientation`. Deliberately unchanged: - the Swift runner wire protocol keeps `command: 'rotate'` — the runner has its own command namespace with no gesture collision, so renaming it would only risk CLI<->installed-runner version skew on physical devices - the `DeviceRotation` value type / `parseDeviceRotation` (names the orientation values, no collision) Note: `client.command.rotate` / `device.system.rotate` and the `RotateCommand*` exported types are removed (the alias only rewrites CLI tokens); SDK consumers must use `orientation`. The JSON `action` value changes `rotate` -> `orientation`. * style: wrap long lines to satisfy oxfmt (orientation rename tests) * fix: add compatibility layer for the rotate->orientation rename Addresses review blockers on the CLI-only alias: `rotate` previously resolved only in CLI token parsing, so command-data/RPC paths that carry the wire command directly failed descriptor validation, and the removed typed SDK surface broke shipped consumers. Central command-alias boundary (was CLI-only): - Promote `cli-command-aliases.ts` to `command-aliases.ts` as the single alias source, applied at each command-name ingress that bypasses the CLI parser: the daemon request boundary (`handleRequest`, covering replay and older remote clients) and the batch step readers (CLI `batch-steps.ts` and daemon `batch-policy.ts`). No hand-synced command tables. Retain deprecated typed SDK surface (shipped v0.18/v0.19): - `RotateCommandOptions` / `RotateCommandResult` type aliases (legacy `action: 'rotate'` contract) and `SystemRotate*` runtime types. - `client.command.rotate` and `device.system.rotate` deprecated wrappers that delegate to `orientation` and restore the legacy response (`action: 'rotate'` / `kind: 'systemRotated'`). ADR 0014: rename `rotate` -> `orientation` in the invalidation guidance (lines 229, 237) so the accepted architecture doc matches the command name. Tests: daemon-boundary rewrite, CLI+daemon batch alias resolution, and the deprecated client/runtime wrappers preserving the legacy contract. Live emulator evidence (emulator-5554): - `orientation landscape-left` -> user_rotation=1 - `rotate portrait` (CLI alias) -> user_rotation=0 - batch step `{command:'rotate'}` (no CLI parser) -> user_rotation=1 * fix: preserve orientation rename compatibility * test: stabilize orientation compatibility formatting * style: format MCP compatibility test * revert: drop cross-surface rotate compatibility, keep the lean rename The rotate->orientation change is a bug fix (name collision with the `gesture rotate` two-finger gesture), not a compatibility feature. The cross-surface command-data compatibility added disproportionate weight (~480 B, dominated by the alias module inlined into the batch bundle) for a command that was only canonical for two minor versions, so shipped batch/ replay/MCP data carrying `rotate` is a rare, documentable break. Removed: - daemon request-boundary command normalization (`request-router.ts`) - batch step alias resolution (`batch-policy.ts`, `cli/batch-steps.ts`) - MCP tool-runner alias/legacy-result handling (`mcp/command-tools.ts`) - the `command-aliases.ts` module rename and cross-surface machinery (reverted to `cli-command-aliases.ts`) - the cross-surface tests Kept (cheap, high value — prevents build breaks for typed consumers): - CLI `rotate` alias (one line, same mechanism as `tap`/`launch`) - deprecated `RotateCommand*` / `SystemRotate*` type aliases and the `client.command.rotate` / `device.system.rotate` wrappers that delegate to `orientation` and restore the legacy response contract Net bundle vs main is now +473 B (was +952 B), almost all the kept SDK wrappers plus the unavoidable longer command name.	2026-07-14 17:17:35 +02:00
Michał Pierzchała	4703915733	fix(replay): resume.from now agrees with repairHint's record-and-heal continuation (#1260) ... * fix(replay): make resume.from agree with repairHint's record-and-heal continuation buildReplayDivergenceResume always reported resume.from as the failed step's index, but the rendered text guidance for repairHint 'record-and-heal' told the agent to continue at step+1 (the corrective step was already performed manually, so re-running the original step re-diverges). A JSON/MCP-first caller following resume.from mechanically would loop on the same divergence forever. resume.from is now computed from the same repairHint the divergence already carries: failedIndex + 1 for record-and-heal, failedIndex unchanged for every other hint. Also handles the case where that shifted index runs past the plan's end (diverged on the last step), reporting allowed:false with an explanatory reason instead of an unusable ordinal. The text renderer now embeds the concrete `replay --from <n> --plan-digest <sha>` command computed from this same value, so text and structured callers always agree. Uncovered and fixed one existing test that was silently asserting the old, wrong behavior for a genuinely record-and-heal-hinted divergence. * fix(replay): legalize the record-and-heal empty-tail resume, guard against a skipped corrective press Review of #1260 found two real issues with resume.from's record-and-heal shift (failedIndex + 1): 1. When the diverged step was the plan's LAST step, from = actions.length + 1 was rejected as out-of-range, with a reason telling the agent to finish with `close` instead. But close only commits when the repair transaction is COMPLETE, and COMPLETE only flips when a replay leg runs to the end — so that guidance walked the agent into `close` aborting and silently discarding the corrective action it just recorded. Fixed by treating `from === actions.length + 1` as a legal EMPTY-TAIL resume: evaluateReplayResumePreflight already proves it safe (it only checks the skipped range, and there's no from-th step to reject), and the runtime loop naturally executes zero steps and reaches the normal end-of-plan completion path, correctly flipping COMPLETE. Relaxed the matching upper-bound check in the actual --from invocation validator (session-replay-runtime-plan.ts) to match. 2. A blind caller resuming at the shifted `from` WITHOUT performing the corrective press would previously re-diverge (loud). With the shift fixed, that same blind resume now silently skips the diverged step — if the tail then completes, `close` commits a healed script with a hole at that step. Added a per-session watermark (`pendingRecordAndHeal`, stamped whenever a record-and-heal divergence reports resume.allowed) plus a runtime guard that rejects a `--from` landing exactly on that target while the session's recorded action count hasn't grown since — proof no corrective action was ever recorded. The watermark self-clears once a resume observes the count having grown, or is overwritten by any later divergence. The old formatResumeCommand placeholder-vs-reason mismatch this out-of-range case caused in the rendered text guidance resolves itself now that the case is allowed:true with a real command. Added an end-to-end test proving the full loop: record-and-heal divergence on the last step -> blind resume rejected -> corrective press recorded -> resume completes with replayed:0 -> transaction flips COMPLETE -> close commits the healed script (with the press, without the never-recorded step). * fix(replay): call sessionStore.set after stamping the pendingRecordAndHeal watermark Reviewer nit on #1260: mutating session.pendingRecordAndHeal in place without a trailing sessionStore.set was harmless in practice (get returns the live reference) but inconsistent with every other session-mutation site in this codebase (e.g. armReplaySaveScriptStep), which all pair a field write with an explicit set. Matches that convention so a future reviewer doesn't have to re-verify the "no set call" is intentional. * fix(replay): scope the empty-tail resume to its own watermark, fix text/reason parity, correct ADR Exact-head review of 2f9d4829 found three real blockers in the empty-tail resume fix: 1. validateReplayResumeRequest accepted `from === actionCount + 1` for ANY session with a matching digest, regardless of whether that session actually carried a pending record-and-heal watermark. A non-record-and-heal repair (or an unrelated session) could therefore be resumed one past its last step, execute zero device actions, reach the completion path, and let `close --save-script` commit while silently omitting the unresolved final step. Fixed by threading the session's `pendingRecordAndHeal` watermark and its recorded action count into `resolveReplayEntryIndex`/ `validateReplayResumeRequest`: `actionCount + 1` is now only in range when it matches THIS session's own watermark, and the "no corrective action recorded" rejection now applies uniformly to any `from` matching that watermark (not just the boundary case), closing the gap the previous runtime-only guard left open. Consumption (clearing the watermark once the action count has grown) moved alongside, in runReplayScriptFile. Added an end-to-end test proving the exploit (`--from actionCount+1` with no watermark) is rejected as out of range. 2. The rendered repair-hint guidance still showed a `replay --from <step+1>` placeholder when `resume.allowed` was false, even though a structured caller reading the same `resume` would be refused — telling a text-only caller to run a command a JSON/MCP-first caller can't. Fixed `buildRepairHintGuidance` to never render a `--from` command when `resume.allowed` is false; it now surfaces `resume.reason` (or a generic non-resumable sentence when no reason is present) instead. Added text assertions covering both the allowed (concrete command) and disallowed (reason surfaced, no --from anywhere) cases. 3. ADR 0012 and the `ReplayDivergenceResume.from` doc comment still described the retired allowed:false/close-instead behavior and the plain failed-step-index semantics. Rewrote both to describe the actual repairHint-dependent `from`, the empty-tail authorization rule, and the uniform unperformed-corrective-press rejection. * test(replay): pin the empty-tail exploit rejection across repair hints, and digest-retry ordering Reviewer follow-up on #1260 asked for two specific regression tests before considering the blocker-1 fix settled: - An armed session diverging with a `state-repair` hint (not `record-and-heal`) at the plan's last step must still reject an unauthorized `--from actionCount+1` exploit attempt — proving the watermark gate applies uniformly across repair hints, not only the `manual`-hint case already covered. - A stale --plan-digest on an otherwise-authorized empty-tail resume must be rejected WITHOUT consuming the pendingRecordAndHeal watermark, so a subsequent retry with the correct digest still succeeds. Verified by hand against the actual code before writing this: the mutation in runReplayScriptFile is gated behind `entryIndex.ok`, which requires validateReplayResumeRequest's digest check to have already passed, so a digest-mismatch leg cannot observe or consume the watermark. Both pass on the first run, confirming the shipped fix (7d044e87d) already has the correct ordering. * fix(replay): satisfy oxfmt formatting and split validateReplayResumeRequest below the complexity gate CI failures on #1260: - Lint & Format: oxfmt --check flagged 3 files with formatting drift from manual edits (line-wrapping only, no semantic change). Fixed by running `pnpm format`. - Fallow Code Quality: validateReplayResumeRequest exceeded the complexity threshold (12 cyclomatic / 8 cognitive / 43.1 CRAP) after the blocker-1 fix folded the empty-tail authorization and unperformed-corrective-press checks into it. Split into four single-purpose describe* functions (describeOutOfRangeResumeFrom, describeUnperformedRecordAndHeal, describeStaleResumeDigest, describeUnsafeResumePreflight), each checked in order by a small dispatcher — same validation order and behavior, verified by the full existing test suite (no test changes needed). Verified locally: `fallow audit --base <merge-base>` now reports "No issues in 12 changed files"; `pnpm format:check` and `pnpm lint` both pass; full suite (1910 tests) and tsc --noEmit both pass.	2026-07-14 16:39:05 +02:00
Michał Pierzchała	cf6a5f12f1	fix(replay): repair-transaction lifecycle — keep-alive, no-partial-emit, close-as-lifecycle, atomic publish (#1235) ... * fix(replay): repair-transaction lifecycle (ADR 0012 decision 6 / #1234) Agent-supervised re-record repair lifecycle, rebased onto #1225's failure-isolated close teardown. Consolidated from the earlier iterative rounds into the final teardown-commits model: - R7 keep-alive keyed off PERSISTED transaction state (repairSessionHeld signal), so a `replay --from` continuation without --save-script is still held on divergence. - Commit gated on transaction COMPLETION (saveScriptComplete/saveScriptCommitted), never on `close` alone — no prefix is ever published. - Single commit path: `commitRepairBeforeClose` runs before #1225's `runSessionCloseTeardown` destructive steps; a repair-armed session skips the teardown's ordinary writeSessionLog, non-repair keeps it. Idle-reap/shutdown commit-on-completion or tombstone via `finalizeRepairTeardown`. - BLOCKER fixes: reaped `replay --from` -> REPAIR_SESSION_EXPIRED; commit failures surfaced (not swallowed) and keep the session for retry (with healed-path reporting on success); race-safe atomic no-clobber publish; minimal `[open, close]` arms the transaction. Integrated with #1225: keeps runSessionCloseTeardown's failure-isolated cleanup + preserved platform-close error; repair commit happens first so a failed commit keeps the session addressable. * fix(replay): make the atomic publish primitive decide the no-clobber race winner BLOCKER 1 (coordinator re-review): after linkSync saw an existing target, the no-clobber publish fell back to an unconditional renameSync once the target was classified "incomplete" — two concurrent writers could both read the SAME pre-existing partial as overwritable and both renameSync over it, each returning success with no signal to the loser. A silent, undetectable clobber. publishNoClobberAtomically now makes every winner decision an atomic primitive: - linkSync is the only way to win outright (EEXIST iff a file is at the target at that instant). - On EEXIST, the existing file is grabbed via an atomic renameSync into a private, uniquely-named quarantine path *before* it is inspected, so the completeness check never races the shared path. A competing writer's own grab racing ours surfaces as ENOENT, and we re-evaluate from the top instead of trusting a stale read. - A COMPLETE quarantined file is restored (best effort) and the publish is refused; a genuinely partial one is discarded and the exclusive linkSync is retried. Every interleaving converges on exactly one winning linkSync and every other writer observing a definitive, thrown "already exists" — never two silent successes, never a torn file. Adds a regression (session-script-writer.test.ts) with both writers starting against the SAME pre-existing partial target, using a renameSync spy to force a genuine interleaving (writer B's whole publish runs inside writer A's grab step) instead of the existing competing-writer test's sequential complete-vs-complete scenario, which never exercised this race. * fix(daemon): preserve failed COMPLETE-transaction commits instead of a generic expiry BLOCKER 2 (coordinator re-review): finalizeRepairTeardown ignored the writer's { written: false, error } outcome, so a COMPLETE transaction whose commit failed at idle-reap/daemon-shutdown teardown (no-clobber refusal, bare-@ref, or a filesystem error) was silently swallowed. Daemon teardown then deleted the session and left a generic "reaped before it was finalized" REPAIR_SESSION_EXPIRED tombstone — losing the only record that a commit was even attempted, let alone why it failed. finalizeRepairTeardown now captures the writer's result. On a real commit failure it writes a distinct commit-failure tombstone (RepairSessionTombstone gains an optional commitFailure: { code, message }); request-router's repairExpiredIfTombstoned surfaces that as a new REPAIR_COMMIT_FAILED error carrying the real cause instead of folding it into REPAIR_SESSION_EXPIRED. Adds a new AppErrorCode REPAIR_COMMIT_FAILED (kernel/errors.ts) with its own hint, a session-store regression proving finalizeRepairTeardown preserves the failure (and leaves the prior complete artifact untouched), and a request-router regression proving the router translates a commit-failure tombstone to REPAIR_COMMIT_FAILED rather than the generic expiry. * fix(daemon): record the skipped terminal close in idle-reap/shutdown auto-commit BLOCKER 3 (coordinator re-review): the source plan's terminal `close` is skipped-while-armed (Fix 3), so it never lands in session.actions. The explicit `close --save-script` path accounts for this by recording a synthetic finalize close (commitRepairBeforeClose) before committing, but finalizeRepairTeardown's auto-commit at idle-reap/daemon-shutdown never runs that handler — its committed healed .ad was missing its own terminal close, so the ADR's "self-contained, fresh-replayable artifact" requirement didn't hold for this path even though the existing auto-commit test only checked existence + the completeness sentinel. finalizeRepairTeardown now calls a new recordRepairFinalizeCloseIfCommitting before writeSessionLog, mirroring commitRepairBeforeClose's recording exactly (same command/positionals/flags shape), but only when the transaction is actually about to be committed (COMPLETE, not yet COMMITTED) — an aborted transaction's write is a no-op regardless. Strengthens the existing auto-commit test (session-replay-repair-transaction .test.ts) to use a source plan with a real terminal close, and to parse the committed script and assert it ends with ['open', 'click', 'close'] with no bare @ref — not just sentinel/existence. Adds a session-store.ts unit regression exercising finalizeRepairTeardown directly for the same self-contained-artifact assertion. * fix(daemon): serialize the no-clobber publish decision behind an exclusive lock BLOCKER 1 (review follow-up): publishNoClobberAtomically's inspect/restore/ publish sequence was atomic per-step but not exclusive as a whole. Writer A could quarantine an existing COMPLETE target, and — before A restored it — writer B could linkSync its own COMPLETE artifact into the now-empty target and return success, only for A's restore (renameSync, which replaces an existing destination per POSIX) to silently stomp B's freshly published bytes. Wrap the whole decide-and-act sequence in an exclusive publish lock (acquireNoClobberLock/releaseNoClobberLock, an atomic linkSync claim over a PID-stamped lock file) so a competing writer for the same scriptPath cannot begin its own decision until the lock holder's sequence has finished and released it. A lock whose PID is provably dead is reclaimed immediately; a lock held by a live process is never stolen, only waited on with a bounded backoff before failing loudly. Adds a deterministic regression that forces the exact reported interleaving via a renameSync spy (mirroring the existing BLOCKER 1 test's technique) and confirms the pre-existing COMPLETE artifact is never clobbered. Also relaxes the older PARTIAL-race test's loser-message assertion, since a losing writer may now fail via lock contention instead of the no-clobber-specific message, depending on interleaving timing. * fix(daemon): report retriable:true for a preserved repair-close failure BLOCKER 3: buildRepairCloseFailureResponse preserves the session specifically so the agent can retry close/close --save-script, but reported details.retriable: false — machine-consistent recovery guidance requires retriable: true whenever the session was kept addressable for a retry. Extends the existing BLOCKER 2b/2c no-clobber-failure test with an assertion on this contract. * fix(daemon): run the repair close's platform close before committing BLOCKER 2: commitRepairBeforeClose recorded a successful terminal `close` and published the healed artifact BEFORE dispatchTargetedPlatformClose ran. If the platform close then failed, the session was torn down and the committed .ad falsely contained a successful close — contradicting the existing failed-close lifecycle contract (a failed close is never recorded as Closed). For a repair-armed session, dispatch the targeted platform close first; only on success does the commit (record + publish) proceed. On failure, return without touching the session at all — same as the existing commit-failure contract, so the agent can fix the cause and retry. runSessionCloseTeardown gains a skipPlatformClose flag so the already-confirmed-successful close is never dispatched a second time during teardown. Adds a regression: a COMPLETE repair whose targeted platform close rejects must not commit a healed .ad, must not record a close action, and must keep the session addressable for retry; a subsequent successful retry then commits cleanly with exactly one terminal close. * fix(daemon): close the no-clobber lock's dead-writer TOCTOU with rename-CAS Two waiters could both observe the same dead-PID publish lock and both decide to reclaim it. If one waiter's reclaim (remove + re-acquire with its own LIVE lock) completed inside the other's decision window, the first waiter's stale rmSync(lockPath) deleted the SECOND waiter's live lock by pathname (not the dead one it actually inspected), letting both enter the exclusive publish section at once. Reclaim is now a rename-based compare-and-swap: renameSync(lockPath, uniquePath) is the atomic claim (only one racer's rename of a given source ever succeeds; the loser gets ENOENT and retries). Only the winner inspects what it actually grabbed at the private claim path — if genuinely dead, discard it; if the claim raced with someone else's fresh reclaim and grabbed their live lock instead, restore it untouched and back off. The live holder's lock is never stolen. Regression drives the exact two-reclaimer interleaving deterministically via a readFileSync spy (writer B reclaims+re-acquires live, inside writer A's reclaim window) and confirms it fails against the prior rmSync-based reclaim. * fix(daemon): surface repair-close retriable/diagnosticId/logPath at the wire top level buildRepairCloseFailureResponse hand-rolled its response shape instead of going through normalizeError, so it put retriable under error.details.retriable — a location neither the router's enrichDaemonError nor the client reads (both read the top-level DaemonError.retriable) — and silently dropped the underlying platform/ commit error's details, diagnosticId, and logPath entirely. Now routes through normalizeError like every other AppError -> DaemonResponse conversion in this codebase, preserving the underlying error's details/diagnosticId/logPath, with retriable forced true at the top level (the session is retained specifically for retry, which must never be contradicted by the underlying error's own classification). Also fixes a companion gap in finalizeDaemonResponse: it rebuilds every handler-RETURNED (non-thrown) failure response into a fresh AppError before re-normalizing, but only carried hint/diagnosticId/logPath through that reconstruction, not retriable/supportedOn — so even a handler setting them correctly at the top level still lost them at this step. Both are now carried through the same way, discovered only by verifying the close fix through the actual router boundary as requested. Regression: an updated handler-level test confirms diagnosticId/ logPath/details survive a repair-close failure, and a new router-level test (through createRequestHandler, not just the raw builder) confirms retriable:true and the platform error's diagnosticId/logPath/details all survive to the client. Both fail against the pre-fix code. * fix(daemon): never re-dispatch an already-succeeded repair-close platform close When a repair-armed close's targeted platform close SUCCEEDED but the subsequent script commit FAILED (no-clobber refusal, a bare-@ref failure, or an fs error), the session was correctly retained for retry -- but nothing recorded that the platform close had already happened. A retry (close --save-script=<other>) dispatched dispatchTargetedPlatformClose again, so a non-idempotent backend could fail or wedge recovery on a second close of an already-closed target. SessionState now carries repairPlatformCloseSucceeded, set the moment the platform close returns success. A subsequent repair close consumes it and skips straight to the commit instead of re-dispatching; it is cleared once the transaction's outcome (commit or abort) is settled, so it never lingers past a single close attempt. Regression: platform close succeeds, commit fails (no-clobber), session is retained; a retry does not re-invoke dispatchTargetedPlatformClose (asserted via call count) and still commits cleanly to the retry path. Fails against the pre-fix code (dispatch called twice). * fix(daemon): enforce complete-artifact protection on explicit --save-script targets The explicit --save-script=<path> publish path bypassed the no-clobber completeness guard entirely (protectComplete only gated on the DEFAULT healed-sibling marker), so it silently overwrote even a sentinel-marked COMPLETE healed artifact at a caller-directed path. An explicit target is caller-DIRECTED (which path to write to), never caller-AUTHORIZED to destroy an unreviewed prior healed diff sitting there. Gate protectComplete on repairArmed instead of the defaulted-path marker, so every repair-armed publish (default sibling or explicit target alike) refuses to clobber a COMPLETE artifact. Ordinary (non-repair) recordings are unaffected: they never carry the completeness sentinel, so the guard never actually engages for them. * fix(daemon): replace PID-liveness lock reclaim with a TTL publish lease reclaimDeadLock (grab lock away -> inspect PID -> restore if live) was structurally race-prone: a three-writer interleaving let waiter A rename waiter B's now-LIVE lock away (to inspect it), waiter C linkSync its own lock into the momentarily-empty path, then A's "restore" (renameSync, which replaces an existing destination) silently clobbered C's freshly acquired lock -- and the pathname-based release could then remove a successor's lock, not the caller's own. Replace it with a TTL lease (LEASE_TTL_MS = 30s). The lock file's content is now a unique owner token plus its own creation timestamp (pid:random:createdAtMs); staleness is judged purely from that embedded timestamp, never by asking the OS whether a PID is alive. A stale lease is stolen via a single atomic renameSync(lockPath, <lockPath>.expired.<id>) -- exactly one caller can ever win that rename for a still-existing source -- and the grabbed content is always discarded outright: there is no restore path at all. verifyOwnership re-checks the lease immediately before the publish critical section, so a writer whose lease gets displaced underneath it (by a stale steal decision racing a concurrent re-acquire) is never fooled into publishing unprotected -- it safely aborts instead. releaseLease only unlinks the lock file when its current token still matches the caller's own, so release can never delete a successor's lock. Regression coverage (session-script-writer.test.ts): a fresh lease is never stolen; an expired lease is stolen and reclaimed cleanly; and the reviewer's exact three-writer interleaving (a stale steal decision grabbing a concurrently-re-acquired fresh lease) is driven deterministically via spies on renameSync/linkSync, asserting exactly one holder ever enters the critical section, no live claim is silently clobbered by a restore, and release never deletes a successor's lock. Confirmed the new tests fail against the old reclaimDeadLock implementation and pass with the lease. * fix(daemon): bind the repair-close platform-close marker to request identity repairPlatformCloseSucceeded was session-wide, not bound to WHICH close request actually succeeded. An untargeted close performs no platform operation (shouldDispatchPlatformClose is false with no positional target), yet the flag was still set as though a real close had run; a retry with a DIFFERENT identity -- a target newly added, or a changed target -- then wrongly skipped the platform close entirely and committed as though it had run. Bind the marker to the request's identity: repairPlatformCloseIdentity records the target (positionals) of the close whose platform close last succeeded -- the only thing that changes what dispatchTargetedPlatformClose actually does (close's other flags, shutdown and saveScript, feed the post-teardown shutdown and the commit path respectively, never the platform close dispatch itself). A retry only skips the platform close when BOTH repairPlatformCloseSucceeded is true AND the identity matches; otherwise it re-runs. Regressions (session-replay-repair-transaction.test.ts): an untargeted-then-targeted retry and a changed-target retry both must re-dispatch the platform close (asserted via the dispatch mock call count/args), not skip it. Confirmed both fail against the prior session-wide boolean and pass with the identity-bound marker. * fix(daemon): surface a shutdown-time repair-commit failure before client cleanup A successful owned one-shot replay --save-script marks the transaction COMPLETE and returns success BEFORE publication -- the actual commit is deferred to daemon teardown (finalizeRepairTeardown), which runs inside the daemon process's own shutdown handler and, on failure, writes a REPAIR_COMMIT_FAILED tombstone. cleanupDaemonAfterRequest then removed the owned ephemeral state dir REGARDLESS of that tombstone, so the caller received success while the failure and its only recovery evidence were deleted in the same breath. session-store.ts exports findUnrecoveredRepairCommitFailure(sessionsDir), scanning every session subdirectory for a non-expired tombstone carrying commitFailure -- the client has no live SessionStore/session name to key off of, only the owned state dir's filesystem path. cleanupDaemonAfterRequest checks for it (after stopDaemonProcessForTakeover, which waits for the daemon to actually exit -- by then any tombstone the daemon's own shutdown handler would write is already on disk) before rmSync'ing the state dir: if found, the state dir is preserved and the response is overridden to a REPAIR_COMMIT_FAILED error instead of the raw success. daemon-client.ts's sendToDaemon now returns cleanup's result rather than the raw request result (restructured as a caught-and-rethrown error rather than a `return` inside `finally`, which oxlint's no-unsafe-finally rejects and which would also swallow a thrown request failure). Regression (daemon-client-lifecycle.test.ts): forces a shutdown-time commit failure by pre-seeding the tombstone in the owned state dir before the client's cleanup runs, and asserts the REPAIR_COMMIT_FAILED response is surfaced and the state dir (with the tombstone) survives. Confirmed it fails without the fix (raw success returned, state dir removed). * fix(daemon): replace the no-clobber publish lock with refuse-on-exist The TTL-lease/reclaim machinery only existed to auto-overwrite a partial healed artifact while never clobbering a complete one — but a concurrent complete-vs-complete race was already correct with a plain exclusive linkSync (first wins, second sees EEXIST), and a leftover partial is a degenerate state, not something to silently replace. Publish is now a single exclusive linkSync: absent target succeeds, ANY pre-existing target (complete or partial, default sibling or explicit --save-script path) is refused. Removes the whole lock/lease/reclaim race class. * docs(daemon): scope refuse-on-exist contract/comments to ordinary recording too PR #1235 review blocker: SessionScriptWriter.write's refuse-on-exist publish is uniform across repair-armed heals AND ordinary (non-repair) open/close --save-script recording, but the ADR contract and several comments still read as if only healed repair publication is refused and ordinary recording keeps the old rename-replace overwrite. Maintainer decision is to keep the behavior uniform and fix the docs/comments/coverage instead of re-scoping. - session-script-writer.ts: clarify isRepairArmedWriteBlocked only gates whether a publish is attempted, not what refuse-on-exist does once it is; fix write()'s catch-block comment, which claimed no AppError was ever raised on the ordinary path (now false since refuse-on-exist is uniform); broaden publishHealedScriptAtomically's doc to state it is write()'s only publish primitive for every target, referencing the removed publishOverwriteAtomically and the future --force/--overwrite (#1258). - session-action-recorder.ts / session-replay-runtime.ts / types.ts: fix comments claiming an explicit --save-script=<path> (or the saveScriptDefaultedHealedPath marker) is exempt from the clobber guard; the guard is uniform regardless of path origin or repair-armed status. - docs/adr/0012-interactive-replay.md: add a "Scope" paragraph making the refusal explicitly uniform across repair and ordinary recording, and extend the decision-6 acceptance-test bullet and migration-plan step 9 bullet to require ordinary-recording no-clobber coverage too. - session-script-writer.test.ts: add the missing existing-target coverage for the ORDINARY (non-repair, no saveScriptBoundary) path — refused with bytes unchanged when the target exists (thrown, since ordinary writes rethrow AppErrors rather than returning them), and confirmed to still succeed against an absent target. No runtime behavior change: publish is still a single uniform exclusive linkSync for every --save-script target.	2026-07-14 14:18:40 +02:00
Michał Pierzchała	66910f1c75	fix: remove Android ADB swipe fallbacks (#1243) ... * fix: remove Android gesture swipe fallback * fix: tighten Android gesture review follow-up * fix: route Android touch actions through gesture helper * test: isolate Android touch provider fixture * test: drop Android swipe fallback assertions * test: provide semantic Android touch in provider scenarios * refactor: drop redundant Android touch planning code * fix: require viewport for Android touch providers * refactor: extract Android touch executor * docs: clarify Android planned touch seam * fix: complete Android gesture failure handling * refactor: tighten Android gesture review fixes * perf: avoid unnecessary Android viewport probes * fix: remove unused Android helper cache export * fix: close Android gesture contract gaps * test: cover max Android helper gesture timeout	2026-07-13 20:12:28 +02:00
Michał Pierzchała	6c416385ca	docs: define session ref-frame lifetime (#1247) ... * docs: define session ref-frame lifetime * docs: address ref-frame ADR review * docs: resolve ref-frame contract blockers	2026-07-13 19:28:56 +02:00
Michał Pierzchała	b5e0e596b8	docs: ADR-0012 Decision 6 — repair-transaction lifecycle (R7 + commit semantics) (#1234) ... * docs: ADR-0012 Decision 6 — repair-transaction lifecycle (R7 + commit semantics) Frames --save-script as a multi-invocation repair TRANSACTION committed only on completion: - R7 (new normative rule): a repair-armed replay returning resume.allowed:true must keep its daemon/session live until close; heal/--from target that same session (strengthens R2). Plain close/teardown/idle-reap while armed = abort/ discard. Bounded-expiry must surface REPAIR_SESSION_EXPIRED, not bare SESSION_NOT_FOUND. Persistent-daemon precondition rejected: fail-fast before step 1, never a later SESSION_NOT_FOUND. - Decision 4 resume: one sentence noting the session is kept addressable so resume.allowed:true is not misleading. - Commit semantics: healed .ad committed only on full-plan completion or explicit close --save-script; never on divergence-only exit, teardown, or idle-reap; atomic temp->publish. R6 defines the slice, this defines when it is complete. - Terminal lifecycle steps: non-target steps (incl. source close, unannotated steps) are already exempt from target-binding divergence per decision 3 (clarification, not a change); prefer SKIPPING the source terminal close while armed and finalize via close --save-script. - Clobber P2: no-clobber guards a COMPLETE (heal-complete sentinel) artifact only; partials are overwritable; auto-versioned names out of scope. Validation + migration step 8 extended accordingly. * docs: ADR-0012 R7 — fix 6 architecture blockers (transaction contract) - C1: R7 keep-alive keys off a DISTINCT resume.repairSessionHeld signal, not resume.allowed (which means plan-resumability and fires for every divergence). - C2: define the ARMED -> COMPLETE -> COMMITTED commit state machine. close before COMPLETE = abort (publish nothing); close at COMPLETE = atomic commit; close after COMMITTED = idempotent teardown, no re-publish. No auto-commit. - C4: precise terminal-source-close contract (last source action == close) — SKIPPED (not dispatched) under armed repair so the session is not deleted; regression required (added to migration step 9). - C5a: REPAIR_SESSION_EXPIRED backed by a bounded tombstone keyed by session key (owner + expiry), cleared by a fresh replay --save-script. - C5b: atomic publication temp file in the target's own directory; race-safe no-clobber via create-exclusive/rename-if-absent. - C6: Status block corrected — Decisions 1-6 base MERGED (#1228 et al.); R7 + commit machine UNIMPLEMENTED, tracked by #1235. Migration split into step 8 (merged) and step 9 (#1235). Validation extended for all of the above. * docs: ADR-0012 R7 — teardown-commits model + persisted-state continuation Resolves the two contract ambiguities blocking merge (aligns with #1235): - Completion model = TEARDOWN-COMMITS (not explicit-close-only). A repair-armed session stays addressable until the transaction ends; ANY teardown (explicit close, idle-reap, daemon shutdown) commits the healed .ad atomically iff the transaction is COMPLETE, else aborts with no publish (never a prefix). An incomplete reap/shutdown leaves the REPAIR_SESSION_EXPIRED tombstone; an explicit close of an incomplete tx just discards. Kept the ARMED->COMPLETE->COMMITTED machine and idempotent post-COMMITTED teardown; removed the "no auto-commit / commit only on explicit close" language. - Continuation by PERSISTED transaction state, not the per-request flag: replay --from <n> --plan-digest <sha> resumes on the persisted repair-armed session WITHOUT repeating --save-script; --save-script appears only on the transaction opener. Implementation MUST key keep-alive/continuation off persisted state. Decision 4 repairSessionHeld updated to match. Edited: R7, Decision 4 resume signal, terminal-close, Emitting, commit state machine, tombstone, migration step 9, and validation.	2026-07-13 15:47:13 +02:00
Michał Pierzchała	f474f0784e	feat: unify gesture planning and multi-touch execution (#1212) ... * feat: unify gesture planning and multi-touch execution * fix: correct unified gesture helper behavior * refactor: tighten unified gesture architecture * fix: preserve gesture routing contracts * test: account for fresh gesture viewport * refactor: remove retired gesture series * fix: preserve example app navigation targets * test: reconcile unified gestures with helper ownership * docs: update Android helper gesture protocol * fix: refresh Maestro percentage swipe frames * refactor: remove stale Maestro frame cache * fix: harden unified gesture execution * fix: model gesture viewport in providers * refactor: remove legacy gesture paths * fix: remove unused swipe preset parser * refactor: tighten unified gesture boundaries * fix: close gesture review gaps * fix: preserve gesture compatibility contracts * fix: preserve multi-touch recording semantics * fix: refresh Apple runner state after app relaunch * test: lock Apple fling fallback route * fix: close Apple runner review gaps * refactor: tighten unified gesture seams * refactor: consolidate gesture planning policy * fix: preserve swipe response compatibility * fix: keep gesture lab aligned with replay coordinates	2026-07-13 13:16:38 +02:00
devin-ai-integration[bot]	4e06304b12	fix: align layering claims, remove app-log ineffective dynamic import, refresh architecture records (#1222) (#1227) ... * fix: align layering claims, remove app-log ineffective dynamic import, refresh architecture records (#1222) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: fix layering spine ordering (cli top), ADR 0009 deferred-scope wording, terse app-log-request-scope comment (#1222) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: show full rank groups in layering spine diagrams; clarify back-edge order vs literal imports (#1222) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: mark replay ADR context historical Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: refresh replay repair and app-log boundary status 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-13 12:22:17 +02:00
Michał Pierzchała	6275ed00c5	docs: remove ready-for-human guidance (#1237)	2026-07-13 12:12:08 +02:00
Michał Pierzchała	e61edf753a	docs: ADR-0012 Decision 6 (agent-supervised re-record repair) (#1226) ... * docs: ADR-0012 Decision 7 — agent-supervised re-record repair Adds "heal-by-doing": when replay diverges on selector drift, the agent performs the failed step's intent with ordinary interactive commands against blessed refs, and the CLI emits the healed .ad from the session's actual successful execution path (session.actions) instead of hand-edited selector text. Folds in five normative protocol rules (R1-R5) from an external design review (verdict: SOUND-WITH-FIXES) covering record-arming timing, --from continuation semantics, mechanical repairHint routing, the writer's bare-@ref fail-close, and the recorded-open requirement. Makes explicit that this reintroduces an EXPLICIT, opt-in heal and is therefore consistent with (not a reversal of) decision 1's retirement of --update's SILENT auto-rewrite. * docs: renumber to Decision 6 and make repairHint the mechanical primary router - Rename 'Decision 7' -> 'Decision 6' (ADR has decisions 1-5; new one is the 6th, placed after 5. Mandatory validation; existing decisions unchanged). - Reframe the two-sub-flows router: the CLI-computed repairHint (mechanical, in-scope, ships with this decision per R3) is the PRIMARY router; the agent follows the hint and uses screen.refs only as an ambiguity override, not as the default router. Removes the agent-judgment-vs-R3 contradiction. * docs: ADR-0012 Decision 6 — daemon-side repairHint (4 kinds) + repair-run boundary (R6) Addresses two P1 review gaps: - P1-A: state that repairHint is computed daemon-side at divergence time from the recorded targetEvidence + the daemon's own full pre-action capture (only the enum crosses the wire, so the flat/capped screen.refs never gate routing); define repairHint for all four divergence kinds (selector-miss, identity- mismatch, identity-unverifiable, action-failure) with the sparse-capture fail-safe to manual. - P1-B: add R6 — --save-script records a boundary watermark (session.actions. length at invocation); the healed script serializes only the post-watermark slice, so a reused session's earlier actions don't pollute it. Clarifications: R4 fails loudly (non-zero exit, never swallowed); exact default output path = <original-stem>.healed.ad sibling; arming sets recordSession AND the watermark before step 1. * docs: ADR-0012 — declare repairHint in the wire contract + make R3 total Addresses three protocol blockers: - Blocker 1: add repairHint to Decision 4's details.divergence field list and spec it as a single bounded enum (record-and-heal|state-repair|caution|manual), present on every divergence, carried at every level, surviving all four projections (text/JSON/client/MCP). - Blocker 2: make R3's mapping total — no recorded targetEvidence (reachable for unannotated action-failure per #1223, or any kind on a legacy script) => manual, generalizing the sparse-capture fail-safe. - Blocker 3: correct capture-timing — target-binding kinds use their PRE-action tree; action-failure uses its POST-response tree (adequate for the container presence test); no new pre-action tree is stored for action-failure. Validation extended for the projection-survival and no-evidence/post-response cases.	2026-07-12 14:28:36 +02:00
Michał Pierzchała	c23d951a58	fix: preserve Maestro coordinate swipes on Android (#1207)	2026-07-11 19:44:51 +02:00
devin-ai-integration[bot]	d585d74172	chore: close out architecture experiments (#1213) ... * chore: close out architecture experiments Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: record unavailable live experiment Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * ci: make Android perf script atomic Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: explain atomic perf workflow Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: simplify back-edge diagnostics 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-11 15:07:32 +02:00
Michał Pierzchała	e2bfed5f9f	feat(replay): ADR 0012 migration steps 5+6 — resume + --update retirement (#1211) ... * feat(replay): ADR 0012 migration steps 5+6 — resume + --update retirement Step 5 (decision 4, resume): replay --from <n> --plan-digest <sha256> resumes at a 1-based plan step, skipping 1..n-1 without executing them. Every divergence report now carries a real resume object (allowed, from, planDigest, reason?) computed by a preflight that rejects INVALID_ARGS before any action when: the plan digest no longer matches the current script (edits/includes/platform-conditioned expansion), --from is out of range, a skipped step can produce outputEnv values, or the skipped range or resume target is runtime control flow (retry/runFlow.when — these are single plan entries, never individually addressable). `test` rejects --from/--plan-digest both at the CLI-schema layer and at the daemon dispatch layer (the original command name is only visible before test rewrites its nested request to `command: 'replay'`). New modules: src/replay/plan-digest.ts (canonical SHA-256 plan digest) and src/daemon/handlers/session-replay-resume.ts (preflight + the report's resume object), kept out of src/replay/ to avoid a replay<->compat import cycle. Step 6 (decision 1, retirement): --update/-u no longer rewrites .ad files. The ADR mandates a no-op, not an error or flag removal: --update now runs identically to a plain replay and returns the same bounded suggestions every divergence already carries. Removed: healReplayAction's retry-and-rewrite arm and its exclusive helpers (collectReplaySelectorCandidates stays — decision 1's suggestions still use it), the write call from the runtime loop, and the env/${VAR}-interpolation/compat-flow refusal guards that existed only to protect that rewrite. writeReplayScript itself keeps its own round-trip tests but is otherwise unused now; deleted after the production-exports gate flagged it as dead. Docs: cli-help.ts workflow topic + --update/--from flag help, AGENTS.md selector pipeline note, maestro-compat-debt-map.md, website replay-e2e.md and commands.md updated for the retired rewrite and the new resume loop. * fix(ci): classify resume flags + provider-scenario resume coverage The Integration Tests job's architecture-progress gate (test:integration:progress:check) requires every public CLI flag to be classified; --from/--plan-digest (replayFrom/replayPlanDigest) were unclassified. Classify them as device-observable workflow flags and add real provider-backed coverage to the Android lifecycle scenario: a full replay diverges on a missing selector, the report's resume object is asserted (allowed/from/planDigest), and resuming at the next index replays only the tail. Also refresh the stale replayUpdate reason ("selector-healing replay update" -> the retired no-op). * fix: bind replay resume digest to execution plan * test: align replay runtime module topology * fix: clear replay CI regressions * docs: clarify replay repair and resume paths * docs: clarify replay resume step semantics * docs(replay): note that ${VAR} values stay out of the plan digest (ADR 0012 + workflow help) Settled decision from the PR #1211 re-review (maintainer-approved): interpolated ${VAR}/--env/AD_VAR_* VALUES are deliberately NOT part of the resume plan digest. Substitution happens after the digest is computed over the still-unsubstituted ${VAR} text, so re-running the same script with different variable values keeps the same digest and stays resumable — supplying the right values on resume is the caller's responsibility. The digest still binds the script/includes, the effective --platform/--target, and per-action runtime hints + target-v1 identity. Documented in ADR 0012 decision 4 and the `help workflow` resume topic. * docs: clarify replay digest interpolation	2026-07-11 15:06:49 +02:00
devin-ai-integration[bot]	d3adea4002	Project navigation contracts and add a network digest (#1208) ... * test: record contract and digest spike selection Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * feat: project navigation client and MCP contracts from executable definitions Collapse the three independent per-command projection declarations (facet clientMethod, public client method signature, MCP output schema) for the typed system navigation subset (home, back, rotate, app-switcher, tv-remote) onto a single colocated projection in src/contracts/navigation.ts. The family builder, public client type, and MCP schema map now derive from those five projections. Refs #1185 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * feat: add opt-in network response digest Preserve every network entry and top-level recovery/actionability signal while dropping only verbose per-entry header, body, and raw-log fields at digest response level. Record deterministic output-economy baselines and parity tests. Refs #1186 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-11 10:42:15 +02:00
devin-ai-integration[bot]	0a8ea3a57b	refactor: consolidate architecture ownership and client results (#1210) ... * refactor: consolidate architecture ownership and client results Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: keep selector parse chunk grouping current Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: update moved architecture breadcrumbs Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: enforce moved selector architecture Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: keep selector guarantee ownership current Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: update selector ownership references 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-11 09:40:24 +02:00
Michał Pierzchała	c93dcdbc90	feat: disclose selector resolution in interaction responses (#1193) ... * feat: disclose selector resolution in interaction responses Implements ADR-0012 migration step 1 (decision 2). Adds an additive `resolution` field to press/click/fill/longpress responses: runtime-selector carries the full pre-action diagnostic shape (unique or disambiguated with matchCount/winnerDiagnostic/tiebreak/bounded alternatives), runtime-ref and native-ref carry the exact ref-provenance shape, direct-ios-selector carries the explicit not-observed marker, and coordinate/maestro-non-hittable-fallback stay inapplicable (no field). The comparator in selectors-resolve.ts now records which criterion (visible/deepest/smallest-area) decided each disambiguation without changing resolveSelectorChain's winner. Extends the ADR-0011 guarantee matrix with the resolutionDisclosure guarantee across all six dispatch paths, wires the shared response builder and MCP output schema, adds digest-level trimming (drops alternatives, keeps the verdict/counts), and proves via contract tests that resolution diagnostics are never ref-issued or MCP-pinned and cannot be reused as @ref targets. * fix: address resolution disclosure review findings * refactor: make resolution-disclosure choices self-evident Replace the direct-iOS/maestro message-sniffing (and its justification paragraph) with an explicit maestroFallback flag passed from the dispatch site that already owns the path decision, and shrink every why-this-is-OK paragraph to one-line constraint statements per the maintainer directive. * fix: usage-based maestro fallback disclosure + spec label-fallback Blocker 1: the runner-payload source now carries maestroFallbackUsed derived from the runner's actual execution outcome (the usedNonHittableFallback message bit RunnerTests+CommandExecution.swift reports, the same signal directIosSelectorFallbackDetails already keys on) instead of the permission flag. A fallback-allowed dispatch that hit its element normally discloses direct-ios/not-observed; only an actually-executed coordinate fallback is the inapplicable maestro cell. Contract tests cover both sides. Blocker 2: ADR-0012 decision 2 now defines the ref/label-fallback disclosure (runtime-ref trailing-label recovery via tryResolveRefNode's fallbackLabel; native-ref stays exact because the backend receives only the ref handle), amends the matrix-cell enumeration, layer-3 coverage list, and validation bullet, and the runtime-ref contract suite proves the label-fallback shape. * fix: honest runtime-ref registry cells for label recovery The disambiguation cell no longer claims refs identify exactly one node by construction — trailing-label recovery is a first-match lookup without the ranking, now an intentional waiver whose outcome the label-fallback disclosure surfaces per-response. resolutionDisclosure.via points at tryResolveRefNode (now exported), the resolver producing both exact and label-fallback, with direct unit coverage of both outcomes. * docs: correct native-ref exactness rationale and tiebreak doc Native-ref forwards fallbackLabel to the backend; exact is justified by non-observability of any backend-side label recovery, not by non-forwarding. The tiebreak doc now states the derived winner-vs-runner-up decisive margin. * fix: disclose Maestro fill fallback usage	2026-07-11 09:09:44 +02:00
devin-ai-integration[bot]	47134bf764	feat: add derived fail-open check:affected selector (#1195) ... * feat: add derived fail-open check:affected selector Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor: simplify selector for complexity gate; add docs Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: fail open on ambiguous non-source fixtures; guard catalog against real package.json/vitest.config Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor: use src/utils/exec.ts process helpers in check:affected runner Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(check:affected): SkillGym ownership, honest catalog, working-tree discovery - Add SkillGym ownership for skills/ and test/skillgym/; stop short-circuiting their Markdown as docs-only (findings 2 & 4). - Drop the fabricated GitHub 'SkillGym' job: it is a local-only gate, now localRunnable with no CI job, guarded by a workflow-existence self-test (3). - Fold working-tree (staged/unstaged/untracked) state into local discovery and disable rename detection so both rename paths classify (1). - Add run.test.ts entrypoint regressions (real diff/status/rename discovery, --run order/skip/stop-on-failure). Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(check:affected): union staged + unstaged diffs so they cannot cancel A single `git diff HEAD` nets index against working tree, so a staged add and an unstaged delete of the same file cancel and hide it. Collect `--cached` (staged) and unstaged diffs separately and union them; add a cancellation regression test. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(check:affected): cover required suite gates * refactor(check:affected): delegate tests to vitest --------- 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-10 17:53:52 +02:00
Michał Pierzchała	f53d572f87	fix: align Maestro swipe semantics across platforms (#1179) ... * fix: preserve explicit Android Maestro swipe lanes * fix: align Maestro swipe semantics across platforms * fix: avoid replaying iOS Maestro gestures * refactor: make swipe coordinate policies explicit	2026-07-10 16:41:54 +02:00
Michał Pierzchała	66fe801377	docs: ADR for interactive replay and resolution disclosure (#1177) ... * docs: add ADR 0012 for interactive replay, resolution disclosure, and retiring --update healing Records the decision to retire --update healing as a silent actor (repurposing its candidate machinery as ranked suggestions), disclose selector disambiguation in every interaction response, verify replay steps against record-time identity evidence, and add an interactive replay --from loop with a structured divergence report for all callers. * docs(adr-0012): ground in live replay evidence; require step provenance for --from Adds hands-on evidence from driving replay on the RN playground (silent text-mode success, app-state divergence heal cannot fix, Maestro step-index shift from runFlow flattening, per-format hint/code inconsistency, recordings carrying zero observation steps), makes step provenance (source file + line, including through Maestro runFlow inlining) a requirement of the divergence report plus an optional replay --list-steps dry-run, and adds a one-line text-mode success summary as decision 4d. * docs: make interactive replay ADR implementable * docs: tighten interactive replay contracts * docs(adr-0012): demote geometry to disambiguation signal, fix matchCount, define matching algorithm Reworks the target-v1 contract per review: identity is recorded id, else role + normalized label, plus a leaf-anchored ancestry prefix (K=8, nearest ancestors kept, root-side truncation only); absolute rects are demoted to never-compared diagnostics with the ±8 tolerance removed rather than tuned; duplicates disambiguate by recorded sibling order among the matching set, then viewport-relative order within the recorded scroll region — never absolute pixels; ties are identity-unverifiable divergences with candidates listed. matchCount is redefined as the replay-time recorded-selector match count (0..N, always present), with selector-miss (0) and identity-mismatch (>=1, no identity candidate) as distinct classes in an explicit six-path verification classification. Also inlines the quantitative benchmark numbers (3.67->1.00 snapshots, 14.3 vs 23.3/26.7 commands, 38/38 in 539s) so the evidence is durable without the external harness directory. * docs(adr-0012): unify positional-signal candidate domains between record and replay Fixes the P1 domain mismatch: sibling becomes a genuine same-parent child index (parent already captured as ancestry[0], no new field; identical by definition on both sides, non-isolating when the same index recurs under different parents); viewportOrder gets one region-scoped domain — the identity set partitioned by scroll region, ordinal within the recorded partition on both sides, unavailable (never compared cross-region) when the recorded region no longer exists; document order (pre-order index) is the canonical total order making every ordering deterministic, including equal rect centers. Residual ties stay identity-unverifiable with candidates listed. Record-time write, replay verification, and mandatory validation updated in lockstep; the six-path classification is unchanged. * docs(adr-0012): conditional matchCount, dependency-ordered migration, writer invariant, suggestion ranking contract	2026-07-10 11:39:45 +02:00
Michał Pierzchała	cf31fb3f7b	fix: harden iOS XCTest recovery paths (#1158)	2026-07-08 21:11:28 +02:00
Michał Pierzchała	9dabe5b1c1	refactor: derive command identity from descriptors (#1151) ... * refactor: derive client-backed cli routing * refactor: derive command identity from descriptors	2026-07-08 17:55:00 +02:00
Michał Pierzchała	b91eaad885	refactor: make iOS synthesized gesture policy explicit (#1152) ... * refactor: make iOS synthesized gesture policy explicit * test: harden settle observation under coverage * fix: preserve first-command synthesized drag behavior * refactor: simplify synthesized frame policy * refactor: inline synthesized command policies * refactor: simplify sequence synthesized context * refactor: clarify synthesized drag fallback policy * refactor: keep synthesized gesture policy runner-local	2026-07-08 17:15:42 +02:00
Michał Pierzchała	8ef4e73408	refactor: derive command exposure lists from descriptors (#1137)	2026-07-07 08:00:33 +02:00
Michał Pierzchała	5c5fa012f7	feat: --settle returns the settled diff in the interaction response (#1101) (#1106) ... * feat: --settle returns the settled diff in the interaction response (#1101) press/click/fill/longpress --settle executes the action, waits for the UI to go quiet (wait stable's loop, shared via stable-capture.ts), and returns the settled diff vs the pre-action tree in the same response — one round trip instead of the interact -> observe pair. - payload: changed lines only (bounded), summary counts, added-line refs, refsGeneration; best-effort (settled:false + hint on never-quiet content, never an action failure); --verify shares the settle captures - ref issuance: the settled tree becomes the session snapshot; a diff-carrying settle response clears snapshotRefsStale and the MCP layer merge-only re-pins added-line refs at the settle generation - grammar: --settle + --settle-quiet <ms> + --timeout <ms> (flag-sourced descriptor budget with new envelope:'widen' semantics mirroring wait) - ADR 0011: new settleObservation guarantee classified on every path with contract scenarios per enforced/delegated cell * test: give the two contention-flaky doctor scenarios explicit budgets The doctor provider scenarios sit at ~5s of real daemon-harness work on a loaded host and flake at vitest's 5s default during full-suite runs (the known contention flake AGENTS.md documents). Same in-file precedent as the Metro-probe scenario's 10s budget. * fix: move SettleParams to contracts to satisfy the layering DAG daemon/handlers/interaction-flags.ts imported the type across the daemon -> commands boundary (R2 commands-floor). The tuning params are part of the interaction contract like SettleObservation, so they live in contracts/interaction.ts and both layers import from there. * feat: keep settle diffs content-first — drop Key nodes, added lines win the cap Bluesky dogfood: a fill that summons the iOS keyboard spent 49 of the 80 capped diff lines spelling out QWERTY keys, and a screen transition with 269 removals could starve out the added lines entirely. Key-type nodes are now filtered from both diff sides (the [keyboard] container line still signals presence), and under truncation added lines — the ones carrying fresh refs — win slots over removals. * docs: state the core loop in the top-level help starting point Benchmarked with headless haiku/sonnet agents given only --help: both models skipped the help-workflow pointer and started with plain snapshot (38KB payloads they then had to re-read from files). One core-loop line at the starting point is what teaches snapshot -i and --settle to models that never read a second help page. * fix: preserve settle digest refs for mcp * fix: reduce settle fallow complexity * fix: surface settle output in CLI text * fix: complete settle handling for longpress * refactor: localize daemon timeout envelopes * refactor: deepen post-action observation * refactor: centralize post-action observation planning * refactor: derive settle capability from descriptors * refactor: trim settle descriptor helpers	2026-07-06 20:18:44 +02:00
Michał Pierzchała	83d54614d8	fix: bound iOS capture stalls and make runner recovery session-preserving (#1105) (#1107) ... * fix: bound iOS capture stalls and make runner recovery session-preserving (#1105) Runner (Swift): - Coalesce duplicate transport sends of one commandId onto the in-flight execution instead of enqueueing them again behind it (capture pileup). - Fail fast with RUNNER_BUSY while watchdog-abandoned main-thread work is draining; escalate to RUNNER_WEDGED past 120s so the daemon recycles. - Carry the capture-plan deadline into the query-sweep and private-AX ladder tiers so chained recovery cannot stack past the watchdog. - Penalize the tree backend after a slow (>5s) or abandoned capture and lead subsequent regular plans with private-AX for that bundle (sticky, 120s), stamped recovered/budget so the deferral stays observable. Daemon (TS): - Per-request runner recycle budget: at most one invalidate+reboot per request, then fail fast with an actionable, session-preserving hint. - RUNNER_WEDGED joins the runner-fatal invalidation reasons. - Interaction commands (click/fill/longpress/press/type/get/is) preserve the daemon on request timeout like snapshot/wait/find: resetting it destroyed every healthy app session the daemon owned. * fix: suppress AX-broken-screen snapshot issues so the runner survives capture XCTest records 'Failed to get matching snapshot: kAXErrorIllegalArgument' issues for every XCUIApplication query on AX-broken screens; after a few of them the test case tears down the moment the in-flight command completes, killing the long-lived runner after every capture of the screen (the restart loop behind #1105). The capture plan already classifies and recovers from AX failures, so this issue class is noise: swallow exactly it in record(_:); everything else still records and still drives XCTEST_RECORDED_FAILURE. * feat: time-slice the XCTest tree capture on a worker thread The tree snapshot XPC is a single blocking call whose duration moves with live content (4s to minutes on Bluesky profile screens); no in-process budget could bound it on the main thread. Run it on a worker bounded to an 8s slice: on timeout the plan penalizes the tree backend, skips the XCTest-backed tiers while the abandoned XPC drains (they would block behind it inside testmanagerd), and recovers through the private AX backend, which does not use testmanagerd. * tune: lower the tree-backend penalty threshold to 3s The Bluesky profile tree grind measures ~4.5s before kAXErrorIllegalArgument, just under the old 5s threshold, so every capture re-paid the doomed grind (9s each). At 3s the second capture onward defers to private AX (2.4s snapshot, 4.9s press on the live repro). * fix: harden the AX-issue suppression per review - Require the kAXError token: 'Failed to get matching snapshot: Timed out while evaluating UI query.' is a genuinely-hung-query signal and must keep recording (and keep driving XCTEST_RECORDED_FAILURE). Sibling AX server codes (kAXErrorCannotComplete, ...) are deliberately included: any AX-server rejection inside a matching-snapshot fetch is the same capture-plan noise. - State honestly that the override is suite-global and why (tap-triggered queries record the same noise; command outcomes stay honest via their own error paths). - Lock-guarded suppressed-issue counter following the file's existing abandoned-work counter pattern, logged with each suppression. - Unit-test the pure classifier (record(_:) itself is not invoked: the must-record variants would record real failures in the test run).	2026-07-05 10:08:15 +02:00
Michał Pierzchała	2557670193	test: slow-test ratchet and speed rules from measured experiments (#1099) ... * test: slow-test ratchet, budget-derived emulator poll, speed guidance from experiments Measured (2026-07-04, full unit suite: 340 files / 3,210 tests / 48s wall): wall clock was bounded by the slowest FILE (44.6s android monolith at ~7x file-level parallelism), and the slowest tests were sleeping through real production budgets (10.8s proving 'times out' by waiting the constant out, 8s emulator polls at 1Hz, real retry backoff). Two config experiments rejected with data: --no-isolate exploded the suite to 205s (module state thrashes across files sharing workers) and --pool=threads changed nothing. - scripts/vitest-slow-test-reporter.ts: the slow-test ratchet. Unit budget 2.5s / integration 15s; failure at 2x budget (the band between reports without failing so host-load variance cannot make the gate cry wolf); 36 pinned offenders, exact keys, ratchet-only pin (tracking #1098). - waitForAndroidEmulatorByAvdName: poll cadence derives from the caller's budget (min 1s, floor 50ms, ~timeout/20) — devices.test.ts 25.6s -> 2.8s (9x) in isolation, and short-budget production calls stop sampling at 1Hz against small budgets. - vitest.config: slowTestThreshold 500 for local visibility; reporter wired; isolation/pool decisions documented with the measurements. - docs/agents/testing.md 'Speed rules' + AGENTS.md testing bullet: the three conversion patterns in preference order (budget-derived cadence, budget-wiring assertion, fake clocks), the no-seam constraint, and the file-granularity Amdahl argument that makes the monolith test split a wall-clock fix, not just navigation. * fix: fallow findings on the slow-test gate — import edge, factory reporter, unit tests The string-path reporter wiring read as a dead file (fallow cannot see vitest's reporter loading); the config now imports the factory, making the edge real and type-checked. The class shape tripped the unused-class-members rule (framework callbacks are invisible to reference analysis) — converted to a factory returning the Reporter object, with the classification and rendering logic extracted as pure exported functions. Those functions now carry their own unit tests (budget bands, integration budgets, pin matching, warn-vs-fail rendering), which also grounds the CRAP estimate in real references. Canary re-verified: unpinned 5.2s sleeper fails the run with exit 1; clean runs exit 0.	2026-07-04 19:13:06 +02:00
Michał Pierzchała	cccd34fb27	docs: refocus AGENTS.md on principles and enforcement gates (#1097) ... * docs: refocus AGENTS.md on principles and gates; index ADRs; extend CONTEXT.md vocabulary AGENTS.md: replace the routing/command-family prose maps (already drifting from the code) with pointers to the self-describing, parity-tested registries; add the two sections agents actually cannot rediscover cheaply — Principles (one line per incident-backed lesson) and Enforcement gates (the classify-don't-suppress index); extend the module-size guidance from raw LOC caps to answer-one-question files, 1:1 test topology mirroring (removing the integration-aggregation exemption that produced 3,400-line test files), sibling fixture modules, claim collocation, and boundary-only barrels; record the dev-loop staleness triple (dist/daemon/adopted-runner), the tsgo typecheck, the Gatekeeper first-node-exec stall, the DEVICE_IN_USE signature, and the contention-flake protocol; append the two gate steps to the new-flag checklist. CONTEXT.md: vocabulary for the ADR 0011 domain (dispatch path, guarantee cell, owned waiver, parity table, coverage manifest, delegation-on-error, ref generation pin) and an architecture paragraph positioning ADR 0011 as ADR 0008's interaction-semantics counterpart. docs/adr: flip 0011 to Accepted (implemented through Layer 3) and add a read-this-when index that names the registries as the living source of truth over ADR prose. * docs: defer versioned-ref references to the implementing PR Review sequencing note on #1097: these lines described #1096 behavior not yet on main. They move to #1096's branch so docs land with the implementation and the two PRs merge in any order.	2026-07-04 19:09:46 +02:00
Michał Pierzchała	c506ddf3e7	RFC: ADR 0011 — interaction guarantee contract (path × guarantee matrix) (#1080) ... * docs+feat: ADR 0011 interaction guarantee contract, Layer-1 registry and gate Design for making interaction guarantees hold across every dispatch path (runtime selector/ref, direct iOS selector, native ref, coordinate, maestro fallback) instead of eroding at path boundaries one incident at a time — every interaction bug this week was a (path, guarantee) cell nobody was watching. Three layers (ADR 0011): declare the path x guarantee matrix as a typed registry whose completeness is a compile error; share one implementation per rule on both sides of the wire with golden fixture tables proving TS/Swift parity; prove every non-waived cell with contract scenarios generated from the registry. This lands Layer 1: the registry with an HONEST initial classification — ten cells are acknowledged gap waivers (direct-path disambiguation/ occlusion/nonHittable/responseFields/errorTaxonomy, native-ref guards, coordinate bounds) — plus the gate test that keeps entries truthful: referenced TS symbols must be exported, runner symbols must exist in the Swift sources, delegations must land on paths that actually enforce the guarantee, and the gap list is pinned so it can only change explicitly in a reviewed diff. * refactor: apply ADR 0011 design review - Frame Layer 1 as an honesty/completeness gate, not a truth gate: it proves every path declared a stance and referenced symbols exist; behavioral parity starts with the Layer-2/3 fixture and scenario work. - Split responseFields into responseConstruction (one shared response construction site — a single Layer-2 refactor) and responseIdentity (which identity fields a path can provide — per-path capability work); note the anticipated errorTaxonomy split (codes vs diagnostics). - Encode the hybrid gap-closure strategy: runner-side parity for geometry-local rules, delegation-on-error for semantic failures (with the explicit caveat that delegation-on-error is NOT success-path parity), and a shared runtime preflight for native-ref where a silent backend success means delegation never triggers. - Gap waivers now require a trackingIssue (gate-enforced URL); all 16 pinned gaps link the umbrella issue #1081. The honest reclassification grew the pin list from 10 to 16 — responseConstruction is a gap on every path including runtime ones, which is exactly the partial progress the coarser guarantee was hiding. - Align ADR wording with the code: parityTable is optional until Layer 3, required once a runner cell claims parity. * fix: address registry review — maestro disambiguation honesty, command-scoped verify 1. maestro-non-hittable-fallback/disambiguation was overclaimed: the guarantee is defined as visible-first/deepest/smallest ranking, but findElement only implements unique-or-ambiguous scanning. Reclassified as an intentional waiver (deliberate Maestro-semantics divergence), mirroring how the direct path keeps its success-path parity gap. 2. verifyEvidence was claimed path-wide on paths that dispatch longpress, which has no --verify. Cells can now be command-scoped via appliesTo (non-empty strict subset of the path's commands, gate-enforced), and the three affected cells scope to press/click/fill.	2026-07-04 14:06:21 +02:00
Michał Pierzchała	9aae457533	fix(errors): close call-site and consumer gaps around the central error system (#1071) ... * fix(errors): close call-site and consumer gaps around the central error system Audit + iOS/Android dogfood findings (see docs/adr/0010-error-system.md): - press/click/fill targets that parse as neither @ref, selector, nor point now fail with INVALID_ARGS grammar guidance (incl. unquoted multi-word selector values) instead of UNKNOWN 'Expected x to be a finite number' - daemon command-input validation throws AppError INVALID_ARGS instead of bare Error surfacing as UNKNOWN - selector-no-match and stale-ref failures carry targeted hints (selectorFailureHint / STALE_REF_HINT) - retriable/supportedOn survive wire rehydration to CLI --json and SDK (previously dropped at throwDaemonError / toDaemonHttpRpcError) - MCP tool errors carry code + hint instead of message-only text - lease busy/capacity use DEVICE_IN_USE (the retriable code) - asAppError(err, fallbackCode) replaces cause-dropping coercions in the Apple runner; new default hints for AMBIGUOUS_MATCH, DEVICE_IN_USE, UNSUPPORTED_PLATFORM, and a distinct UNKNOWN hint - ADR 0010 documents the error-system conventions * fix: format touched files and clear fallow audit gate - privatize SELECTOR_NO_MATCH_HINT / SELECTOR_NOT_UNIQUE_HINT (consumed only via selectorFailureHint in the same module) and integerSchema (only used inside command-input.ts) - dedupe the resolved-node return tail in interaction resolution into describeResolvedNode - extract stringDetail/booleanDetail readers so normalizeError stays under the complexity threshold * fix: reject unquoted trailing text after interaction selectors press/click/longpress positionals like 'press text=Gesture lab' used to silently drop the leftover tokens and act on the truncated selector (text=Gesture), potentially hitting the wrong element. Reject non-empty splitSelectorFromArgs rest with INVALID_ARGS guidance that suggests the merged quoted form (text="Gesture lab"). Fill keeps consuming rest as its text payload; wait/is/replay-heal already handle rest explicitly.	2026-07-04 12:32:21 +02:00