From 295367b0b062dc7ff65394514173bd38c972318e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 3 Aug 2026 12:10:50 +0200 Subject: [PATCH 1/4] fix(daemon): distrust post-gesture stability that matches the pre-gesture baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1542 defect 2: post-gesture-stabilization.ts treated two consecutive matching AX-signature polls as proof the screen settled. On iOS's AX-free synthesized gesture lane, XCTest's tree can serve a stale-but-internally- consistent read for a window after a scroll/swipe, so that "match" can be false: the daemon then evaluates pre-gesture node positions on the very next interaction. Fix: capture the interaction-surface signature before the gesture dispatches (reusing session.snapshot, no extra capture), and when a quiet poll-to-poll match still equals that baseline, don't trust it — keep polling past the normal 1.5s deadline up to a bounded 3.5s cap. On cap expiry with the signature still identical, accept the result (a genuine no-op gesture is the honest answer) but flag it via a new post_gesture_snapshot_stale_accept diagnostic so a stale-accept is observable in ndjson. Baseline comparison is subset-tolerant (interactionSurfaceMatchesBaseline) rather than whole-array equality: the pre-gesture baseline and the post-gesture capture are routinely fetched with different snapshot scopes, so naive equality reported "changed" from scope drift alone and never caught the real staleness on first implementation — live-verified and fixed before shipping. Platform-scoped to Apple only (requiresPostGestureBaselineDistrust): Android's persistent helper clears its accessibility-node cache before every capture (AccessibilityTreeCapture.capture, #1254/#1259), so an Android post-gesture read is fresh by construction and never computes a baseline signature — latency and semantics unchanged, confirmed live (checkout-form-android.ad + gesture-lab-android.ad 2/2 on Pixel_7_CI). Does not close #1542: live validation on checkout-form.ad still fails at step 11, but now for a distinct reason this fix correctly surfaces rather than causes — a corrupted ScrollView-ancestor viewport frame ((18,381,366,109) vs the true (18,62,366,729)) that the off-screen guard's findNearestScrollableAncestorRect trusts, independent of whether the signature matches the baseline. gesture-lab.ad (iOS) remains 2/2 clean, confirming no regression on the passing scenario. _Generated by Claude Code_ --- .../interaction-outcome-policy.test.ts | 104 +++++ .../post-gesture-stabilization.test.ts | 389 +++++++++++++++++- src/daemon/interaction-outcome-policy.ts | 33 ++ src/daemon/post-gesture-stabilization.ts | 118 +++++- src/daemon/types.ts | 17 + 5 files changed, 652 insertions(+), 9 deletions(-) diff --git a/src/daemon/__tests__/interaction-outcome-policy.test.ts b/src/daemon/__tests__/interaction-outcome-policy.test.ts index a1049455fd..4a3098cd6d 100644 --- a/src/daemon/__tests__/interaction-outcome-policy.test.ts +++ b/src/daemon/__tests__/interaction-outcome-policy.test.ts @@ -4,6 +4,7 @@ import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { buildInteractionSurfaceSignature, classifyInteractionSurfaceChange, + interactionSurfaceMatchesBaseline, markPendingInteractionOutcome, stripInternalInteractionFlags, } from '../interaction-outcome-policy.ts'; @@ -38,6 +39,87 @@ test('classifyInteractionSurfaceChange detects material layout movement', () => assert.equal(classifyInteractionSurfaceChange(before, after), 'changed'); }); +// --------------------------------------------------------------------------- +// interactionSurfaceMatchesBaseline (#1542 defect 2): subset-tolerant 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 — this is the comparison that has to see through that scope +// drift. +// --------------------------------------------------------------------------- + +test('interactionSurfaceMatchesBaseline matches identical signatures', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes); + const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes); + + assert.equal(interactionSurfaceMatchesBaseline(baseline, current), true); +}); + +test('interactionSurfaceMatchesBaseline treats an empty side as no evidence', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes); + + assert.equal(interactionSurfaceMatchesBaseline([], baseline), false); + assert.equal(interactionSurfaceMatchesBaseline(baseline, []), false); + assert.equal(interactionSurfaceMatchesBaseline([], []), false); +}); + +test('interactionSurfaceMatchesBaseline matches through a broader baseline scope when the shared 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(interactionSurfaceMatchesBaseline(baseline, current), true); +}); + +test('interactionSurfaceMatchesBaseline matches through a broader current scope when the shared element is frozen', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500).nodes); + const current = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes); + + assert.equal(interactionSurfaceMatchesBaseline(baseline, current), true); +}); + +test('interactionSurfaceMatchesBaseline detects real movement even through a scope difference', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes); + const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 120).nodes); + + assert.equal(interactionSurfaceMatchesBaseline(baseline, current), false); +}); + +test('interactionSurfaceMatchesBaseline is ambiguous (no match) when the signatures share no key', () => { + 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(interactionSurfaceMatchesBaseline(baseline, current), false); +}); + +test('interactionSurfaceMatchesBaseline tolerates tiny rect drift on the shared element', () => { + const baseline = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes); + const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500.4).nodes); + + assert.equal(interactionSurfaceMatchesBaseline(baseline, current), true); +}); + test('markPendingInteractionOutcome stores retry state only for explicit retry flags', () => { const session = makeSession(); markPendingInteractionOutcome({ @@ -128,3 +210,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.test.ts b/src/daemon/__tests__/post-gesture-stabilization.test.ts index b2c5484124..2c04c97d30 100644 --- a/src/daemon/__tests__/post-gesture-stabilization.test.ts +++ b/src/daemon/__tests__/post-gesture-stabilization.test.ts @@ -1,7 +1,14 @@ 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 { makeSnapshotState } from '../../__tests__/test-utils/index.ts'; +import { countDiagnosticEventsByPhase, withDiagnosticsScope } from '../../utils/diagnostics.ts'; +import { buildInteractionSurfaceSignature } from '../interaction-outcome-policy.ts'; +import { + capturePostGestureStabilizedResult, + decidePostGestureStabilityVerdict, + markPostGestureStabilization, +} from '../post-gesture-stabilization.ts'; import type { SessionState } from '../types.ts'; afterEach(() => { @@ -48,6 +55,386 @@ test('markPostGestureStabilization ignores non-swipe gesture sessions', () => { assert.equal(session.postGestureStabilization, undefined); }); +// --------------------------------------------------------------------------- +// #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. +// --------------------------------------------------------------------------- + +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, []); +}); + +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', + ); +}); + +// --------------------------------------------------------------------------- +// capturePostGestureStabilizedResult: the async loop wired to the pure +// decision above. 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); +}); + +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 }, + }, + ]); +} + +// 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. +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 }, + }, + ], + }; +} + function makeSession(platform: 'ios' | 'android' = 'ios'): SessionState { return { name: platform, diff --git a/src/daemon/interaction-outcome-policy.ts b/src/daemon/interaction-outcome-policy.ts index 5ae02b3d82..b86256acee 100644 --- a/src/daemon/interaction-outcome-policy.ts +++ b/src/daemon/interaction-outcome-policy.ts @@ -187,6 +187,39 @@ export function areInteractionSurfaceSignaturesStable( return true; } +/** + * Subset-tolerant variant of {@link areInteractionSurfaceSignaturesStable} for + * comparing a signature against a baseline captured by a DIFFERENT query (used + * by post-gesture baseline distrust, #1542 defect 2). 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. + * + * Matches when every semantic key present in BOTH signatures still has the + * same rect (within tolerance), and at least one key is shared — an empty + * intersection is ambiguous (no comparable evidence), not a match. + */ +export function interactionSurfaceMatchesBaseline( + baseline: InteractionSurfaceSignature, + current: InteractionSurfaceSignature, +): boolean { + if (baseline.length === 0 || current.length === 0) return false; + const baselineByKey = new Map(baseline.map((entry) => [entry.key, entry])); + let comparedCount = 0; + for (const entry of current) { + const baselineEntry = baselineByKey.get(entry.key); + if (!baselineEntry) continue; + comparedCount += 1; + if (Math.abs(baselineEntry.x - entry.x) > RECT_TOLERANCE_PX) return false; + if (Math.abs(baselineEntry.y - entry.y) > RECT_TOLERANCE_PX) return false; + if (Math.abs(baselineEntry.width - entry.width) > RECT_TOLERANCE_PX) return false; + if (Math.abs(baselineEntry.height - entry.height) > RECT_TOLERANCE_PX) return false; + } + return comparedCount > 0; +} + function supportsInteractionOutcomePolicy(session: SessionState): boolean { return isMobilePlatform(session.device); } diff --git a/src/daemon/post-gesture-stabilization.ts b/src/daemon/post-gesture-stabilization.ts index 3c976183db..25e00414df 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, + interactionSurfaceMatchesBaseline, + 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,6 +57,51 @@ 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 match uses `interactionSurfaceMatchesBaseline` (subset- + * tolerant), not whole-array equality: 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. Live + * evidence (#1542 checkout-form.ad): whole-array equality made this verdict + * `trust` on the very first quiet match every time, because the arrays never + * lined up — never once catching the actual staleness the check exists for. + */ +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) return 'trust'; + if (!baselineSignature || baselineSignature.length === 0) return 'trust'; + if (!interactionSurfaceMatchesBaseline(baselineSignature, quietSignature)) return 'trust'; + return elapsedMs < distrustCapMs ? 'distrust' : 'accept-stale'; +} + export async function capturePostGestureStabilizedResult(params: { session: SessionState | undefined; capture: () => Promise; @@ -44,28 +114,47 @@ export async function capturePostGestureStabilizedResult(params: { 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); + // 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)) { + const elapsedMs = Date.now() - startedAt; + const verdict = decidePostGestureStabilityVerdict({ + needsBaselineDistrust, + baselineSignature: pending.baselineSignature, + quietSignature: currentSignature, + elapsedMs, + distrustCapMs: STABILIZATION_DISTRUST_DEADLINE_MS, + }); + if (verdict === 'distrust') { + effectiveDeadlineMs = STABILIZATION_DISTRUST_DEADLINE_MS; + previous = current; + previousSignature = currentSignature; + continue; + } clearPostGestureStabilization(session); emitDiagnostic({ - level: attempts > 2 ? 'info' : 'debug', - phase: 'post_gesture_snapshot_stabilized', + level: verdict === 'accept-stale' ? 'warn' : attempts > 2 ? 'info' : 'debug', + phase: + verdict === 'accept-stale' + ? 'post_gesture_snapshot_stale_accept' + : 'post_gesture_snapshot_stabilized', data: { action: pending.action, attempts, - durationMs: Date.now() - startedAt, + durationMs: elapsedMs, + ...(verdict === 'accept-stale' ? { matchedPreGestureBaseline: true } : {}), }, }); return current; @@ -101,3 +190,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 2bb308bf3d..b8a4fea213 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -197,6 +197,23 @@ 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; + }>; }; export type PendingInteractionOutcome = { From a4ad668496d532bf696b0ea9f517e38642d26f5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 3 Aug 2026 12:17:38 +0200 Subject: [PATCH 2/4] refactor(daemon): decompose the stabilization loop's diagnostics and capture pair The distrust integration pushed capturePostGestureStabilizedResult over the complexity gate (cyclomatic 15, cognitive 24); extracting the settle-diagnostic branching and the capture+signature pair restores a clean fallow pass with no behavior change. --- src/daemon/post-gesture-stabilization.ts | 64 +++++++++++++++--------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/src/daemon/post-gesture-stabilization.ts b/src/daemon/post-gesture-stabilization.ts index 25e00414df..74eda8fb93 100644 --- a/src/daemon/post-gesture-stabilization.ts +++ b/src/daemon/post-gesture-stabilization.ts @@ -102,13 +102,45 @@ export function decidePostGestureStabilityVerdict(params: { 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()); @@ -117,8 +149,7 @@ export async function capturePostGestureStabilizedResult(params: { 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; @@ -126,41 +157,26 @@ export async function capturePostGestureStabilizedResult(params: { 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)) { + 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: currentSignature, + quietSignature: current.signature, elapsedMs, distrustCapMs: STABILIZATION_DISTRUST_DEADLINE_MS, }); if (verdict === 'distrust') { effectiveDeadlineMs = STABILIZATION_DISTRUST_DEADLINE_MS; previous = current; - previousSignature = currentSignature; continue; } clearPostGestureStabilization(session); - emitDiagnostic({ - level: verdict === 'accept-stale' ? 'warn' : attempts > 2 ? 'info' : 'debug', - phase: - verdict === 'accept-stale' - ? 'post_gesture_snapshot_stale_accept' - : 'post_gesture_snapshot_stabilized', - data: { - action: pending.action, - attempts, - durationMs: elapsedMs, - ...(verdict === 'accept-stale' ? { matchedPreGestureBaseline: true } : {}), - }, - }); - return current; + emitPostGestureSettleDiagnostic(verdict, pending.action, attempts, elapsedMs); + return current.value; } previous = current; - previousSignature = currentSignature; } clearPostGestureStabilization(session); @@ -173,7 +189,7 @@ export async function capturePostGestureStabilizedResult(params: { durationMs: Date.now() - startedAt, }, }); - return previous; + return previous.value; } function isPostGestureStabilizingAction( From 191c35120788cdc8a28b6c38631fbc154055023e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 3 Aug 2026 12:51:46 +0200 Subject: [PATCH 3/4] fix(daemon): require discriminating overlap for a post-gesture baseline match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR review on #1563 (P1): interactionSurfaceMatchesBaseline returned true whenever ANY shared entry was frozen, including the application/window viewport root, whose rect is invariant under any gesture. In the exact scope-drift case this PR supports, a broad pre-gesture baseline and a narrow post-gesture selector capture can share only that root after a real, successful scroll — the boolean predicate called that a baseline match and extended the interaction to the 3.5s stale-read cap on zero real evidence. Fix: replace the boolean with classifyBaselineSurfaceEvidence, a subset-tolerant classifier reusing this module's existing InteractionSurfaceChange vocabulary ('changed' | 'unchanged' | 'ambiguous') instead of a bespoke boolean or an Application-only special case. An entry only counts as evidence when it is `discriminating` — excludes the viewport root (minimal local equivalent of snapshot-occlusion.ts's isViewportRoot) and keyboard chrome (minimal local equivalent of snapshot-chrome.ts's keyboard-container check), both computed once at signature-build time since the flat signature-entry representation has no ref/parentIndex to reuse those modules' full ancestor-walk classifiers directly. Zero discriminating overlap is now 'ambiguous' (insufficient evidence) rather than a match, and decidePostGestureStabilityVerdict falls through 'ambiguous' to 'trust' — the safe default, same as 'changed'. Tests: the reviewer's exact shape (signatures sharing only the Application root, with the real content swapped) at three layers — classifyBaselineSurfaceEvidence directly, decidePostGestureStabilityVerdict, and the full capturePostGestureStabilizedResult async loop (proving no cap-tax: settles in 2 capture attempts, not 3.5s). Also: root+one real element both frozen still matches (guards against over-excluding), and keyboard chrome excluded from discriminating overlap. All prior tests kept green unchanged. Counterfactual: reverted to the old boolean predicate and reran — 5 tests went red, including the async regression test, which didn't just fail an assertion but timed out after 5s because the boolean predicate extended the interaction to the 3.5s distrust cap the test's 1s timer advance never covered — exactly the "extends to cap" failure mode the review predicted. Restored, 37/37 green. _Generated by Claude Code_ --- .../interaction-outcome-policy.test.ts | 142 +++++++++++++++--- .../post-gesture-stabilization.test.ts | 105 +++++++++++++ .../__tests__/snapshot-handler.test.ts | 2 + src/daemon/interaction-outcome-policy.ts | 112 +++++++++++--- src/daemon/post-gesture-stabilization.ts | 42 ++++-- src/daemon/types.ts | 8 + 6 files changed, 353 insertions(+), 58 deletions(-) diff --git a/src/daemon/__tests__/interaction-outcome-policy.test.ts b/src/daemon/__tests__/interaction-outcome-policy.test.ts index 4a3098cd6d..df347e20be 100644 --- a/src/daemon/__tests__/interaction-outcome-policy.test.ts +++ b/src/daemon/__tests__/interaction-outcome-policy.test.ts @@ -3,8 +3,8 @@ import { test } from 'vitest'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { buildInteractionSurfaceSignature, + classifyBaselineSurfaceEvidence, classifyInteractionSurfaceChange, - interactionSurfaceMatchesBaseline, markPendingInteractionOutcome, stripInternalInteractionFlags, } from '../interaction-outcome-policy.ts'; @@ -40,55 +40,65 @@ test('classifyInteractionSurfaceChange detects material layout movement', () => }); // --------------------------------------------------------------------------- -// interactionSurfaceMatchesBaseline (#1542 defect 2): subset-tolerant 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 — this is the comparison that has to see through that scope -// drift. +// 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('interactionSurfaceMatchesBaseline matches identical signatures', () => { +test('classifyBaselineSurfaceEvidence reports unchanged for identical signatures', () => { const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes); const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes); - assert.equal(interactionSurfaceMatchesBaseline(baseline, current), true); + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged'); }); -test('interactionSurfaceMatchesBaseline treats an empty side as no evidence', () => { +test('classifyBaselineSurfaceEvidence is ambiguous when either side is empty', () => { const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes); - assert.equal(interactionSurfaceMatchesBaseline([], baseline), false); - assert.equal(interactionSurfaceMatchesBaseline(baseline, []), false); - assert.equal(interactionSurfaceMatchesBaseline([], []), false); + assert.equal(classifyBaselineSurfaceEvidence([], baseline), 'ambiguous'); + assert.equal(classifyBaselineSurfaceEvidence(baseline, []), 'ambiguous'); + assert.equal(classifyBaselineSurfaceEvidence([], []), 'ambiguous'); }); -test('interactionSurfaceMatchesBaseline matches through a broader baseline scope when the shared element is frozen', () => { +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(interactionSurfaceMatchesBaseline(baseline, current), true); + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged'); }); -test('interactionSurfaceMatchesBaseline matches through a broader current scope when the shared element is frozen', () => { +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(interactionSurfaceMatchesBaseline(baseline, current), true); + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged'); }); -test('interactionSurfaceMatchesBaseline detects real movement even through a scope difference', () => { +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(interactionSurfaceMatchesBaseline(baseline, current), false); + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'changed'); }); -test('interactionSurfaceMatchesBaseline is ambiguous (no match) when the signatures share no key', () => { +test('classifyBaselineSurfaceEvidence is ambiguous when the signatures share no key at all', () => { const baseline = buildInteractionSurfaceSignature([ { ref: 'e1', @@ -110,16 +120,100 @@ test('interactionSurfaceMatchesBaseline is ambiguous (no match) when the signatu }, ]); - assert.equal(interactionSurfaceMatchesBaseline(baseline, current), false); + assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous'); }); -test('interactionSurfaceMatchesBaseline tolerates tiny rect drift on the shared element', () => { +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(interactionSurfaceMatchesBaseline(baseline, current), true); + 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 keyboard chrome 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'); +}); + +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({ diff --git a/src/daemon/__tests__/post-gesture-stabilization.test.ts b/src/daemon/__tests__/post-gesture-stabilization.test.ts index 2c04c97d30..879749a949 100644 --- a/src/daemon/__tests__/post-gesture-stabilization.test.ts +++ b/src/daemon/__tests__/post-gesture-stabilization.test.ts @@ -209,6 +209,56 @@ test('decidePostGestureStabilityVerdict accepts a baseline-matching signature on ); }); +// --- #1563 review regression: 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', + ); +}); + +function applicationRootNode() { + return { + ref: 'e-root', + index: 0, + type: 'Application', + label: 'App', + rect: { x: 0, y: 0, width: 390, height: 844 }, + }; +} + // --------------------------------------------------------------------------- // capturePostGestureStabilizedResult: the async loop wired to the pure // decision above. Fake timers keep these instant despite the real 200ms poll @@ -400,6 +450,44 @@ test('capturePostGestureStabilizedResult catches a frozen target even when the b assert.equal(settled, 0); }); +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); +}); + function pickupSnapshot(y = 500) { return makeSnapshotState([ { index: 0, type: 'Application', label: 'App', rect: { x: 0, y: 0, width: 390, height: 844 } }, @@ -414,6 +502,23 @@ function pickupSnapshot(y = 500) { ]); } +// 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. +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. diff --git a/src/daemon/handlers/__tests__/snapshot-handler.test.ts b/src/daemon/handlers/__tests__/snapshot-handler.test.ts index 74fbc0cdc4..9af140ed25 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 b86256acee..67d281f69d 100644 --- a/src/daemon/interaction-outcome-policy.ts +++ b/src/daemon/interaction-outcome-policy.ts @@ -2,6 +2,7 @@ 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 { emitDiagnostic } from '../utils/diagnostics.ts'; +import { normalizeType } from '../utils/text-surface.ts'; import { contextFromFlags } from './context.ts'; import type { SessionState } from './types.ts'; @@ -188,36 +189,61 @@ export function areInteractionSurfaceSignaturesStable( } /** - * Subset-tolerant variant of {@link areInteractionSurfaceSignaturesStable} for - * comparing a signature against a baseline captured by a DIFFERENT query (used - * by post-gesture baseline distrust, #1542 defect 2). 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. + * 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. * - * Matches when every semantic key present in BOTH signatures still has the - * same rect (within tolerance), and at least one key is shared — an empty - * intersection is ambiguous (no comparable evidence), not a match. + * The evidence rule: only shared entries flagged `discriminating` (i.e. NOT + * the viewport root or keyboard 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 interactionSurfaceMatchesBaseline( +export function classifyBaselineSurfaceEvidence( baseline: InteractionSurfaceSignature, current: InteractionSurfaceSignature, -): boolean { - if (baseline.length === 0 || current.length === 0) return false; +): InteractionSurfaceChange { + if (baseline.length === 0 || current.length === 0) return 'ambiguous'; const baselineByKey = new Map(baseline.map((entry) => [entry.key, entry])); - let comparedCount = 0; + let discriminatingOverlap = 0; for (const entry of current) { const baselineEntry = baselineByKey.get(entry.key); if (!baselineEntry) continue; - comparedCount += 1; - if (Math.abs(baselineEntry.x - entry.x) > RECT_TOLERANCE_PX) return false; - if (Math.abs(baselineEntry.y - entry.y) > RECT_TOLERANCE_PX) return false; - if (Math.abs(baselineEntry.width - entry.width) > RECT_TOLERANCE_PX) return false; - if (Math.abs(baselineEntry.height - entry.height) > RECT_TOLERANCE_PX) return false; + // 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 comparedCount > 0; + return discriminatingOverlap > 0 ? 'unchanged' : 'ambiguous'; } function supportsInteractionOutcomePolicy(session: SessionState): boolean { @@ -247,9 +273,53 @@ function buildInteractionSurfaceEntry( y: Math.round(node.rect.y), width: Math.round(node.rect.width), height: Math.round(node.rect.height), + discriminating: !isNonDiscriminatingSurfaceNode(node), }; } +/** + * 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: both checks below reuse this + * repo's existing kind classifications rather than inventing a new list. + */ +function isNonDiscriminatingSurfaceNode(node: SnapshotNode): boolean { + return isViewportRootKind(node) || isKeyboardChromeKind(node); +} + +/** + * 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'); +} + +/** + * Minimal local equivalent of the keyboard-chrome container test in + * `src/core/snapshot-chrome.ts` (source of truth, `collectKeyboardChrome`) — + * that module additionally walks ancestor/descendant `parentIndex` chains to + * classify a keyboard window's whole subtree, which this flat signature-entry + * comparison has no access to (entries carry no ref/parentIndex). Catching + * the `[Keyboard]` container itself via the same per-node type test is the + * invariant-rect anchor that matters here: the container's own frame never + * moves for a scroll/swipe gesture. + */ +function isKeyboardChromeKind(node: Pick): boolean { + return normalizeType(node.type ?? '') === 'keyboard'; +} + 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 74eda8fb93..d6965a5e4b 100644 --- a/src/daemon/post-gesture-stabilization.ts +++ b/src/daemon/post-gesture-stabilization.ts @@ -6,7 +6,7 @@ import { sleep } from '../utils/timeouts.ts'; import { areInteractionSurfaceSignaturesStable, buildInteractionSurfaceSignature, - interactionSurfaceMatchesBaseline, + classifyBaselineSurfaceEvidence, type InteractionSurfaceSignature, } from './interaction-outcome-policy.ts'; import type { SessionState } from './types.ts'; @@ -77,15 +77,30 @@ export type PostGestureStabilityVerdict = 'trust' | 'distrust' | 'accept-stale'; * 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 match uses `interactionSurfaceMatchesBaseline` (subset- - * tolerant), not whole-array equality: 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. Live - * evidence (#1542 checkout-form.ad): whole-array equality made this verdict - * `trust` on the very first quiet match every time, because the arrays never - * lined up — never once catching the actual staleness the check exists for. + * 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; @@ -96,9 +111,10 @@ export function decidePostGestureStabilityVerdict(params: { }): PostGestureStabilityVerdict { const { needsBaselineDistrust, baselineSignature, quietSignature, elapsedMs, distrustCapMs } = params; - if (!needsBaselineDistrust) return 'trust'; - if (!baselineSignature || baselineSignature.length === 0) return 'trust'; - if (!interactionSurfaceMatchesBaseline(baselineSignature, quietSignature)) return 'trust'; + if (!needsBaselineDistrust || !baselineSignature?.length) return 'trust'; + if (classifyBaselineSurfaceEvidence(baselineSignature, quietSignature) !== 'unchanged') { + return 'trust'; + } return elapsedMs < distrustCapMs ? 'distrust' : 'accept-stale'; } diff --git a/src/daemon/types.ts b/src/daemon/types.ts index b8a4fea213..9f44122c22 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -213,6 +213,13 @@ export type PostGestureStabilization = { 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; }>; }; @@ -229,6 +236,7 @@ export type PendingInteractionOutcome = { y: number; width: number; height: number; + discriminating: boolean; }>; }; From 12a7caebca2e44761351fa1d8839b246a9ab06ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 3 Aug 2026 13:15:53 +0200 Subject: [PATCH 4/4] fix(daemon): exclude keyboard descendants (not just the container) from baseline evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR review on #1563 (two findings, blocking merge): 1. isKeyboardChromeKind excluded only the [Keyboard] container node itself. collectKeyboardChrome (src/core/snapshot-chrome.ts, the established source of truth) classifies the WHOLE keyboard window/subtree — keys, AND the "Next keyboard"/"Dictate" assistant buttons, which are documented siblings of the container, not descendants, so a container-descendant walk alone provably misses them. In the scope-drift case this PR supports, a successful scroll can leave only those keyboard descendants shared between a baseline and a later capture, and the narrower check called that a baseline match — extending a fresh result to the 3.5s stale-read cap. Fixed by exporting a narrow predicate, collectKeyboardChromeRefs(nodes), from snapshot-chrome.ts (returns collectKeyboardChrome(nodes).refs, no Android union — this caller has no appBundleId in scope and only needs the iOS half). buildInteractionSurfaceSignature computes it once per signature build and threads it into buildInteractionSurfaceEntry, so discriminating is now `!isViewportRootKind(node) && !keyboardChromeRefs .has(node.ref)` — reusing the real ancestor-walk classification instead of a per-node type check, no ancestry needed in the signature entries themselves. 2. post-gesture-stabilization.test.ts had grown to 550 LOC, past the repository's 500-line extraction tripwire (AGENTS.md: "past 500, extract before adding behavior... Tests are not exempt"). Split along subject lines: the pure decidePostGestureStabilityVerdict coverage moved to a new sibling post-gesture-stabilization-verdict.test.ts, and shared fixtures (pickupSnapshot, deliverySnapshot, applicationRootNode, keyboardWindowNodes, makeSession) moved to a new non-test post-gesture-stabilization-fixtures.ts. The async capturePostGesture- StabilizedResult loop tests stay in the original file. Assertions unchanged, only relocation, plus the new regression tests below. Resulting LOC: post-gesture-stabilization.test.ts 381, -verdict.test.ts 208, -fixtures.ts 129 (interaction-outcome-policy.test.ts grew to 413, still under the tripwire). Tests: the reviewer's exact regression — a shared overlap consisting only of keyboard descendants (a key + the "Next keyboard" sibling button, NOT the container) plus real content that changed (Pickup -> Delivery) — at three layers: classifyBaselineSurfaceEvidence directly (ambiguous), the verdict function (trust, elapsedMs: 0), and the full async capture loop (settles in 2 attempts, no cap tax). Counterfactual: reverted isNonDiscriminatingSurfaceNode to a container-only check (normalizeType(node.type) === 'keyboard') and reran — 3 of the new tests went red across all three layers, including the async test, which timed out after 5s (not just a failed assertion) because the container-only exclusion genuinely extended the interaction to the 3.5s distrust cap the test's 1s timer advance never covers — the same "extends to cap" failure shape as the review's finding 1. Restored, 40/40 green. _Generated by [Claude Code](https://claude.ai/code)_ --- src/core/snapshot-chrome.ts | 20 ++ .../interaction-outcome-policy.test.ts | 87 +++++- .../post-gesture-stabilization-fixtures.ts | 129 +++++++++ ...post-gesture-stabilization-verdict.test.ts | 208 +++++++++++++ .../post-gesture-stabilization.test.ts | 273 ++++-------------- src/daemon/interaction-outcome-policy.ts | 40 ++- 6 files changed, 513 insertions(+), 244 deletions(-) create mode 100644 src/daemon/__tests__/post-gesture-stabilization-fixtures.ts create mode 100644 src/daemon/__tests__/post-gesture-stabilization-verdict.test.ts diff --git a/src/core/snapshot-chrome.ts b/src/core/snapshot-chrome.ts index 8bec0342d9..be48c36e2c 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 df347e20be..cd38b55960 100644 --- a/src/daemon/__tests__/interaction-outcome-policy.test.ts +++ b/src/daemon/__tests__/interaction-outcome-policy.test.ts @@ -175,7 +175,7 @@ test('classifyBaselineSurfaceEvidence is ambiguous when the current capture is t assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous'); }); -test('classifyBaselineSurfaceEvidence excludes keyboard chrome from discriminating overlap', () => { +test('classifyBaselineSurfaceEvidence excludes the keyboard container from discriminating overlap', () => { const keyboardNode = { ref: 'e3', index: 2, @@ -192,6 +192,91 @@ test('classifyBaselineSurfaceEvidence excludes keyboard chrome from discriminati 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 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 0000000000..f93b97e6df --- /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 0000000000..96a7eb851b --- /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 879749a949..2e7147cc34 100644 --- a/src/daemon/__tests__/post-gesture-stabilization.test.ts +++ b/src/daemon/__tests__/post-gesture-stabilization.test.ts @@ -1,15 +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 { makeSnapshotState } from '../../__tests__/test-utils/index.ts'; import { countDiagnosticEventsByPhase, withDiagnosticsScope } from '../../utils/diagnostics.ts'; import { buildInteractionSurfaceSignature } from '../interaction-outcome-policy.ts'; import { capturePostGestureStabilizedResult, - decidePostGestureStabilityVerdict, markPostGestureStabilization, } from '../post-gesture-stabilization.ts'; -import type { SessionState } from '../types.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(); @@ -55,17 +63,6 @@ test('markPostGestureStabilization ignores non-swipe gesture sessions', () => { assert.equal(session.postGestureStabilization, undefined); }); -// --------------------------------------------------------------------------- -// #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. -// --------------------------------------------------------------------------- - test('markPostGestureStabilization captures the pre-gesture baseline signature on iOS', () => { const session = makeSession('ios'); session.snapshot = makeSnapshotState([ @@ -113,156 +110,11 @@ test('markPostGestureStabilization tolerates a missing pre-gesture snapshot on i assert.deepEqual(session.postGestureStabilization?.baselineSignature, []); }); -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 regression: 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', - ); -}); - -function applicationRootNode() { - return { - ref: 'e-root', - index: 0, - type: 'Application', - label: 'App', - rect: { x: 0, y: 0, width: 390, height: 844 }, - }; -} - // --------------------------------------------------------------------------- // capturePostGestureStabilizedResult: the async loop wired to the pure -// decision above. Fake timers keep these instant despite the real 200ms poll -// interval and (for the distrust path) the 3.5s cap. +// 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 () => { @@ -450,6 +302,8 @@ test('capturePostGestureStabilizedResult catches a frozen target even when the b 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 @@ -488,63 +342,40 @@ test('capturePostGestureStabilizedResult trusts immediately (no cap tax) when a assert.equal(capture.mock.calls.length, 2); }); -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 }, - }, - ]); -} +// --- #1563 review, finding 2: keyboard DESCENDANTS (not just the container) must not read as evidence --- -// 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. -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. -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 }, - }, - ], - }; -} - -function makeSession(platform: 'ios' | 'android' = 'ios'): SessionState { - return { - name: platform, - device: platform === 'android' ? ANDROID_EMULATOR : IOS_SIMULATOR, - createdAt: Date.now(), - actions: [], - }; -} +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/interaction-outcome-policy.ts b/src/daemon/interaction-outcome-policy.ts index 67d281f69d..c1ac29d432 100644 --- a/src/daemon/interaction-outcome-policy.ts +++ b/src/daemon/interaction-outcome-policy.ts @@ -1,6 +1,7 @@ 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'; @@ -153,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); } @@ -200,8 +205,8 @@ export function areInteractionSurfaceSignaturesStable( * staleness. * * The evidence rule: only shared entries flagged `discriminating` (i.e. NOT - * the viewport root or keyboard chrome — see `isNonDiscriminatingSurfaceNode`) - * count as evidence. + * 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 @@ -259,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; @@ -273,7 +279,7 @@ function buildInteractionSurfaceEntry( y: Math.round(node.rect.y), width: Math.round(node.rect.width), height: Math.round(node.rect.height), - discriminating: !isNonDiscriminatingSurfaceNode(node), + discriminating: !isNonDiscriminatingSurfaceNode(node, keyboardChromeRefs), }; } @@ -284,11 +290,15 @@ function buildInteractionSurfaceEntry( * regardless of what happened. `classifyBaselineSurfaceEvidence` excludes * them from the discriminating-overlap count for exactly this reason. * - * Not a special case for "Application" alone: both checks below reuse this - * repo's existing kind classifications rather than inventing a new list. + * 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): boolean { - return isViewportRootKind(node) || isKeyboardChromeKind(node); +function isNonDiscriminatingSurfaceNode( + node: SnapshotNode, + keyboardChromeRefs: ReadonlySet, +): boolean { + return isViewportRootKind(node) || (node.ref !== undefined && keyboardChromeRefs.has(node.ref)); } /** @@ -306,20 +316,6 @@ function isViewportRootKind(node: Pick): boolean { - return normalizeType(node.type ?? '') === 'keyboard'; -} - function interactionSurfaceSemanticKey(node: SnapshotNode): string | undefined { const semanticKey = [ node.identifier,