diff --git a/src/core/snapshot-chrome.ts b/src/core/snapshot-chrome.ts index 8bec0342d..be48c36e2 100644 --- a/src/core/snapshot-chrome.ts +++ b/src/core/snapshot-chrome.ts @@ -325,6 +325,26 @@ export function collectSettleChromeRefs( return collectSettleChrome(nodes, appBundleId).refs; } +/** + * Refs of iOS keyboard-window chrome ONLY — no Android union, so callers with + * no `appBundleId` in scope can still reuse the real subtree/window-aware + * classification (`collectKeyboardChrome`) instead of a narrower per-node type + * check. Container-descendant walk alone provably misses the "Next keyboard" + * / "Dictate" assistant buttons (siblings of the `[Keyboard]` container, not + * descendants — see `collectKeyboardChrome`'s doc comment), so a caller that + * only excludes nodes whose OWN type is `keyboard` still leaks every key and + * assistant control as "discriminating" evidence. + * + * Used by `src/daemon/interaction-outcome-policy.ts`'s post-gesture + * baseline-distrust discriminating-overlap classification (#1542 defect 2, + * #1563 review): that comparison operates on flat signature entries with no + * ref-selection budget of its own, so it needs the ref set directly rather + * than a node-filtering helper like `withoutSettleChrome`. + */ +export function collectKeyboardChromeRefs(nodes: SnapshotNode[]): ReadonlySet { + return collectKeyboardChrome(nodes).refs; +} + /** * Windows eligible for whole-window chrome classification: nearest `[window]` * ancestor of each `[Keyboard]` container, minus windows hosting editable diff --git a/src/daemon/__tests__/interaction-outcome-policy.test.ts b/src/daemon/__tests__/interaction-outcome-policy.test.ts index a1049455f..cd38b5596 100644 --- a/src/daemon/__tests__/interaction-outcome-policy.test.ts +++ b/src/daemon/__tests__/interaction-outcome-policy.test.ts @@ -3,6 +3,7 @@ import { test } from 'vitest'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { buildInteractionSurfaceSignature, + classifyBaselineSurfaceEvidence, classifyInteractionSurfaceChange, markPendingInteractionOutcome, stripInternalInteractionFlags, @@ -38,6 +39,266 @@ test('classifyInteractionSurfaceChange detects material layout movement', () => assert.equal(classifyInteractionSurfaceChange(before, after), 'changed'); }); +// --------------------------------------------------------------------------- +// classifyBaselineSurfaceEvidence (#1542 defect 2, #1563 review): subset- +// tolerant, three-valued baseline comparison. Live evidence on +// checkout-form.ad showed the pre-gesture baseline (captured by an earlier +// `wait`, a broad query) and the post-gesture quiet signature (captured by +// the click's interactive-only selector resolution) never line up as whole +// arrays even when the target element never moved — the first version of +// this check has to see through that scope drift. +// +// The #1563 review then caught a SECOND failure mode in that first version +// (a plain "any shared entry frozen" boolean): the viewport root +// (Application/Window) is always present and its rect is invariant under any +// gesture, so a broad baseline and a narrow post-gesture capture can share +// ONLY the root even after a real, successful scroll — and the boolean +// predicate called that a match. `classifyBaselineSurfaceEvidence` requires +// at least one DISCRIMINATING shared entry (excluding the viewport root and +// keyboard chrome) before calling it `'unchanged'`; a root-only (or +// no-discriminating-evidence) overlap is `'ambiguous'` instead. +// --------------------------------------------------------------------------- + +test('classifyBaselineSurfaceEvidence reports unchanged for identical signatures', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes); + const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged'); +}); + +test('classifyBaselineSurfaceEvidence is ambiguous when either side is empty', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes); + + assert.equal(classifyBaselineSurfaceEvidence([], baseline), 'ambiguous'); + assert.equal(classifyBaselineSurfaceEvidence(baseline, []), 'ambiguous'); + assert.equal(classifyBaselineSurfaceEvidence([], []), 'ambiguous'); +}); + +test('classifyBaselineSurfaceEvidence reports unchanged through a broader baseline scope when the shared discriminating element is frozen', () => { + // The exact live shape: the baseline came from a broader capture (extra + // "Loading" text node the interactive-only capture never sees), but the + // shared "primary-action" button never moved. + const baseline = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes); + const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500).nodes); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged'); +}); + +test('classifyBaselineSurfaceEvidence reports unchanged through a broader current scope when the shared discriminating element is frozen', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500).nodes); + const current = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged'); +}); + +test('classifyBaselineSurfaceEvidence detects real movement even through a scope difference', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes); + const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 120).nodes); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'changed'); +}); + +test('classifyBaselineSurfaceEvidence is ambiguous when the signatures share no key at all', () => { + const baseline = buildInteractionSurfaceSignature([ + { + ref: 'e1', + index: 0, + type: 'Button', + identifier: 'checkout-only-button', + label: 'Checkout', + rect: { x: 0, y: 0, width: 100, height: 40 }, + }, + ]); + const current = buildInteractionSurfaceSignature([ + { + ref: 'e1', + index: 0, + type: 'Button', + identifier: 'settings-only-button', + label: 'Settings', + rect: { x: 0, y: 0, width: 100, height: 40 }, + }, + ]); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous'); +}); + +test('classifyBaselineSurfaceEvidence tolerates tiny rect drift on the shared discriminating element', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes); + const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500.4).nodes); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged'); +}); + +// --- #1563 review regression: root-only overlap must NOT read as evidence --- + +test('classifyBaselineSurfaceEvidence is ambiguous (NOT unchanged) when a real scroll leaves only the application root shared — the reviewer-caught false-distrust shape', () => { + // baseline = {Application, Pickup@y=500}; current = {Application, + // OtherButton@...} — a genuine, successful scroll replaced every real + // element in view, so the only entry the two signatures still share is the + // always-present, always-identical viewport root. A boolean "any shared + // entry frozen" predicate calls this a baseline match (the root always + // "matches") and would extend the interaction to the 3.5s stale-read + // deadline on zero real evidence — exactly the bug this test pins. + const baseline = buildInteractionSurfaceSignature([ + applicationRootNode(), + { + ref: 'e2', + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-pickup', + label: 'Pickup', + rect: { x: 20, y: 500, width: 200, height: 44 }, + }, + ]); + const current = buildInteractionSurfaceSignature([ + applicationRootNode(), + { + ref: 'e2', + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-delivery', + label: 'Delivery', + rect: { x: 20, y: 120, width: 200, height: 44 }, + }, + ]); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous'); +}); + +test('classifyBaselineSurfaceEvidence is ambiguous when the current capture is the application root alone', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500).nodes); + const current = buildInteractionSurfaceSignature([applicationRootNode()]); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous'); +}); + +test('classifyBaselineSurfaceEvidence excludes the keyboard container from discriminating overlap', () => { + const keyboardNode = { + ref: 'e3', + index: 2, + parentIndex: 0, + type: 'Keyboard', + rect: { x: 0, y: 500, width: 390, height: 300 }, + }; + const baseline = buildInteractionSurfaceSignature([applicationRootNode(), keyboardNode]); + // The keyboard's own container rect never changes; only the app content + // does. A capture sharing just the root and the keyboard container (no + // real content) must not read as a baseline match. + const current = buildInteractionSurfaceSignature([applicationRootNode(), keyboardNode]); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous'); +}); + +// #1563 review, finding 2: a container-only exclusion still misses keyboard +// DESCENDANTS (individual keys) and SIBLINGS (assistant buttons like "Next +// keyboard"/"Dictate", which live outside the container per +// src/core/snapshot-chrome.ts's collectKeyboardChrome doc comment — a +// container-descendant walk alone provably misses them, hence the whole- +// window classification that module reuses here via collectKeyboardChromeRefs). +test('classifyBaselineSurfaceEvidence excludes keyboard DESCENDANTS and window SIBLINGS, not just the container, from discriminating overlap', () => { + const shared = keyboardWindowNodes(); // window + [Keyboard] container + a key + a sibling "Next keyboard" button + const baseline = buildInteractionSurfaceSignature([ + applicationRootNode(), + { + ref: 'e-pickup', + index: 20, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-pickup', + label: 'Pickup', + rect: { x: 20, y: 500, width: 200, height: 44 }, + }, + ...shared, + ]); + // Real content changed (Pickup -> Delivery, a genuine successful scroll); + // the keyboard subtree is identical — a keyboard does not move when app + // content scrolls. + const current = buildInteractionSurfaceSignature([ + applicationRootNode(), + { + ref: 'e-delivery', + index: 20, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-delivery', + label: 'Delivery', + rect: { x: 20, y: 120, width: 200, height: 44 }, + }, + ...shared, + ]); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous'); +}); + +/** + * A keyboard-window subtree: a `[Keyboard]` container plus a SIBLING "Next + * keyboard" assistant button under the same window — matches the shape in + * `src/daemon/__tests__/post-gesture-stabilization-fixtures.ts`'s + * `keyboardWindowNodes` (kept local here rather than imported: this file's + * fixtures are raw node literals consumed directly by + * `buildInteractionSurfaceSignature`, not `SnapshotState`-wrapped like that + * module's). + */ +function keyboardWindowNodes() { + return [ + { + ref: 'e-kb-window', + index: 10, + parentIndex: 0, + type: 'Window', + rect: { x: 0, y: 400, width: 390, height: 444 }, + }, + { + ref: 'e-kb-container', + index: 11, + parentIndex: 10, + type: 'Keyboard', + rect: { x: 0, y: 500, width: 390, height: 300 }, + }, + { + ref: 'e-kb-key-a', + index: 12, + parentIndex: 11, // descendant of the container + type: 'Key', + label: 'A', + rect: { x: 10, y: 520, width: 30, height: 40 }, + }, + { + ref: 'e-kb-next', + index: 13, + parentIndex: 10, // sibling of the container, NOT a descendant + type: 'Button', + label: 'Next keyboard', + rect: { x: 340, y: 520, width: 40, height: 40 }, + }, + ]; +} + +test('classifyBaselineSurfaceEvidence still reports unchanged when the root AND a real discriminating element both match (guards against over-excluding)', () => { + // Root-sharing alone is not disqualifying — it just cannot be the ONLY + // evidence. Once a real, frozen discriminating element is also shared + // (the ordinary "genuinely stuck" case), the verdict must still be + // 'unchanged', not swing to 'ambiguous' just because the root is present. + const snapshotNodes = makeSnapshot('Inbox', 500).nodes; // [Application, primary-action Button] + const baseline = buildInteractionSurfaceSignature(snapshotNodes); + const current = buildInteractionSurfaceSignature(snapshotNodes); + + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged'); +}); + +function applicationRootNode() { + return { + ref: 'e1', + index: 0, + type: 'Application', + label: 'App', + rect: { x: 0, y: 0, width: 390, height: 844 }, + }; +} + test('markPendingInteractionOutcome stores retry state only for explicit retry flags', () => { const session = makeSession(); markPendingInteractionOutcome({ @@ -128,3 +389,25 @@ function makeSnapshot(label: string, y = 100): SnapshotState { backend: 'xctest', }; } + +// A broader-scope variant of makeSnapshot: the same Application + Button +// entries, plus a non-interactive text node an interactive-only capture would +// never return. Models the real shape mismatch between a pre-gesture baseline +// snapshot and a post-gesture interactive-only selector-resolution capture. +function makeSnapshotWithExtraText(label: string, y = 100): SnapshotState { + const base = makeSnapshot(label, y); + return { + ...base, + nodes: [ + ...base.nodes, + { + ref: 'e3', + index: 2, + parentIndex: 0, + type: 'Text', + label: 'Loading', + rect: { x: 20, y: 20, width: 200, height: 20 }, + }, + ], + }; +} diff --git a/src/daemon/__tests__/post-gesture-stabilization-fixtures.ts b/src/daemon/__tests__/post-gesture-stabilization-fixtures.ts new file mode 100644 index 000000000..f93b97e6d --- /dev/null +++ b/src/daemon/__tests__/post-gesture-stabilization-fixtures.ts @@ -0,0 +1,129 @@ +import { ANDROID_EMULATOR, IOS_SIMULATOR } from '../../__tests__/test-utils/device-fixtures.ts'; +import { makeSnapshotState } from '../../__tests__/test-utils/index.ts'; +import type { SessionState } from '../types.ts'; + +/** + * Shared fixtures for post-gesture-stabilization.test.ts (the async capture + * loop) and post-gesture-stabilization-verdict.test.ts (the pure + * verdict/classifier coverage) — split by subject per #1563 review, to stay + * under the repo's 500-line test-file tripwire (AGENTS.md). Not a `.test.ts` + * file, so vitest never tries to run it directly. + */ + +export function pickupSnapshot(y = 500) { + return makeSnapshotState([ + { index: 0, type: 'Application', label: 'App', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-pickup', + label: 'Pickup', + rect: { x: 20, y, width: 200, height: 44 }, + }, + ]); +} + +// Same Application root as pickupSnapshot, but a DIFFERENT real element — +// models a genuine, successful scroll that swapped every real element in +// view, so the only entry shared with a pickupSnapshot baseline is the root. +export function deliverySnapshot(y = 500) { + return makeSnapshotState([ + { index: 0, type: 'Application', label: 'App', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-delivery', + label: 'Delivery', + rect: { x: 20, y, width: 200, height: 44 }, + }, + ]); +} + +// Broader-scope variant: adds a non-interactive text node an interactive-only +// capture would never return, modeling the real pre-gesture-baseline vs +// post-gesture-selector-capture scope mismatch. +export function pickupSnapshotWithExtraText(y = 500) { + const base = pickupSnapshot(y); + return { + ...base, + nodes: [ + ...base.nodes, + { + ref: 'e3', + index: 2, + parentIndex: 0, + type: 'Text', + label: 'Delivery choices', + rect: { x: 20, y: 300, width: 200, height: 20 }, + }, + ], + }; +} + +export function applicationRootNode() { + return { + ref: 'e-root', + index: 0, + type: 'Application', + label: 'App', + rect: { x: 0, y: 0, width: 390, height: 844 }, + }; +} + +/** + * A keyboard-window subtree modeling the #1563 review's second finding: a + * `[Keyboard]` container PLUS a sibling "Next keyboard" assistant button + * under the SAME window — a container-descendant-only walk provably misses + * the sibling (see `collectKeyboardChrome`'s doc comment in + * src/core/snapshot-chrome.ts, the source of truth this fixture's shape is + * drawn from: "a SIBLING subtree holding the 'Next keyboard' and 'Dictate' + * buttons — siblings of the container, so a container-descendant walk alone + * provably misses them"). Neither entry is the container itself, so sharing + * only these between a baseline and a later capture is the exact + * keyboard-descendants-only regression shape. + */ +export function keyboardWindowNodes() { + return [ + { + ref: 'e-kb-window', + index: 10, + parentIndex: 0, + type: 'Window', + rect: { x: 0, y: 400, width: 390, height: 444 }, + }, + { + ref: 'e-kb-container', + index: 11, + parentIndex: 10, + type: 'Keyboard', + rect: { x: 0, y: 500, width: 390, height: 300 }, + }, + { + ref: 'e-kb-key-a', + index: 12, + parentIndex: 11, // descendant of the container + type: 'Key', + label: 'A', + rect: { x: 10, y: 520, width: 30, height: 40 }, + }, + { + ref: 'e-kb-next', + index: 13, + parentIndex: 10, // sibling of the container, NOT a descendant + type: 'Button', + label: 'Next keyboard', + rect: { x: 340, y: 520, width: 40, height: 40 }, + }, + ]; +} + +export function makeSession(platform: 'ios' | 'android' = 'ios'): SessionState { + return { + name: platform, + device: platform === 'android' ? ANDROID_EMULATOR : IOS_SIMULATOR, + createdAt: Date.now(), + actions: [], + }; +} diff --git a/src/daemon/__tests__/post-gesture-stabilization-verdict.test.ts b/src/daemon/__tests__/post-gesture-stabilization-verdict.test.ts new file mode 100644 index 000000000..96a7eb851 --- /dev/null +++ b/src/daemon/__tests__/post-gesture-stabilization-verdict.test.ts @@ -0,0 +1,208 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { buildInteractionSurfaceSignature } from '../interaction-outcome-policy.ts'; +import { decidePostGestureStabilityVerdict } from '../post-gesture-stabilization.ts'; +import { + applicationRootNode, + keyboardWindowNodes, + pickupSnapshot, +} from './post-gesture-stabilization-fixtures.ts'; + +// --------------------------------------------------------------------------- +// #1542 defect 2: baseline-comparison distrust. +// +// After an AX-free synthesized gesture, XCTest's AX tree isn't proactively +// resynced by the synthesized touch, so it can serve a stale-but-internally- +// consistent read: two consecutive polls agree with each other while still +// exactly matching the PRE-gesture tree. `decidePostGestureStabilityVerdict` +// is the pure decision that catches this; the tests below are its exhaustive +// truth table. +// +// Split out of post-gesture-stabilization.test.ts per #1563 review (the pure +// verdict coverage, alongside its own shared fixtures, moved to this sibling +// module so the async-loop test file stays under the repo's 500-line +// tripwire — see post-gesture-stabilization-fixtures.ts). +// --------------------------------------------------------------------------- + +test('decidePostGestureStabilityVerdict trusts immediately when the platform does not need baseline distrust', () => { + const signature = buildInteractionSurfaceSignature(pickupSnapshot().nodes); + + assert.equal( + decidePostGestureStabilityVerdict({ + needsBaselineDistrust: false, + baselineSignature: signature, + quietSignature: signature, + elapsedMs: 0, + distrustCapMs: 3_500, + }), + 'trust', + ); +}); + +test('decidePostGestureStabilityVerdict trusts when there is no usable baseline', () => { + const signature = buildInteractionSurfaceSignature(pickupSnapshot().nodes); + + assert.equal( + decidePostGestureStabilityVerdict({ + needsBaselineDistrust: true, + baselineSignature: undefined, + quietSignature: signature, + elapsedMs: 0, + distrustCapMs: 3_500, + }), + 'trust', + ); + assert.equal( + decidePostGestureStabilityVerdict({ + needsBaselineDistrust: true, + baselineSignature: [], + quietSignature: signature, + elapsedMs: 0, + distrustCapMs: 3_500, + }), + 'trust', + ); +}); + +test('decidePostGestureStabilityVerdict trusts a quiet signature that differs from the baseline', () => { + const baseline = buildInteractionSurfaceSignature(pickupSnapshot(500).nodes); + const moved = buildInteractionSurfaceSignature(pickupSnapshot(120).nodes); + + assert.equal( + decidePostGestureStabilityVerdict({ + needsBaselineDistrust: true, + baselineSignature: baseline, + quietSignature: moved, + elapsedMs: 0, + distrustCapMs: 3_500, + }), + 'trust', + ); +}); + +test('decidePostGestureStabilityVerdict distrusts a quiet signature matching the baseline before the cap', () => { + const signature = buildInteractionSurfaceSignature(pickupSnapshot().nodes); + + assert.equal( + decidePostGestureStabilityVerdict({ + needsBaselineDistrust: true, + baselineSignature: signature, + quietSignature: signature, + elapsedMs: 3_499, + distrustCapMs: 3_500, + }), + 'distrust', + ); +}); + +test('decidePostGestureStabilityVerdict accepts a baseline-matching signature once the cap expires', () => { + const signature = buildInteractionSurfaceSignature(pickupSnapshot().nodes); + + assert.equal( + decidePostGestureStabilityVerdict({ + needsBaselineDistrust: true, + baselineSignature: signature, + quietSignature: signature, + elapsedMs: 3_500, + distrustCapMs: 3_500, + }), + 'accept-stale', + ); + assert.equal( + decidePostGestureStabilityVerdict({ + needsBaselineDistrust: true, + baselineSignature: signature, + quietSignature: signature, + elapsedMs: 9_000, + distrustCapMs: 3_500, + }), + 'accept-stale', + ); +}); + +// --- #1563 review, finding 1: a root-only shared overlap must trust immediately, not tax the cap --- + +test('decidePostGestureStabilityVerdict trusts immediately when a real scroll leaves only the application root shared (no cap tax)', () => { + const baseline = buildInteractionSurfaceSignature([ + applicationRootNode(), + { + ref: 'e2', + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-pickup', + label: 'Pickup', + rect: { x: 20, y: 500, width: 200, height: 44 }, + }, + ]); + const quiet = buildInteractionSurfaceSignature([ + applicationRootNode(), + { + ref: 'e2', + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-delivery', + label: 'Delivery', + rect: { x: 20, y: 120, width: 200, height: 44 }, + }, + ]); + + assert.equal( + decidePostGestureStabilityVerdict({ + needsBaselineDistrust: true, + baselineSignature: baseline, + quietSignature: quiet, + elapsedMs: 0, // first quiet match, well before any cap + distrustCapMs: 3_500, + }), + 'trust', + ); +}); + +// --- #1563 review, finding 2: keyboard DESCENDANTS (not just the container) must not read as evidence --- + +test('decidePostGestureStabilityVerdict trusts immediately when the overlap is only keyboard descendants, not the container', () => { + // Same keyboard subtree in both baseline and quiet (unmoved — a keyboard + // does not move when app content scrolls), but the real content behind it + // changed (Pickup -> Delivery). The shared overlap is Application + the + // keyboard window + the keyboard container + a key + the "Next keyboard" + // assistant button — none of which is real, discriminating evidence. + const baseline = buildInteractionSurfaceSignature([ + applicationRootNode(), + { + ref: 'e2', + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-pickup', + label: 'Pickup', + rect: { x: 20, y: 500, width: 200, height: 44 }, + }, + ...keyboardWindowNodes(), + ]); + const quiet = buildInteractionSurfaceSignature([ + applicationRootNode(), + { + ref: 'e2', + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-delivery', + label: 'Delivery', + rect: { x: 20, y: 120, width: 200, height: 44 }, + }, + ...keyboardWindowNodes(), // identical: the keyboard itself never moves + ]); + + assert.equal( + decidePostGestureStabilityVerdict({ + needsBaselineDistrust: true, + baselineSignature: baseline, + quietSignature: quiet, + elapsedMs: 0, // first quiet match, well before any cap + distrustCapMs: 3_500, + }), + 'trust', + ); +}); diff --git a/src/daemon/__tests__/post-gesture-stabilization.test.ts b/src/daemon/__tests__/post-gesture-stabilization.test.ts index b2c548412..2e7147cc3 100644 --- a/src/daemon/__tests__/post-gesture-stabilization.test.ts +++ b/src/daemon/__tests__/post-gesture-stabilization.test.ts @@ -1,8 +1,23 @@ import assert from 'node:assert/strict'; import { afterEach, test, vi } from 'vitest'; -import { ANDROID_EMULATOR, IOS_SIMULATOR } from '../../__tests__/test-utils/device-fixtures.ts'; -import { markPostGestureStabilization } from '../post-gesture-stabilization.ts'; -import type { SessionState } from '../types.ts'; +import { makeSnapshotState } from '../../__tests__/test-utils/index.ts'; +import { countDiagnosticEventsByPhase, withDiagnosticsScope } from '../../utils/diagnostics.ts'; +import { buildInteractionSurfaceSignature } from '../interaction-outcome-policy.ts'; +import { + capturePostGestureStabilizedResult, + markPostGestureStabilization, +} from '../post-gesture-stabilization.ts'; +import { + deliverySnapshot, + keyboardWindowNodes, + makeSession, + pickupSnapshot, + pickupSnapshotWithExtraText, +} from './post-gesture-stabilization-fixtures.ts'; + +// Pure verdict/classifier coverage (decidePostGestureStabilityVerdict) lives +// in the sibling post-gesture-stabilization-verdict.test.ts — split per +// #1563 review to stay under the repo's 500-line test-file tripwire. afterEach(() => { vi.useRealTimers(); @@ -48,11 +63,319 @@ test('markPostGestureStabilization ignores non-swipe gesture sessions', () => { assert.equal(session.postGestureStabilization, undefined); }); -function makeSession(platform: 'ios' | 'android' = 'ios'): SessionState { - return { - name: platform, - device: platform === 'android' ? ANDROID_EMULATOR : IOS_SIMULATOR, - createdAt: Date.now(), - actions: [], - }; -} +test('markPostGestureStabilization captures the pre-gesture baseline signature on iOS', () => { + const session = makeSession('ios'); + session.snapshot = makeSnapshotState([ + { index: 0, type: 'Application', label: 'App', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + identifier: 'shipping-pickup', + label: 'Pickup', + rect: { x: 20, y: 500, width: 200, height: 44 }, + }, + ]); + + markPostGestureStabilization(session, 'scroll'); + + assert.deepEqual( + session.postGestureStabilization?.baselineSignature, + buildInteractionSurfaceSignature(session.snapshot.nodes), + ); + assert.ok((session.postGestureStabilization?.baselineSignature?.length ?? 0) > 0); +}); + +test('markPostGestureStabilization does not compute a baseline signature on Android', () => { + const session = makeSession('android'); + session.snapshot = makeSnapshotState([ + { + index: 0, + type: 'android.widget.Button', + label: 'Pickup', + rect: { x: 20, y: 500, width: 200, height: 44 }, + }, + ]); + + markPostGestureStabilization(session, 'scroll'); + + assert.equal(session.postGestureStabilization?.baselineSignature, undefined); +}); + +test('markPostGestureStabilization tolerates a missing pre-gesture snapshot on iOS', () => { + const session = makeSession('ios'); + + markPostGestureStabilization(session, 'scroll'); + + assert.deepEqual(session.postGestureStabilization?.baselineSignature, []); +}); + +// --------------------------------------------------------------------------- +// capturePostGestureStabilizedResult: the async loop wired to the pure +// decision in post-gesture-stabilization-verdict.test.ts. Fake timers keep +// these instant despite the real 200ms poll interval and (for the distrust +// path) the 3.5s cap. +// --------------------------------------------------------------------------- + +test('capturePostGestureStabilizedResult keeps polling past the normal deadline when the AX tree is stuck at the pre-gesture baseline (iOS)', async () => { + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = pickupSnapshot(500); + markPostGestureStabilization(session, 'scroll'); + + let captureCount = 0; + const capture = vi.fn(async () => { + captureCount += 1; + return pickupSnapshot(500); // identical to the pre-gesture baseline, every time + }); + + const resultPromise = withDiagnosticsScope({}, async () => { + const result = await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + result, + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + settled: countDiagnosticEventsByPhase(['post_gesture_snapshot_stabilized']), + }; + }); + + await vi.advanceTimersByTimeAsync(10_000); + const { staleAccepts, settled } = await resultPromise; + + assert.equal(staleAccepts, 1); + assert.equal(settled, 0); + assert.equal(session.postGestureStabilization, undefined); + // Proves it kept polling well past the OLD 1.5s accept point (2 attempts, + // ~200ms) instead of trusting the first quiet match. + assert.ok(captureCount > 8, `expected sustained polling, saw ${captureCount} captures`); +}); + +test('capturePostGestureStabilizedResult trusts a quiet signature once content genuinely differs from the baseline (iOS)', async () => { + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = pickupSnapshot(500); // pre-gesture: Pickup below the fold + markPostGestureStabilization(session, 'scroll'); + + const capture = vi.fn(async () => pickupSnapshot(120)); // post-gesture: scrolled into view, every read agrees + + const resultPromise = withDiagnosticsScope({}, async () => { + const result = await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + result, + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + settled: countDiagnosticEventsByPhase(['post_gesture_snapshot_stabilized']), + }; + }); + + await vi.advanceTimersByTimeAsync(1_000); + const { staleAccepts, settled } = await resultPromise; + + assert.equal(settled, 1); + assert.equal(staleAccepts, 0); + // Accepted at the first quiet match (initial capture + one poll = 2 + // attempts): no distrust cost for a genuine settle. + assert.equal(capture.mock.calls.length, 2); +}); + +test('capturePostGestureStabilizedResult trusts an Android baseline match immediately (no distrust cost)', async () => { + vi.useFakeTimers(); + const session = makeSession('android'); + session.snapshot = pickupSnapshot(500); + markPostGestureStabilization(session, 'scroll'); + assert.equal(session.postGestureStabilization?.baselineSignature, undefined); + + const capture = vi.fn(async () => pickupSnapshot(500)); // identical throughout, like the iOS stale case + + const resultPromise = withDiagnosticsScope({}, async () => { + const result = await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + result, + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + settled: countDiagnosticEventsByPhase(['post_gesture_snapshot_stabilized']), + }; + }); + + await vi.advanceTimersByTimeAsync(1_000); + const { staleAccepts, settled } = await resultPromise; + + assert.equal(settled, 1); + assert.equal(staleAccepts, 0); + // Android has no baseline to distrust, so it accepts on the first quiet + // match (initial capture + one poll = 2 attempts) — Android's latency is + // untouched by the fix. + assert.equal(capture.mock.calls.length, 2); +}); + +test('capturePostGestureStabilizedResult keeps the ordinary never-quiet timeout at the original 1.5s budget (iOS)', async () => { + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = pickupSnapshot(500); + markPostGestureStabilization(session, 'scroll'); + + let toggle = 0; + const capture = vi.fn(async () => { + toggle += 1; + // Never quiet: alternates every poll, so consecutive reads never agree. + return pickupSnapshot(toggle % 2 === 0 ? 120 : 300); + }); + + const resultPromise = withDiagnosticsScope({}, async () => { + const result = await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + result, + timeouts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stabilization_timeout']), + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + }; + }); + + // Advance just past the original 1.5s deadline (one 200ms poll of slack): + // if the distrust extension wrongly applied here (it must not — the + // signature never goes quiet), the loop would still be polling past this + // point and the assertions below would see zero timeouts instead of one. + await vi.advanceTimersByTimeAsync(1_700); + const { timeouts, staleAccepts } = await resultPromise; + + assert.equal(timeouts, 1); + assert.equal(staleAccepts, 0); + // 1500ms / 200ms poll interval = 7 loop iterations plus the initial + // capture: bounded by the ORIGINAL 1.5s deadline, not the 3.5s distrust + // cap the accept-stale test above needs (>8 captures) to reach its verdict. + assert.ok( + capture.mock.calls.length <= 9, + `expected the original ~1.5s budget, saw ${capture.mock.calls.length} captures`, + ); +}); + +test('capturePostGestureStabilizedResult catches a frozen target even when the baseline came from a broader-scope capture than the post-gesture reads (iOS, live regression)', async () => { + // Live shape (checkout-form.ad): the pre-gesture baseline is whatever + // `session.snapshot` held from an earlier broad capture (e.g. a text-search + // `wait`), while the post-gesture reads are the click's interactive-only + // selector-resolution captures — a strictly narrower shape. Both still see + // the "Pickup" button frozen at the same pre-scroll position. + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = pickupSnapshotWithExtraText(500); + markPostGestureStabilization(session, 'scroll'); + assert.ok( + (session.postGestureStabilization?.baselineSignature?.length ?? 0) > + buildInteractionSurfaceSignature(pickupSnapshot(500).nodes).length, + 'the baseline must carry the extra text entry the post-gesture reads never see', + ); + + const capture = vi.fn(async () => pickupSnapshot(500)); // narrower shape, same frozen position + + const resultPromise = withDiagnosticsScope({}, async () => { + const result = await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + result, + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + settled: countDiagnosticEventsByPhase(['post_gesture_snapshot_stabilized']), + }; + }); + + await vi.advanceTimersByTimeAsync(10_000); + const { staleAccepts, settled } = await resultPromise; + + // A whole-array baseline comparison would report "changed" purely from the + // scope drift and accept on the first quiet match (settled=1) — exactly the + // live failure this test pins. + assert.equal(staleAccepts, 1); + assert.equal(settled, 0); +}); + +// --- #1563 review, finding 1: a root-only shared overlap must trust immediately, not tax the cap --- + +test('capturePostGestureStabilizedResult trusts immediately (no cap tax) when a real scroll leaves only the application root shared — #1563 review regression', async () => { + // The reviewer's exact false-distrust shape end to end: the baseline is + // Application + Pickup; every post-gesture read is Application + a + // DIFFERENT button (Delivery) — a genuine, successful scroll that swapped + // every real element. A boolean "any shared entry frozen" predicate would + // call the shared, always-identical Application root a baseline match and + // extend this to the 3.5s stale-read cap on zero real evidence. + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = pickupSnapshot(500); + markPostGestureStabilization(session, 'scroll'); + + const capture = vi.fn(async () => deliverySnapshot(120)); // consistent from the first read: quiet immediately + + const resultPromise = withDiagnosticsScope({}, async () => { + const result = await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + result, + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + settled: countDiagnosticEventsByPhase(['post_gesture_snapshot_stabilized']), + }; + }); + + await vi.advanceTimersByTimeAsync(1_000); + const { staleAccepts, settled } = await resultPromise; + + assert.equal(settled, 1); + assert.equal(staleAccepts, 0); + // Trusted at the first quiet match (initial capture + one poll = 2 + // attempts): root-only overlap is ambiguous, not a baseline match, so it + // never pays the 3.5s distrust cap. + assert.equal(capture.mock.calls.length, 2); +}); + +// --- #1563 review, finding 2: keyboard DESCENDANTS (not just the container) must not read as evidence --- + +test('capturePostGestureStabilizedResult trusts immediately (no cap tax) when the overlap is only keyboard descendants, not the container', async () => { + // Same end-to-end shape as the root-only regression above, but the shared + // non-evidence is a keyboard's descendants (a key, the "Next keyboard" + // assistant button — siblings of the [Keyboard] container, not inside it) + // instead of the viewport root. A container-only exclusion still counts + // these as real, frozen evidence and extends to the 3.5s cap. + vi.useFakeTimers(); + const session = makeSession('ios'); + session.snapshot = makeSnapshotState([...pickupSnapshot(500).nodes, ...keyboardWindowNodes()]); + markPostGestureStabilization(session, 'scroll'); + + const capture = vi.fn(async () => + makeSnapshotState([...deliverySnapshot(120).nodes, ...keyboardWindowNodes()]), + ); + + const resultPromise = withDiagnosticsScope({}, async () => { + const result = await capturePostGestureStabilizedResult({ + session, + capture, + readSnapshot: (snapshot) => snapshot, + }); + return { + result, + staleAccepts: countDiagnosticEventsByPhase(['post_gesture_snapshot_stale_accept']), + settled: countDiagnosticEventsByPhase(['post_gesture_snapshot_stabilized']), + }; + }); + + await vi.advanceTimersByTimeAsync(1_000); + const { staleAccepts, settled } = await resultPromise; + + assert.equal(settled, 1); + assert.equal(staleAccepts, 0); + assert.equal(capture.mock.calls.length, 2); +}); diff --git a/src/daemon/handlers/__tests__/snapshot-handler.test.ts b/src/daemon/handlers/__tests__/snapshot-handler.test.ts index 74fbc0cdc..9af140ed2 100644 --- a/src/daemon/handlers/__tests__/snapshot-handler.test.ts +++ b/src/daemon/handlers/__tests__/snapshot-handler.test.ts @@ -1137,6 +1137,7 @@ test('captureSnapshot lazily retries pending no-change touch before returning fr y: 120, width: 160, height: 48, + discriminating: true, }, ], }; @@ -1279,6 +1280,7 @@ test('captureSnapshot retries pending tap outcome before post-gesture stabilizat y: 1301, width: 476, height: 110, + discriminating: true, }, ], }; diff --git a/src/daemon/interaction-outcome-policy.ts b/src/daemon/interaction-outcome-policy.ts index 5ae02b3d8..c1ac29d43 100644 --- a/src/daemon/interaction-outcome-policy.ts +++ b/src/daemon/interaction-outcome-policy.ts @@ -1,7 +1,9 @@ import { dispatchCommand, type CommandFlags } from '../core/dispatch.ts'; import { isMobilePlatform } from '@agent-device/kernel/device'; import type { SnapshotNode, SnapshotState } from '@agent-device/kernel/snapshot'; +import { collectKeyboardChromeRefs } from '../core/snapshot-chrome.ts'; import { emitDiagnostic } from '../utils/diagnostics.ts'; +import { normalizeType } from '../utils/text-surface.ts'; import { contextFromFlags } from './context.ts'; import type { SessionState } from './types.ts'; @@ -152,9 +154,13 @@ export function buildInteractionSurfaceSignature( ): InteractionSurfaceSignature { const occurrenceCounts = new Map(); const entries: InteractionSurfaceSignature = []; + // Computed once per signature build (needs the whole tree for the + // ancestor/descendant walk `collectKeyboardChrome` does — see + // `isNonDiscriminatingSurfaceNode`), not per node. + const keyboardChromeRefs = collectKeyboardChromeRefs(nodes); for (const node of nodes) { - const entry = buildInteractionSurfaceEntry(node, occurrenceCounts); + const entry = buildInteractionSurfaceEntry(node, occurrenceCounts, keyboardChromeRefs); if (entry) entries.push(entry); } @@ -187,6 +193,64 @@ export function areInteractionSurfaceSignaturesStable( return true; } +/** + * Subset-tolerant baseline classifier for post-gesture baseline distrust + * (#1542 defect 2), reusing this module's existing three-valued vocabulary + * (`InteractionSurfaceChange`) instead of a bespoke boolean. The pre-gesture + * baseline and the post-gesture quiet capture routinely come from different + * snapshot scopes (e.g. a broad text-search capture vs. an interactive-only + * selector capture), so their signatures can differ in length/membership even + * when the element that matters never moved — whole-array equality would + * report "changed" purely from scope drift and never catch the real + * staleness. + * + * The evidence rule: only shared entries flagged `discriminating` (i.e. NOT + * the viewport root or keyboard-window chrome — see + * `isNonDiscriminatingSurfaceNode`) count as evidence. + * + * - `'ambiguous'`: the shared overlap has zero discriminating entries — this + * includes an empty overlap AND an overlap that is only structurally fixed + * chrome (e.g. two signatures sharing nothing but the Application/Window + * root after a successful scroll swapped every real element — the exact + * live shape #1563's review caught: treating that as a match would extend + * every such interaction to the stale-read cap on zero real evidence). + * Ambiguous is NOT a match — insufficient evidence is its own first-class + * outcome, the same way `classifyInteractionSurfaceChange` already treats + * an empty side. + * - `'changed'`: at least one discriminating shared entry moved beyond + * tolerance — real movement occurred. + * - `'unchanged'`: every discriminating shared entry (and there is at least + * one) still matches — this is the actual "stale, matches baseline" signal + * the distrust check exists to catch. + */ +export function classifyBaselineSurfaceEvidence( + baseline: InteractionSurfaceSignature, + current: InteractionSurfaceSignature, +): InteractionSurfaceChange { + if (baseline.length === 0 || current.length === 0) return 'ambiguous'; + const baselineByKey = new Map(baseline.map((entry) => [entry.key, entry])); + let discriminatingOverlap = 0; + for (const entry of current) { + const baselineEntry = baselineByKey.get(entry.key); + if (!baselineEntry) continue; + // Shared but non-discriminating (viewport root / keyboard chrome): this + // pair carries no evidence either way, so it neither counts toward the + // overlap nor is checked for movement (its rect is invariant by + // definition and comparing it would be pure noise). + if (!entry.discriminating || !baselineEntry.discriminating) continue; + discriminatingOverlap += 1; + if ( + Math.abs(baselineEntry.x - entry.x) > RECT_TOLERANCE_PX || + Math.abs(baselineEntry.y - entry.y) > RECT_TOLERANCE_PX || + Math.abs(baselineEntry.width - entry.width) > RECT_TOLERANCE_PX || + Math.abs(baselineEntry.height - entry.height) > RECT_TOLERANCE_PX + ) { + return 'changed'; + } + } + return discriminatingOverlap > 0 ? 'unchanged' : 'ambiguous'; +} + function supportsInteractionOutcomePolicy(session: SessionState): boolean { return isMobilePlatform(session.device); } @@ -200,6 +264,7 @@ function retryCommandForTap(command: string): string | undefined { function buildInteractionSurfaceEntry( node: SnapshotNode, occurrenceCounts: Map, + keyboardChromeRefs: ReadonlySet, ): InteractionSurfaceSignature[number] | undefined { if (!node.rect) return undefined; if (!isFiniteRect(node.rect)) return undefined; @@ -214,9 +279,43 @@ function buildInteractionSurfaceEntry( y: Math.round(node.rect.y), width: Math.round(node.rect.width), height: Math.round(node.rect.height), + discriminating: !isNonDiscriminatingSurfaceNode(node, keyboardChromeRefs), }; } +/** + * Structurally fixed elements whose rect is invariant under a scroll/swipe by + * construction — sharing only these between a baseline and a later capture is + * NOT evidence the screen is unchanged, since they would read identically + * regardless of what happened. `classifyBaselineSurfaceEvidence` excludes + * them from the discriminating-overlap count for exactly this reason. + * + * Not a special case for "Application" alone, and not a container-only + * special case for the keyboard either: both checks below reuse this repo's + * existing kind classifications rather than inventing a narrower one. + */ +function isNonDiscriminatingSurfaceNode( + node: SnapshotNode, + keyboardChromeRefs: ReadonlySet, +): boolean { + return isViewportRootKind(node) || (node.ref !== undefined && keyboardChromeRefs.has(node.ref)); +} + +/** + * Minimal local equivalent of `isViewportRoot` in + * `src/snapshot/snapshot-occlusion.ts` (source of truth) — that function is + * module-private and keyed off the broader `RawSnapshotNode` shape used by + * occlusion/viewport resolution, so it is reimplemented here rather than + * exported solely for this caller. Same normalized-kind substring test; keep + * the two in lockstep if the underlying AX vocabulary changes. + */ +function isViewportRootKind(node: Pick): boolean { + const normalizedKind = [node.type, node.role, node.subrole] + .map((value) => normalizeType(value ?? '')) + .join(' '); + return normalizedKind.includes('application') || normalizedKind.includes('window'); +} + function interactionSurfaceSemanticKey(node: SnapshotNode): string | undefined { const semanticKey = [ node.identifier, diff --git a/src/daemon/post-gesture-stabilization.ts b/src/daemon/post-gesture-stabilization.ts index 3c976183d..d6965a5e4 100644 --- a/src/daemon/post-gesture-stabilization.ts +++ b/src/daemon/post-gesture-stabilization.ts @@ -1,11 +1,13 @@ import { emitDiagnostic } from '../utils/diagnostics.ts'; -import { isMobilePlatform } from '@agent-device/kernel/device'; +import { isApplePlatform, isMobilePlatform } from '@agent-device/kernel/device'; import type { CommandFlags } from '../core/dispatch.ts'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { sleep } from '../utils/timeouts.ts'; import { areInteractionSurfaceSignaturesStable, buildInteractionSurfaceSignature, + classifyBaselineSurfaceEvidence, + type InteractionSurfaceSignature, } from './interaction-outcome-policy.ts'; import type { SessionState } from './types.ts'; @@ -13,6 +15,22 @@ const STABILIZATION_DEADLINE_MS = 1_500; const STABILIZATION_INTERVAL_MS = 200; const STABILIZATION_MIN_ATTEMPTS = 2; +/** + * Defect 2 (#1542): a bounded extra budget used ONLY when a quiet AX-signature + * match (two consecutive polls agree) still equals the pre-gesture baseline on + * the Apple synthesized-gesture lane (see `requiresPostGestureBaselineDistrust`). + * XCTest's AX tree isn't proactively resynced by a synthesized touch, so it + * can serve a stale-but-internally-consistent read that two polls agree on + * without the screen having moved. + * + * 2s of real margin over both the poll interval (200ms) and the normal + * deadline (1.5s) — a near-zero margin between a poll interval and a quiet + * window is a proven flake source in this codebase (see + * settle-zero-margin-flake, a week-long contention-flake root cause), so this + * cap is sized to never come close to that trap. + */ +const STABILIZATION_DISTRUST_DEADLINE_MS = STABILIZATION_DEADLINE_MS + 2_000; + export function markPostGestureStabilization( session: SessionState, action: string, @@ -24,6 +42,13 @@ export function markPostGestureStabilization( session.postGestureStabilization = { action, markedAt: Date.now(), + // No extra capture: `session.snapshot` is still whatever was captured + // before this gesture dispatched (this call happens post-dispatch, + // pre-capture — the same "last known pre-action snapshot" idiom + // `markPendingInteractionOutcome` already relies on). + ...(requiresPostGestureBaselineDistrust(session.device) + ? { baselineSignature: buildInteractionSurfaceSignature(session.snapshot?.nodes ?? []) } + : {}), }; } @@ -32,46 +57,142 @@ function clearPostGestureStabilization(session: SessionState | undefined): void session.postGestureStabilization = undefined; } +export type PostGestureStabilityVerdict = 'trust' | 'distrust' | 'accept-stale'; + +/** + * Pure decision at the heart of defect 2's fix. Called only once a quiet + * AX-signature match has already been observed (two consecutive post-gesture + * polls agree); decides whether that agreement is trustworthy "settled" + * evidence or a stale-but-consistent AX read that happens to still equal the + * pre-gesture baseline. + * + * - `trust`: accept immediately — the platform doesn't need baseline distrust + * (Android is fresh by construction), there is no usable baseline, or the + * quiet signature genuinely differs from the pre-gesture baseline (real + * movement occurred). + * - `distrust`: the quiet signature still equals the baseline AND the bounded + * distrust cap has not expired — keep polling, do not accept as final. + * - `accept-stale`: the distrust cap expired and the signature still equals + * the baseline. A genuinely inert gesture (e.g. scroll already at an edge) + * is the honest read at this point, so it is accepted — but flagged, so a + * stale-accept is distinguishable from an ordinary settle in diagnostics. + * + * The baseline comparison is `classifyBaselineSurfaceEvidence` — a + * subset-tolerant, three-valued classifier reusing this codebase's existing + * `InteractionSurfaceChange` vocabulary (`'changed' | 'unchanged' | + * 'ambiguous'`), not whole-array equality and not a boolean. Two reasons, + * both live-verified on #1542 checkout-form.ad before shipping: + * + * 1. Scope drift: the pre-gesture baseline and the post-gesture quiet capture + * are routinely fetched by different callers with different snapshot + * scopes (e.g. a broad text-search capture vs. an interactive-only + * selector capture), so their signatures can differ in length/membership + * even when the element that matters never moved. Whole-array equality + * made the verdict `trust` on the very first quiet match every time, + * because the arrays never lined up — never once catching the real + * staleness this check exists for. + * 2. Non-discriminating overlap: a shared-any-entry boolean match is fooled + * the opposite way — the viewport root (Application/Window) is always + * present and its rect is invariant under any gesture, so a broad + * pre-gesture baseline and a narrow post-gesture capture can share ONLY + * the root even after a real, successful scroll swapped every actual + * element. `classifyBaselineSurfaceEvidence` excludes the root and + * keyboard chrome from the overlap it counts as evidence + * (`isNonDiscriminatingSurfaceNode`), so that case classifies as + * `'ambiguous'` (no comparable evidence) rather than `'unchanged'` (a + * match) — `'ambiguous'` falls through to `trust` below, same as `'changed'`. + */ +export function decidePostGestureStabilityVerdict(params: { + needsBaselineDistrust: boolean; + baselineSignature: InteractionSurfaceSignature | undefined; + quietSignature: InteractionSurfaceSignature; + elapsedMs: number; + distrustCapMs: number; +}): PostGestureStabilityVerdict { + const { needsBaselineDistrust, baselineSignature, quietSignature, elapsedMs, distrustCapMs } = + params; + if (!needsBaselineDistrust || !baselineSignature?.length) return 'trust'; + if (classifyBaselineSurfaceEvidence(baselineSignature, quietSignature) !== 'unchanged') { + return 'trust'; + } + return elapsedMs < distrustCapMs ? 'distrust' : 'accept-stale'; +} + +type CapturedSurface = { value: T; signature: InteractionSurfaceSignature }; + +async function captureInteractionSurface( + capture: () => Promise, + readSnapshot: (result: T) => SnapshotState, + initial?: T, +): Promise> { + const value = initial ?? (await capture()); + return { value, signature: buildInteractionSurfaceSignature(readSnapshot(value).nodes) }; +} + +function emitPostGestureSettleDiagnostic( + verdict: 'trust' | 'accept-stale', + action: string, + attempts: number, + durationMs: number, +): void { + if (verdict === 'accept-stale') { + emitDiagnostic({ + level: 'warn', + phase: 'post_gesture_snapshot_stale_accept', + data: { action, attempts, durationMs, matchedPreGestureBaseline: true }, + }); + return; + } + emitDiagnostic({ + level: attempts > 2 ? 'info' : 'debug', + phase: 'post_gesture_snapshot_stabilized', + data: { action, attempts, durationMs }, + }); +} + export async function capturePostGestureStabilizedResult(params: { session: SessionState | undefined; capture: () => Promise; readSnapshot: (result: T) => SnapshotState; initial?: T; }): Promise { - const { session, capture } = params; + const { session, capture, readSnapshot } = params; const pending = session?.postGestureStabilization; if (!session || !supportsPostGestureStabilization(session.device) || !pending) { return params.initial ?? (await capture()); } + const needsBaselineDistrust = requiresPostGestureBaselineDistrust(session.device); const startedAt = Date.now(); let attempts = 1; - let previous = params.initial ?? (await capture()); - let previousSignature = buildInteractionSurfaceSignature(params.readSnapshot(previous).nodes); + let previous = await captureInteractionSurface(capture, readSnapshot, params.initial); + // Extended past STABILIZATION_DEADLINE_MS only when the distrust verdict + // fires below; the ordinary (non-distrust) timeout path is unaffected. + let effectiveDeadlineMs = STABILIZATION_DEADLINE_MS; - while ( - attempts < STABILIZATION_MIN_ATTEMPTS || - Date.now() - startedAt < STABILIZATION_DEADLINE_MS - ) { + while (attempts < STABILIZATION_MIN_ATTEMPTS || Date.now() - startedAt < effectiveDeadlineMs) { await sleep(STABILIZATION_INTERVAL_MS); attempts += 1; - const current = await capture(); - const currentSignature = buildInteractionSurfaceSignature(params.readSnapshot(current).nodes); - if (areInteractionSurfaceSignaturesStable(previousSignature, currentSignature)) { - clearPostGestureStabilization(session); - emitDiagnostic({ - level: attempts > 2 ? 'info' : 'debug', - phase: 'post_gesture_snapshot_stabilized', - data: { - action: pending.action, - attempts, - durationMs: Date.now() - startedAt, - }, + const current = await captureInteractionSurface(capture, readSnapshot); + if (areInteractionSurfaceSignaturesStable(previous.signature, current.signature)) { + const elapsedMs = Date.now() - startedAt; + const verdict = decidePostGestureStabilityVerdict({ + needsBaselineDistrust, + baselineSignature: pending.baselineSignature, + quietSignature: current.signature, + elapsedMs, + distrustCapMs: STABILIZATION_DISTRUST_DEADLINE_MS, }); - return current; + if (verdict === 'distrust') { + effectiveDeadlineMs = STABILIZATION_DISTRUST_DEADLINE_MS; + previous = current; + continue; + } + clearPostGestureStabilization(session); + emitPostGestureSettleDiagnostic(verdict, pending.action, attempts, elapsedMs); + return current.value; } previous = current; - previousSignature = currentSignature; } clearPostGestureStabilization(session); @@ -84,7 +205,7 @@ export async function capturePostGestureStabilizedResult(params: { durationMs: Date.now() - startedAt, }, }); - return previous; + return previous.value; } function isPostGestureStabilizingAction( @@ -101,3 +222,16 @@ function isPostGestureStabilizingAction( function supportsPostGestureStabilization(device: SessionState['device']): boolean { return isMobilePlatform(device); } + +/** + * Apple-only gate for defect 2's baseline-distrust check (#1542). Android's + * persistent helper clears its accessibility-node cache before every capture + * (`AccessibilityTreeCapture.capture` → `clearAccessibilityCache`, + * #1254/#1259), so an Android post-gesture read is fresh by construction and + * cannot reproduce the stale-but-internally-consistent AX tree this check + * exists to catch. Gating here keeps Android's stabilization latency and + * semantics untouched — this only ever adds cost on the Apple lane. + */ +function requiresPostGestureBaselineDistrust(device: SessionState['device']): boolean { + return isApplePlatform(device.platform); +} diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 2bb308bf3..9f44122c2 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -197,6 +197,30 @@ export type AndroidSnapshotFreshness = { export type PostGestureStabilization = { action: string; markedAt: number; + /** + * Pre-gesture interaction-surface signature, captured from the session's + * last-known snapshot before the gesture dispatched (no extra capture — see + * `markPostGestureStabilization`). Populated only when + * `requiresPostGestureBaselineDistrust` is true for the session's device + * (Apple mobile only, #1542 defect 2): a post-gesture quiet-poll match that + * still equals this baseline is a stale-but-internally-consistent AX read, + * not proof the screen settled. Android's persistent helper clears its a11y + * cache before every capture (#1254/#1259) and needs no baseline check. + */ + baselineSignature?: Array<{ + key: string; + x: number; + y: number; + width: number; + height: number; + /** + * False for structurally fixed elements (the viewport root, keyboard + * chrome) whose rect is invariant regardless of any gesture — shared + * evidence limited to these never counts toward a baseline match. See + * `classifyBaselineSurfaceEvidence` in interaction-outcome-policy.ts. + */ + discriminating: boolean; + }>; }; export type PendingInteractionOutcome = { @@ -212,6 +236,7 @@ export type PendingInteractionOutcome = { y: number; width: number; height: number; + discriminating: boolean; }>; };