Commit 1235216
authored
fix(ios): double-check off-screen click refusals against a direct element read (#1566)
* fix(ios): double-check off-screen click refusals against a direct element read
#1542: after an AX-free scroll on iOS, the off-screen interaction guard can
refuse a click even though the target is genuinely on-screen, because it
trusts a scroll-container ancestor's rect from the bulk accessibility tree,
which a keyboard-dismiss content-offset correction can leave stale/corrupted
while the target's own rect is already correct.
When the guard is about to refuse on iOS, it now takes a single fresh,
tree-independent XCUITest read of the target element (querySelector) and
trusts that read's live `hittable` + rect-vs-root-viewport signal instead,
if it positively confirms on-screen. Any failure to unambiguously re-resolve
the element (no id/label, not found, ambiguous, transport error) fails
closed exactly as before. Genuinely off-screen targets, and every other
platform, are unchanged: the backend method is gated to local (non-provider)
iOS sessions only, and only ever runs on the about-to-fail path.
The decision itself is a pure function (decideOffscreenRefusalDoubleCheck in
mobile-snapshot-semantics.ts) with counterfactual-proven tests: hardcoding it
to always trust the bulk verdict turns the rescue test red, and hardcoding
it to always trust the direct read (including on "unavailable") turns the
fail-closed/genuine-refusal test red.
Live-validated on a fresh-boot iOS simulator: checkout-form.ad 2/2 passes
(previously failing at step 11), gesture-lab.ad 2/2 (regression), and the
Android checkout-form/gesture-lab suite passes unchanged, proving no
cross-platform behavior change.
* fix(ios): tap the live rect after a rescued offscreen refusal; collapse the double-check to one backend hook
Review blockers 1+2 (interleaved by design — the soundness fix is expressed
through the collapsed hook's contract):
1. SOUNDNESS: a rescued refusal now returns the node PATCHED WITH THE LIVE
RECT the backend confirmed, and every downstream use (tap point, response)
reads from that returned node — never the original. In the frozen-tree
manifestation (the whole bulk tree pinned at pre-gesture values), the
original rect can be stale even when the rescue verdict is correct;
tapping it would have silently landed at the wrong coordinate. New
regression: offscreen-double-check.test.ts's frozen-tree case, with a
counterfactual (revert to computing the point from the pre-guard node)
proven red then reverted.
2. SURFACE: collapsed to ONE optional backend hook,
`confirmOffscreenTargetVisible?(context, node, rootViewport): Promise<Rect
| null>` — conceptually a boolean, but returns the live rect so item 1's
fix has something to act on. Deleted decideOffscreenRefusalDoubleCheck,
the OffscreenRefusalDoubleCheckSignal/Reading ADT, and resolution.ts's
dual-signal reconciliation shell: the bulk side was hardcoded 'off-screen'
at the only call site, so the two-signal model was dead weight. The shared
guard is now: bulk-off-screen -> ask the hook -> a live rect proceeds
(patched), anything else (including no hook) throws exactly as before.
The pure geometry boundary that decision reduces to (`isConfirmedOnScreenProbe`
in mobile-snapshot-semantics.ts, replacing the deleted ADT) is unit-tested
with two counterfactuals: ignoring `hittable` and ignoring the viewport
containment check each turn a test red (proved, then reverted).
`throwIfOffscreenInteractionTarget` is now exported (ADR 0011 registry
honesty, see the contracts commit) and directly unit-tested in
resolution.test.ts, mirroring the existing tryResolveRefNode pattern.
* refactor(ios): direct-ios-selector.ts back to pure gate/parse; reuse queryDirectIosSelector
Review blocker 3 (BOUNDARIES):
- direct-ios-selector.ts no longer does any runner I/O — it's back to pure
gate/parse (readSimpleIosSelectorTarget, deriveDirectIosNodeSelector,
isDirectIosSelectorFallbackError) plus the ONE shared eligibility
predicate, isLocalIosRunnerSession(session, { skipPendingPostGestureStabilization
}). Both the direct-selector tap fast path and the new offscreen
double-check probe call this same function; the one behavioral difference
between them (the tap fast path skips a session with a pending
postGestureStabilization, the double-check does not) is now an explicit
parameter instead of two separately-written gates.
- The probe I/O moved to a new sibling, src/daemon/offscreen-target-probe.ts,
which reuses selector-runtime.ts's `queryDirectIosSelector` (now exported
and decoupled from SelectorRuntimeParams — it takes a session + a bare
{key, value} selector + AppleRunnerRequestOptions) rather than opening a
second querySelector client. Node extraction (`readDirectIosSelectorNode`,
the one `as SnapshotNode` cast) stays singular, inside selector-runtime.ts.
- interaction-runtime.ts wires confirmOffscreenTargetVisible only when
isLocalIosRunnerSession(session, { skipPendingPostGestureStabilization:
false }) — deliberately NOT skipping a pending post-gesture stabilization,
since that is exactly the window the double-check exists to cover.
* docs(contracts): name the iOS offscreen rescue hook as part of the guarantee matrix
Review blocker 4 (GUARANTEE HONESTY): the shared offscreen cell
(RUNTIME_TREE_SHARED_GUARANTEES.offscreen, used by runtime-selector and
runtime-ref) and the native-ref path's offscreen cell still named
isNodeVisibleOnScreen as sole enforcement after #1542's double-check landed —
that understates what actually enforces the guarantee now.
Both cells' `via` now point at throwIfOffscreenInteractionTarget (exported
from resolution.ts in the prior commit for exactly this), the real
end-to-end enforcement point: isNodeVisibleOnScreen is the bulk-tree
decision it starts from, and on iOS a would-be refusal can still be
confirmed via the optional AgentDeviceBackend.confirmOffscreenTargetVisible
hook before erroring. The cell's comment states the rescue-only, fail-closed
shape explicitly per ADR 0011's matrix rules — this does not weaken the
cell, it extends its description to match reality.
iOS rescue policy stays OUT of resolution.ts's shared docstrings (the
"spine"): this registry file is where per-path enforcement detail belongs,
and the optional-method wiring in interaction-runtime.ts remains the only
cross-platform touch.
The registry's own gate test (interaction-guarantees.test.ts) still passes:
every `via` resolves to a real exported symbol.
* test(ios): move #1542 offscreen double-check tests out of interaction.test.ts
Review blocker 5 (TEST HOMES): AGENTS.md forbids adding to
daemon/handlers/__tests__/interaction.test.ts (it predates the
test-mirrors-source-topology rule and shrinks opportunistically). Reverts
the 172 lines added there in the original PR version; interaction.test.ts is
back to its pre-#1542 baseline (81 tests, unchanged).
The same assertions now live in their proper homes (see the prior three
commits for the sources they cover):
- pure decision pin: src/utils/__tests__/mobile-snapshot-semantics.test.ts
(isConfirmedOnScreenProbe, with the two counterfactuals)
- direct-guard pin: src/commands/interaction/runtime/resolution.test.ts
(throwIfOffscreenInteractionTarget, mirroring tryResolveRefNode)
- probe unit tests: src/daemon/__tests__/selector-runtime.test.ts
(queryDirectIosSelector) and src/daemon/__tests__/direct-ios-selector.test.ts
(isLocalIosRunnerSession, deriveDirectIosNodeSelector)
- probe integration: src/daemon/__tests__/offscreen-target-probe.test.ts
(confirmIosOffscreenTargetVisible, mocked runner)
- end-to-end rescue/refuse, including the frozen-tree live-geometry
regression + its counterfactual: new sibling
src/commands/interaction/runtime/offscreen-double-check.test.ts (next to
resolution.ts, using the same createInteractionDevice harness
resolution.test.ts already uses)
* style: oxfmt formatting for resolution.test.ts1 parent f8617a2 commit 1235216
15 files changed
Lines changed: 903 additions & 47 deletions
File tree
- packages/contracts/src
- src
- commands/interaction/runtime
- __tests__/test-utils
- daemon
- __tests__
- handlers
- snapshot
- utils/__tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
147 | 147 | | |
148 | 148 | | |
149 | 149 | | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
150 | 159 | | |
151 | 160 | | |
152 | | - | |
| 161 | + | |
153 | 162 | | |
154 | 163 | | |
155 | 164 | | |
| |||
291 | 300 | | |
292 | 301 | | |
293 | 302 | | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
294 | 310 | | |
295 | 311 | | |
296 | | - | |
| 312 | + | |
297 | 313 | | |
298 | 314 | | |
299 | 315 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
431 | 431 | | |
432 | 432 | | |
433 | 433 | | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
434 | 465 | | |
435 | 466 | | |
436 | 467 | | |
| |||
Lines changed: 7 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
298 | 298 | | |
299 | 299 | | |
300 | 300 | | |
| 301 | + | |
301 | 302 | | |
302 | 303 | | |
303 | 304 | | |
| |||
327 | 328 | | |
328 | 329 | | |
329 | 330 | | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
330 | 337 | | |
331 | 338 | | |
332 | 339 | | |
| |||
Lines changed: 149 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| |||
614 | 614 | | |
615 | 615 | | |
616 | 616 | | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
0 commit comments