Skip to content

Commit 2bdbef3

Browse files
authored
fix(daemon): distrust post-gesture stability that matches the pre-gesture baseline (#1563)
* fix(daemon): distrust post-gesture stability that matches the pre-gesture baseline #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_ * 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. * fix(daemon): require discriminating overlap for a post-gesture baseline match 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_ * fix(daemon): exclude keyboard descendants (not just the container) from baseline evidence 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)_
1 parent 1235216 commit 2bdbef3

9 files changed

Lines changed: 1258 additions & 35 deletions

src/core/snapshot-chrome.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,26 @@ export function collectSettleChromeRefs(
325325
return collectSettleChrome(nodes, appBundleId).refs;
326326
}
327327

328+
/**
329+
* Refs of iOS keyboard-window chrome ONLY — no Android union, so callers with
330+
* no `appBundleId` in scope can still reuse the real subtree/window-aware
331+
* classification (`collectKeyboardChrome`) instead of a narrower per-node type
332+
* check. Container-descendant walk alone provably misses the "Next keyboard"
333+
* / "Dictate" assistant buttons (siblings of the `[Keyboard]` container, not
334+
* descendants — see `collectKeyboardChrome`'s doc comment), so a caller that
335+
* only excludes nodes whose OWN type is `keyboard` still leaks every key and
336+
* assistant control as "discriminating" evidence.
337+
*
338+
* Used by `src/daemon/interaction-outcome-policy.ts`'s post-gesture
339+
* baseline-distrust discriminating-overlap classification (#1542 defect 2,
340+
* #1563 review): that comparison operates on flat signature entries with no
341+
* ref-selection budget of its own, so it needs the ref set directly rather
342+
* than a node-filtering helper like `withoutSettleChrome`.
343+
*/
344+
export function collectKeyboardChromeRefs(nodes: SnapshotNode[]): ReadonlySet<string> {
345+
return collectKeyboardChrome(nodes).refs;
346+
}
347+
328348
/**
329349
* Windows eligible for whole-window chrome classification: nearest `[window]`
330350
* ancestor of each `[Keyboard]` container, minus windows hosting editable

src/daemon/__tests__/interaction-outcome-policy.test.ts

Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { test } from 'vitest';
33
import type { SnapshotState } from '@agent-device/kernel/snapshot';
44
import {
55
buildInteractionSurfaceSignature,
6+
classifyBaselineSurfaceEvidence,
67
classifyInteractionSurfaceChange,
78
markPendingInteractionOutcome,
89
stripInternalInteractionFlags,
@@ -38,6 +39,266 @@ test('classifyInteractionSurfaceChange detects material layout movement', () =>
3839
assert.equal(classifyInteractionSurfaceChange(before, after), 'changed');
3940
});
4041

42+
// ---------------------------------------------------------------------------
43+
// classifyBaselineSurfaceEvidence (#1542 defect 2, #1563 review): subset-
44+
// tolerant, three-valued baseline comparison. Live evidence on
45+
// checkout-form.ad showed the pre-gesture baseline (captured by an earlier
46+
// `wait`, a broad query) and the post-gesture quiet signature (captured by
47+
// the click's interactive-only selector resolution) never line up as whole
48+
// arrays even when the target element never moved — the first version of
49+
// this check has to see through that scope drift.
50+
//
51+
// The #1563 review then caught a SECOND failure mode in that first version
52+
// (a plain "any shared entry frozen" boolean): the viewport root
53+
// (Application/Window) is always present and its rect is invariant under any
54+
// gesture, so a broad baseline and a narrow post-gesture capture can share
55+
// ONLY the root even after a real, successful scroll — and the boolean
56+
// predicate called that a match. `classifyBaselineSurfaceEvidence` requires
57+
// at least one DISCRIMINATING shared entry (excluding the viewport root and
58+
// keyboard chrome) before calling it `'unchanged'`; a root-only (or
59+
// no-discriminating-evidence) overlap is `'ambiguous'` instead.
60+
// ---------------------------------------------------------------------------
61+
62+
test('classifyBaselineSurfaceEvidence reports unchanged for identical signatures', () => {
63+
const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes);
64+
const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes);
65+
66+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged');
67+
});
68+
69+
test('classifyBaselineSurfaceEvidence is ambiguous when either side is empty', () => {
70+
const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox').nodes);
71+
72+
assert.equal(classifyBaselineSurfaceEvidence([], baseline), 'ambiguous');
73+
assert.equal(classifyBaselineSurfaceEvidence(baseline, []), 'ambiguous');
74+
assert.equal(classifyBaselineSurfaceEvidence([], []), 'ambiguous');
75+
});
76+
77+
test('classifyBaselineSurfaceEvidence reports unchanged through a broader baseline scope when the shared discriminating element is frozen', () => {
78+
// The exact live shape: the baseline came from a broader capture (extra
79+
// "Loading" text node the interactive-only capture never sees), but the
80+
// shared "primary-action" button never moved.
81+
const baseline = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes);
82+
const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500).nodes);
83+
84+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged');
85+
});
86+
87+
test('classifyBaselineSurfaceEvidence reports unchanged through a broader current scope when the shared discriminating element is frozen', () => {
88+
const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500).nodes);
89+
const current = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes);
90+
91+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged');
92+
});
93+
94+
test('classifyBaselineSurfaceEvidence detects real movement even through a scope difference', () => {
95+
const baseline = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes);
96+
const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 120).nodes);
97+
98+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'changed');
99+
});
100+
101+
test('classifyBaselineSurfaceEvidence is ambiguous when the signatures share no key at all', () => {
102+
const baseline = buildInteractionSurfaceSignature([
103+
{
104+
ref: 'e1',
105+
index: 0,
106+
type: 'Button',
107+
identifier: 'checkout-only-button',
108+
label: 'Checkout',
109+
rect: { x: 0, y: 0, width: 100, height: 40 },
110+
},
111+
]);
112+
const current = buildInteractionSurfaceSignature([
113+
{
114+
ref: 'e1',
115+
index: 0,
116+
type: 'Button',
117+
identifier: 'settings-only-button',
118+
label: 'Settings',
119+
rect: { x: 0, y: 0, width: 100, height: 40 },
120+
},
121+
]);
122+
123+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous');
124+
});
125+
126+
test('classifyBaselineSurfaceEvidence tolerates tiny rect drift on the shared discriminating element', () => {
127+
const baseline = buildInteractionSurfaceSignature(makeSnapshotWithExtraText('Inbox', 500).nodes);
128+
const current = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500.4).nodes);
129+
130+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged');
131+
});
132+
133+
// --- #1563 review regression: root-only overlap must NOT read as evidence ---
134+
135+
test('classifyBaselineSurfaceEvidence is ambiguous (NOT unchanged) when a real scroll leaves only the application root shared — the reviewer-caught false-distrust shape', () => {
136+
// baseline = {Application, Pickup@y=500}; current = {Application,
137+
// OtherButton@...} — a genuine, successful scroll replaced every real
138+
// element in view, so the only entry the two signatures still share is the
139+
// always-present, always-identical viewport root. A boolean "any shared
140+
// entry frozen" predicate calls this a baseline match (the root always
141+
// "matches") and would extend the interaction to the 3.5s stale-read
142+
// deadline on zero real evidence — exactly the bug this test pins.
143+
const baseline = buildInteractionSurfaceSignature([
144+
applicationRootNode(),
145+
{
146+
ref: 'e2',
147+
index: 1,
148+
parentIndex: 0,
149+
type: 'Button',
150+
identifier: 'shipping-pickup',
151+
label: 'Pickup',
152+
rect: { x: 20, y: 500, width: 200, height: 44 },
153+
},
154+
]);
155+
const current = buildInteractionSurfaceSignature([
156+
applicationRootNode(),
157+
{
158+
ref: 'e2',
159+
index: 1,
160+
parentIndex: 0,
161+
type: 'Button',
162+
identifier: 'shipping-delivery',
163+
label: 'Delivery',
164+
rect: { x: 20, y: 120, width: 200, height: 44 },
165+
},
166+
]);
167+
168+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous');
169+
});
170+
171+
test('classifyBaselineSurfaceEvidence is ambiguous when the current capture is the application root alone', () => {
172+
const baseline = buildInteractionSurfaceSignature(makeSnapshot('Inbox', 500).nodes);
173+
const current = buildInteractionSurfaceSignature([applicationRootNode()]);
174+
175+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous');
176+
});
177+
178+
test('classifyBaselineSurfaceEvidence excludes the keyboard container from discriminating overlap', () => {
179+
const keyboardNode = {
180+
ref: 'e3',
181+
index: 2,
182+
parentIndex: 0,
183+
type: 'Keyboard',
184+
rect: { x: 0, y: 500, width: 390, height: 300 },
185+
};
186+
const baseline = buildInteractionSurfaceSignature([applicationRootNode(), keyboardNode]);
187+
// The keyboard's own container rect never changes; only the app content
188+
// does. A capture sharing just the root and the keyboard container (no
189+
// real content) must not read as a baseline match.
190+
const current = buildInteractionSurfaceSignature([applicationRootNode(), keyboardNode]);
191+
192+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous');
193+
});
194+
195+
// #1563 review, finding 2: a container-only exclusion still misses keyboard
196+
// DESCENDANTS (individual keys) and SIBLINGS (assistant buttons like "Next
197+
// keyboard"/"Dictate", which live outside the container per
198+
// src/core/snapshot-chrome.ts's collectKeyboardChrome doc comment — a
199+
// container-descendant walk alone provably misses them, hence the whole-
200+
// window classification that module reuses here via collectKeyboardChromeRefs).
201+
test('classifyBaselineSurfaceEvidence excludes keyboard DESCENDANTS and window SIBLINGS, not just the container, from discriminating overlap', () => {
202+
const shared = keyboardWindowNodes(); // window + [Keyboard] container + a key + a sibling "Next keyboard" button
203+
const baseline = buildInteractionSurfaceSignature([
204+
applicationRootNode(),
205+
{
206+
ref: 'e-pickup',
207+
index: 20,
208+
parentIndex: 0,
209+
type: 'Button',
210+
identifier: 'shipping-pickup',
211+
label: 'Pickup',
212+
rect: { x: 20, y: 500, width: 200, height: 44 },
213+
},
214+
...shared,
215+
]);
216+
// Real content changed (Pickup -> Delivery, a genuine successful scroll);
217+
// the keyboard subtree is identical — a keyboard does not move when app
218+
// content scrolls.
219+
const current = buildInteractionSurfaceSignature([
220+
applicationRootNode(),
221+
{
222+
ref: 'e-delivery',
223+
index: 20,
224+
parentIndex: 0,
225+
type: 'Button',
226+
identifier: 'shipping-delivery',
227+
label: 'Delivery',
228+
rect: { x: 20, y: 120, width: 200, height: 44 },
229+
},
230+
...shared,
231+
]);
232+
233+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'ambiguous');
234+
});
235+
236+
/**
237+
* A keyboard-window subtree: a `[Keyboard]` container plus a SIBLING "Next
238+
* keyboard" assistant button under the same window — matches the shape in
239+
* `src/daemon/__tests__/post-gesture-stabilization-fixtures.ts`'s
240+
* `keyboardWindowNodes` (kept local here rather than imported: this file's
241+
* fixtures are raw node literals consumed directly by
242+
* `buildInteractionSurfaceSignature`, not `SnapshotState`-wrapped like that
243+
* module's).
244+
*/
245+
function keyboardWindowNodes() {
246+
return [
247+
{
248+
ref: 'e-kb-window',
249+
index: 10,
250+
parentIndex: 0,
251+
type: 'Window',
252+
rect: { x: 0, y: 400, width: 390, height: 444 },
253+
},
254+
{
255+
ref: 'e-kb-container',
256+
index: 11,
257+
parentIndex: 10,
258+
type: 'Keyboard',
259+
rect: { x: 0, y: 500, width: 390, height: 300 },
260+
},
261+
{
262+
ref: 'e-kb-key-a',
263+
index: 12,
264+
parentIndex: 11, // descendant of the container
265+
type: 'Key',
266+
label: 'A',
267+
rect: { x: 10, y: 520, width: 30, height: 40 },
268+
},
269+
{
270+
ref: 'e-kb-next',
271+
index: 13,
272+
parentIndex: 10, // sibling of the container, NOT a descendant
273+
type: 'Button',
274+
label: 'Next keyboard',
275+
rect: { x: 340, y: 520, width: 40, height: 40 },
276+
},
277+
];
278+
}
279+
280+
test('classifyBaselineSurfaceEvidence still reports unchanged when the root AND a real discriminating element both match (guards against over-excluding)', () => {
281+
// Root-sharing alone is not disqualifying — it just cannot be the ONLY
282+
// evidence. Once a real, frozen discriminating element is also shared
283+
// (the ordinary "genuinely stuck" case), the verdict must still be
284+
// 'unchanged', not swing to 'ambiguous' just because the root is present.
285+
const snapshotNodes = makeSnapshot('Inbox', 500).nodes; // [Application, primary-action Button]
286+
const baseline = buildInteractionSurfaceSignature(snapshotNodes);
287+
const current = buildInteractionSurfaceSignature(snapshotNodes);
288+
289+
assert.equal(classifyBaselineSurfaceEvidence(baseline, current), 'unchanged');
290+
});
291+
292+
function applicationRootNode() {
293+
return {
294+
ref: 'e1',
295+
index: 0,
296+
type: 'Application',
297+
label: 'App',
298+
rect: { x: 0, y: 0, width: 390, height: 844 },
299+
};
300+
}
301+
41302
test('markPendingInteractionOutcome stores retry state only for explicit retry flags', () => {
42303
const session = makeSession();
43304
markPendingInteractionOutcome({
@@ -128,3 +389,25 @@ function makeSnapshot(label: string, y = 100): SnapshotState {
128389
backend: 'xctest',
129390
};
130391
}
392+
393+
// A broader-scope variant of makeSnapshot: the same Application + Button
394+
// entries, plus a non-interactive text node an interactive-only capture would
395+
// never return. Models the real shape mismatch between a pre-gesture baseline
396+
// snapshot and a post-gesture interactive-only selector-resolution capture.
397+
function makeSnapshotWithExtraText(label: string, y = 100): SnapshotState {
398+
const base = makeSnapshot(label, y);
399+
return {
400+
...base,
401+
nodes: [
402+
...base.nodes,
403+
{
404+
ref: 'e3',
405+
index: 2,
406+
parentIndex: 0,
407+
type: 'Text',
408+
label: 'Loading',
409+
rect: { x: 20, y: 20, width: 200, height: 20 },
410+
},
411+
],
412+
};
413+
}

0 commit comments

Comments
 (0)