diff --git a/packages/ad-script/src/internal/target-annotation-identity.ts b/packages/ad-script/src/internal/target-annotation-identity.ts index d8b2a1695e..5ca1f826c7 100644 --- a/packages/ad-script/src/internal/target-annotation-identity.ts +++ b/packages/ad-script/src/internal/target-annotation-identity.ts @@ -126,7 +126,7 @@ export function demoteNonUniqueLocalIdentity( * role/id/label, NOT element identity, so two sibling rows both reading `label="Item"` are one * identity — and an ambiguous identity answers nothing, the same fail-closed direction * `demoteNonUniqueLocalIdentity` takes for a non-unique id. `#3471`'s moving-target gate pairs this - * with `movingTargetRectsStable` to ask whether one node held its position between two trees. + * with `areCapturedRectsAtSamePosition` to ask whether one node held its position between two trees. */ export function nodeRectByIdentity( nodes: readonly IdentityPositionedNode[], diff --git a/packages/contracts/src/interaction.ts b/packages/contracts/src/interaction.ts index cde4faf5a0..3768568600 100644 --- a/packages/contracts/src/interaction.ts +++ b/packages/contracts/src/interaction.ts @@ -150,6 +150,9 @@ export type SurfaceScopedNodes = { surfaceBundleId?: string; }; +/** Which tree a resolved `@ref`'s node was read from (ADR 0014 step 5). */ +export type RefObservationSource = 'observation' | 'authorized-frame'; + export type ResolvedInteractionTarget = | { kind: 'point'; @@ -168,6 +171,16 @@ export type ResolvedInteractionTarget = resolution?: ResolutionDisclosure; recordingTarget?: RecordingTargetOverride; preAction?: SurfaceScopedNodes; + /** + * ADR 0014 step 5: which tree the dispatched node was read from, reported by the + * reconciliation that chose it. `'observation'` means the session's newest capture, so the + * node's rect is a position that capture measured; `'authorized-frame'` means the older tree + * the ref was authorized against, whose rect says nothing about where the target sits now. + * Reported rather than re-derived downstream from node or rect identity, so no caller can + * disagree with the rule after the rule has decided. Absent on a node a caller carried in + * (#1654), which no reconciliation chose. + */ + adoptedFrom?: RefObservationSource; } | { kind: 'selector'; diff --git a/packages/contracts/src/moving-target-gate.test.ts b/packages/contracts/src/moving-target-gate.test.ts index 3f635181a1..27f5da1d66 100644 --- a/packages/contracts/src/moving-target-gate.test.ts +++ b/packages/contracts/src/moving-target-gate.test.ts @@ -1,25 +1,18 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; import { - MOVING_TARGET_GATE_RECT_TOLERANCE_PX, MOVING_TARGET_GATE_SAMPLE_GAP_MS, MOVING_TARGET_GATE_WINDOW_MS, movingTargetGateDecision, - movingTargetRectsStable, - targetStillMovingHint, } from './moving-target-gate.ts'; // #3471: the two judgements the moving-target gate makes, isolated from both callers so the window -// and the stability rule have exactly one reading. The runtime half (what a press does with these +// and the stability rule have exactly one reading. Whether two captures count as one position is +// kernel's `areCapturedRectsAtSamePosition`, tested beside the band it owns +// (`packages/kernel/src/rect-center.test.ts`). The runtime half (what a press does with these // answers) is `src/commands/interaction/runtime/moving-target-gate.ts`; the Layer-3 scenarios behind // the ADR 0011 `targetStability` cells drive the whole press. -const RECT = { x: 100, y: 200, width: 120, height: 44 }; - -function shifted(over: Partial = {}) { - return { ...RECT, ...over }; -} - // --- is a second observation owed? --- test('a session with no mutation clock is idle and owes nothing', () => { @@ -123,61 +116,5 @@ test('a clock skew ahead of the mutation does not produce a negative wait', () = assert.ok(decision.waitMs >= MOVING_TARGET_GATE_SAMPLE_GAP_MS); }); -// --- has the target stopped? --- - -test('two observations at the same rect are a resting position', () => { - assert.equal(movingTargetRectsStable(RECT, shifted()), true); -}); - -test('capture rounding within the declared tolerance is not motion', () => { - assert.equal(MOVING_TARGET_GATE_RECT_TOLERANCE_PX, 1); - assert.equal(movingTargetRectsStable(RECT, shifted({ x: RECT.x + 1 })), true); - assert.equal(movingTargetRectsStable(RECT, shifted({ height: RECT.height + 1 })), true); -}); - -test('one axis moving is enough to still be moving', () => { - // The slide-in case from the #3471 repro (ADB Android): a translate animation keeps width and - // height constant, so only one axis says the target moved. - assert.equal(movingTargetRectsStable(RECT, shifted({ x: RECT.x + 2 })), false); - assert.equal(movingTargetRectsStable(RECT, shifted({ y: RECT.y + 2 })), false); - assert.equal(movingTargetRectsStable(RECT, shifted({ width: RECT.width + 2 })), false); - assert.equal(movingTargetRectsStable(RECT, shifted({ height: RECT.height + 2 })), false); -}); - -test('an observation that could not measure the target never agrees with one that could', () => { - // Absence is the surface having changed under the point, which is the exposure the gate exists - // to catch — so it can never be the observation that certifies rest. - assert.equal(movingTargetRectsStable(RECT, undefined), false); - assert.equal(movingTargetRectsStable(undefined, RECT), false); - assert.equal(movingTargetRectsStable(undefined, undefined), false); - assert.equal(movingTargetRectsStable(RECT, shifted({ width: Number.NaN })), false); -}); - -test('the stability comparison is symmetric', () => { - assert.equal( - movingTargetRectsStable(shifted({ y: 205 }), RECT), - movingTargetRectsStable(RECT, shifted({ y: 205 })), - ); -}); - -// --- the refusal's published recovery --- - -test('a refused selector press names the wait that recovers it', () => { - const hint = targetStillMovingHint({ selector: 'label="Slide target"' }); - - assert.match(hint, /wait 'label="Slide target"'/); - assert.match(hint, /same selector/); - assert.match(hint, /--settle/); - assert.match(hint, /nothing was tapped/); -}); - -test('a refused @ref press is sent to a selector, because the wait would retire its frame', () => { - // ADR 0014: the capture a `wait` takes expires the ref frame, so telling the caller to retry the - // same ref would trade a motion refusal for a stale-ref refusal. - const hint = targetStillMovingHint({ selector: undefined }); - - assert.match(hint, /snapshot -i/); - assert.match(hint, /selector/); - assert.match(hint, /ref frame/); - assert.doesNotMatch(hint, /wait '/); -}); +// The refusal's published recovery is asserted in the runtime gate's suite, where the hint lives +// beside the refusal that publishes it (`movingTargetHint`); this module decides, it does not speak. diff --git a/packages/contracts/src/moving-target-gate.ts b/packages/contracts/src/moving-target-gate.ts index 055053d847..37644ee721 100644 --- a/packages/contracts/src/moving-target-gate.ts +++ b/packages/contracts/src/moving-target-gate.ts @@ -1,20 +1,24 @@ -import type { Rect } from '@agent-device/kernel/snapshot'; -import { normalizeRect } from '@agent-device/kernel/rect-center'; import type { SurfaceScopedNodes } from './interaction.ts'; /** - * The moving-target gate (#3471): a press must not dispatch at a point an in-flight transition has - * already vacated. A capture is one instant's photograph of a surface that may still be rearranging - * itself, and the ADB-backed Android helper is measured reporting frames in flight — so a press - * issued right after an action that moved the UI can resolve a point the target has already left and - * report success on the vacancy. The exposure is the runtime path's own capture-to-dispatch - * distance, so the gate is backend-neutral. + * The pure half of the moving-target gate (#3471): a press must not dispatch at a point an + * in-flight transition has already vacated. A capture is one instant's photograph of a surface that + * may still be rearranging itself, and the ADB-backed Android helper is measured reporting frames in + * flight — so a press issued right after an action that moved the UI can resolve a point the target + * has already left and report success on the vacancy. The exposure is the runtime path's own + * capture-to-dispatch distance, so the gate is backend-neutral. * - * Two judgements decide when it is real, both pure over values the caller already holds so neither - * is re-derived per platform: {@link movingTargetGateDecision} asks whether this position is still - * owed a second observation, {@link movingTargetRectsStable} asks whether the target has stopped. - * The window is anchored on the mutation's END, so the gate waits out only what is LEFT of it — - * which is also why the refusal's hint names `--settle` as the caller-opted superset. + * Two judgements decide when it is real: {@link movingTargetGateDecision} asks whether this position + * is still owed a second observation, and kernel's `areCapturedRectsAtSamePosition` asks whether the + * target has stopped — that one lives in `@agent-device/kernel/rect-center` because the post-gesture + * no-effect comparison judges the same pair of capture rects and must not hold its own band. Both + * are pure over values the caller already holds, so neither is re-derived per platform. The window is + * anchored on the mutation's END, so the gate waits out only what is LEFT of it — which is also why + * the refusal's hint names `--settle` as the caller-opted superset. + * + * What cannot be decided over values — spending the window, taking the re-observations, and the + * refusal with its reason (`INTERACTION_ERROR_REASONS.targetStillMoving`) and hint — lives in + * `src/commands/interaction/runtime/moving-target-gate.ts`, which produces all three together. */ /** How long after a mutation its target positions stay owed a second observation. */ @@ -28,13 +32,6 @@ export const MOVING_TARGET_GATE_WINDOW_MS = 500; */ export const MOVING_TARGET_GATE_SAMPLE_GAP_MS = 50; -/** - * How far a rect may shift between the two observations and still count as the same position. - * Matches the tolerance the post-gesture surface comparison uses - * (`src/daemon/interaction-surface-signature.ts`), so capture rounding never reads as motion. - */ -export const MOVING_TARGET_GATE_RECT_TOLERANCE_PX = 1; - /** * The tree a mutation resolved against, kept on the session with the instant that mutation * finished, so the next action can ask whether its own coordinates are still in play. @@ -86,51 +83,3 @@ export function movingTargetGateDecision(params: { waitMs: Math.max(MOVING_TARGET_GATE_WINDOW_MS - ageMs, MOVING_TARGET_GATE_SAMPLE_GAP_MS), }; } - -/** - * Whether two consecutive observations put the target at the same position. A rect either - * observation could not measure is not stable: absence is the surface having changed under the - * point, which is the exposure the gate exists to catch. - */ -export function movingTargetRectsStable( - first: Rect | undefined, - second: Rect | undefined, -): boolean { - // `normalizeRect` is the repository's one "this rect reports a position" rule, so the gate can - // never measure a target the tap-point resolver itself would refuse to place. The tolerance stays - // here rather than in kernel's 0.5 px `areRectsApproximatelyEqual`: that one asks whether two - // values denote one point, this one asks whether capture rounding counts as motion, and 1 px is - // the band the post-gesture surface comparison already uses. - const a = normalizeRect(first); - const b = normalizeRect(second); - if (!a || !b) return false; - return ( - Math.abs(a.x - b.x) <= MOVING_TARGET_GATE_RECT_TOLERANCE_PX && - Math.abs(a.y - b.y) <= MOVING_TARGET_GATE_RECT_TOLERANCE_PX && - Math.abs(a.width - b.width) <= MOVING_TARGET_GATE_RECT_TOLERANCE_PX && - Math.abs(a.height - b.height) <= MOVING_TARGET_GATE_RECT_TOLERANCE_PX - ); -} - -/** - * The hint published beside a `target_still_moving` refusal — the recovery that works, with its - * wording owned here so every producer publishes one text. Pairing the two is a convention rather - * than something this module enforces: the reason is a literal in the vocabulary every interaction - * adapter reads (`INTERACTION_ERROR_REASONS.targetStillMoving` in - * `packages/selectors/src/interaction-error.ts`), so a producer could raise it without this hint. - * The runtime moving-target gate is the only producer today and publishes both together. - * - * A selector re-resolves against a fresh capture and can be retried as-is. An `@ref` cannot survive - * the wait: the capture a `wait` takes retires the ref frame (ADR 0014), which is the same reason - * the off-screen guard sends its `@ref` refusals to a selector. - */ -export function targetStillMovingHint(params: { selector: string | undefined }): string { - const reobserve = params.selector - ? `Run wait '${params.selector}' and retry with the same selector` - : 'Run snapshot -i and retry with a selector (text=/id=) rather than this @ref — re-observing retires the ref frame'; - return ( - 'The target was still moving when the transition window after the previous action ran out, so no resting ' + - `position could be confirmed and nothing was tapped. ${reobserve}; or pass --settle to have the action itself ` + - 'wait for the UI to go quiet.' - ); -} diff --git a/packages/kernel/src/rect-center.test.ts b/packages/kernel/src/rect-center.test.ts index a6e025c825..744279f296 100644 --- a/packages/kernel/src/rect-center.test.ts +++ b/packages/kernel/src/rect-center.test.ts @@ -1,5 +1,9 @@ import { expect, test } from 'vitest'; -import { interiorCoordinate, pointInsideRect } from './rect-center.ts'; +import { + areCapturedRectsAtSamePosition, + interiorCoordinate, + pointInsideRect, +} from './rect-center.ts'; test('interiorCoordinate preserves one-pixel edge controls', () => { expect(interiorCoordinate(0, 1)).toBe(0); @@ -11,3 +15,52 @@ test('pointInsideRect clamps center point inside the rect bounds', () => { y: 13, }); }); + +const CAPTURED = { x: 100, y: 200, width: 120, height: 44 }; + +function shifted(over: Partial = {}) { + return { ...CAPTURED, ...over }; +} + +test('two captures at the same rect report one position', () => { + expect(areCapturedRectsAtSamePosition(CAPTURED, shifted())).toBe(true); +}); + +test('a one-pixel shift is capture rounding, not motion', () => { + expect(areCapturedRectsAtSamePosition(CAPTURED, shifted({ x: CAPTURED.x + 1 }))).toBe(true); + expect(areCapturedRectsAtSamePosition(CAPTURED, shifted({ height: CAPTURED.height + 1 }))).toBe( + true, + ); +}); + +test('the band is one pixel on every axis, and one axis is enough to move', () => { + // The slide-in case from the #3471 repro (ADB Android): a translate animation keeps width and + // height constant, so only one axis says the target moved. + expect(areCapturedRectsAtSamePosition(CAPTURED, shifted({ x: CAPTURED.x + 2 }))).toBe(false); + expect(areCapturedRectsAtSamePosition(CAPTURED, shifted({ y: CAPTURED.y + 2 }))).toBe(false); + expect(areCapturedRectsAtSamePosition(CAPTURED, shifted({ width: CAPTURED.width + 2 }))).toBe( + false, + ); + expect(areCapturedRectsAtSamePosition(CAPTURED, shifted({ height: CAPTURED.height + 2 }))).toBe( + false, + ); +}); + +test('a rect that reports no position never reports one position with another', () => { + // Absence is the surface having changed under the point, so it can never be the observation that + // certifies rest, for either the moving-target gate or the post-gesture no-effect claim. + expect(areCapturedRectsAtSamePosition(CAPTURED, undefined)).toBe(false); + expect(areCapturedRectsAtSamePosition(undefined, CAPTURED)).toBe(false); + expect(areCapturedRectsAtSamePosition(undefined, undefined)).toBe(false); + expect(areCapturedRectsAtSamePosition(CAPTURED, shifted({ width: Number.NaN }))).toBe(false); + expect(areCapturedRectsAtSamePosition(CAPTURED, shifted({ width: -1 }))).toBe(false); +}); + +test('the comparison is symmetric', () => { + expect(areCapturedRectsAtSamePosition(shifted({ y: 201 }), CAPTURED)).toBe( + areCapturedRectsAtSamePosition(CAPTURED, shifted({ y: 201 })), + ); + expect(areCapturedRectsAtSamePosition(shifted({ y: 202 }), CAPTURED)).toBe( + areCapturedRectsAtSamePosition(CAPTURED, shifted({ y: 202 })), + ); +}); diff --git a/packages/kernel/src/rect-center.ts b/packages/kernel/src/rect-center.ts index 5663b42d1d..cc2608f639 100644 --- a/packages/kernel/src/rect-center.ts +++ b/packages/kernel/src/rect-center.ts @@ -3,6 +3,14 @@ import { centerOfRect, type Point, type Rect } from '@agent-device/kernel/snapsh const RECT_COMPARE_FIELDS = ['x', 'y', 'width', 'height'] as const; const RECT_EQUALITY_TOLERANCE = 0.5; +/** + * The band within which two independently captured rects count as ONE position. Wider than + * {@link areRectsApproximatelyEqual}'s band, which asks whether two values denote one point: this + * one asks whether capture rounding counts as motion, and 1 px is the drift a backend's own + * rounding can produce on a node that never moved. + */ +const CAPTURE_RECT_TOLERANCE_PX = 1; + export function resolveRectCenter(rect: Rect | undefined): { x: number; y: number } | null { const normalized = normalizeRect(rect); if (!normalized) return null; @@ -39,6 +47,26 @@ export function areRectsApproximatelyEqual( ); } +/** + * Whether two observations put a target at the SAME POSITION — the one rule that decides whether + * capture rounding counts as motion, for every comparison of rects captured at different instants. + * + * Normalizes through {@link normalizeRect} first, so a rect that reports no position (absent, + * non-finite, inverted) never compares equal to anything: absence is the surface having changed + * under the point, never agreement. + */ +export function areCapturedRectsAtSamePosition( + first: Rect | undefined, + second: Rect | undefined, +): boolean { + const left = normalizeRect(first); + const right = normalizeRect(second); + if (!left || !right) return false; + return RECT_COMPARE_FIELDS.every( + (key) => Math.abs(left[key] - right[key]) <= CAPTURE_RECT_TOLERANCE_PX, + ); +} + export function pointInsideRect(rect: Rect): Point { return { x: interiorCoordinate(rect.x, rect.width), diff --git a/packages/selectors/src/interaction-error.ts b/packages/selectors/src/interaction-error.ts index 3e73b71a90..d02455a144 100644 --- a/packages/selectors/src/interaction-error.ts +++ b/packages/selectors/src/interaction-error.ts @@ -30,11 +30,13 @@ export const INTERACTION_ERROR_REASONS = { /** The target resolved, but another visible element covers it. */ targetCovered: 'target_covered', /** - * A press or click refused because its target's rect was still changing when the transition window - * after the session's last mutating action ran out. `details.selector` or `details.ref` names the - * target, `details.rect` where the command resolved it and `details.movedToRect` where the last - * re-observation found it. Branch on this reason, not the message: the recovery is to wait for the - * surface and retry, never to retry the same point immediately. The owner is + * A press or click refused because it could not confirm a resting position for its target: the + * target's rect was still changing when the transition window after the session's last mutating + * action ran out, or a re-observation could not measure the position at all. `details.selector` or + * `details.ref` names the target, `details.rect` is where the command resolved it, and + * `details.movedToRect` where the last re-observation found it — present only when that look + * measured one. Branch on this reason, not the message: the recovery is to wait for the surface + * and retry, never to retry the same point immediately. The owner is * `src/commands/interaction/runtime/moving-target-gate.ts`, which raises the reason beside the * hint that names that recovery. */ diff --git a/src/commands/interaction/runtime/moving-target-gate.test.ts b/src/commands/interaction/runtime/moving-target-gate.test.ts index fbf8463fc5..f951fd9289 100644 --- a/src/commands/interaction/runtime/moving-target-gate.test.ts +++ b/src/commands/interaction/runtime/moving-target-gate.test.ts @@ -25,25 +25,28 @@ const RESTING_X = 10; const BUTTON_Y = 240; /** One button, reported at `x`: the same control at two positions is the whole fixture. */ -function slideTree(x: number): SnapshotState { - return makeSnapshotState([ - { - index: 0, - depth: 0, - type: 'Application', - label: 'App', - rect: { x: 0, y: 0, width: 400, height: 800 }, - }, - { - index: 1, - depth: 1, - parentIndex: 0, - type: 'Button', - label: 'Slide target', - rect: { x, y: BUTTON_Y, width: 120, height: 50 }, - hittable: true, - }, - ]); +function slideTree(x: number, over?: { label?: string; surfaceBundleId?: string }): SnapshotState { + return makeSnapshotState( + [ + { + index: 0, + depth: 0, + type: 'Application', + label: 'App', + rect: { x: 0, y: 0, width: 400, height: 800 }, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'Button', + label: over?.label ?? 'Slide target', + rect: { x, y: BUTTON_Y, width: 120, height: 50 }, + hittable: true, + }, + ], + over?.surfaceBundleId ? { iosSystemSurfaceBundleId: over.surfaceBundleId } : undefined, + ); } function centerOf(x: number): Point { @@ -66,6 +69,7 @@ function createGuardedDevice(params: { refFrame?: SnapshotState; }): { press: (target: 'selector' | 'ref') => Promise; + pressSelector: (expression: string) => Promise; pressPoint: () => Promise; captures: () => number; taps: () => Point[]; @@ -79,7 +83,14 @@ function createGuardedDevice(params: { ? undefined : { finishedAt: clock.now() - params.baselineAgeMs, - tree: { nodes: baselineTree.nodes }, + // The baseline carries its surface the way a real one does: `surfaceScopedNodes` reads it + // off the capture, so a fixture that wants a system-surface baseline asks for one. + tree: { + nodes: baselineTree.nodes, + ...(baselineTree.iosSystemSurfaceBundleId + ? { surfaceBundleId: baselineTree.iosSystemSurfaceBundleId } + : {}), + }, }; const device = createAgentDevice({ backend: { @@ -131,6 +142,10 @@ function createGuardedDevice(params: { return { press, + pressSelector: async (expression: string) => { + await device.interactions.press(selector(expression), { session: 'default' }); + return taps; + }, pressPoint: async () => { await device.interactions.press( { kind: 'point', x: centerOf(RESTING_X).x, y: centerOf(RESTING_X).y }, @@ -256,12 +271,53 @@ test('a ref the second observation cannot re-measure is refused, never self-agre const details = (error as { details?: Record }).details; assert.equal(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving); assert.equal(details?.dispatched, 'no'); + // Nothing was measured, so nothing is reported: a caller reading `movedToRect` would take the + // stale frame position for a fresh observation of where the target went. + assert.equal(details?.movedToRect, undefined); + // The `@ref` branch of the recovery, asserted on a real refusal rather than on the string: the + // hint's only test was the contracts unit suite, and the hint now lives beside this refusal. + assert.match(String(details?.hint), /snapshot -i/); + assert.match(String(details?.hint), /ref frame/); return true; }, ); assert.deepEqual(device.taps(), [], 'an unmeasured position licenses no tap'); }); +test('a surface that arrives between the two looks is not rest', async () => { + // Rest is one position on one surface. The screening already refuses to compare positions across + // surfaces for exactly the #2438 reason — a system sheet can carry the same control at the same + // rect — and the two looks that certify rest are the same comparison. Without the surface check, a + // sheet presented during the sample gap would let a press land on it while reporting agreement. + const sheet = 'com.apple.springboard'; + const device = createGuardedDevice({ + stored: slideTree(MID_FLIGHT_X), + trees: [ + // The resolution's own capture, then the two looks: the first on the app's content at rest, + // the second on a sheet presented during the sample gap — same control, same rect, another + // surface. + slideTree(MID_FLIGHT_X), + slideTree(RESTING_X), + slideTree(RESTING_X, { surfaceBundleId: sheet }), + ], + ...DISTURBING_HISTORY, + }); + + await assert.rejects( + () => device.press('selector'), + (error: unknown) => { + const details = (error as { details?: Record }).details; + assert.equal(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving); + assert.equal(details?.dispatched, 'no'); + // The sheet's rect is not somewhere the app's target moved to, so the refusal reports no + // position rather than one measured on a presentation this press never asked about. + assert.equal(details?.movedToRect, undefined); + return true; + }, + ); + assert.deepEqual(device.taps(), [], 'the sheet never received a tap'); +}); + test('a target the wait loses is refused by resolution, not by a motion refusal', async () => { // The recheck asked the same question again and the answer was "no such element". That is a more // accurate account of a vanished target than "still moving" would be, and the caller's retry is a @@ -277,9 +333,115 @@ test('a target the wait loses is refused by resolution, not by a motion refusal' () => device.press('ref'), (error: unknown) => { const details = (error as { details?: Record }).details; - assert.notEqual(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving); + // Named, not merely "not the motion refusal": any unrelated rejection would pass otherwise. + assert.equal(details?.reason, INTERACTION_ERROR_REASONS.refNotFound); + assert.equal(details?.dispatched, 'no'); return true; }, ); assert.deepEqual(device.taps(), []); }); + +test('a target newly arrived on a different surface is re-observed, not certified stationary', async () => { + // The screening compares positions, so it is only meaningful inside one surface: an iOS system + // sheet arriving over the app can carry the same-identity node at the same rect, and that is the + // positional coincidence ADR 0014 refuses rather than evidence of rest. The press pays the + // re-observations instead of tapping on the strength of the other surface's tree. + const sheet = 'com.apple.springboard'; + const device = createGuardedDevice({ + stored: slideTree(MID_FLIGHT_X, { surfaceBundleId: sheet }), + trees: [ + slideTree(MID_FLIGHT_X, { surfaceBundleId: sheet }), + slideTree(RESTING_X, { surfaceBundleId: sheet }), + slideTree(RESTING_X, { surfaceBundleId: sheet }), + ], + baselineAgeMs: 100, + // The baseline shows the target exactly where the press resolved it — the `stationary` shape — + // but it is the app's tree, not the sheet's. + baselineTree: slideTree(MID_FLIGHT_X), + }); + + assert.deepEqual(await device.press('selector'), [centerOf(RESTING_X)]); + assert.equal( + device.captures(), + 3, + 'resolution plus both re-observations, not the screening alone', + ); +}); + +test('a replacement that took the ref at the frame coordinates is refused, not agreed', async () => { + // ADR 0014 keeps the authorized frame node when the captured node's identity differs, so both + // looks land on the frame rect — and a gate that read rect agreement alone would certify rest and + // dispatch onto a control it never measured. Same ref, same position, different element: the + // look measured something, but not the target. + const frame = slideTree(RESTING_X); + const device = createGuardedDevice({ + stored: frame, + refFrame: frame, + trees: [replacementAtRestingRect(), replacementAtRestingRect()], + ...DISTURBING_HISTORY, + }); + + await assert.rejects( + () => device.press('ref'), + (error: unknown) => { + const details = (error as { details?: Record }).details; + assert.equal(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving); + // A look that measured no target reports no position: the frame rect is the value the whole + // window exists to doubt, and publishing it as `movedToRect` would read as a fresh observation. + assert.equal(details?.movedToRect, undefined); + return true; + }, + ); + assert.deepEqual(device.taps(), [], 'one look that measured no target ends the recheck'); +}); + +/** The `@e2` position, held by a control with a different local identity. */ +function replacementAtRestingRect(): SnapshotState { + return makeSnapshotState([ + { + index: 0, + depth: 0, + type: 'Application', + label: 'App', + rect: { x: 0, y: 0, width: 400, height: 800 }, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + ref: 'e2', + type: 'Button', + label: 'Something else', + rect: { x: RESTING_X, y: BUTTON_Y, width: 120, height: 50 }, + hittable: true, + }, + ]); +} + +test('the refused selector press publishes a wait that survives the shell', async () => { + // The hint is copied into a shell, and a naive single-quote wrap over a selector carrying an + // apostrophe would end the word early — teaching a recovery that cannot run. + const label = "It's moving"; + const device = createGuardedDevice({ + stored: slideTree(MID_FLIGHT_X, { label }), + // Still moving at the end of the window: the two re-observations disagree. + trees: [ + slideTree(MID_FLIGHT_X, { label }), + slideTree(MID_FLIGHT_X + 40, { label }), + slideTree(MID_FLIGHT_X + 80, { label }), + ], + baselineAgeMs: 100, + baselineTree: slideTree(-400, { label }), + }); + + await assert.rejects( + () => device.pressSelector('label="It\'s moving"'), + (error: unknown) => { + const details = (error as { details?: Record }).details; + assert.equal(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving); + assert.match(String(details?.hint), /wait 'label="It'\\''s moving"'/); + return true; + }, + ); +}); diff --git a/src/commands/interaction/runtime/moving-target-gate.ts b/src/commands/interaction/runtime/moving-target-gate.ts index e57cbafb36..57e6c82714 100644 --- a/src/commands/interaction/runtime/moving-target-gate.ts +++ b/src/commands/interaction/runtime/moving-target-gate.ts @@ -1,14 +1,19 @@ import { MOVING_TARGET_GATE_SAMPLE_GAP_MS, movingTargetGateDecision, - movingTargetRectsStable, - targetStillMovingHint, + type MovingTargetBaseline, type MovingTargetGateDecision, } from '@agent-device/contracts/moving-target-gate'; import { INTERACTION_ERROR_REASONS } from '@agent-device/selectors/interaction-error'; -import type { ResolvedInteractionTarget } from '@agent-device/contracts/interaction'; +import type { + InteractionTarget, + ResolvedInteractionTarget, + SurfaceScopedNodes, +} from '@agent-device/contracts/interaction'; import type { Rect, SnapshotNode } from '@agent-device/kernel/snapshot'; +import { areCapturedRectsAtSamePosition } from '@agent-device/kernel/rect-center'; import { AppError, discloseDispatch } from '@agent-device/kernel/errors'; +import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; import type { AgentDeviceRuntime, CommandContext, @@ -17,8 +22,8 @@ import type { import { nodeRectByIdentity } from '@agent-device/ad-script'; import { now, sleep } from '../../runtime-common.ts'; import { captureInteractionSnapshot } from './interaction-snapshot-capture.ts'; -import { tryResolveRefNode } from './ref-target-resolution.ts'; -import { resolveInteractionTarget, type InteractionTarget } from './resolution.ts'; +import { surfaceScopedNodes } from './post-action-surface.ts'; +import { resolveInteractionTarget } from './resolution.ts'; import type { ResolveInteractionTargetParams } from './interaction-resolution-request.ts'; /** @@ -53,7 +58,7 @@ export async function guardResolvedTargetAgainstMotion( // picked. Re-resolving under either swaps evidence for a different observation, so this asks // before spending the session read the answer would then discard. if (params.expectedResolvedTarget || params.preresolvedTarget) return resolved; - const gate = await readMovingTargetGate(runtime, options, resolved.node); + const gate = await readMovingTargetGate(runtime, options, resolved.node, resolved.preAction); if (!gate) return resolved; return await resolveAtRestTarget(runtime, options, params, resolved, gate); } @@ -70,22 +75,49 @@ async function readMovingTargetGate( runtime: AgentDeviceRuntime, options: CommandContext, resolvedNode: SnapshotNode, + resolvedSurface: SurfaceScopedNodes | undefined, ): Promise { const baseline = await readGateBaseline(runtime, options); if (!baseline) return undefined; - // Screening is the one thing the press gets for free: the last action's own tree, never a new - // observation. `nodeRectByIdentity` answers it fail-closed — absent, ambiguous and unmeasurable - // all read as "not shown at rest" — because a guard whose failure mode is a refused tap must never - // grant rest on positional coincidence. - const carried = nodeRectByIdentity(baseline.tree.nodes, resolvedNode); const decision = movingTargetGateDecision({ baselineFinishedAtMs: baseline.finishedAt, - targetShownAtRest: carried !== undefined && movingTargetRectsStable(carried, resolvedNode.rect), + targetShownAtRest: baselineShowsTargetAtRest(baseline, resolvedNode, resolvedSurface), nowMs: now(runtime), }); return decision.kind === 'recheck' ? decision : undefined; } +/** + * Whether the previous action's own tree already shows this target where this press resolved it — + * the one thing the press gets for free, since that tree is the observation the action resolved + * against and this asks nothing new of the surface. + * + * The surfaces have to be the same one before its positions mean anything: an iOS system sheet + * arriving over the app publishes a tree of a different surface, and a same-identity node at the + * same rect there is exactly the positional coincidence ADR 0014 refuses. Comparing + * `surfaceBundleId` draws the same line `post-action-surface.ts` draws between two captures. + * + * `nodeRectByIdentity` then answers fail-closed — absent, ambiguous and unmeasurable all read as + * "not shown at rest" — because a guard whose failure mode is a refused tap must never grant rest + * on a guess. + * + * The claim is an inference from one tree, not stability evidence: an animation that has not moved + * the control yet reads here exactly as a finished transition does, and no observation available at + * this instant separates them. The tier that carries evidence is `resolveAtRestTarget`, which spends + * two fresh observations and requires them to agree on a position and a surface. This one buys the + * case it can decide for free — the previous transition already finished, which is what the large + * majority of presses inside the window are — and sends everything else to be measured. + */ +function baselineShowsTargetAtRest( + baseline: MovingTargetBaseline, + resolvedNode: SnapshotNode, + resolvedSurface: SurfaceScopedNodes | undefined, +): boolean { + if (baseline.tree.surfaceBundleId !== resolvedSurface?.surfaceBundleId) return false; + const carried = nodeRectByIdentity(baseline.tree.nodes, resolvedNode); + return carried !== undefined && areCapturedRectsAtSamePosition(carried, resolvedNode.rect); +} + /** * The baseline the gate screens against. A session store that carries none is the non-daemon case — * an in-process runtime owns no mutation history — and a gate with no history has nothing to worry @@ -120,28 +152,54 @@ async function resolveAtRestTarget( const waitedFrom = now(runtime); await sleep(runtime, gate.waitMs); const first = await reObserveTarget(runtime, options, params); - if (!first.measured || !first.rect) - throw motionRefusal(resolved, undefined, now(runtime) - waitedFrom); + if (!first.measured) throw motionRefusal(resolved, undefined, now(runtime) - waitedFrom); await sleep(runtime, MOVING_TARGET_GATE_SAMPLE_GAP_MS); - const second = await reObserveTarget(runtime, options, params); - if (second.measured && movingTargetRectsStable(first.rect, second.rect)) return second.resolved; - throw motionRefusal( - resolved, - { firstRect: first.rect, secondRect: second.rect }, - now(runtime) - waitedFrom, - ); + const second = onSameSurface(first, await reObserveTarget(runtime, options, params)); + if (second.measured && areCapturedRectsAtSamePosition(first.rect, second.rect)) + return second.resolved; + throw motionRefusal(resolved, second.rect, now(runtime) - waitedFrom); } -/** One look at the target, and whether that look actually measured the surface. */ -type MovingTargetObservation = { +/** + * Rest is one position on one surface, which is why the screening refuses to compare positions + * across surfaces at all (#2438). A look taken on another presentation — a system sheet arriving + * during the sample gap can carry the same control at the same rect — therefore measures nothing + * about where this target came to rest, and is demoted to the shape that already means that: it + * cannot certify agreement, and no position from it is reported as somewhere the target moved to. + */ +function onSameSurface( + first: MovingTargetObservation & { measured: true }, + next: MovingTargetObservation, +): MovingTargetObservation { + return next.measured && next.surfaceBundleId === first.surfaceBundleId + ? next + : { measured: false, rect: undefined, resolved: next.resolved }; +} + +/** One look at the target: what it resolved to, and whether that look measured a position. */ +type MovingTargetObservation = MovingTargetMeasurement & { resolved: ResolvedInteractionTarget; - rect: Rect | undefined; +}; + +/** Whether a look produced a position of its own, or nothing to compare against. */ +type MovingTargetMeasurement = + /** The position this observation's own capture reported for the target, on that capture's surface. */ + | MovingTargetMeasured /** - * Whether the rect came from the capture this observation took. A resolution that kept its - * authorized ref frame node instead measured nothing, and two of those would "agree" trivially — - * rest certified by a non-measurement, the one outcome the gate must never produce. + * No position of its own. Two of these would "agree" trivially — rest certified by a + * non-measurement, the one outcome the gate must never produce — so the first of them ends the + * recheck, and a refusal reports no `movedToRect` rather than a stale one. */ - measured: boolean; + | { measured: false; rect: undefined }; + +/** + * A look that measured the target: where it put it, and on which surface. The surface rides along + * because a rect is only comparable to one measured on the same presentation (#2438). + */ +type MovingTargetMeasured = { + measured: true; + rect: Rect; + surfaceBundleId: string | undefined; }; /** @@ -151,16 +209,9 @@ type MovingTargetObservation = { * A `@ref` resolution deliberately spends no capture (ADR 0014 binds a ref to its authorized frame), * so the ref branch takes the observation first: publishing it into the session is what lets * ADR 0014's reconciliation adopt the capture's coordinates for a same-identity node, which is the - * sanctioned way a ref's position moves. - * - * Whether that adoption happened is already what the resolution reports: it publishes the tree it - * resolved against as its `preAction`, and that tree IS this capture when the coordinates were - * adopted. Reading provenance rather than re-matching the node answers the question the gate - * actually asks — "did this position come from the capture I just took" — and keeps the answer - * correct for the nodes whose identity is not unique in the tree (an unlabeled button row is one - * role and no label), which is most of what a screen offers. A resolution that kept its authorized - * frame node measured nothing here, and the gate refuses rather than comparing one stale rect to - * itself. + * sanctioned way a ref's position moves. `measureRefObservation` then asks whether that adoption is + * what actually got dispatched — a resolution that kept its authorized frame node measured nothing + * here, and the gate refuses rather than comparing one stale rect to itself. */ async function reObserveTarget( runtime: AgentDeviceRuntime, @@ -169,20 +220,41 @@ async function reObserveTarget( ): Promise { if (options.target.kind !== 'ref') { const resolved = await resolveInteractionTarget(runtime, options, params); - return { resolved, rect: resolvedRect(resolved), measured: true }; + const rect = resolvedRect(resolved); + // A selector resolution measured its target on the surface it resolved against, which it + // publishes as `preAction` along with the tree. + const surfaceBundleId = resolved.preAction?.surfaceBundleId; + return rect + ? { measured: true, resolved, rect, surfaceBundleId } + : { measured: false, resolved, rect: undefined }; } const { snapshot } = await captureInteractionSnapshot(runtime, options, true); const resolved = await resolveInteractionTarget(runtime, options, params); - const resolvedNodeRect = resolvedRect(resolved); - const inCapture = tryResolveRefNode(snapshot.nodes, options.target.ref, { - fallbackLabel: options.target.fallbackLabel ?? '', - }); - const observedRect = inCapture.kind === 'resolved' ? inCapture.resolved.node.rect : undefined; - return { - resolved, - rect: observedRect, - measured: observedRect !== undefined && movingTargetRectsStable(observedRect, resolvedNodeRect), - }; + return { resolved, ...measureRefObservation(surfaceScopedNodes(snapshot), resolved) }; +} + +/** + * Whether this look measured the target, stated over the provenance the resolution itself reports. + * + * Rect agreement alone is not enough, and this is the hole that makes it so: ADR 0014 keeps the + * authorized frame node when the captured node's identity differs, so a DIFFERENT control that took + * over the ref at the frame's own coordinates would produce two agreeing "observations" while the + * dispatch went to the stale point. The look measured something only when that reconciliation + * REPORTED reading the dispatched node out of the observation — the one account of what got + * dispatched that cannot drift from the rule it applied, whatever the rule becomes. The surface + * comes from the same capture through `surfaceScopedNodes`, the way the selector branch reads its + * resolution's `preAction`. + */ +function measureRefObservation( + capture: SurfaceScopedNodes, + resolved: ResolvedInteractionTarget, +): MovingTargetMeasurement { + const rect = resolvedRect(resolved); + const measured = + rect !== undefined && 'adoptedFrom' in resolved && resolved.adoptedFrom === 'observation'; + return measured + ? { measured: true, rect, surfaceBundleId: capture.surfaceBundleId } + : { measured: false, rect: undefined }; } function resolvedRect(resolved: ResolvedInteractionTarget): Rect | undefined { @@ -190,13 +262,37 @@ function resolvedRect(resolved: ResolvedInteractionTarget): Rect | undefined { } /** - * The refusal, keyed on the shared reason constant and raised beside the guard that owns it. `rect` - * is the position the caller's own resolution produced and `movedToRect` where the gate's - * re-observations found the target instead — the pair that says which way the surface is going. + * The hint published beside every `target_still_moving` refusal, owned by the refusal's module so + * the reason cannot be published without its recovery. + * + * A selector re-resolves against a fresh capture and can be retried as-is. An `@ref` cannot survive + * the wait: the capture a `wait` takes retires the ref frame (ADR 0014), which is the same reason + * the off-screen guard sends its `@ref` refusals to a selector. The selector is quoted for the + * caller's shell the way the daemon's other recovery hints quote values (`shellQuoteIfNeeded`), so + * a label carrying an apostrophe cannot turn the suggested command into a copy-paste failure. + */ +function targetStillMovingHint(params: { selector: string | undefined }): string { + const reobserve = params.selector + ? `Run wait ${shellQuoteIfNeeded(params.selector)} and retry with the same selector` + : 'Run snapshot -i and retry with a selector (text=/id=) rather than this @ref — re-observing retires the ref frame'; + return ( + 'The target was still moving when the transition window after the previous action ran out, so no resting ' + + `position could be confirmed and nothing was tapped. ${reobserve}; or pass --settle to have the action itself ` + + 'wait for the UI to go quiet.' + ); +} + +/** + * The refusal, keyed on the reason the whole interaction surface shares and raised beside the guard + * that owns it. `rect` is the position the caller's own resolution produced and `movedToRect` where + * the last re-observation found the target — the pair that says which way the surface is going. + * `movedToRect` is the one optional member: a re-observation that measured nothing (the ref kept its + * authorized frame node) has no position to name, and an absent value is the honest report rather + * than a stale one. */ function motionRefusal( resolved: Exclude, - disagreement: { firstRect: Rect; secondRect: Rect | undefined } | undefined, + movedToRect: Rect | undefined, waitedMs: number, ): Error { const selector = resolved.target.kind === 'selector' ? resolved.target.selector : undefined; @@ -211,7 +307,7 @@ function motionRefusal( reason: INTERACTION_ERROR_REASONS.targetStillMoving, ...(selector !== undefined ? { selector } : { ref }), ...(resolved.node?.rect ? { rect: resolved.node.rect } : {}), - ...(disagreement?.secondRect ? { movedToRect: disagreement.secondRect } : {}), + ...(movedToRect ? { movedToRect } : {}), waitedMs, hint: targetStillMovingHint({ selector }), }, diff --git a/src/commands/interaction/runtime/ref-target-resolution.ts b/src/commands/interaction/runtime/ref-target-resolution.ts index f3c0143e1e..5c08d51696 100644 --- a/src/commands/interaction/runtime/ref-target-resolution.ts +++ b/src/commands/interaction/runtime/ref-target-resolution.ts @@ -19,6 +19,7 @@ import { surfaceScopedNodes } from './post-action-surface.ts'; import type { InteractionTarget, PreresolvedInteractionTarget, + RefObservationSource, ResolvedInteractionTarget, SurfaceScopedNodes, } from '@agent-device/contracts/interaction'; @@ -43,6 +44,11 @@ type RefResolution = { resolved: ResolvedRefNode; /** The keyboard band the capture of `tree.nodes` measured, when it measured one (#2660). */ keyboard?: SnapshotKeyboardBandFact; + /** + * Which tree `resolved.node` was read from, as the reconciliation decided it (ADR 0014 step 5). + * Absent for a caller-carried node (#1654), which no reconciliation of this module chose. + */ + adoptedFrom?: RefObservationSource; }; /** @@ -90,6 +96,7 @@ async function readRefResolution( return { tree: surfaceScopedNodes(capture.snapshot), ...(capture.snapshot.keyboard ? { keyboard: capture.snapshot.keyboard } : {}), + ...(capture.adoptedFrom ? { adoptedFrom: capture.adoptedFrom } : {}), resolved: capture.resolved, }; } @@ -100,7 +107,7 @@ export async function resolveRefInteractionTarget( target: Extract, params: ResolveInteractionTargetParams, ): Promise { - const { tree, keyboard, resolved } = params.preresolvedTarget + const { tree, keyboard, resolved, adoptedFrom } = params.preresolvedTarget ? adoptPreresolvedRefTarget(target, params.preresolvedTarget) : await readRefResolution(runtime, options, target); const nodes = tree.nodes; @@ -128,6 +135,7 @@ export async function resolveRefInteractionTarget( kind: 'ref', point, target: { kind: 'ref', ref: `@${resolved.ref}` }, + ...(adoptedFrom ? { adoptedFrom } : {}), ...describeResolvedInteractionNode( runtime, visibleNode, @@ -138,11 +146,18 @@ export async function resolveRefInteractionTarget( }; } +/** What the frame model decided for one `@ref`: the node to act on, the tree it came from, and why. */ +type RefObservationReconciliation = InteractionSnapshot & { + resolved: ResolvedRefNode; + /** Which of the candidate trees `resolved.node` was read from: what the rule chose. */ + adoptedFrom: RefObservationSource; +}; + async function resolveSnapshotForRef( runtime: AgentDeviceRuntime, options: CommandContext, target: Extract, -): Promise { +): Promise { const { session, snapshot: frameTree } = await requireSnapshotSession(runtime, options.session); const fallbackLabel = target.fallbackLabel ?? ''; @@ -180,11 +195,14 @@ function reconcileFreshObservation(params: { target: Extract; fallbackLabel: string; authorized: ResolvedRefNode; -}): InteractionSnapshot & { resolved: ResolvedRefNode } { +}): RefObservationReconciliation { const { session, frameTree, target, fallbackLabel, authorized } = params; const observation = session.snapshot; - if (!observation || observation === frameTree) { - return { snapshot: frameTree, resolved: authorized }; + if (!observation) { + return { snapshot: frameTree, resolved: authorized, adoptedFrom: 'authorized-frame' }; + } + if (observation === frameTree) { + return { snapshot: frameTree, resolved: authorized, adoptedFrom: 'observation' }; } const observed = tryResolveRefNode(observation.nodes, target.ref, { fallbackLabel }); if ( @@ -194,9 +212,9 @@ function reconcileFreshObservation(params: { readNodeLocalIdentity(observed.resolved.node), ) ) { - return { snapshot: observation, resolved: observed.resolved }; + return { snapshot: observation, resolved: observed.resolved, adoptedFrom: 'observation' }; } - return { snapshot: frameTree, resolved: authorized }; + return { snapshot: frameTree, resolved: authorized, adoptedFrom: 'authorized-frame' }; } /** The runtime-ref resolver: `exact` for a resolved `@ref`, `label-fallback` for trailing-label recovery. */ diff --git a/src/commands/interaction/runtime/stable-capture-signal.test.ts b/src/commands/interaction/runtime/stable-capture-signal.test.ts index a99505b33c..cb2f5255c4 100644 --- a/src/commands/interaction/runtime/stable-capture-signal.test.ts +++ b/src/commands/interaction/runtime/stable-capture-signal.test.ts @@ -164,6 +164,26 @@ test('stable semantic projection stays linear on flat XCTest trees', () => { ); }); +test('sub-pixel geometry is quantized before the band, so the band stays in whole pixels', () => { + // The band this loop runs is measured in whole pixels: the two captures sit 1.89 px apart raw and + // one apart once quantized, and the straddle is what keeps the comparison honest — dropping the + // quantization would restart the quiet window on a pair this loop has always called at rest. + assert.equal( + stableCaptureSignalsEqual( + stableCaptureSignal(snapshot(0.51, 900)), + stableCaptureSignal(snapshot(2.4, 900)), + ), + true, + ); + assert.equal( + stableCaptureSignalsEqual( + stableCaptureSignal(snapshot(0.51, 900)), + stableCaptureSignal(snapshot(3.4, 900)), + ), + false, + ); +}); + test('regular snapshot stability preserves rounded geometry tolerance', () => { const left = snapshot(0, 900); const right = snapshot(0.6, 900); diff --git a/src/commands/interaction/runtime/stable-capture-signal.ts b/src/commands/interaction/runtime/stable-capture-signal.ts index 36377c3141..7544e7f2b0 100644 --- a/src/commands/interaction/runtime/stable-capture-signal.ts +++ b/src/commands/interaction/runtime/stable-capture-signal.ts @@ -5,8 +5,8 @@ import type { SnapshotState, } from '@agent-device/kernel/snapshot'; import { createSnapshotVisibility, isViewportRootNode } from '@agent-device/contracts/snapshot'; +import { areCapturedRectsAtSamePosition } from '@agent-device/kernel/rect-center'; -const GEOMETRY_TOLERANCE_PX = 1; export type StableCaptureSignal = { captureBackend?: SnapshotCaptureBackend; nodes: SignalNode[]; @@ -45,13 +45,7 @@ export function stableCaptureSignalsEqual( const candidate = right.nodes[index]; if (!candidate || node.identity !== candidate.identity) return false; if (!node.rect || !candidate.rect) return node.rect === candidate.rect; - const nodeRect = node.rect; - const candidateRect = candidate.rect; - return (['x', 'y', 'width', 'height'] as const).every( - (coordinate) => - Math.abs(Math.round(nodeRect[coordinate]) - Math.round(candidateRect[coordinate])) <= - GEOMETRY_TOLERANCE_PX, - ); + return areCapturedRectsAtSamePosition(node.rect, candidate.rect); }); } @@ -77,12 +71,23 @@ export function stableCaptureSignalsHaveBroadReplacement( return sharedNodes / Math.max(left.nodes.length, right.nodes.length) < 0.5; } +/** + * Geometry enters the signal quantized to the pixel, which is what the shared capture band + * (`areCapturedRectsAtSamePosition`, 1 px) has always effectively been compared against here: the + * quiet window restarts on more than one WHOLE pixel of drift. Quantizing at the one place a signal + * mints a rect keeps that band while leaving the comparison itself in `@agent-device/kernel`. + */ function stableCaptureNodeSignal(node: SnapshotNode): SignalNode { return { identity: `${node.type ?? ''}#${stableSemanticIdentity(node)}`, ...(node.rect ? { - rect: { ...node.rect }, + rect: { + x: Math.round(node.rect.x), + y: Math.round(node.rect.y), + width: Math.round(node.rect.width), + height: Math.round(node.rect.height), + }, } : {}), }; diff --git a/src/daemon/interaction-surface-signature.ts b/src/daemon/interaction-surface-signature.ts index 191171a342..cd05b27222 100644 --- a/src/daemon/interaction-surface-signature.ts +++ b/src/daemon/interaction-surface-signature.ts @@ -1,11 +1,10 @@ import type { Rect, SnapshotNode, SnapshotState } from '@agent-device/kernel/snapshot'; +import { areCapturedRectsAtSamePosition } from '@agent-device/kernel/rect-center'; import { collectKeyboardChromeRefs } from '@agent-device/capture-kit/snapshot-chrome'; import { stateMarkers } from '@agent-device/capture-kit/snapshot-lines'; import { isViewportRootNode } from '@agent-device/contracts/snapshot'; import type { InteractionSurfaceEntry } from './session-state.ts'; -const RECT_TOLERANCE_PX = 1; - export type InteractionSurfaceSignature = InteractionSurfaceEntry[]; export type InteractionSurfaceChange = 'changed' | 'unchanged' | 'ambiguous'; @@ -39,23 +38,6 @@ export function snapshotSurfaceComparisonKey( return snapshot?.comparisonKey ?? snapshot?.snapshotQuality?.backend; } -/** - * Shared rect-tolerance comparison for the surface-stability checks in this - * module. Entry rects are already rounded by `buildInteractionSurfaceEntry`, - * so one `RECT_TOLERANCE_PX` band absorbs residual drift consistently. - */ -function rectsWithinTolerance( - a: Pick, - b: Pick, -): boolean { - return ( - Math.abs(a.x - b.x) <= RECT_TOLERANCE_PX && - Math.abs(a.y - b.y) <= RECT_TOLERANCE_PX && - Math.abs(a.width - b.width) <= RECT_TOLERANCE_PX && - Math.abs(a.height - b.height) <= RECT_TOLERANCE_PX - ); -} - export function areInteractionSurfaceSignaturesStable( left: InteractionSurfaceSignature, right: InteractionSurfaceSignature, @@ -65,7 +47,7 @@ export function areInteractionSurfaceSignaturesStable( const a = left[index]; const b = right[index]; if (!a || !b || a.key !== b.key) return false; - if (!rectsWithinTolerance(a, b)) return false; + if (!areCapturedRectsAtSamePosition(a, b)) return false; } return true; } @@ -128,7 +110,7 @@ export function classifyBaselineSurfaceEvidence( continue; } shared += 1; - if (!rectsWithinTolerance(seen, now)) return 'changed'; + if (!areCapturedRectsAtSamePosition(seen, now)) return 'changed'; } if (shared === 0) return 'ambiguous'; const addedSinceBaseline = after.size > shared; @@ -189,7 +171,7 @@ export function haveIdenticalDiscriminatingSurfaces( for (const entry of leftEntries) { const other = rightByKey.get(entry.key); if (!other) return false; - if (!rectsWithinTolerance(entry, other)) return false; + if (!areCapturedRectsAtSamePosition(entry, other)) return false; } return true; } @@ -222,7 +204,7 @@ export function summarizeDiscriminatingSurfaceDivergence( } currentByKey.delete(entry.key); shared += 1; - if (!rectsWithinTolerance(entry, other)) rectMismatched += 1; + if (!areCapturedRectsAtSamePosition(entry, other)) rectMismatched += 1; } return { onlyInBaseline, onlyInCurrent: currentByKey.size, rectMismatched, shared }; } @@ -249,7 +231,7 @@ export function discriminatingSurfaceChangedWithinRect( for (const [content, entry] of beforeInRect) { const other = afterInRect.get(content); if (!other) return true; - if (!rectsWithinTolerance(entry, other)) return true; + if (!areCapturedRectsAtSamePosition(entry, other)) return true; afterInRect.delete(content); } return afterInRect.size > 0;