diff --git a/docs/adr/0004-ios-snapshot-backend-strategy.md b/docs/adr/0004-ios-snapshot-backend-strategy.md index 5e8fe6e66a..dc46f6db6c 100644 --- a/docs/adr/0004-ios-snapshot-backend-strategy.md +++ b/docs/adr/0004-ios-snapshot-backend-strategy.md @@ -102,16 +102,16 @@ those runtimes; it does not imply that Swift and TypeScript share an implementat The macOS XCTest runner is the desktop-surface exception: its already-presented nodes bypass the iOS presentation engine and continue through neutral snapshot assembly. -The same split now holds for the three remaining Wave 4 policies tracked by #1983, so -`src/snapshot/` is the host-side owner of snapshot policy generally rather than of presentation -alone: +The same split now holds for the three remaining Wave 4 policies tracked by #1983, so the host-side +facet in `@agent-device/capture-kit` owns snapshot policy generally rather than presentation alone: - **Freshness recovery.** The freshness window, the Android staleness classification and its - thresholds, and the retry loop live in `src/snapshot/snapshot-freshness/`. The loop is - parameterized by a classifier and a retry schedule, so "how long may a backend lag behind a real - transition" is a policy input rather than a constant the loop owns. The schedule is stated as a - duration budget; the loop derives the deadline from the window's `markedAt` itself, so the - budget is always spent from the action and a caller has no absolute instant it could get wrong. + thresholds, and the retry loop live in `packages/capture-kit/src/snapshot/snapshot-freshness/`. + The loop is parameterized by a classifier and a retry schedule, so "how long may a backend lag + behind a real transition" is a policy input rather than a constant the loop owns. The schedule is + stated as a duration budget; the loop derives the deadline from the window's `markedAt` itself, so + the budget is always spent from the action and a caller has no absolute instant it could get + wrong. `src/daemon/session-snapshot-freshness.ts` keeps only what needs a session: reading and retiring the window on store-owned `SessionState`, and choosing the comparison baseline from snapshot lineage. It remains the declared R7 owner of `androidSnapshotFreshness`. @@ -125,8 +125,8 @@ alone: reclassifying. No message shape is consulted anywhere on that path, so rewording helper or wrapper prose cannot move the reason, and prose that merely reads like a timeout does not become one — both directions are asserted end to end against the real producer. - `src/snapshot/snapshot-timeout-policy.ts` reads the reason; the human-facing hint is derived - from it rather than decided alongside it. + `packages/capture-kit/src/snapshot/snapshot-timeout-policy.ts` reads the reason; the + human-facing hint is derived from it rather than decided alongside it. The published `details.androidSnapshotTimeoutScreenshot` payload is vocabulary in `@agent-device/contracts/snapshot-timeout-evidence`, a union whose arms encode which claims can @@ -136,13 +136,15 @@ alone: The daemon keeps the ordering that genuinely needs it: resolving a bound screenshot runtime, writing the artifact, annotating it from the stored observation, and emitting the diagnostics. - **Screenshot-overlay policy.** Which Android nodes earn an overlay ref, and what rectangle an - overlay for one of them covers, live in `src/snapshot/screenshot-overlay/`. The daemon keeps - approved artifact and ref assembly only: ranking, projection to screenshot pixels, drawing, and - PNG IO. - -`scripts/layering/snapshot-presentation-boundary.test.ts` enforces the direction for the whole -facet: nothing under `src/snapshot/` may import `src/daemon/`. It carries a positive control, -because a filter that stopped matching would look identical to a boundary being obeyed. + overlay for one of them covers, live in `packages/capture-kit/src/screenshot-overlay*.ts`. The + daemon keeps approved artifact and ref assembly only: ranking, projection to screenshot pixels, + drawing, and PNG IO. + +`scripts/layering/snapshot-presentation-boundary.test.ts` enforces the direction, but only across +the roots `snapshot-policy` declares in `scripts/layering/architecture-ownership.ts`: nothing in +that snapshot tree may import `src/daemon/`, while the overlay modules beside the tree sit outside +those roots and are outside that gate. It carries a positive control, because a filter that stopped +matching would look identical to a boundary being obeyed. The residual call sites #1983 also named are audited and deliberately left in place. `src/daemon/direct-ios-selector.ts` carries no presentation policy: `isLocalIosRunnerSession` and diff --git a/docs/adr/0005-ios-runner-interaction-lifecycle.md b/docs/adr/0005-ios-runner-interaction-lifecycle.md index 888af655c8..5c539144ce 100644 --- a/docs/adr/0005-ios-runner-interaction-lifecycle.md +++ b/docs/adr/0005-ios-runner-interaction-lifecycle.md @@ -65,12 +65,27 @@ therefore drained), and left intact by a transport failure or an unstamped recov healthy `ok` is never read as proof of drain because a private-AX snapshot can be served while an abandoned tree crawl still grinds on the XCTest main thread (#2552). +What a `RUNNER_BUSY` refusal then costs the caller turns on the daemon's own per-command read-only +trait, not on any runner-side classification of the command: `RUNNER_COMMAND_TRAITS` in +`packages/platform-apple/src/runner/runner-command-traits.ts`, read through +`isReadOnlyRunnerCommand` where `runAppleRunnerCommand` decides whether to wrap the send in a resend +loop. The asymmetry is shipped, and asserted in `runner-command-busy-resend.test.ts`: a read-only +command treats the refusal as *not yet*, waiting the drain out on the busy-specific budget sized to +outlast it and resending until the runner answers or the window ends; a mutating command treats the +same refusal as *not mine to send again* — one attempt, and the refusal reaches the caller unwrapped +with nothing replayed. Neither path pays a status probe, because a structured reply already answered +"did my command run?" (the bypass in `runner-lifecycle.ts`). The same trait already decides the +startup-preflight skip above, so it is the single place a change to that classification lands. + Close that would retain a runner for reuse first stops it when that occupancy is set, awaiting the -stop so the lease is released before the next request, because a runner still finishing -watchdog-abandoned work refuses every command until it drains or escalates to `RUNNER_WEDGED`; pooling -it back to the next `open` hands the same stalled runner to the caller and `close` recovers nothing. -Killing the process is the only way to abort uncancellable XCTest work. This is the `RUNNER_WEDGED` -restart from #1105 applied at the close boundary rather than after the wedge threshold elapses. +stop so the lease is released before the next request. A runner still finishing watchdog-abandoned +work does refuse every command sent to it until it drains or escalates to `RUNNER_WEDGED`, and that +fact is stated here for the close-and-pooling decision only: the read-only resend above is one live +caller waiting the same refusal out on its own budget, and `close` does not inherit that wait on the +pool's behalf. Pooling the stalled runner back hands it to the next `open` and `close` recovers +nothing. Killing the process is the only way to abort uncancellable XCTest work. This is the +`RUNNER_WEDGED` restart from #1105 applied at the close boundary rather than after the wedge +threshold elapses. When XCTest reports a root accessibility snapshot failure such as `kAXErrorIllegalArgument`, the runner treats the cached app target as suspect. Interactive snapshots fail closed to a truncated diff --git a/docs/adr/0026-scroll-clip-authority.md b/docs/adr/0026-scroll-clip-authority.md index c4c0bc1d59..f265b4dab1 100644 --- a/docs/adr/0026-scroll-clip-authority.md +++ b/docs/adr/0026-scroll-clip-authority.md @@ -2,8 +2,12 @@ ## Status -Proposed (2026-09-22). Refines ADR 0004's fact/interpretation boundary for scroll geometry; does not -supersede it. Implementation contract: #2754. +Accepted (2026-09-22). The ownership decision is shipped: #2758 reads an indicator's owner off the +parent edge, climbing only same-frame ancestors — the tolerance the #2754 step 3 survey justified on +real captures — and #2759 pairs the two eligibility lists and settles `ScrollArea` for macOS. Two +#2754 steps stay open work rather than part of this decision: the runner-stage differential arm, and +the typed ejection disposition the "every removed source" row below names. Refines ADR 0004's +fact/interpretation boundary for scroll geometry; does not supersede it. An interactive iOS snapshot of a list dropped every row after a row holding selectable text. A `UITextView` is a UIScrollView and XCTest publishes its scroll indicator inside the text, so the @@ -22,7 +26,7 @@ walking up past the indicator's parent, and that guess decided which nodes exist | Removing a node **because a scroll band hides it** | Requires evidence: the clip fold's result, or a band whose owner is the indicator's parent. A guess about ownership may not do it | | Removing a node by semantic delegation — a collapsed row, a duplicate label, a wrapper's content | Out of scope here. That is compaction, and it keeps its own authority | | Weaker evidence than a parent edge | Directional hints (`hiddenContentAbove` / `hiddenContentBelow`) and nothing that changes membership | -| Every removed source | Ends with a typed disposition. Internal evidence, never wire vocabulary | +| Every removed source | Must end with a typed disposition — the open #2754 step 5 contract, not shipped code. Internal evidence, never wire vocabulary | ## Contracts @@ -47,8 +51,12 @@ neither eligible set contains it, and the iOS runner never emits it — it origi `AXScrollArea` mapping. That helper's trees do reach these rules, through `snapshot-desktop-surface.ts` → `ios-snapshot-runtime.ts` → `publishIosSnapshot`, so on that surface a `ScrollArea` host can be dropped by eligibility while its children re-parent past it, which leaves a parent-edge lookup with no owner. -Reusing this rule for macOS therefore needs a `ScrollArea` eligibility decision made there, not carried -over from the iOS claim above. +Reusing this rule for macOS therefore needed a `ScrollArea` eligibility decision made there, not +carried over from the iOS claim above, and #2759 made it with cases rather than by admitting the +type for iOS's sake: `scrollarea` stays out of both eligible sets, a `ScrollArea` carrying content +survives eligibility on those trees and owns its band, and a label-less one is dropped, leaving its +indicator with no owner to band — which under-clips (safe) instead of mis-clipping the enclosing +list (`eligibility-parity.test.ts`). **Why the band exists at all.** XCTest reports a scroll view's frame spanning the bars and the safe area, not the visible track. In the pinned Settings tree the `CollectionView` frame is the whole screen, @@ -56,16 +64,17 @@ area, not the visible track. In the pinned Settings tree the `CollectionView` fr cannot express visibility, and deleting indicator clipping would return content that is scrolled under the chrome. The band is necessary; only *who owns it* was ever in question. -**Visibility ejection needs evidence, and every ejection is recorded.** Most of the engine's 23 removal -sites delegate semantics or drop decoration — a collapsed row, a duplicate label, a wrapper's content, a -system indicator — and this ADR does not touch their authority. It governs the four visibility-driven -sites among them, the scroll band's one and the keyboard band's three, alongside the clip fold's own -inclusion decision in `geometry-policy.ts`. Of those, the scroll band is the one a weak signal can reach, -and directional hints are its safe outlet when nothing is the indicator's parent. Every source index ends -either presented, with its representatives, or removed with a typed reason, which makes the ledger complete -by construction rather than gated. The existing `presentedIndexesBySourceIndex` in -`ios-snapshot-engine/semantic-index.ts` is the shape to extend; a second parallel ledger would be a second -source of truth. +**Visibility ejection needs evidence, and every ejection must be recorded.** Most of the engine's 23 +removal sites delegate semantics or drop decoration — a collapsed row, a duplicate label, a +wrapper's content, a system indicator — and this ADR does not touch their authority. It governs the +four visibility-driven sites among them, the scroll band's one and the keyboard band's three, +alongside the clip fold's own inclusion decision in `geometry-policy.ts`. Of those, the scroll band +is the one a weak signal can reach, and directional hints are its safe outlet when nothing is the +indicator's parent. Every source index must end either presented, with its representatives, or +removed with a typed reason, which makes the ledger complete by construction rather than gated; +#2754 step 5 still owes that reason, and `presentedIndexesBySourceIndex` in +`ios-snapshot-engine/semantic-index.ts` records representatives and no removal reason today. That +ledger is the shape to extend; a second parallel ledger would be a second source of truth. ## Refuted alternatives @@ -118,10 +127,12 @@ source of truth. upward. So `findScrollIndicatorContainer` returns null when the node is itself a scroll type; `runner-presentation.test.ts` pins a scroll host labelled as an indicator leaving its parent list's band intact. -- **macOS is already a second consumer of these rules.** Desktop capture runs the engine through - `snapshot-desktop-surface.ts` → `ios-snapshot-runtime.ts` → `publishIosSnapshot`, and its trees carry - `ScrollArea`, which neither eligible set admits. The parent-edge rule needs its own decision there - before it is reused, not a carry-over from the iOS claim. +- **macOS is already a second consumer of these rules, and #2759 settled its `ScrollArea` + question.** Desktop capture runs the engine through `snapshot-desktop-surface.ts` → + `ios-snapshot-runtime.ts` → `publishIosSnapshot`, and its trees carry `ScrollArea`, which neither + eligible set admits: a content-bearing one survives eligibility and owns its band; a label-less + one is dropped and leaves its indicator with no owner, under-clipping instead of mis-clipping the + list. `eligibility-parity.test.ts` pins both outcomes and the TS/Swift eligibility pair. - **Ejection inventory precedes any API change.** 23 suppression sites across ten rules, and two of them (`scroll`, `transitions`) already rewrite rects on other nodes in the same pass that ejects — the combination the proposed API split claimed to make impossible. diff --git a/docs/adr/README.md b/docs/adr/README.md index 51e2a8af42..c68b299dc7 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -27,7 +27,7 @@ | [0023 End-State Entry-to-Platform Hop Trace](0023-end-state-hop-trace.md) | the hop counting definition, the ordered `press`/Android and dual-arm `snapshot`/iOS chains, per-hop roles, and the deletion test behind the numbers ADR 0019 and ADR 0022 quote | | [0024 Screen Recording — One Coordinator, Two Facts (Proposed)](0024-screen-recording-provable-signal.md) | `record start`/`stop` lifecycle, the export-exists / recorder-stopped split, backend stop-and-collect contract, recovery through the coordinator, the ADR 0019 §5 amendment and the failed-finish cleanup fix | | [0025 Foldable Apple Panels — Capture the Lit Panel](0025-foldable-apple-panels.md) | iPhone Duo outer/inner panels, CoreDevice display info as the only screen authority, why `simctl`'s implicit display default captures a black panel, derived (never asserted) pose, and why no pose-control command exists | -| [0026 Scroll Clip Authority — Ownership Is the Parent Edge (Proposed)](0026-scroll-clip-authority.md) | iOS scroll indicators and the visible band, why ownership is read from the parent edge instead of an ancestor walk, what may remove a node from an interactive snapshot, and ejection dispositions | +| [0026 Scroll Clip Authority — Ownership Is the Parent Edge](0026-scroll-clip-authority.md) | iOS scroll indicators and the visible band, why ownership is read from the parent edge instead of an ancestor walk, what may remove a node from an interactive snapshot, and ejection dispositions | | [0027 Descriptor Root Size vs Eager-Closure Budget (Proposed)](0027-descriptor-root-vs-eager-closure-budget.md) | splitting `packages/command-registry/src/registry.ts`, the ADR-0019 eager-closure module-count budget, and why a byte-neutral hub split is currently unshippable | | [0028 Capability-Family Cell Vocabulary — One Runtime Source (Proposed)](0028-capability-family-cell-vocabulary.md) | adding a capability operation family, `UnavailablePlatformRuntimeFacts` / `UNAVAILABLE_CELLS`, `INTERACTOR_OPERATIONS`, and why an eight-package fan-out recurs |