Commit 6efe544
authored
fix: unify divergence screen capture with snapshot's full-window scope (#1265)
* fix: unify divergence screen capture with snapshot's full-window scope
Route captureDivergenceObservation through captureSnapshotData — the same
function the snapshot command itself builds its capture with (Android's
snapshot-helper full-window route with its graceful app-scoped fallback,
iOS's bounded system-modal probe path, macOS/Linux surface-scoped branches)
— instead of a parallel hand-rolled dispatchCommand call. The chrome filter
and meaningful-target filter stay layered on top as filters over that full
capture, never as a scoping.
Amends ADR-0012 decision 4 to state the invariant: an agent must never see a
healthier `screen` in a divergence report than a plain `snapshot` would show
it, so a separate-window system overlay (volume dialog, quick-settings
shade, permission dialog) must survive into `screen.refs` exactly as
`snapshot` would present it.
Also fixes the synthetic `volume_dialog_slider` id in
snapshot-chrome-android-statusbar.test.ts to the real, live-verified
`volume_new_ringer_active_icon_container` id and rewords the test comment to
read as a filter-logic unit test rather than a live-capture-path claim, and
adds unit coverage for the invariant itself.
Fixes #1264
* fix: rank divergence screen.refs within the cap so overlays are not buried
The #1264 root cause is cap burial, not capture scope: buildReplayDivergenceScreenRefs
sliced candidates in document order, so a fully-captured separate-window
overlay (volume dialog, QS shade, permission dialog) that enumerates after
the app window's ~77 nodes lands past position 20 and is truncated away —
the report shows a healthy-looking app under a covering overlay it cannot see
(archived evidence: screen.truncated: true, zero volume refs).
- Rank within the cap instead of document-order slicing: foreign-window
(non-app-bundleId) hittable nodes — the dismiss targets for whatever covers
the app — are promoted ahead of app content, otherwise stable (document
order preserved within each tier; equal-priority app nodes never reshuffled).
The 20-cap is a byte bound, not a first-20-in-tree-order policy.
- Occlusion fallback: when a system overlay mass-covers the app (every app
node annotated interactionBlocked: 'covered'), surface those covered nodes
rather than emitting an empty screen.refs — a report whose capture holds
meaningful nodes but whose refs is empty is broken by construction.
- repairHint/suggestions consume the full captured node list, not the capped
refs slice, so hint routing is unaffected; only screen.refs selection changes.
Detection keys off node.bundleId (Android-only, from the a11y package); iOS/macOS
leave per-node bundleId undefined, so ranking degrades to document order there
(safe — those platforms surface modals via the probe path, not by cap-competing).
Guarded on a known appBundleId so a sessionless capture never reorders.
Tests: replaces the small-fixture #1264 test (which the overlay fit inside the
cap regardless of order, so it did not prove the invariant) with a realistic
full fixture (24 app controls + overlay dismiss-target captured LAST) that
fails on document-order slicing and passes with ranking; plus occlusion tests
(mass-covered app -> overlay surfaces, refs non-empty; bare-scrim fallback ->
covered app nodes surfaced, refs non-empty). Both were verified to fail before
the fix. ADR-0012 decision 4 amendment reworded to cover ref-selection ranking
and the occlusion guarantee, not only capture scope.
Refs #1264
* fix: route divergence capture through captureSnapshot wrapper + clean flags policy
Completes the #1264 capture unification. The prior round routed
captureDivergenceObservation through captureSnapshotData (the inner single-shot
capture), but plain `snapshot`'s backend calls the HIGHER captureSnapshot
wrapper, which owns Android freshness + post-action retry. A divergence could
therefore consume the first stale/app-scoped dump while a plain `snapshot`
retries to the fresh full-window tree — a divergence staler/narrower than
`snapshot`, violating the invariant.
- Route the divergence capture through the same `captureSnapshot` wrapper as
plain snapshot, so it inherits freshness/post-action retry parity. No fork:
the wrapper's params (device, session, flags, logPath) are all suppliable
from the divergence path.
- Build the divergence capture's flags from a clean, fixed policy
(`divergenceCaptureFlags`: full-window, non-raw, default depth) instead of
spreading the failed action's flags — so a failed `snapshot --raw`/scoped/`-d`
action can no longer narrow the diagnostic tree. Only the interactive-only
policy is carried (extracted as a helper so captureDivergenceObservation
stays within complexity budget).
Tests: a freshness-retry regression (session carries an active Android
freshness marker; capture-1 is a stale near-empty dump that trips sharp-drop,
capture-2 holds the overlay — asserts the divergence uses the retried fresh
tree and dispatched twice), and a clean-flags regression (a failed
raw/scoped/depth action — asserts the snapshot dispatch context drops
snapshotRaw/scope/depth while still applying interactive-only). Both verified
to fail on the pre-fix code. ADR-0012 decision 4 amendment updated to state the
same-wrapper (freshness parity) and clean-flags guarantees.
Live overlay acceptance remains a maintainer device step (env down): unit
fixtures prove ref SELECTION after nodes are supplied, not that the Android
helper returns the separate-window overlay at divergence time.
Refs #1264
* test: stub the freshness-retry sleep so the capture-parity test doesn't real-wait
The #1264 capture-parity regression exercised the real Android sharp-drop
retry, which awaited the real ~250 ms `sleep` delay — repo guidance forbids
real-time waits in unit tests. Mock `sleep` (the delay the retry path in
snapshot-capture.ts awaits) to a no-op at the module level, so the retry
BRANCH still executes (loop runs, retries, re-captures) without a wall-clock
wait. The test still proves the branch: two on-device dispatches and use of
the retried fresh tree (overlay present). Verified it still fails on the
pre-fix single-shot path (1 dispatch) with the delay stubbed, so the stub does
not make it vacuous. No production change; the delay stub needs no DI seam
since `sleep` is a plain module export.
Refs #1264
* fix: reconcile divergence ref selection with #1257 ADR-0014 partial ref frame
Rebase reconciliation. #1257 (ADR-0014 session ref-frame lifetime) landed on
main and changed captureDivergenceObservation to activate a PARTIAL ref frame
(markSessionPartialRefsIssued) authorizing exactly the divergence screen's
emitted refs — computing that "digestBodies" set with its own document-order,
non-covered-only filter. My #1264 change made buildReplayDivergenceScreenRefs
emit a DIFFERENT set (ranked, occlusion-fallback, meaningful-filtered), so the
authorized frame would no longer match the shown screen: in the mass-covered
fallback the screen surfaces covered refs that #1257's non-covered-only frame
filter excluded, leaving the agent a ref the screen advertised but the frame
rejects.
Extract selectDivergenceScreenRefNodes as the single source of truth for which
nodes screen.refs publishes and in what order. Both the rendered digest
(buildReplayDivergenceScreenRefs) and the partial-frame authorization
(captureDivergenceObservation -> markSessionPartialRefsIssued) derive from it,
so the frame authorizes exactly the emitted set — preserving BOTH #1257's
ADR-0014 intent and #1264's ranking/occlusion intent. Also refresh the
captureDivergenceObservation doc to the partial-frame sequence.
Test: assert the partial ref frame scope (session.refFrameScope) equals the
emitted screen.refs set in the mass-covered fallback (covered refs included) —
verified to fail on #1257's original non-covered-only digestBodies.
Refs #1264 #12571 parent 54977f3 commit 6efe544
4 files changed
Lines changed: 783 additions & 61 deletions
File tree
- docs/adr
- src
- core/__tests__
- daemon/handlers
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
15 | 22 | | |
16 | 23 | | |
17 | 24 | | |
| |||
438 | 445 | | |
439 | 446 | | |
440 | 447 | | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
441 | 484 | | |
442 | 485 | | |
443 | 486 | | |
| |||
Lines changed: 17 additions & 12 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
104 | 104 | | |
105 | 105 | | |
106 | 106 | | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | | - | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
115 | 117 | | |
116 | 118 | | |
117 | 119 | | |
| |||
125 | 127 | | |
126 | 128 | | |
127 | 129 | | |
128 | | - | |
129 | | - | |
| 130 | + | |
| 131 | + | |
130 | 132 | | |
131 | 133 | | |
132 | 134 | | |
133 | 135 | | |
134 | 136 | | |
135 | 137 | | |
136 | 138 | | |
137 | | - | |
| 139 | + | |
| 140 | + | |
138 | 141 | | |
139 | 142 | | |
140 | 143 | | |
141 | 144 | | |
142 | 145 | | |
143 | | - | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
144 | 149 | | |
145 | 150 | | |
146 | 151 | | |
| |||
0 commit comments