mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
v0.20.7
1372 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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> |
||
|
|
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). |
||
|
|
236016ed8a |
fix(ios): support remote-hosted alerts on physical devices (#1232)
* fix(ios): probe remote-hosted system modals (AccessorySetupKit picker) when the springboard mirror yields no hittable actions * fix(ios): fail closed on host state, guard dismissal re-query, unit-test probe routing Addresses review on #1232: - Gate the remote-host probe to a foreground host (RemoteHostedSystemModalPolicy.isEligibleHostState); background/unknown hosts fail closed instead of substituting an unrelated action tree. - Wrap the alert-resolution fallback query in safeElementsQuery so a dismissed remote host raising kAXErrorServerNotFound is absorbed. - Extract routing/gating into RemoteHostedSystemModalPolicy and add simulator-free unit tests under AGENT_DEVICE_RUNNER_UNIT_TESTS. * refactor(ios): centralize blocking system modal resolution * fix(ios): bound alert dismissal rechecks * feat(ios): enable alerts on physical devices * fix(ios): bound alert system modal resolution * test(ios): add AccessorySetupKit picker fixture * fix(ios): validate remote-hosted system modal interactions * chore: keep pnpm checks non-interactive * fix(ios): share alert command deadline --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> |
||
|
|
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.
|
||
|
|
62fd46abd8 |
fix: Android status/nav-bar systemui chrome leaks into non-raw captures (#1251) (#1256)
* fix: recognize Android status/nav-bar leaf ids as systemui chrome (#1251) The non-raw Android walk (walkUiHierarchyNode/shouldIncludeStructuralAndroidNode in ui-hierarchy.ts) drops unlabeled/unidentified structural nodes, re-parenting their children upward. That silently swallows the status_bar*/navigation_bar* WRAPPER nodes carrying the marker ids collectAndroidSystemChromeRunIndexes keys on, leaving only their labeled/identified LEAVES (clock, battery, wifi/mobile icons, nav buttons) in a non-raw capture. Those leaves' own ids have no status_bar/navigation_bar prefix, so the systemui run loses its marker and is no longer dropped -- leaking status-bar chrome into --settle and replay divergence screen.refs. --raw keeps the wrapper markers, so it was unaffected. Recognize the surviving leaves directly by EXACT resource-id (not prefix, to stay tight -- actionable systemui overlays like the volume dialog or a media picker must keep surviving). Test derives a faithful non-raw tree from a real --raw Android capture (checkout-form fixture app, Gboard + status bar) by simulating the walk's drop+reparent for the specific marker-bearing wrappers, then asserts: every surviving status-bar leaf is classified as chrome, the whole systemui run drops, app fields and the IME keyboard are handled unchanged, and a synthetic volume-dialog run still survives. A second synthetic case covers the nav-bar leaves (no real nav-bar capture was available on the gesture-nav test device). * fix(replay): surface only meaningful divergence refs, dropping unlabeled structural nodes The get/is/wait divergence uses a full (non-interactive) capture so static-text targets survive, but that also pulls in unlabeled structural containers (ViewGroups/ComposeViews) that carry a ref yet no identity and aren't tappable. On deeply-nested RN trees they consume the SCREEN_REF_CAPTURE_LIMIT budget ahead of the actionable controls (and the app content the excluded status/nav chrome just freed room for). Filter divergence screen.refs to nodes an agent could actually re-target: identifiable (display label/value/non-generic id) or interactive (hittable). * fix(test): run Android status-bar fixture through the real non-raw walk The `simulateNonRawWalk` helper in snapshot-chrome-android-statusbar.test.ts only hand-removed status_bar*/navigation_bar* wrapper nodes, while production (shouldIncludeStructuralAndroidNode in ui-hierarchy.ts) also drops other unlabeled/generic-id structural nodes that aren't hittable and have no hittable descendant. That let the synthetic, non-hittable com.android.systemui:id/home_handle node survive the fixture and get asserted as chrome, when the real walk drops it entirely. Replace the hand simulation with a shared `walkNonRawAndroidFixture` test util that reconstructs the `AndroidUiHierarchy` tree and calls the real `buildUiHierarchySnapshot(tree, undefined, { raw: false })`, so every inclusion/drop decision in the fixture is production's. Update the status-bar leaf assertions to the identifiers that actually survive the walk, and assert `home_handle` is absent (not chrome-classified). Add an Android divergence-route test (`buildReplayFailureDivergence` with `makeAndroidSession`) that feeds the mocked dispatch the real walked tree, covering the target-binding divergence route the previous iOS-only tests missed. Verified the rewritten tests fail when `ANDROID_SYSTEM_CHROME_MARKER_LEAF_IDS` is reverted and pass with it restored. |
||
|
|
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 |
||
|
|
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. |
||
|
|
139153ce64 |
fix: bound the iOS system-modal snapshot probe to the capture deadline (#1244) (#1248)
* fix: bound the iOS system-modal snapshot probe to the capture deadline (#1244) The pre-plan SpringBoard system-modal probe (`blockingSystemAlertSnapshot`) ran before `runSnapshotCapturePlan`, outside the 20s plan budget, the bounded main-thread watchdog, and the XCTest recovery envelope. A slow `springboard.alerts`/`sheets`/descendant enumeration (seen on ASWebAuthenticationSession consent and notification-permission dialogs) could therefore stall `snapshot -i` for 30-39s. Run the probe as a bounded capture tier instead: it shares the snapshot plan deadline and executes on the same `runMainThreadWork` watchdog as the tree and query backends (`systemModalProbeBudget`, clamped by the remaining deadline). On timeout the abandoned probe is tracked like an abandoned tree capture, so the plan recovers through the independent (private-AX on simulator) backend, later commands fail fast as busy instead of queueing behind it, and the runner is never wedged. Diagnostics identify the probe and its elapsed time. The shared abandonment bookkeeping used by both the tree capture and the modal probe is extracted into retain/release helpers. * fix: don't re-enter main for bookkeeping behind an abandoned snapshot probe (#1244) After the bounded system-modal probe times out, runMainThreadWork abandons its query but that query keeps grinding on the main thread. The capture plan recovers through the independent private-AX backend and returns — but executeSnapshotDispatched then ran its didRecordXCTestFailure/retry bookkeeping through another runMainThreadWork hop, which queues behind the same abandoned query and re-stalls the command for up to the 30s execution watchdog (or throws), reintroducing the very stall the recovery avoided. Skip that bookkeeping while abandoned XCTest work is outstanding — the policy setNeedsPostSnapshotInteractionDelay already uses — so the recovered response returns immediately and a subsequent command still reports RUNNER_BUSY until the work drains. Adds an in-bundle regression that fails if the guard is removed. The snapshot recovery loop is factored into executeDispatchedWithRecovery so the guard is exercisable without a live capture. The alert command's SpringBoard detection is intentionally left on its existing 30s command watchdog here; giving alert its own physical-iOS capability is tracked as a focused follow-up (#1231). * test: force the bounded system-modal probe timeout through snapshotFast Adds a minimal probeWork seam to boundedBlockingSystemAlertSnapshot (threaded through snapshotFast/snapshotRaw, defaulting to today's exact production closure) so a test can substitute a blocking probe body while still running the real runMainThreadWork wrap and the real onAbandoned/onDrained hooks. The new regression test drives snapshotFast (the real entry point, not boundedBlockingSystemAlertSnapshot directly) with an injected probeWork that blocks past the probe's slice, forcing a genuine timeout, and asserts in order: busy/penalty accounting once the timeout fires, a recovered payload returned while the probe is still abandoned (before drain), and release (hasAbandonedTreeCapture() false / idle) once the probe drains. Verified revert-sensitive: bypassing the runMainThreadWork wrap, or dropping the onAbandoned/onDrained hooks, each turn this test red. * ci(ios): run the #1244 system-modal probe regressions so a wrapper/hook revert is caught * fix: gate the #1244 probe-timeout test seam behind the unit-test flag Addresses PR #1248 re-review blockers: 1. `probeWork` was a test-only DI parameter on the production signatures of `snapshotFast`, `snapshotRaw`, and `boundedBlockingSystemAlertSnapshot`. Gate it behind `#if AGENT_DEVICE_RUNNER_UNIT_TESTS` (the same active compilation condition the build script already sets via AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS, and the same one this file already uses to gate its unit-test methods): production callers now compile the original, unparameterized signatures byte-identical to pre-`probeWork`. Both overloads of `boundedBlockingSystemAlertSnapshot` delegate to a new private `boundedBlockingSystemAlertSnapshotBody`, which is the only place the real `runMainThreadWork` wrap and the real `onAbandoned`/`onDrained` hooks are defined, so a revert there fails through both the production and the test-seam overload. 2. The regression test previously asserted `.idle`/`hasAbandonedTreeCapture() == false` right after signaling the probe's release semaphore, racing the drain instead of synchronizing on it. It now polls `hasAbandonedTreeCapture()` (bounded) on the same background queue after signaling release, fulfilling a dedicated `drained` expectation that the test `wait(for:timeout:)`s on before the release assertions, so a slow or missing drain fails deterministically instead of racing. 3. The regression only drove `snapshotFast`, leaving a `snapshotRaw`-only wrapper regression uncaught. The test body is now a private helper parameterized over the entry point, called once for `snapshotFast` (existing test, unchanged name) and once for `snapshotRaw` (new `...ForSnapshotRaw` test), keeping ios.yml's `-only-testing` list in sync. Verified revert-sensitive for both entry points: temporarily bypassing the bounding wrap in `snapshotFast` or `snapshotRaw` turns each entry point's own test red (and only that one); dropping the onAbandoned/onDrained hooks in the shared body turns both tests red. Restoring the code turns all green again. * fix: collapse snapshotFast/snapshotRaw to one production entry point Reviewer blocker on #1248: the previous #if/#else split compiled a unit-test overload with a probeWork parameter that the regression tests called, while shipping builds compiled a separate #else implementation that no test ever exercised. Reverting the shipped snapshotFast/snapshotRaw/ boundedBlockingSystemAlertSnapshot to bypass the bounded probe would have left the tests green. Collapse each command to a single, always-compiled production implementation (no probeWork parameter anywhere), and move the only injectable seam to a tiny systemModalProbeOverrideForTesting property (stored on RunnerTests since extensions can't hold stored properties) consulted from inside boundedBlockingSystemAlertSnapshot's probe closure. Tests now call the real snapshotFast/snapshotRaw entry points and set the override instead of passing probeWork. |
||
|
|
7c935faaf9 |
fix: exclude keyboard/IME chrome from replay divergence screen.refs (#1233)
* fix: exclude keyboard/IME chrome from replay divergence screen.refs On a keyboard-open screen, target-binding divergence refs were dominated by keyboard KEY/window chrome, pushing the real actionable target past the 20-ref cap. Reuse settle's existing structural keyboard/IME chrome classifier (collectSettleChromeRefs, #1198/#1200) to filter divergence refs before the cap, instead of duplicating the classification. * refactor: move settle chrome classifier to core/ for cross-layer reuse daemon/ importing collectSettleChromeRefs from commands/ violated the layering DAG (R2 commands-floor). Extract the pure SnapshotNode[] keyboard/IME/system-chrome classifier into src/core/snapshot-chrome.ts (below both commands/ and daemon/); settle.ts and session-replay-divergence.ts both import it from there. No logic change. * fix: preserve app-owned keyboard accessory controls in chrome filter The iOS classifier treated an entire keyboard WINDOW as chrome, so a button-only inputAccessoryView/toolbar (e.g. a "Send" button) the app hosts in that window was wrongly stripped from divergence screen.refs — hiding a control the agent must heal against. Narrow it structurally: the keyboard's own chrome (keys, shift/Emoji/return, Next keyboard/Dictate) renders within the keyboard container's frame at the bottom of the screen, while an inputAccessoryView renders as a bar ABOVE the keys, so non-keyboard nodes above the keyboard's top edge survive classification. Structural spine nodes that contain the keyboard are never exempted, so genuine key-only keyboard windows classify exactly as before. Android scope unchanged (app dialog / unmarked SystemUI controls already kept). Adds core classifier unit tests (iOS accessory survives, genuine keyboard unchanged, Android app/SystemUI controls kept) plus a divergence-level regression that an app inputAccessoryView control stays in screen.refs. |
||
|
|
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 |
||
|
|
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 |
||
|
|
59a75d25ae |
fix: reject stale iOS refs after navigation (#1241)
* fix: validate stale iOS refs before touch * fix: preserve stale ref presentation mode * fix: reject stale iOS mutation refs |
||
|
|
ea3813d3b3 |
fix(daemon): isolate disconnect cancellation and resource teardown (#1225)
* fix(daemon): isolate disconnect cancellation and resource teardown Cancel HTTP requests that lose their client before response headers, and scope disconnect cancellation to the affected request/device/session instead of a global Apple runner abort. Make session resource teardown failure-isolated so one rejected step no longer skips later cleanup, while preserving lease release and session deletion. Closes #1220 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(daemon,runner): request-scoped prep cancellation and platform-close error preservation Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(daemon): split session-close teardown to satisfy complexity gate Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(daemon,runner): require pre-close runner stop and add integration prep-cancellation coverage Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(apple): preserve request cancellation during runner build Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(daemon): close request cancellation isolation gaps Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(request): keep cancellation cleanup owned 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> |
||
|
|
b8fa83c53d |
fix(record): validate finalized iOS device MP4 before reporting record stop success (#1238)
* fix(record): validate finalized iOS device MP4 before reporting record stop success Physical-iOS `record stop` (runner AVAssetWriter backend) could return a successful `screen-recording` artifact for an MP4 that was never finalized (no `moov` atom), because it was the only native recording backend that did not validate the copied file before reporting success. Mirror the simulator and Android paths: after the devicectl copy succeeds, run `deps.waitForStableFile` + `deps.isPlayableVideo` on the output and return a `COMMAND_FAILED` response (no artifact) when the file is not a playable video. Also capture the runner-side stop result (`runnerStopOk`) that `stopRunnerRecordingBestEffort` previously swallowed, fold it into the failure message, and emit a `record_stop_ios_invalid_video` diagnostic. Closes #1229 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M1ZTEJFKv3BAW36VmBwe2v * test(record): cover iOS video validation through provider flow * fix(record): surface iOS runner stop failures --------- Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Michał Pierzchała <thymikee@gmail.com> |
||
|
|
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. |
||
|
|
c7c5be24ef |
fix: support physical iOS close with Xcode 26 (#1236)
* fix: support physical iOS app close * refactor: share iOS device process resolution * fix: tolerate exited iOS process during close * fix: force physical iOS app termination |
||
|
|
3305b6868c |
fix: write Android debug_http_host to the RN default-preferences file (#1242)
* fix: write Android debug_http_host to the RN default-preferences file React Native's PackagerConnectionSettings/DevInternalSettings read debug_http_host via PreferenceManager.getDefaultSharedPreferences(context), which resolves to shared_prefs/<packageName>_preferences.xml — not ReactNativeDevPrefs.xml. Writing only the latter meant open --metro-port, open --bundle-url, and metro prepare/reload silently had no effect on modern (new-arch) RN Android apps. Verified live: a new-arch RN app ignored the ReactNativeDevPrefs.xml write and kept requesting its default host, but immediately picked up the chosen port once debug_http_host was written to <packageName>_preferences.xml. No evidence found (RN 0.76-0.85 source, current and historical) that ReactNativeDevPrefs.xml is ever read, but we keep writing it alongside the default file in case some RN fork/version does. Both files are read-merge-written so unrelated existing keys survive, and the clear path removes debug_http_host/dev_server_https from both. * fix: default the Metro host for port-only hints on emulator/simulator `--metro-port <port>` without `--metro-host` previously resolved to no transport (resolveRuntimeTransportHints needs both host and port), so applyRuntimeHintsToApp bailed and nothing was written — the app kept its built-in default dev-server host. The filename fix only took effect when both flags were passed. Default the host in the device-aware layer (resolveOpenRuntimeHints, which has DeviceInfo) via applyDeviceDefaultMetroHost: Android emulator -> 10.0.2.2, iOS simulator -> 127.0.0.1. The default is baked into the resolved runtime, so it flows to both the app-pref write and the stored session dev-server binding. Physical devices stay host-ambiguous and still require an explicit --metro-host (no LAN-IP guessing). bundleUrl hints carry their own host and are left untouched. Help text for --metro-port updated to match. |
||
|
|
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 |
||
|
|
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> |
||
|
|
6275ed00c5 | docs: remove ready-for-human guidance (#1237) | ||
|
|
ddae892430 |
feat(replay): ADR 0012 decision 6 — agent-supervised re-record repair (#1228)
* feat(replay): ADR 0012 decision 6 — agent-supervised re-record repair Implements "heal-by-doing": `replay --save-script` arms recording from the first attempt (R1), a boundary watermark scopes the healed `.ad` to the repair run (R6), the daemon computes a total `repairHint` enum at divergence time via genuine ancestor-containment over the daemon's own tree (R3), and the writer fails loud on a bare `@ref` in a repair-armed session instead of emitting an unreplayable script (R4). - session-replay-runtime.ts: `armReplaySaveScript` arms `session.recordSession` + `saveScriptBoundary` before every step; `--save-script`'s absence leaves replay byte-identical to today. - session-replay-repair-hint.ts (new): `computeReplayRepairHint` — a total mapping over the 4 divergence kinds, with two fail-safes to `manual` (no recorded evidence; sparse/unavailable capture). Wired into both divergence-minting sites (session-replay-divergence.ts, session-replay-target-verification.ts) and threaded through divergence.ts's bounding/minimal-fallback/text-report paths so it survives every response level and projection. - session-script-writer.ts: `saveScriptBoundary` slices the healed script to the repair run; a bare unresolved `@ref` throws in a repair-armed session (ordinary open/close --save-script keeps its existing refLabel fallback). - cli-help.ts: documents the repairHint-routed repair sub-flows and the close --save-script / <stem>.healed.ad promotion step. Tests: repairHint mapping (all 4 kinds x container-present/absent x fail-safes, with genuine-containment fixtures), R1 fresh evidence, R6 sticky boundary across --from legs, --from ordering with no prefix duplication, opt-in byte-identical behavior, no-record/thrown-failure exclusion, R4 fail-loud, and a full fresh-session replay acceptance test. * fix(replay): adversarial-review fixes for ADR 0012 decision 6 Addresses P0-P2 semantic holes + CI found in review of the repair loop: P0 (session-replay-repair-hint.ts): the scrollRegion container test was a near-universal false positive. An UNIDENTIFIED scrollRegion (RN's default ScrollView/FlatList with no testID) matched "any anonymous scrollview exists," routing unrelated screens to record-and-heal (the exact mis-binding --update retirement prevents). Now an identified region must ALSO satisfy the recorded ancestry containment (AND); an unidentified/absent region falls back to ancestry alone, walking the WHOLE recorded chain (not just ancestry[0]). P1 (session-replay-runtime.ts): the boundary watermark was captured pre-loop from the current session, but step-1 open --relaunch can REPLACE the session with a fresh actions:[] one, amputating the healed open/prefix. Reworked into a per-step armer that stamps the boundary on the session that actually accumulates the run (firstArm captures prior actions; a later arm on an unset boundary means open replaced the session → boundary 0). Added R2 preflight: a fresh full replay --save-script on an already-armed session is rejected (INVALID_ARGS) rather than silently duplicating the recorded prefix. P1 (session-replay.ts): test now rejects --save-script (replay-only), matching the sibling --from/--plan-digest guard, so buildNestedReplayFlags can't fan it across a suite run. P2 (session-script-writer.ts): the default <stem>.healed.ad is never silently clobbered — a second repair against the same original fails loud unless an explicit --save-script=<path> is given, so an unreviewed diff survives. CI (session-replay-runtime.ts): extracted the per-step verify/dispatch/guard body into resolveReplayStepResponse (runReplayScriptFile 205→163 lines) and the arming into preflight/armer helpers; refactored the branchy test mock invokes into a shared makeRecordingReplayInvoke fixture — fallow complexity is green. New/updated tests: unidentified-scrollRegion false-positive (state-repair/ manual) + labeled positive + full-chain ancestry; R6 no-amputation (open replaces) + preserved-session prior-action exclusion; R2 full-replay rejection; test --save-script rejection; P2 clobber refusal + explicit-path overwrite. * fix(replay): close three residual ADR 0012 decision 6 review blockers B1 (session-replay-runtime.ts): R2 was bypassable by OMITTING --save-script — the session stays repair-armed (recordSession + saveScriptBoundary), so a plain full replay re-appended the prefix. preflightReplayAgainstActiveRepair now fires for ANY full replay (entryIndex 0) on an armed session, regardless of the flag; --from resumes stay allowed. B2 (session-replay-runtime.ts + session-action-recorder.ts): an explicit --save-script=<path> at BOTH the replay-arming and close (recordActionEntry) sites now clears saveScriptDefaultedHealedPath, so the clobber guard no longer wrongly refuses a caller-directed overwrite. Invariant: marker set iff the current saveScriptPath was defaulted. B3 (session-replay-repair-hint.ts): a root-level target with NO usable structural container signal (empty ancestry AND no identified scrollRegion) was reading as "present" on any non-empty screen -> record-and-heal on an unrelated screen. computeReplayRepairHint now fails such no-signal cases to manual (same class as the unidentified-region P0); empty ancestry + IDENTIFIED region still uses the region test. Regressions: plain full replay on an armed session -> INVALID_ARGS + zero dispatches + no prefix duplication; close --save-script=<explicit existing path> overwrites (marker cleared) while a defaulted path still fails loud; no-ancestry/no-region on an unrelated screen -> manual (selector-miss and action-failure) with the identified-region positive kept. |
||
|
|
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. |
||
|
|
70bb9b6701 |
fix: wrap replay selector-miss as REPLAY_DIVERGENCE (repair-loop regression) (#1223)
* fix: wrap replay selector-miss as REPLAY_DIVERGENCE (repair-loop regression)
A dispatch failure thrown by a replay action (e.g. a selector-miss during
press/click/fill/longpress) escaped runReplayScriptFile's `if (!response.ok)`
divergence wrapping and fell through to the outer catch, returning a bare
COMMAND_FAILED with the legacy diagnostics shape instead of the ADR 0012
REPLAY_DIVERGENCE report (no resume, no screen refs, no suggestions) —
breaking the interactive repair loop for the common drift case.
Fix at the invocation layer: invokeReplayAction (the single choke point every
replay action dispatch funnels through — the top-level loop, retry blocks,
and nested Maestro runFlow invocations) now normalizes a thrown AppError into
a `{ok:false}` response before returning, matching DaemonInvokeFn's own
never-reject contract. This is more localized than wrapping the per-action
loop body in session-replay-runtime.ts and additionally closes the same gap
for retry/control-flow and Maestro nested dispatch, which funnel through this
exact function and previously had no throw protection either.
Adds regression coverage for the exact gap: end-to-end replay tests asserting
REPLAY_DIVERGENCE (not COMMAND_FAILED) with resume + screen refs for an
annotated press, an unannotated press, and a fill selector-miss thrown at
dispatch.
* style: wrap over-width line in replay selector-miss test (oxfmt)
* fix: narrow replay dispatch catch to AppError, normalize via normalizeError
invokeReplayAction's catch previously coerced ANY thrown dispatch failure
(via asAppError) into a {ok:false} response so the replay loop's divergence
wrapping applies — including plain TypeError/ReferenceError from programmer
bugs, which asAppError silently reclassified as a repairable
REPLAY_DIVERGENCE ("heal a selector"), masking a real crash.
Narrow the catch to only wrap actual AppError dispatch failures; anything
else re-throws to the outer internal-error path. Switch the wrap itself from
errorResponse(appErr.code, appErr.message, appErr.details) to
{ ok: false, error: normalizeError(dispatchErr) } — the canonical daemon
thrown->response pattern (request-generic-dispatch.ts, selector-runtime.ts)
— so retriable/supportedOn and hint/diagnosticId/logPath hoisting survive
on the thrown path instead of being dropped.
|
||
|
|
ae21067920 |
fix(mcp): advertise public Apple platform leaves in boot/shutdown/prepare output schemas (#1224)
The boot and shutdown MCP outputSchemas (and the non-exposed prepare schema) enumerated the internal PLATFORMS vocabulary (apple | android | linux | web), while the command contracts and runtime emit PublicPlatform (ios | macos | android | linux | web). Point the schemas at PUBLIC_PLATFORMS so a valid ios/macos result validates and the internal apple identity is never advertised on these public result fields. Add a focused, non-strict JSON Schema validator and use it to validate representative ios/macos structured content against the complete advertised schema (enums/consts + nested required), with negative coverage for the internal apple value, unknown enum values, the booted const, and a missing nested field. Closes #1219 Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
|
260ec45b34 |
feat(replay): ADR 0012 migration step 4 — target-binding verification enforcement (#1209)
* feat(replay): ADR 0012 step 4 target-binding verification + post-resolution guard + step-5 resume wiring Rebuilt onto current main (#1210 selector relocation, #1211 resume + --update retirement, #1213, #1215) as one coherent commit — the prior 4-commit history collided structurally with #1211's own session-replay-runtime split. Step 4 (decision 3, enforcement): every annotated resolved-target replay/test action is verified against a fresh pre-action snapshot BEFORE dispatch (verifyReplayActionTarget → classifyReplayTarget, decision 3's six paths). Only a verified outcome sends the action; selector-miss / identity-mismatch / identity-unverifiable each return a complete target-binding REPLAY_DIVERGENCE built from its own capture, with targetBinding {classification==kind, matchCount per the presence rule, recorded/observed, mismatches, candidates} through #1211's shared failure-response + bounding/sanitizer pipeline. Post-resolution guard: a verified outcome mints internal.replayTargetGuard (the verified member's identity); dispatch's own resolution (occlusion/ visibility guards verification does not replicate) must land on the SAME element or assertExpectedResolvedTarget refuses pre-action, which the loop converts to an identity-mismatch divergence. Guarded dispatch skips the direct-iOS and native-ref fast paths so a tree node exists to check. Step-5 resume wiring (this rebase): the target-binding divergence resume stub is replaced with #1211's buildReplayDivergenceResume — a pre-action divergence resumes AT the failed step (from = step index, real SHA-256 planDigest, allowed unless preflight rejects the skipped range). buildTargetBindingDivergenceResponse is the single resume site; planActions/planDigest are threaded from the loop. --update healing is retired by #1211, so the in-branch "verify healed action" work is dropped (its module and test removed as moot). Tests: classification (13), guard (4), token (5), runtime end-to-end (9, incl. old-script pass-through, guard threading + mismatch conversion, and a real computed resume assertion), plus target-binding wire tests in divergence.test.ts (classification==kind, matchCount presence, overflow-fallback targetBinding, computed-resume-not-stub, text report). Also repoints daemon-http-disconnect.test.ts already handled upstream by #1215. * test(replay): assert real computed resume on identity-mismatch and guard-mismatch paths Non-blocking review follow-up: only the selector-miss runtime test asserted the computed resume {allowed, from, planDigest}. All three target-binding paths funnel through buildTargetBindingDivergenceResponse (one shared resume site), so this is coverage not a bug — but pin resume on the identity-mismatch and guard-mismatch tests too so no path can regress its resume wiring silently. * fix(replay): carry structural denotation into the post-resolution guard (P1) The guard carried only LOCAL identity {id, role, label}. ADR path 6 isolates ONE member among several SAME-local-identity nodes via sibling / region-scoped viewportOrder — a discriminator lost at the guard. So if verification denoted duplicate A but dispatch's occlusion/visibility filter selected duplicate B (identical id/role/label), the guard passed and SENT the action on the wrong element — the exact verified-but-taps-different-node mis-binding step 4 exists to prevent. Fix: replayTargetGuard now carries a structural denotation of the verified member — its pre-order document index (decision 3's canonical total order, `node.index` after the shared reindex pipeline) plus its same-parent sibling ordinal. `assertExpectedResolvedTarget` compares BOTH local identity AND structural denotation against dispatch's pre-promotion capture, refusing pre-action when EITHER differs. Two distinct duplicates always differ in document order, so a different-member resolution can no longer pass. The promotion exemption is preserved (comparison is on the verified LEAF, and duplicates are distinct leaves). New helpers live in target-identity-node.ts (replay zone) so writer/classifier/guard share one sibling-ordinal implementation; computeSiblingOrdinal now delegates to it. The guard-mismatch divergence surfaces a `position:` mismatch line (from the structural denotations) so `mismatches` is never empty when local identity is identical. END-TO-END regression (session-replay-target-guard.test.ts): two "Save" buttons with IDENTICAL id/role/label, A covered+deeper (verification's winner), B visible (dispatch's winner) — driven through the real interaction runtime (`device.interactions.press`), the guard refuses pre-action with ZERO backend taps AND zero native tapTargets, the refusal driven purely by the structural denotation (both observedStructural/expectedStructural asserted). Plus a same-structural passing case, a runtime-level `position:`-mismatch divergence test, and updated guard-threading assertions for the {identity, structural} shape. Wire contract unchanged: replayTargetGuard is DaemonRequestInternal (never on the wire); decision 4's targetBinding {recorded, observed} stay LOCAL identity per decision 3's Identity tier — the structural denotation is an internal guard concern, surfaced only as a `mismatches` string. No ADR change needed. |
||
|
|
c23d951a58 | fix: preserve Maestro coordinate swipes on Android (#1207) | ||
|
|
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> |
||
|
|
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
|
||
|
|
8157b37efd |
fix: repoint tests to relocated request-progress/cancel modules (#1215)
main was red: this test still imported src/daemon/request-progress.ts and src/daemon/request-cancel.ts, which no longer exist after the relocation to src/request/progress.ts and src/request/cancel.ts. Import-only fix, no behavior change. |
||
|
|
7571b226a3 |
fix(macos): allow XCTest teardown before runner kill (#1206)
* fix(macos): allow XCTest teardown before runner kill * fix(macos): abort runner on HTTP disconnect |
||
|
|
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> |
||
|
|
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> |
||
|
|
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 |
||
|
|
952bc3704a |
refactor: keep command and daemon-route owner-file claims tooling-only (#1178) (#1192)
* refactor(command-descriptor): keep owner-file claims tooling-only Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(daemon): keep daemon-route owner-file claims tooling-only Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): guard against re-adding owner-file paths to the production route chain Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(command-descriptor): derive owner-file projection from colocated RAW_COMMAND_DESCRIPTORS - Keep ownerFiles on each RAW_COMMAND_DESCRIPTORS entry as the source of truth. - Add tooling-only __OWNER_FILES__ build flag so production bundles omit the ownerFiles properties entirely. - Derive COMMAND_OWNER_FILES from RAW_COMMAND_DESCRIPTORS instead of a hand-maintained parallel table. - Guard command-explain tests against leaking ownerFiles into production descriptor objects. - Enable treeshake.propertyReadSideEffects: false in tsdown to help drop the dead ownerFiles branch from production bundles. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: apply oxfmt formatting Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(build): guard tooling metadata exclusion Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(command-descriptor): drop global treeshake option and add bundle guard - Remove treeshake.propertyReadSideEffects from tsdown.config.ts; the __OWNER_FILES__ define + conditional spread already keeps owner files out of the bundle, so the global DCE lever is unnecessary and scope-creeping. - Add a comment on the __OWNER_FILES__ global declaration explaining the deliberate type-versus-runtime mismatch. - Add test/output-economy/owner-files-no-leak.test.ts to build dist and assert that no command or daemon-route owner-file path appears in the emitted JS. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(build): remove owner metadata property reads Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(command-descriptor): enforce owner claim totality 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> |
||
|
|
973df8a8a8 |
feat(replay): ADR 0012 migration step 2 — structured replay divergence report (#1197)
* feat(replay): thread runFlow include provenance through Maestro parsing
ADR 0012 migration step 2. The Maestro parser previously lost per-action
source location in two stages: convertRootCommands stamped every action
produced by inlining a runFlow with the PARENT runFlow: command's own line,
and parseRunFlowFile's callers discarded the included file's line table and
path entirely. This is the live-reproduced bug: "Replay failed at step 5
(__maestroTapOn ...)" with no file or line anywhere in the failure.
ParsedReplayScript gains an optional actionSourcePaths array parallel to
actionLines (undefined means "the top-level replay file"). The Maestro
conversion pipeline now carries {path, line} through runFlow inlining,
including multi-level (include-of-include) nesting, via a transient
SessionAction.replaySource field that convertRootCommands reads back into
the flat arrays and strips before an action leaves the parser.
Regression coverage: a flow including ../launch.yml where a failure inside
the include must report the include's own path+line, plus a nested
include-of-include case.
* feat(replay): structured REPLAY_DIVERGENCE report for step failures
ADR 0012 migration step 2 (report only; kind: "action-failure"). Every
replay/test step failure now returns ok:false with code REPLAY_DIVERGENCE
and a bounded, redacted details.divergence v1 object instead of the
underlying failure's own code:
- step: 1-based plan index + source {path, line}, now accurate through
Maestro runFlow includes (previous commit) and already-correct for .ad.
- cause: the original code/message/hint, preserved verbatim.
- screen: a fresh post-failure interactive snapshot digest, captured and
blessed exactly like settle's stored-tree pattern (setSessionSnapshot +
markSessionSnapshotRefsIssued + session.snapshotGeneration), so refs are
immediately actionable. Capture/sparse failure degrades to
{state:"unavailable", reason, hint} and never masks the original cause.
- suggestions: collectReplaySelectorCandidates + resolveSelectorChain
re-resolution (session-replay-heal.ts), reused READ-ONLY to rank up to 5
candidates by identity-component strength (id > role+label > label >
other) then document order. The same-scrollRegion-as-recorded tier from
decision 1's total order is not evaluated here — it depends on decision
3's recorded target evidence, which this step does not consume.
--update's actual rewrite behavior is unchanged.
- resume: always {allowed:false, reason}, no planDigest/from key until
step 5 lands.
- Bounds: 8/24/64 KiB (digest/default/full) on the serialized
details.divergence object, 256-UTF-8-byte per-field caps, 8/20 screen
refs, 5 suggestions. Overflow writes the fuller detail to a session-scoped
artifact and returns {omittedBytes, artifactPath}, or artifactUnavailable
if the write itself fails — the cause is never dropped either way.
- Success path gains a one-line text summary ("Replayed N steps in X.Xs"),
closing the silent-success gap (text replay previously printed nothing).
Existing replay-failure test fixtures that didn't mock dispatchCommand now
do, since every failure attempts a post-failure snapshot capture.
* feat(cli): render replay divergence as a compact text report
ADR 0012 migration step 2, end-to-end preservation (daemon -> Node client ->
CLI leg). printHumanError now renders a compact divergence summary (step
location, screen availability, ranked suggestions or an omitted-count hint,
overflow artifact pointer) unconditionally — not gated behind --debug, which
previously only showed step+action+selector+a generic hint with no screen
evidence. --json already carried the full structured error via
normalizeError, which was already preserving details.divergence through
stripDiagnosticMeta/redactDiagnosticData (only hint/diagnosticId/logPath/
retriable/supportedOn are stripped there); this commit adds contract tests
proving that survival, plus the throwDaemonError -> AppError leg.
* feat(mcp): treat a replay divergence error as a ref-issuing result
ADR 0012 migration step 2, MCP leg. Previously any command failure (daemon
or otherwise) reached callTool's outer catch, which built a text-only
ToolResult with no structuredContent at all — a divergence's repair data
(screen refs, suggestions, cause) was completely lost over MCP.
The tool executor's own execute() now catches command failures directly, so
it can run the same ref-merge/pin pipeline successful ref-issuing responses
already get: an error's details.divergence.screen (when "available") is
merged and pinned at refsGeneration exactly like a snapshot/find response,
merge-only like every other pin path (an unrelated command error, e.g.
INVALID_ARGS with no divergence, never clears existing pins). The result is
isError:true, structuredContent is the full normalized error, and content
carries the same compact text summary the CLI renders. router.ts's own
catch is now only the fallback for failures outside a resolved command call
(unknown tool name, malformed params) and reuses the same normalize/format
helpers (extracted to mcp/tool-error.ts) rather than duplicating them.
* fix(replay): carry include provenance through retry/runFlow.when wrappers
Review blocker on #1197 (empirically probed): a `runFlow: file:` include
nested under `retry:` or a runtime `runFlow.when: {visible|notVisible}`
conditional broke provenance on both sides. Parse side: the transient
SessionAction.replaySource field was only stripped by the top-level
convertRootCommands path — nested actions under replayControl.actions kept
it, contradicting the field's own "never reaches dispatch" doc comment.
Runtime side: invokeReplayActionBlock invoked every nested action with the
WRAPPING control action's line, so a failure inside such a wrapped include
reported the retry:/runFlow.when: line of the root file — reproducing
exactly the provenance bug this PR fixes, on the single most common real
failure site (the RN suite's launch include is retry-wrapped).
Fix, both sides:
- Parse: stripNestedActionSources removes replaySource from every action a
control-flow wrapper carries and records it as the new parallel
replayControl.actionSources array (undefined entry = "the wrapper's own
file"). Deep leak check test proves no replaySource survives anywhere in
parsed output.
- Runtime: invokeReplayActionBlock/invokeReplayRetryBlock consult
actionSources per nested action; invokeReplayAction threads a sourcePath
(falling back to the wrapper's, then the root file) and attaches the
failing action's resolved {path, line} to the error as a
transport-internal details.replaySource marker — deepest failure wins, an
outer wrapper never overwrites it. Trace events gain sourcePath when it
differs from the root replay file.
Regression tests are the reviewer's probe scenario: a retry-wrapped include
and a runtime-notVisible-wrapped include, each failing INSIDE the include,
asserting divergence.step.source.path/line point into the include (and that
the internal marker is stripped from the flat details). Parser-level tests
assert the strip + actionSources shape for both wrapper kinds.
* fix(replay): single divergence capture; consume nested failure provenance
Review findings on #1197 (needs-work review, findings 1-3 + 5):
- ONE post-failure snapshot now serves both the screen digest and the
suggestion re-resolution. The previous per-purpose double capture had the
second setSessionSnapshot advance the generation past the one the report
had just advertised (screen.refsGeneration=G1, live session at G2), so
MCP pinned the caller's next ref at a stale generation and the daemon
warned "re-run snapshot" — defeating the ADR's ref-issuing contract in
exactly the case a selector suggestion exists. Single capture = one
generation, refs and suggestions minted from the same stored tree, and
one fewer device round trip per failure (heal path drops from 3 captures
to 2, plain failure from 2 to 1).
- Blessing follows the settle/find choke-point sequence with the store
ordered explicitly after markSessionSnapshotRefsIssued, replacing the
ineffective flag flip the review flagged; session-snapshot.ts's
caller-list doc gains the new caller.
- Sparse captures no longer write back (per the selector-capture
reliability contract): a sparse verdict degrades the whole observation to
screen unavailable + no suggestions.
- Capture flavor: interactive-only, except for the non-rect selector reads
(get/is/wait) where heal's suggestion semantics always used a full tree.
- withReplayFailureContext consumes the transport-internal
details.replaySource marker (deepest nested failure wins over the
wrapper's top-level source) and strips it from the flat details; the wire
shaping moved into a pure helper to stay under the complexity gate.
- step.source.line fallback is now ?? 1 (matching the parsers' own
fallbacks) instead of ?? 0, which rendered as "path:0" in the CLI report.
Runtime regression tests cover the reviewer's probe scenario end-to-end
(retry-wrapped and runFlow.when-wrapped includes failing inside the
include) plus a construction-path redaction assertion.
* fix(replay): redact divergence fields before truncation, per the ADR
Review should-fix on #1197: ADR 0012 decision 4 specifies "all rendered
strings and any overflow artifact pass through the central diagnostics
redactor BEFORE truncation"; the implementation truncated at construction
time and relied on normalizeError's later whole-details redaction — i.e.
truncate-then-redact, whose safety depended on the regex redactor's
incidental robustness to secrets split across the cut.
sanitizeReplayDivergenceField (redactDiagnosticData -> truncateUtf8Field ->
marker) is now the sanctioned field sanitizer, used at every divergence
string construction site (cause message/hint, action summary, source path,
screen ref roles/labels, suggestion selectors/labels) and by the minimal
overflow fallback. Tests: a password assignment whose value straddles the
256-byte boundary must yield no secret fragment and a [REDACTED] marker,
plus a no-truncation bearer-token case.
Also picks up the outstanding format-only diff on output.ts (the CI
format:check blocker).
* refactor(replay): one provenance representation through the Maestro pipeline
Maintainer directive on #1197: a workaround needing a paragraph-long
justification comment marks a redesign spot. The named suspect was the
transient SessionAction.replaySource field — attach in runFlow conversion,
strip at two different exits (top-level convertRootCommands, control-flow
stripNestedActionSources), with a doc comment narrating that lifecycle.
The conversion pipeline now returns provenance in a parallel structure
everywhere instead: MaestroConvertedActions { actions, sources } is the
uniform result of convertMaestroCommandWithLine / convertCommandList /
convertRunFlow / convertRepeat / convertRetry. runFlow includes produce
concrete sources; everything else produces undefined ("this file");
control wrappers put nested sources straight into
replayControl.actionSources. SessionAction.replaySource is deleted along
with both attach/strip helpers and their lifecycle docs — one
representation, nothing transient to strip, nothing to narrate.
Comment self-audit across the PR diff: acceptability arguments trimmed to
one-line constraint statements (redaction ordering, single-capture
rationale, --debug gating history, minimal-fallback defensiveness, resume
scope); the arguments live in the PR thread. Behavior unchanged — the
parser/runtime provenance regression tests (retry-wrapped and when-wrapped
includes, deep no-per-action-provenance leak check) pass unmodified in
substance, renamed to describe the carried-structure design.
* fix(replay): categorical fill-text exclusion, repair data on all text surfaces, sanitize/dedupe gaps
Maintainer review blockers on #1197, all three:
1. Fill text never serialized (categorical, not redact-if-secret-shaped).
formatDivergenceActionLabel replaces formatScriptActionSummary (now
deleted — no other consumer remained) for the divergence action field and
the top-level failure message: typing commands (fill/type) drop the typed
value for a `<text>` marker, keeping only the identifying target token
(selector/@ref/point). The flat details.positionals now goes through the
event log's categorical buildDisplayPositionals (`<text:N chars>`).
divergence.cause carries only {code, message, hint} — the fill
verification detail that legitimately holds `expected` is a nested cause
detail and is structurally never serialized, and platform fill failure
messages are static strings. Tests: a sentinel-bearing fill divergence at
every response level asserts the sentinel appears nowhere in the
serialized divergence, flat details, or message; unit matrix for the
label across fill selector/@ref/point and type.
2. Every text surface carries the repair data. The compact divergence
report moved from a CLI-private helper into the shared
formatReplayDivergenceReport (src/replay/divergence.ts) and is now
rendered by all three text surfaces: CLI printHumanError (as before),
MCP text content (tool-error.ts — previously code+hint only), and the
`test` default reporter's failure body (previously message+hint only).
--json and MCP structuredContent were already complete. Tests assert
step location, screen refs line, and ranked suggestions on the MCP text
and test reporter surfaces.
3. Sanitization + dedupe gaps. (a) The capture-failed screen hint
interpolated the raw capture error message — every `unavailable` screen
field now passes through sanitizeReplayDivergenceField (audited all
construction sites; cause/action/source/screen refs/suggestions were
already covered). Test: a secret in the capture error is redacted in
screen.hint. (b) Suggestion dedupe now keys on the node's unique tree
index and keeps the STRONGEST basis per the ADR (previously first-seen
basis won and ref-less nodes could collide/miss). Test: a node reachable
via a label-basis and an id-basis candidate appears once, tagged id.
Also: rebased onto origin/main (ReplaySuiteTestFailed.session fixture
field); the new production-unused-exports gate is clean.
* fix(replay): strip arbitrary cause details from the public divergence error
Maintainer P1 on #1197 (STRIP decision on the escalated scope question):
the flat REPLAY_DIVERGENCE details previously spread the underlying
command error's own details verbatim — and a real fill-verification
failure carries the entered text there (`expected`, unmasked fields, per
the fill-diagnostics contract). Per the ADR ("arbitrary nested cause
details are never serialized"), the transport now drops the cause's
details categorically; only the machine-dispatchable signals
(reason/retriable/supportedOn) survive alongside the transport's own
fields. The documented details-borne meta keys (hint/diagnosticId/
logPath) are hoisted onto the error fields first so repair guidance is
not lost (one pre-existing test updated to the hoisted location).
Privacy regression strengthened to the leak class the selector-miss test
could not see: the failing fill now returns details.expected/actual
carrying the sentinel, and the assertion covers the WHOLE serialized
public error at every response level.
* fix(replay): render repair refs on text surfaces; scrub expanded variables
Review blockers (devin-ai-integration on #1197), both:
1. Text-only repair data was incomplete: formatReplayDivergenceReport
rendered only the ref COUNT for an available screen and dropped the
unavailable hint, so a text-only CLI/MCP/test caller still needed a
follow-up snapshot or a switch to JSON — against the ADR's "no caller
gets a text-only divergence that loses its repair data". The report now
lists a bounded ref/role/label subset (8 lines, matching the digest ref
cap, with a "... N more" remainder) and carries the unavailable-screen
hint. Covered on all three text surfaces (CLI printHumanError, MCP text
content, test default reporter) plus report-level bound/hint tests.
2. Expanded replay variables were not categorically excluded: the generic
redactor cannot know replay-scope values, so `press 'label="${SECRET}"'`
failing with the daemon echoing the RESOLVED selector serialized the
expanded value in divergence.cause and the top-level message (and the
Maestro path bakes expansions into positionals at parse time, reaching
the action label the same way). Every divergence string now passes
through a per-report sanitizer that scrubs all non-builtin replay-scope
values (file env, -e CLI entries, AD_VAR_* shell env, and runtime
outputEnv merges — collected at failure time) to a `<var:NAME>` marker
BEFORE generic redaction/truncation; the top-level message/hint get the
same scrub. Marker replacement, not a drop: the caller still sees which
variable was interpolated. Regression: a selector error echoing an
expanded sentinel (message + hint) asserts the sentinel absent from the
whole public error and `<var:SECRET>` present; plus scrub unit tests
(longest-value-first, non-secret-shaped values).
Fallow follow-ups: the three failure call sites collapsed into one
failStep closure (duplication), divergenceScreenLine split per state
(complexity).
* fix: preserve replay divergence recovery and privacy signals
|
||
|
|
d4146c7f1b |
feat: add Android test IME helper for deterministic text entry (#1198) (#1201)
* feat: add Android test IME helper for deterministic text entry (#1198) Ships a headless InputMethodService (android-ime-helper) as a third Android helper APK, replacing the visible system keyboard during automated sessions. Renders zero accessibility nodes and accepts Unicode/CJK/emoji text over a base64-encoded broadcast channel, fixing both the settle-diff IME-chrome flood and the ASCII-only adb-shell text entry limit in one structural fix. - android-ime-helper/: InputMethodService + build/package scripts on the existing helper-APK toolchain (javac+d8+aapt2+zipalign+apksigner). - src/platforms/android/ime-helper.ts, ime-lifecycle.ts: install/version lifecycle (shared with the other two helpers via the new helper-package-install.ts), activation on session open, and on-device restore-hygiene (previous IME persisted to a device settings key so any daemon/state-dir can recover it; restored on close, daemon teardown, and daemon startup for orphans left by a crashed run). - input-actions.ts: fill/type route through the helper's broadcast channel when active, unicode-safe; unchanged ASCII-shell fallback otherwise. - doctor: new android-test-ime check flags a stuck helper IME with a copy-pasteable `adb shell ime set` remediation command. - Gating: default-on for emulators, opt-in via `open --test-ime` on real devices. - Dead-weight: rewrote the manual ADBKeyBoard workaround doc, dropped the now-provably-live skillgym non-ASCII eval case, updated the ASCII fallback's error message to point at the helper instead of dead-ending. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * fix(#1201 review): permission-gate the IME receiver, fix CI, add opt-out Addresses the independent review's blockers and should-fixes. SECURITY (blocker 1): the text-injection receiver was RECEIVER_EXPORTED with no gate — any co-installed app could inject text into the focused field while the test IME was active. Fixed by requiring the WRITE_SECURE_SETTINGS sender permission on the (in-process, dynamically-registered) receiver: adb shell holds it, third-party apps cannot. The reviewer's suggested exported=false + explicit-component approach was tried first but empirically breaks delivery on API 36 (adb shell cannot reach a non-exported receiver there) — documented in the helper README. Live-verified: a purpose-built rogue APK's broadcasts (implicit and package-scoped, no permission) are silently dropped, field unchanged; adb shell's bare broadcast still injects. Added ime-helper-security.test.ts asserting the permission gate and that no permissionless exported registration returns. CI (blocker 2): (a) added `testIme` to integration-progress-model flag buckets (Integration Tests was red on the unclassified flag). (b) mocked resolveAndroidImeHelperArtifact in session-doctor-android / ime-lifecycle / input-actions-test-ime tests so they no longer depend on android-ime-helper/dist existing on disk (Coverage was red on a fresh checkout); verified by running them with dist removed. Should-fixes: added `--no-test-ime` to opt out on emulators (tri-state gating, parser-tested); PR body's "byte-identical" claim corrected to size/CRC-match. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * docs(#1201): pin the API-36 exported-receiver constraint in a comment The RECEIVER_EXPORTED flag cannot express why it must stay exported. Add a one-line note so a future hardening pass doesn't switch to RECEIVER_NOT_EXPORTED and silently break the CLI (adb shell can't deliver explicit broadcasts to non-exported components on API 36+; WRITE_SECURE_SETTINGS is the actual gate). 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * fix(#1201 review): harden IME restore lifecycle (blockers 1 & 2) Blocker 1 — a failed restore no longer deletes the recovery value. restore now reads back default_input_method after `ime set` and only clears the persisted previous-IME record on a confirmed-successful restore; a failed set keeps the value so a later retry / startup recovery / doctor remediation can still un-strand the user off the helper IME. Blocker 2 — startup orphan-recovery no longer overwrites/races user state. It only restores when the device's CURRENT default IME is still our helper (so a user who legitimately switched away is left alone), and skips any device a live session in this process owns (the fire-and-forget startup vs. concurrent `open` race — activate now marks the device active BEFORE the `ime set`, so any recovery pass that could observe the helper active also observes the flag and skips). Never persists the helper itself as the previous IME. activate also verifies its own switch via read-back. Exported ANDROID_IME_HELPER_SERVICE_COMPONENT so restore compares the active IME without reading the packaged artifact from disk. Tests: failed-restore keeps the value (+ later recovery succeeds), startup no-op when current != helper, startup skips a live-owned device. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * chore(#1201): delete unused ACTION_ENTER path, baseline test-only export seams Rebased onto main (#1202 production-unused-exports gate). Two follow-ups: - Deleted the unused ACTION_ENTER broadcast end-to-end (TS sendAndroidImeHelperEnter + its test, Java handler, README): nothing routes through it — `keyboard enter` uses the keyevent ENTER path — so the new production-exports gate flagged it as dead production code. Removed rather than grandfathered. - Added the three legitimate test-only seams (resetAndroidImeHelperInstallCache, resetAndroidTestImeActivationCacheForTests, setAndroidTestImeActiveForTests) to fallow-baselines/production-unused-exports.json, matching how the sibling helper reset functions (resetAndroidMultiTouchHelperInstallCache, ...) are already grandfathered there. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * fix(#1201): stop daemon-startup adb spawn on non-Android hosts (macOS Smoke) Root cause of the red macOS Smoke shard (proven, not hand-waved): the fire-and-forget restoreOrphanedAndroidTestImeOnDaemonStartup ran `adb devices` at EVERY daemon startup, on every platform. GitHub macOS runners ship the Android SDK, so this cold-started the adb server mid-replay and destabilized the macOS System Settings replay timing — the failed job's cleanup shows "Terminate orphan process: pid (N) (adb)"; main's green runs spawn no adb. Fix: gate the startup orphan scan behind a host-side marker written in the daemon state dir when a session activates the test IME (mirrors the managed-web-browser orphan-cleanup `installed` gate). A host that never uses the Android test IME — the macOS CI runner included — never writes the marker and so never spawns adb at startup. The marker is cleared once nothing is left stuck. Adds SessionStore.resolveStateDir(); tests: startup recovery does not scan adb when no marker exists (+ marker cleared after a clean scan). 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * chore(#1201): suppress fallow class-member false-positive on state-dir accessor CI's Fallow audit flags SessionStore.resolveDaemonStateDir as an unused class member, but it is called via sessionStore.resolveDaemonStateDir() in session-open.ts — fallow's class-member tracer just doesn't resolve a method call sited inside a call argument. Renamed for clarity (avoids the collision with config.ts's free resolveStateDir) and added the localized fallow-ignore-next-line unused-class-member suppression. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * fix(#1201 review): durable persist before switch + device-scoped recovery markers Addresses devin-ai-integration's two P1 restore-safety blockers on 19cbce79d. P1.1 — durably persist the restore target BEFORE the global IME switch. writePersistedPreviousIme now checks the `settings put` exit code AND reads the value back, returning a boolean. activate persists first and, if it cannot be persisted, fails open to the existing input path WITHOUT switching — a rejected `settings put` can no longer strand the user on the helper with no restore target. Regression test added. P1.2 — close the marker crash/offline blind spot. Recovery intent is now recorded per device, BEFORE the switch (ordering: durable record -> marker -> ime set), eliminating the post-switch/pre-marker crash window. Markers are device-scoped and each is retained until that device is actually observed clean: an offline/disconnected-but-stuck device keeps its marker and is recovered on reconnect instead of being cleared because the current `adb devices` scan saw no set-failed. Close-time restore clears only that device's marker (stateDir plumbed through teardown/close). Tests cover the persist-failure, post-switch/pre-marker crash, offline-then-reconnect, live-session-owned, and user-switched-away cases. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
ee117c4080 |
fix: scope Android settle diffs to app content and collapse IME chrome (#1200)
* fix: scope Android settle diffs to app content and collapse IME chrome Android --settle diffs were flooded by system chrome: status-bar churn (clock/signal/battery) appeared as +/- lines in every settled diff, and summoning the keyboard added dozens of IME ref-bearing nodes that suppressed the unchanged-interactive tail (it fired zero times across the 2026-07-10 Android benchmark, regressing haiku to 2/3 success). Every Android accessibility node carries its owning package (bundleId), so settle chrome is now classified per node: IME-owned nodes collapse to one surviving line per contiguous run (the iOS [Keyboard] analog), and with a session appBundleId all other foreign-package nodes are dropped from both diff sides, mirroring record --scope app (#1163). Classification deliberately never walks parentIndex chains: Android's interactive-only pruning can chain an app node's parent through another window's node (locked in by a real-capture regression test). Inert on iOS/macOS, whose nodes never set bundleId. Benchmark (haiku, settle arm, 3 runs): 2/3 -> 3/3 success, snapshots per run 21/5/6 -> 7/1/3, settle uses 3/9/8 -> 10/11/11. * fix: keep unknown-foreign Android packages in settle output PR #1200 review blocker (live-reproduced with a real Sharesheet): the initial blanket foreign-package drop made settle blind to blocking system dialogs — a ResolverActivity over the app produced an empty diff and a tail pointing at covered buttons, relocating the death-spiral failure mode from IME noise to system dialogs. Invert the default to three per-node tiers: IME packages still collapse to one line per contiguous run; persistent system chrome (exactly com.android.systemui, the status-bar churn that motivated the fix) drops from both diff sides; every other foreign package is kept in full — a permission prompt's or share sheet's buttons stay actionable. appBundleId is no longer a classification input, only the never-drop-the-app-under-test guard, which also defuses the pre-action staleness noted in review. Regression tests use the real captured Sharesheet (raw 41-node + interactive 10-node shapes, package "android" throughout). * fix: drop only status/nav-bar systemui runs from settle output Second review round on PR #1200: com.android.systemui hosts actionable overlays (volume panel, media/output pickers, some system prompts), so package-level dropping reintroduced dialog blindness for that class. Key the drop on window-runs instead, live-verified on the emulator: the status-bar window carries com.android.systemui:id/status_bar* markers throughout while the VolumeDialog window carries only volume_dialog* ids. A systemui run (contiguous same-package parent chain) drops only when a member matches the status_bar*/navigation_bar* marker prefixes; every other systemui surface is kept with actionable refs. Run grouping never crosses packages, preserving the per-node cross-window guarantee. Also corrects the tracking-issue references (#1178 was an unrelated registry issue; the Android settle-noise issue is #1198) and adds a real-capture VolumeDialog regression test: status bar drops, volume dialog survives in both diff and tail. |
||
|
|
d968a27dc3 |
fix: honor session metro hints and expo dev-client bundle urls (#1199)
* fix: honor session metro hints and expo dev-client bundle urls - metro reload now resolves against the dev server the session's last metro prepare bound (via a per-session hint file), instead of silently defaulting to localhost:8081 and reloading an unrelated project. Explicit --metro-host/--metro-port/--bundle-url still win. - metro prepare --metro-kind expo (detected or forced) now hints the virtual-metro-entry bundle URL instead of index.bundle, which 404s against Expo dev servers in monorepos (live-verified against react-navigation's example app). Package-manager detection for --install-deps now walks up from --project-root to the nearest lockfile so Yarn/pnpm workspace monorepos don't wrongly fall back to npm and hit EUNSUPPORTEDPROTOCOL on workspace: deps; install failures now hint at --no-install-deps and the detected PM. - open now accepts --metro-host/--metro-port/--bundle-url/--launch-url as session-hint setters (folded into the same runtime object the daemon already persists for open), so a fresh session doesn't need a throwaway reload-first call just to seed hints. Updates help text for metro and open to match. * fix: make the metro-sessions file the single reload hint store Review follow-up (PR #1199): - open's --metro-host/--metro-port/--bundle-url now also record the session's dev-server binding in the metro-sessions file (the one local store metro reload resolves against), so a later plain reload actually reuses what open set. The daemon runtime-hints write stays for device-native dev-server prefs. - The binding now carries bundleUrl (prepare persists the local-flow bundle URL; bridge runtimes are excluded). - clearMetroSessionHints is wired: session close drops the binding (teardown intent — even when the daemon call fails), and a hintless open that creates the session clears a leftover same-name binding. The open result carries sessionReused so the client can tell fresh from reused sessions. Regression test pins prepare -> close -> flagless reload resolving to the default, never the stale port. - Package-manager detection recognizes bun.lock (text lockfile, default since Bun 1.2) and bounds the lockfile walk-up at the nearest .git entry. - Comment audit per maintainer directive: multi-line acceptability arguments trimmed to one-line constraint statements; the store's lifecycle is documented once on MetroSessionHints. * fix: preserve bundle-url mount prefix and broadcast expo reloads over /message Maintainer review follow-ups (PR #1199): - Reload/message endpoint URLs keep the bound bundle URL's mount prefix (e.g. /tenant-42/index.bundle -> /tenant-42/reload) instead of collapsing to the host root. The Expo virtual entry (.expo/.virtual-metro-entry.bundle) is an entry-module path, not a server mount, so it maps to the server-root endpoints (verified live: Expo serves /message at the root). - When the dev server has no HTTP /reload route and answers with the app page (Expo), metro reload now broadcasts {"version":2,"method":"reload"} over the server's /message websocket (the channel dev-server CLIs use for the r key) instead of reporting the app-page 200 as a successful reload. The result carries a transport field (http | message-socket). Live-verified: a flagless metro reload against the running Expo server made the app re-fetch its JS bundle. - Endpoint resolution moved to src/metro/metro-reload-endpoints.ts so the seam tests use is production-imported (keeps the new production-unused-exports ratchet clean). - Docs reconciled: help metro and website commands.md describe the single session binding store, open's hint flags, prefix preservation, and the message-socket fallback. * fix: preserve Expo reload mount prefixes |
||
|
|
58995bf51b |
feat: parse and preserve .ad target-v1 evidence (ADR 0012 migration step 3) (#1196)
* feat: parse and preserve .ad target-v1 evidence (ADR 0012 migration step 3)
Recording now emits a `# agent-device:target-v1 {...}` comment immediately
before every click/press/longpress/fill action that resolves through the
tree (ref or selector), carrying the identity/ancestry/sibling/scrollRegion/
viewportOrder tuple from decision 3's record-time write algorithm plus a
record-time self-check (verified/unverifiable). The parser accepts known
fields in any order, NFC-normalizes, rejects malformed/oversized annotations
with INVALID_ARGS, binds an annotation only to the immediately-next action
line, and treats unknown future target-vN comments as ordinary comments.
writeReplayScript's read-then-rewrite (heal) path preserves v1 annotations
in canonical form. Nothing consumes the parsed evidence yet beyond
preservation — replay-time enforcement is a later migration step.
The identity tuple's own node/tree only ever lived on the internal
visualization/session-history payload (never the public response); it is
converted to the compact target-v1 evidence and stripped before anything is
persisted or returned.
* fix: bound candidate identity before self-check comparison
The identity-set scan in computeTargetEvidence compared an untruncated
candidate identity against the 256-byte-bounded recorded identity, so a
node whose own id/label exceeds the field cap could fail to match itself,
corrupting the record-time self-check. Compare through the same bounding
on both sides instead.
* fix: address PR #1196 review — contract boundary, formatting, path-6 reasons, worst-case sizing
- native-ref contract test: deliberately updated to assert the new ADR 0012
boundary — the preflight's node/preActionNodes ride the INTERNAL runtime
result and visualization payload only; the public responseData never
carries node/preActionNodes/targetEvidence (asserted directly against
buildInteractionResponseData).
- oxfmt formatting on all touched files.
- classifyTargetBindingMatch path 6 now distinguishes decision 3's two
spec-distinct outcomes: a signal isolating a member that differs from the
winner ('signal-isolated-wrong', the paths-4/5 comparison class, future
identity-mismatch) vs true fall-through ('no-signal-isolation', future
identity-unverifiable), so step 4 can consume the reasons directly.
- writer reduction loop sizes every candidate against the worst-case
verification value ("unverifiable" is 4 bytes longer than "verified"), so
a fail-closed self-check downgrade can never push an accepted payload over
the 4 KiB cap; pinned by a 1-byte-granularity sweep across the boundary
that fails against the old placeholder-sized check.
* fix: reject missing role keys in target-v1 annotations, correct data-flow comment
Second-review nits: the writer emits `role` unconditionally (top level,
ancestry entries, scrollRegion) — possibly as the empty string for typeless
nodes, which stays accepted — so a MISSING role key can only come from a
hand-edited/adversarial annotation and is now rejected with INVALID_ARGS
instead of silently parsing as an implicit empty role, which step-4
enforcement could otherwise match against anonymous wrapper nodes. Also
corrects the interaction-touch-response comment that overstated where the
raw node/tree flows (finalizeTouchInteraction strips it before both session
history and touch overlay telemetry).
* fix: record iOS selector target evidence
* refactor: make the record-time evidence channel structural, drop argument-comments
Maintainer directive: comments that argue a workaround is acceptable mark
code to redesign. Applied to the whole PR diff:
- The record-time node/tree now travel on a typed `recordedTarget` side
channel of InteractionResponsePayloads instead of being smuggled through
the visualization Record and stripped later. The construction site routes
them there exclusively, finalizeTouchInteraction consumes the channel
directly, and extractTargetEvidenceForRecording (the strip helper and its
justifying doc) is deleted — the public/internal split is now enforced by
the type shape, so the contract test asserts it in two lines instead of a
paragraph.
- findNearestScrollableContainer/findNearestAncestor made generic over the
node type, removing a cast plus its safety-argument comment.
- computeLocalIdentity/boundedLocalIdentity collapsed into one always-capped
identity reader — the raw/bounded split was the root of the earlier
self-match bug and existed only to be explained.
- parseReplayScriptDetailed's rejectUnbound takes the pending annotation as
a parameter, removing a non-null assertion and its comment.
- Remaining paragraph-length argument-comments trimmed to one-line
constraint statements (module docs, sizing/floor comments, regex/role
rationales, test comments); spec-mapping docs stay.
* chore: oxfmt the selector-evidence test files
* fix: record get target evidence, fail closed on broken parent linkage
Maintainer blockers on ADR 0012 step 3:
- get text/attrs now records target-v1 evidence: GetCommandResult carries
the resolution tree (preActionNodes, internal — neither the recorded
result nor the public payload copies it), recordIfSession gains the typed
recordedTarget channel, and dispatchGetViaRuntime threads the capture
through. The direct-iOS get query is gated during recording so the
snapshot path supplies the evidence tree, mirroring the tap/fill gating.
find/is stay uncovered: their results are match-set shaped (found flags,
predicate booleans over possibly-many matches), not a single resolved
winner — noted in the PR.
- buildAncestryChain now reports a broken parent walk (dangling parentIndex
or cycle) instead of silently producing a root-like chain; the writer
fails the annotation closed to 'unverifiable' per decision 3's
capture-anomaly rule, and broken candidates cannot prove an ancestry
prefix. Regression tests for both anomaly shapes.
- New ADR 0012 recording tests live in focused files
(interaction-target-evidence.test.ts) instead of growing the
interaction.test.ts aggregation; the earlier additions moved out.
* refactor: extract direct-iOS get selector guards below the complexity threshold
|
||
|
|
46d2931bf0 | refactor: remove redundant facade exports (#1204) | ||
|
|
fb7dbfe6fe | 0.19.3 v0.19.3 | ||
|
|
6e21fedc08 | refactor: remove production-unused exports (#1203) | ||
|
|
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> |
||
|
|
fb1117f229 |
ci: ratchet against production-unused exports (#1202)
* ci: ratchet against test-only exports Three exported-and-unit-tested-but-unreferenced-in-production incidents this week (#1166 getNearestCommandNames, #1167 buildSettleTail, #1199 clearMetroSessionHints) — the first two were caught by fallow's dead-code check because they had zero importers anywhere; #1199 was missed because a test file imports the export, and fallow's default reachability graph counts a test import as "used". Adds a second, stricter pass reusing fallow's own --production mode (entry.exclude test/story/dev files) via scripts/test-only-exports/check.ts: an export alive in fallow's default graph but dead in its production graph, with no other reference anywhere in its own file, has no production call site — exactly the #1199 shape. Ratchets against a checked-in baseline (scripts/test-only-exports-baseline.json, 77 entries); new findings fail `pnpm check:test-only-exports` (wired into CI's Fallow job and check:tooling). A `// test-seam: <reason>` comment above an export is the escape hatch for intentional test seams. Also extends .fallowrc.json's ignoreExports for seven daemon route handlers (src/daemon/handlers/*.ts) that are genuinely production-reachable through request-handler-chain.ts's `typeof import()` lazy-load pattern, which fallow's static import graph can't trace as a named-export consumer — without this they were false positives in the production-mode pass. * fix: harden test-only-exports ratchet per review Addresses the two should-fixes and all five minors from the independent review of #1202: - Replace the regex own-file occurrence count with an oxc-parser AST walk (typescript@7 ships no JS scanner API, so the review's fallback tool suggestion is the primary): identifiers are counted as AST nodes deduped by source span, so mentions in JSDoc/block comments, strings, and template-literal text no longer masquerade as call sites (review finding 1, both constructed cases re-verified fixed), and a `//` inside a string no longer hides real usages (finding 6). Span dedupe keeps barrel re-exports (`export { x } from`) counting once. The sharper count surfaced one organic false negative on main: `selector` in src/commands/index.ts was previously exempted because the regex matched "selector" inside the './...selector-read.ts' import path string; it is now baselined alongside its sibling `ref` (same re-export line). - Make the baseline shrink-only (finding 2): --update-baseline refuses new findings with the same wire/delete/annotate message, so the `// test-seam:` annotation in the reviewed source diff is the only acceptance path; CONTRIBUTING no longer documents baseline regeneration as an acceptance option and now describes baseline growth as a deliberate manual edit. - Stale baseline entries now emit a `::warning` CI annotation (finding 3). - Commit a re-runnable fixture test (finding 4): check.test.ts mirrors scripts/layering/model.test.ts, builds a synthetic package with a clearMetroSessionHints-shaped export (JSDoc self-mention included), asserts it is flagged, and asserts the annotated twin passes; wired before the check in pnpm check:test-only-exports. - Mark the unreadable/unparseable-file fallbacks CONSERVATIVE: per CONTRIBUTING's convention (finding 5). - Document the dynamic property access (obj[name]) blind spot in the script header and CONTRIBUTING (finding 7). * fix: harden test-only export ratchet * refactor: use native Fallow export gate * chore: refresh production export baseline |
||
|
|
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 |
||
|
|
db492cbaed |
test(output-economy): add routine-workflow output-behavior oracle (#1190)
* test(output-economy): add routine-workflow output-behavior oracle Add a deterministic routine-workflow measurement (#1180) that pairs response bytes with follow-up behavior: fallback-observation count, retry count, and whether an actionable failure preserves the session. Refs chain across one recorded checkout session and counts derive from the real formatters, so dropping settled-diff refs, the unchanged- interactive tail, or a recovery handle fails the suite. Adds a matching non-gating help-conformance next-command case. Response defaults unchanged. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(output-economy): make routine-workflow ref-surfacing depend on rendered output Address review on #1190: - Drop the raw mutation-confirm/failure MCP data samples that leaked e4/e5 and mislabeled the surface; e4/e5 now surface only from the rendered CLI settled-diff, so dropping added refs genuinely raises the fallback count. - Track the failure once as its projection-invariant normalized payload (workflow.failure.shared.json) instead of duplicate cli/mcp raw copies. - Reuse the shared REF_TOKEN_PATTERN from economy-metrics.ts. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(output-economy): reuse shared fixtures in routine-workflow oracle Address review finding #2 on #1190: chain the workflow onto the shared per-surface fixtures instead of copy-pasting them. - orient/recheck reuse SNAPSHOT_RESULT + SNAPSHOT_DAEMON_RESULT; the first mutation reuses SETTLE_ADDED_REF_RESULT, so session identity and ref generations come from ./fixtures.ts and the two suites cannot drift. - Only the genuinely workflow-specific pieces remain local: the unchanged recheck, a tail retargeted onto a settled-diff ref (SETTLE_TAIL_RESULT taps an unsurfaced @e6 and can't chain), the in-session timeout failure, and its recovered retry. routine-workflow.ts drops ~110 LOC. - Rendered-output ref guard preserved: @e5 (settled diff) and @e7 (tail) surface only from formatter output; recovery semantics unchanged. 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> |
||
|
|
1f14e224d3 |
refactor(daemon): route perf metrics sampling body through PlatformPlugin facet (#1191)
* refactor(daemon): route perf metrics sampling body through PlatformPlugin facet Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): cover the shipped perf sampler dispatch path via the facet Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * ci: re-trigger checks (flaky settle-observation integration test) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): pin perf sampler selection to the facet tag, not the platform 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> |
||
|
|
ae74c51abd |
chore: add agent-efficiency regression guards (#1174)
* chore: ratchet architecture dependency graph Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: ratchet agent-facing output economy Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * feat: derive command navigation explanations Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: keep efficiency checks fallow-clean Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(layering): enforce back-edge ceiling monotonicity and cover root src files Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * ci(layering): pin back-edge-ceiling ratchet to PR merge-base Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(output-economy): baseline-independent actionability floors, policy-derived error, like-for-like screenshot surfaces Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(explain): resolve true CLI aliases, canonical usage, and derived owners Surface true CLI aliases from parser normalization (long-press, metrics, tap, launch, relaunch) distinct from catalog keys, preserving implied-flag semantics (relaunch => open --relaunch). Extract the canonical single-line usage builder to src/utils/cli-usage.ts so schemas without usageOverride include positionals and flags. Replace guessed handler paths with a completeness-checked daemon-route owner map keyed by the closed DaemonCommandRoute union, fixing silently-dropped non-kebab routes (reactNative, recordTrace) and generic dispatch. Add table-driven coverage for aliases, synthesized usage, split-family/route-variant/dispatch owners, structured output, and explain:command CLI exit/stdout/stderr. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: enforce exact ratchets and compact command explain Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: colocate command ownership metadata Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: bind daemon owners to production routes Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: preserve generic dispatch bundling Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: enforce monotonic output budgets 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> |
||
|
|
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 |
||
|
|
b3b0f590b3 |
fix: add curated suggestions for open-url and close-session (#1175)
Last night's help-conformance benchmark showed agents guessing open-url and close-session with no "did you mean" hint (the Levenshtein fallback doesn't cover either token — both exceed the edit-distance threshold for their length). Add curated entries pointing them at `open <url>` and `close`. |