From 4f866fb7b14e655f9643517eac5b48f0f3876310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 12 Oct 2026 18:51:55 +0200 Subject: [PATCH 1/3] feat(interaction): add the moving-target gate the tap path consults (#3471) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A press resolves a point from one capture, and nothing between that capture and the dispatch looked at the surface twice. A press issued right after an action that animated the UI can therefore resolve a point the control has already vacated and report success on the vacancy — measured on ADB Android, whose helper reports frames in flight. This adds the judgement: a pure window/stability decision in @agent-device/contracts, the same-node identity lookup it screens with in @agent-device/ad-script, and the runtime seam that spends the window in front of the tap, re-resolving once and tapping where the target came to rest. The session does not yet record the history the gate reads, so every press still answers `idle` and pays nothing. --- packages/ad-script/src/index.ts | 1 + .../target-annotation-identity.test.ts | 46 +++ .../internal/target-annotation-identity.ts | 23 +- .../contracts/src/moving-target-gate.test.ts | 188 ++++++++++++ packages/contracts/src/moving-target-gate.ts | 135 +++++++++ packages/contracts/src/runtime-contract.ts | 9 + packages/selectors/src/interaction-error.ts | 10 + .../interaction/runtime/interactions.ts | 30 +- .../runtime/moving-target-gate.test.ts | 285 ++++++++++++++++++ .../interaction/runtime/moving-target-gate.ts | 221 ++++++++++++++ 10 files changed, 940 insertions(+), 8 deletions(-) create mode 100644 packages/contracts/src/moving-target-gate.test.ts create mode 100644 packages/contracts/src/moving-target-gate.ts create mode 100644 src/commands/interaction/runtime/moving-target-gate.test.ts create mode 100644 src/commands/interaction/runtime/moving-target-gate.ts diff --git a/packages/ad-script/src/index.ts b/packages/ad-script/src/index.ts index 53c4fe2cf2..a2c0a30a93 100644 --- a/packages/ad-script/src/index.ts +++ b/packages/ad-script/src/index.ts @@ -39,6 +39,7 @@ export { localIdentitiesEqual, matchesAncestryPrefix, matchesLocalIdentity, + nodeRectByIdentity, readNodeLocalIdentity, readNodeStructuralDenotation, siblingOrdinal, diff --git a/packages/ad-script/src/internal/__tests__/target-annotation-identity.test.ts b/packages/ad-script/src/internal/__tests__/target-annotation-identity.test.ts index 05b4c927e2..79fcb568c7 100644 --- a/packages/ad-script/src/internal/__tests__/target-annotation-identity.test.ts +++ b/packages/ad-script/src/internal/__tests__/target-annotation-identity.test.ts @@ -6,6 +6,7 @@ import { idMatchCountInTree, matchesAncestryPrefix, matchesLocalIdentity, + nodeRectByIdentity, readNodeLocalIdentity, } from '../target-annotation-identity.ts'; @@ -124,3 +125,48 @@ test('demoteNonUniqueLocalIdentity passes through identities without an id', () assert.equal(identity.id, undefined); assert.deepEqual(demoteNonUniqueLocalIdentity(identity, nodes), identity); }); + +// --------------------------------------------------------------------------- +// nodeRectByIdentity: the fail-closed same-node lookup #3471's gate screens with +// --------------------------------------------------------------------------- + +const RECT = { x: 10, y: 240, width: 120, height: 50 }; + +test('nodeRectByIdentity reports the position a list names once', () => { + const nodes: RawSnapshotNode[] = [{ index: 0, type: 'Button', label: 'Continue', rect: RECT }]; + + assert.deepEqual(nodeRectByIdentity(nodes, nodes[0]!), RECT); +}); + +test('nodeRectByIdentity answers nothing for an absent or unmeasurable node', () => { + const target: RawSnapshotNode = { index: 0, type: 'Button', label: 'Continue', rect: RECT }; + const frameless: RawSnapshotNode = { index: 0, type: 'Button', label: 'Continue' }; + + assert.equal(nodeRectByIdentity([], target), undefined); + assert.equal(nodeRectByIdentity([frameless], target), undefined); +}); + +test('nodeRectByIdentity answers nothing when an identity names several nodes', () => { + // Local identity is role/id/label, so two sibling rows both reading `label="Item"` are ONE + // identity. Guessing between them is positional coincidence — the same fail-closed direction + // `demoteNonUniqueLocalIdentity` takes for a non-unique id. + const rows: RawSnapshotNode[] = [ + { index: 0, type: 'Cell', label: 'Item', rect: { x: 0, y: 100, width: 400, height: 60 } }, + { index: 1, type: 'Cell', label: 'Item', rect: { x: 0, y: 160, width: 400, height: 60 } }, + ]; + + assert.equal(nodeRectByIdentity(rows, rows[0]!), undefined); +}); + +test('nodeRectByIdentity keys on the exact identity triple, so a relabel is a different node', () => { + const before: RawSnapshotNode = { + index: 0, + type: 'Button', + identifier: 'save-action', + label: 'Save', + rect: RECT, + }; + const after: RawSnapshotNode = { ...before, label: 'Saved' }; + + assert.equal(nodeRectByIdentity([before], after), undefined); +}); diff --git a/packages/ad-script/src/internal/target-annotation-identity.ts b/packages/ad-script/src/internal/target-annotation-identity.ts index 9126e32444..d8b2a1695e 100644 --- a/packages/ad-script/src/internal/target-annotation-identity.ts +++ b/packages/ad-script/src/internal/target-annotation-identity.ts @@ -15,7 +15,7 @@ * here, not through `@agent-device/ad-replay`'s façade (#1555 review). */ -import type { RawSnapshotNode } from '@agent-device/kernel/snapshot'; +import type { RawSnapshotNode, Rect } from '@agent-device/kernel/snapshot'; import { normalizeType } from '@agent-device/contracts/snapshot'; import type { LocalIdentity, @@ -120,6 +120,27 @@ export function demoteNonUniqueLocalIdentity( return { role, ...(label !== undefined ? { label } : {}) }; } +/** + * Where `nodes` puts the node carrying `node`'s local identity, or nothing when the list cannot + * answer: absent, named by more than one node, or named by a node carrying no rect. Local identity is + * 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. + */ +export function nodeRectByIdentity( + nodes: readonly IdentityPositionedNode[], + node: IdentityPositionedNode, +): Rect | undefined { + const identity = readNodeLocalIdentity(node); + const carriers = nodes.filter((candidate) => + localIdentitiesEqual(identity, readNodeLocalIdentity(candidate)), + ); + return carriers.length === 1 ? carriers[0]?.rect : undefined; +} + +type IdentityPositionedNode = IdentityTreeNode & { rect?: Rect }; + type StructuralNode = Pick; /** Zero-based ordinal among the node's own parent's children, in document order. */ diff --git a/packages/contracts/src/moving-target-gate.test.ts b/packages/contracts/src/moving-target-gate.test.ts new file mode 100644 index 0000000000..1edee0a161 --- /dev/null +++ b/packages/contracts/src/moving-target-gate.test.ts @@ -0,0 +1,188 @@ +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, + TARGET_STILL_MOVING_REASON, + 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 +// 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', () => { + // `targetShownAtRest` is deliberately the disturbing kind here: with no clock there is nothing to + // wait out, and an absent clock must not read as "a mutation just finished". + const decision = movingTargetGateDecision({ + baselineFinishedAtMs: undefined, + targetShownAtRest: false, + nowMs: 10_000, + }); + + assert.deepEqual(decision, { kind: 'idle' }); +}); + +test('a target the previous action left in place is answered without a re-observation', () => { + const decision = movingTargetGateDecision({ + baselineFinishedAtMs: 9_900, + targetShownAtRest: true, + nowMs: 10_000, + }); + + assert.deepEqual(decision, { kind: 'stationary' }); +}); + +test('the window is measured from the mutation END, so only what is LEFT of it is waited out', () => { + // The action finished 400 ms ago; a full window of waiting would be a 900 ms pause on a press + // whose surface has already been quiet for 400 ms. + const decision = movingTargetGateDecision({ + baselineFinishedAtMs: 9_600, + targetShownAtRest: false, + nowMs: 10_000, + }); + + assert.deepEqual(decision, { kind: 'recheck', waitMs: 100 }); +}); + +test('the declared window is the 500 ms transition budget, and its edge decides the branch', () => { + // The edge is exclusive: at exactly the budget the resolved point already stands, because an + // action that finished a full window ago has no claim on this press. + assert.equal(MOVING_TARGET_GATE_WINDOW_MS, 500); + const atEdge = movingTargetGateDecision({ + baselineFinishedAtMs: 10_000 - MOVING_TARGET_GATE_WINDOW_MS, + targetShownAtRest: false, + nowMs: 10_000, + }); + const inside = movingTargetGateDecision({ + baselineFinishedAtMs: 9_600, + targetShownAtRest: false, + nowMs: 10_000, + }); + const outside = movingTargetGateDecision({ + baselineFinishedAtMs: 9_400, + targetShownAtRest: false, + nowMs: 10_000, + }); + + assert.deepEqual(atEdge, { kind: 'not-recent' }); + assert.equal(inside.kind, 'recheck'); + assert.equal(outside.kind, 'not-recent'); +}); + +test('inside the window, only a tree that shows the target at rest excuses the re-observation', () => { + // One bit carries the whole screening verdict, because that is all this branch can use: absent, + // ambiguous, and moved all mean the same thing here — the session cannot show where the target + // came to rest, so it owes the look. + for (const targetShownAtRest of [false, true] as const) { + const decision = movingTargetGateDecision({ + baselineFinishedAtMs: 9_950, + targetShownAtRest, + nowMs: 10_000, + }); + + assert.equal( + decision.kind, + targetShownAtRest ? 'stationary' : 'recheck', + `targetShownAtRest ${String(targetShownAtRest)}`, + ); + } +}); + +test('the wait never drops below one sample gap, even as the window closes', () => { + // Re-observing immediately would compare two captures the same animation frame could produce, + // and agreement would then be a timing accident rather than the target having stopped. + const decision = movingTargetGateDecision({ + baselineFinishedAtMs: 9_520, + targetShownAtRest: false, + nowMs: 10_000, + }); + + assert.deepEqual(decision, { kind: 'recheck', waitMs: MOVING_TARGET_GATE_SAMPLE_GAP_MS }); +}); + +test('a clock skew ahead of the mutation does not produce a negative wait', () => { + const decision = movingTargetGateDecision({ + baselineFinishedAtMs: 10_100, + targetShownAtRest: false, + nowMs: 10_000, + }); + + assert.ok(decision.kind === 'recheck'); + 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('the reason is the one shared constant, not a per-producer literal', () => { + assert.equal(TARGET_STILL_MOVING_REASON, 'target_still_moving'); +}); + +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 '/); +}); diff --git a/packages/contracts/src/moving-target-gate.ts b/packages/contracts/src/moving-target-gate.ts new file mode 100644 index 0000000000..51bed8829d --- /dev/null +++ b/packages/contracts/src/moving-target-gate.ts @@ -0,0 +1,135 @@ +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. + * + * 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. + */ + +/** How long after a mutation its target positions stay owed a second observation. */ +export const MOVING_TARGET_GATE_WINDOW_MS = 500; + +/** + * How far apart the two observations must be for agreeing to mean "at rest". Below one capture + * round trip, two observations of a still-moving target agree by timing rather than by the target + * having stopped, and the gate would certify a position it never measured. The gate therefore waits + * `max(remaining window, this)` before re-observing. + */ +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. + * + * A {@link SurfaceScopedNodes} rather than a whole capture: the gate compares node positions on one + * surface, and the surface scoping is what keeps an iOS system-sheet tree from ever being compared + * against app content (#2438). + */ +export type MovingTargetBaseline = Readonly<{ + /** + * When the action finished, not when it began: the gate waits out what is LEFT of the window. The + * daemon stamps it with `Date.now()`; the gate compares it against `now(runtime)`, so a runtime + * that installs a clock of its own (a scenario pinning the window) states this value in that + * clock's time too. Production installs none, so both sides read the wall clock. + */ + finishedAt: number; + /** The pre-action tree — the observation the action's own resolution came out of. */ + tree: SurfaceScopedNodes; +}>; + +export type MovingTargetGateDecision = + /** The session holds no baseline: nothing has resolved a mutation recently enough to matter. */ + | { kind: 'idle' } + /** The last resolving mutation finished at or beyond the window ago; the resolved point stands. */ + | { kind: 'not-recent' } + /** Inside the window, but the pre-action tree already shows this node at the resolved rect. */ + | { kind: 'stationary' } + /** Inside the window and disturbed: re-observe at the end of it before dispatching. */ + | { kind: 'recheck'; waitMs: number }; + +/** + * Whether a resolved rect is still owed a second observation, from the session's mutation clock and + * its pre-action tree's verdict on the resolved node. A missing verdict (no baseline tree) can only + * mean "not shown at rest", so it rechecks. + */ +export function movingTargetGateDecision(params: { + /** When the session's last resolving mutation finished; `undefined` when the session is idle. */ + baselineFinishedAtMs: number | undefined; + /** Whether the action's own pre-action tree already shows the resolved node at this rect. */ + targetShownAtRest: boolean; + nowMs: number; +}): MovingTargetGateDecision { + if (params.baselineFinishedAtMs === undefined) return { kind: 'idle' }; + const ageMs = params.nowMs - params.baselineFinishedAtMs; + if (ageMs >= MOVING_TARGET_GATE_WINDOW_MS) return { kind: 'not-recent' }; + if (params.targetShownAtRest) return { kind: 'stationary' }; + return { + kind: 'recheck', + 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 one reason a press refuses because it cannot name a position its target is at rest in. */ +export const TARGET_STILL_MOVING_REASON = 'target_still_moving'; + +/** + * The hint published beside every {@link TARGET_STILL_MOVING_REASON} refusal — the recovery that + * works, owned here so no producer can publish the reason without it. + * + * 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/contracts/src/runtime-contract.ts b/packages/contracts/src/runtime-contract.ts index f35d994f71..4e006945d4 100644 --- a/packages/contracts/src/runtime-contract.ts +++ b/packages/contracts/src/runtime-contract.ts @@ -1,5 +1,6 @@ import type { AgentDeviceBackend } from './backend.ts'; import type { ArtifactAdapter } from './artifact-adapter.ts'; +import type { MovingTargetBaseline } from './moving-target-gate.ts'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; export type CommandPolicy = { @@ -23,6 +24,14 @@ export type CommandSessionRecord = { * pre-frame sessions and non-daemon runtimes, which resolve against `snapshot`. */ refFrameSnapshot?: SnapshotState; + /** + * #3471: the tree the session's last resolving mutation acted through, with the instant that + * mutation finished. The moving-target gate screens a freshly resolved rect against it to decide + * whether the position is still owed a re-observation, which is why the pre-action tree — and not + * just a timestamp — has to cross this boundary: the session's newest observation is by + * construction the one that cannot prove a target held its position. + */ + movingTargetBaseline?: MovingTargetBaseline; metadata?: Record; }; diff --git a/packages/selectors/src/interaction-error.ts b/packages/selectors/src/interaction-error.ts index 888e3aa2dd..b862b31767 100644 --- a/packages/selectors/src/interaction-error.ts +++ b/packages/selectors/src/interaction-error.ts @@ -1,3 +1,4 @@ +import { TARGET_STILL_MOVING_REASON } from '@agent-device/contracts/moving-target-gate'; import type { Rect, SnapshotCoveringNode, SnapshotNode } from '@agent-device/kernel/snapshot'; /** Machine-readable `error.details.reason` values shared by interaction producers and adapters. */ @@ -22,6 +23,15 @@ export const INTERACTION_ERROR_REASONS = { captureSparse: 'capture_sparse', /** 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 it was resolved and `details.movedToRect` where the 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 `moving-target-gate.ts` in this + * package, which raises the reason beside the hint that names that recovery. + */ + targetStillMoving: TARGET_STILL_MOVING_REASON, } as const; /** The part of a covered node a refusal reports. */ diff --git a/src/commands/interaction/runtime/interactions.ts b/src/commands/interaction/runtime/interactions.ts index 85d502e458..7b53b2aa04 100644 --- a/src/commands/interaction/runtime/interactions.ts +++ b/src/commands/interaction/runtime/interactions.ts @@ -20,8 +20,12 @@ import { type PostActionObservationOptions, } from './post-action-observation.ts'; import { resolveInteractionTarget, type InteractionTarget } from './resolution.ts'; +import { guardResolvedTargetAgainstMotion } from './moving-target-gate.ts'; import { dispatchNativeRefInteraction } from './native-ref-interaction.ts'; -import type { ExpectedResolvedTarget } from './interaction-resolution-request.ts'; +import type { + ExpectedResolvedTarget, + ResolveInteractionTargetParams, +} from './interaction-resolution-request.ts'; export { focusCommand, hoverCommand, longPressCommand } from './gestures.ts'; export type { @@ -147,7 +151,9 @@ async function tapCommand( : await maybeTapRefTarget(runtime, options, action); if (nativeRefTap) return nativeRefTap; - const resolved = await resolveInteractionTarget(runtime, options, { + // One request object for the resolution and for the re-resolutions #3471's gate may spend, so the + // second look is the same question asked again rather than a differently-configured one. + const resolutionRequest: ResolveInteractionTargetParams = { action, requireInteractive: true, pipeline: SELECTOR_PIPELINE_POLICIES.promotedTarget, @@ -155,11 +161,21 @@ async function tapCommand( expectedResolvedTarget: options.expectedResolvedTarget, preresolvedTarget: options.preresolvedTarget, readinessTimeoutMs: options.readinessTimeoutMs, - }); + }; + const resolved = await resolveInteractionTarget(runtime, options, resolutionRequest); if (!runtime.backend.tap) { throw new AppError('UNSUPPORTED_OPERATION', 'tap is not supported by this backend'); } - const point = requireResolvedPoint(resolved); + // The moving-target gate (#3471): after every pre-dispatch guard has accepted the target and + // before the device is touched. A press issued straight after an action that animated the UI can + // otherwise resolve a point the control has already vacated and report success on the vacancy. + const atRest = await guardResolvedTargetAgainstMotion( + runtime, + options, + resolutionRequest, + resolved, + ); + const point = requireResolvedPoint(atRest); let backendResult; try { backendResult = await runtime.backend.tap(toBackendContext(runtime, options), point, { @@ -174,16 +190,16 @@ async function tapCommand( // Resolution is complete before the backend call. Preserve it out of // band so a daemon-level failure corroboration can still record the same // target identity if the backend reports an ambiguous tap outcome. - attachResolvedInteractionTarget(error, resolved); + attachResolvedInteractionTarget(error, atRest); throw error; } const formattedBackendResult = toBackendResult(backendResult); return await applyPostActionObservation( runtime, options, - resolved, + atRest, { - ...resolved, + ...atRest, ...(formattedBackendResult ? { backendResult: formattedBackendResult } : {}), }, observation, diff --git a/src/commands/interaction/runtime/moving-target-gate.test.ts b/src/commands/interaction/runtime/moving-target-gate.test.ts new file mode 100644 index 0000000000..fbf8463fc5 --- /dev/null +++ b/src/commands/interaction/runtime/moving-target-gate.test.ts @@ -0,0 +1,285 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import type { AgentDeviceBackend } from '@agent-device/contracts/backend'; +import type { PreresolvedInteractionTarget } from '@agent-device/contracts/interaction'; +import type { MovingTargetBaseline } from '@agent-device/contracts/moving-target-gate'; +import type { Point, SnapshotState } from '@agent-device/kernel/snapshot'; +import { readNodeLocalIdentity, readNodeStructuralDenotation } from '@agent-device/ad-script'; +import { makeSnapshotState } from '@agent-device/selectors/snapshot-geometry-fixtures'; +import { INTERACTION_ERROR_REASONS } from '@agent-device/selectors/interaction-error'; +import { createLocalArtifactAdapter } from '../../../io.ts'; +import { createAgentDevice } from '../../../runtime.ts'; +import { localCommandPolicy } from '@agent-device/contracts/command-policy'; +import { createMemorySessionStore } from '../../../command-runtime/runtime-factory.ts'; +import { ref, selector } from './selector-read-utils.ts'; +import { createFakeClock } from './__tests__/settle-device-fixtures.ts'; + +// #3471, the gate's own claims: what a press pays when the session is idle, and which callers are +// never asked to pay anything. The accepted re-aim and the typed refusal are proven end-to-end by +// the ADR 0011 `targetStability` scenarios under test/integration/interaction-contract/ (both paths, +// on the real command surface), so what stays here is the accounting and the exemptions — the cases +// where re-resolving would replace one guard's evidence with another observation's. + +const MID_FLIGHT_X = 240; +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 centerOf(x: number): Point { + return { x: x + 60, y: BUTTON_Y + 25 }; +} + +/** + * A press harness with a mutation history: `trees` answers captures in order, and `baseline` is the + * session state a just-finished mutation leaves behind. `ageMs` is stated relative to the scenario + * clock, which is the only clock the gate reads. + */ +function createGuardedDevice(params: { + stored: SnapshotState; + trees: SnapshotState[]; + baselineAgeMs?: number; + baselineTree?: SnapshotState; + preresolvedTarget?: PreresolvedInteractionTarget; + expectedResolvedTarget?: boolean; + /** The daemon-only ADR 0014 authorized frame: the tree a `@ref` resolves against for identity. */ + refFrame?: SnapshotState; +}): { + press: (target: 'selector' | 'ref') => Promise; + pressPoint: () => Promise; + captures: () => number; + taps: () => Point[]; +} { + const clock = createFakeClock(); + let captureCount = 0; + const taps: Point[] = []; + const baselineTree = params.baselineTree ?? slideTree(MID_FLIGHT_X); + const baseline: MovingTargetBaseline | undefined = + params.baselineAgeMs === undefined + ? undefined + : { + finishedAt: clock.now() - params.baselineAgeMs, + tree: { nodes: baselineTree.nodes }, + }; + const device = createAgentDevice({ + backend: { + platform: 'android', + captureSnapshot: async () => { + const snapshot = params.trees[Math.min(captureCount, params.trees.length - 1)]!; + captureCount += 1; + return { snapshot }; + }, + tap: async (_context, point) => { + taps.push(point); + return { ok: true }; + }, + } satisfies AgentDeviceBackend, + artifacts: createLocalArtifactAdapter(), + sessions: createMemorySessionStore([ + { + name: 'default', + snapshot: params.stored, + ...(params.refFrame ? { refFrameSnapshot: params.refFrame } : {}), + ...(baseline ? { movingTargetBaseline: baseline } : {}), + }, + ]), + policy: localCommandPolicy(), + clock, + }); + + async function press(target: 'selector' | 'ref'): Promise { + await device.interactions.press( + target === 'selector' ? selector('label="Slide target"') : ref('@e2'), + { + session: 'default', + ...(params.preresolvedTarget ? { preresolvedTarget: params.preresolvedTarget } : {}), + ...(params.expectedResolvedTarget + ? { + expectedResolvedTarget: { + identity: readNodeLocalIdentity(slideTree(MID_FLIGHT_X).nodes[1]!), + structural: readNodeStructuralDenotation( + slideTree(MID_FLIGHT_X).nodes[1]!, + slideTree(MID_FLIGHT_X).nodes, + ), + }, + } + : {}), + }, + ); + return taps; + } + + return { + press, + pressPoint: async () => { + await device.interactions.press( + { kind: 'point', x: centerOf(RESTING_X).x, y: centerOf(RESTING_X).y }, + { session: 'default' }, + ); + return taps; + }, + captures: () => captureCount, + taps: () => taps, + }; +} + +// The disturbing history every exemption below has to survive: a mutation that just finished, with +// a pre-action tree that does not show the target at the position being resolved. +const DISTURBING_HISTORY = { baselineAgeMs: 100, baselineTree: slideTree(-400) }; + +test('a session with no mutation history pays the gate nothing', async () => { + const device = createGuardedDevice({ + stored: slideTree(RESTING_X), + trees: [slideTree(RESTING_X)], + }); + + assert.deepEqual(await device.press('selector'), [centerOf(RESTING_X)]); + assert.equal(device.captures(), 1, 'the resolution capture, and nothing from the gate'); +}); + +test('a mutation outside the window leaves the resolved point standing', async () => { + const device = createGuardedDevice({ + stored: slideTree(MID_FLIGHT_X), + trees: [slideTree(MID_FLIGHT_X), slideTree(RESTING_X)], + baselineAgeMs: 5_000, + baselineTree: slideTree(-400), + }); + + assert.deepEqual(await device.press('selector'), [centerOf(MID_FLIGHT_X)]); + assert.equal(device.captures(), 1, 'no re-observation once the window has closed'); +}); + +test('the @ref path takes its own observations, because its resolution spends none', async () => { + // ADR 0014 binds a ref to its authorized frame, so re-asking it would otherwise hand back the same + // coordinates twice and the gate would certify a position it never re-measured. Two observations + // is the claim; where they send the tap is the runtime-ref `targetStability` scenario's claim. + const device = createGuardedDevice({ + stored: slideTree(MID_FLIGHT_X), + trees: [slideTree(RESTING_X), slideTree(RESTING_X)], + ...DISTURBING_HISTORY, + }); + + assert.deepEqual(await device.press('ref'), [centerOf(RESTING_X)]); + assert.equal(device.captures(), 2, 'both re-observations, none for the first resolution'); +}); + +test('a raw-coordinate press has no resolved element to re-observe', async () => { + const device = createGuardedDevice({ + stored: slideTree(RESTING_X), + trees: [slideTree(RESTING_X)], + ...DISTURBING_HISTORY, + }); + + assert.deepEqual(await device.pressPoint(), [centerOf(RESTING_X)]); + assert.equal(device.captures(), 0, 'a point press neither captures nor re-observes'); +}); + +test('a replay step keeps its own identity guard as the last word on the resolved node', async () => { + // ADR 0012 step 4 judges THIS resolution against the verification tree. Re-resolving would swap + // that evidence for a different observation, so the gate stays out and the press dispatches at the + // point the replay step's guard accepted. + const device = createGuardedDevice({ + stored: slideTree(MID_FLIGHT_X), + trees: [slideTree(MID_FLIGHT_X), slideTree(RESTING_X)], + ...DISTURBING_HISTORY, + expectedResolvedTarget: true, + }); + + assert.deepEqual(await device.press('selector'), [centerOf(MID_FLIGHT_X)]); + assert.equal(device.captures(), 1); +}); + +test("a mutating find's carried node is not second-guessed by position", async () => { + // #1654: the node came out of the locator's own capture, which is the evidence its caller has. + const carried = slideTree(MID_FLIGHT_X); + const device = createGuardedDevice({ + stored: carried, + trees: [carried, slideTree(RESTING_X)], + ...DISTURBING_HISTORY, + preresolvedTarget: { ref: '@e2', node: carried.nodes[1]!, nodes: carried.nodes }, + }); + + assert.deepEqual(await device.press('ref'), [centerOf(MID_FLIGHT_X)]); + assert.equal(device.captures(), 0, 'neither the lookup nor a re-observation happens'); +}); + +test('a ref the second observation cannot re-measure is refused, never self-agreed', async () => { + // ADR 0014 lets a ref fall back to its authorized frame node when a newer observation does not + // carry the same local identity — a positional coincidence must not retarget it. The gate has to + // survive that: the frame's own rect is the one thing a rest judgement cannot be made from, since + // it is the position the whole window exists to doubt. Here both looks land on the frame rect, so + // a gate that mistook the fallback for an observation would "confirm" rest and dispatch. + const frame = slideTree(RESTING_X); + const device = createGuardedDevice({ + stored: frame, + refFrame: frame, + trees: [ + // The first re-observation names the same identity and is a real measurement; the second finds + // a screen that names it nowhere, so resolution hands back the frame node. + slideTree(RESTING_X), + makeSnapshotState([ + { + index: 0, + depth: 0, + type: 'StaticText', + label: 'Done', + rect: { x: 0, y: 0, width: 100, height: 40 }, + }, + ]), + ], + ...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); + assert.equal(details?.dispatched, 'no'); + return true; + }, + ); + assert.deepEqual(device.taps(), [], 'an unmeasured position licenses no 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 + // fresh judgement rather than the same one. + const device = createGuardedDevice({ + stored: slideTree(MID_FLIGHT_X), + // The observation the re-observation takes no longer names the target at all. + trees: [makeSnapshotState([])], + ...DISTURBING_HISTORY, + }); + + await assert.rejects( + () => device.press('ref'), + (error: unknown) => { + const details = (error as { details?: Record }).details; + assert.notEqual(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving); + return true; + }, + ); + assert.deepEqual(device.taps(), []); +}); diff --git a/src/commands/interaction/runtime/moving-target-gate.ts b/src/commands/interaction/runtime/moving-target-gate.ts new file mode 100644 index 0000000000..2f079a4055 --- /dev/null +++ b/src/commands/interaction/runtime/moving-target-gate.ts @@ -0,0 +1,221 @@ +import { + MOVING_TARGET_GATE_SAMPLE_GAP_MS, + TARGET_STILL_MOVING_REASON, + movingTargetGateDecision, + movingTargetRectsStable, + targetStillMovingHint, + type MovingTargetGateDecision, +} from '@agent-device/contracts/moving-target-gate'; +import type { ResolvedInteractionTarget } from '@agent-device/contracts/interaction'; +import type { Rect, SnapshotNode } from '@agent-device/kernel/snapshot'; +import { AppError, discloseDispatch } from '@agent-device/kernel/errors'; +import type { + AgentDeviceRuntime, + CommandContext, + CommandSessionRecord, +} from '@agent-device/contracts/runtime-contract'; +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 type { ResolveInteractionTargetParams } from './interaction-resolution-request.ts'; + +/** + * The moving-target gate (#3471): the seam that spends the window, in front of the tap and behind + * every pre-dispatch guard that already judged the target (occlusion, off-screen, keyboard), so it + * re-decides none of them. The decision and its budget live in + * `@agent-device/contracts/moving-target-gate`; what lives here is the part that can only be done + * with a runtime — taking the second and third observations, and putting the tap at the position two + * of them agree on. + * + * Agreement dispatches rather than refuses: a control that settled inside the window is the tap the + * caller asked for, at where it ended up. The refusal is reserved for a target the window could not + * settle, where a coordinate would be a guess. + * + * Re-resolving rather than re-pointing is deliberate: the recheck spends the whole resolution path, + * so every shared guard re-runs against the fresh observation and the response's identity fields + * describe the node actually dispatched at. + */ +export async function guardResolvedTargetAgainstMotion( + runtime: AgentDeviceRuntime, + options: CommandContext & { target: InteractionTarget }, + params: ResolveInteractionTargetParams, + resolved: ResolvedInteractionTarget, +): Promise { + // A raw-coordinate press has no resolved element that could be at rest, and a target the capture + // gave no measurable rect for has already met the guard that owns that case. + if (resolved.kind === 'point') return resolved; + if (!resolved.node?.rect) return resolved; + // Two guards own the identity of what they already resolved and refuse a second look at it: a + // replay step's post-resolution identity guard (ADR 0012 step 4) judges THIS resolution against its + // own verification tree, and a mutating `find`'s carried node (#1654) is the node its locator just + // 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); + if (!gate) return resolved; + return await resolveAtRestTarget(runtime, options, params, resolved, gate); +} + +/** A press owed a re-observation: the recheck decision, which carries the wait it owes. */ +type MovingTargetRecheck = Extract; + +/** + * The gate's idle-path cost, stated as code: one session-record read, no capture. A press whose + * target the last action's own tree already shows at the resolved rect is `stationary`, and one + * taken after the window has closed is `not-recent`; both dispatch at the point already resolved. + */ +async function readMovingTargetGate( + runtime: AgentDeviceRuntime, + options: CommandContext, + resolvedNode: SnapshotNode, +): 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), + nowMs: now(runtime), + }); + return decision.kind === 'recheck' ? decision : undefined; +} + +/** + * 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 + * about. + */ +async function readGateBaseline( + runtime: AgentDeviceRuntime, + options: CommandContext, +): Promise { + const session: CommandSessionRecord | undefined = await runtime.sessions.get( + options.session ?? 'default', + ); + return session?.movingTargetBaseline; +} + +/** + * Re-resolves past the end of the window and one sample gap later; agreement is the resting + * position. A re-resolution that fails outright is left to propagate: the resolution path's own typed + * refusal (`selector_not_found`, `ref_not_found`, both `dispatched: 'no'`) describes a target the + * wait lost more accurately than a motion refusal would, and the window has by then passed, so the + * caller's retry is a fresh judgement rather than the same one. + */ +async function resolveAtRestTarget( + runtime: AgentDeviceRuntime, + options: CommandContext & { target: InteractionTarget }, + params: ResolveInteractionTargetParams, + resolved: Exclude, + gate: MovingTargetRecheck, +): Promise { + // Measured, not arithmetic: the refused press reports the wait it actually paid, which is what a + // caller tuning `--settle` against this error needs. + 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); + 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, + ); +} + +/** One look at the target, and whether that look actually measured the surface. */ +type MovingTargetObservation = { + resolved: ResolvedInteractionTarget; + rect: Rect | undefined; + /** + * 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. + */ + measured: boolean; +}; + +/** + * One observation of the same target, asked as the original question rather than a narrower one. + * + * A selector re-resolution re-observes by itself — capture is the first thing its resolution spends. + * 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. + */ +async function reObserveTarget( + runtime: AgentDeviceRuntime, + options: CommandContext & { target: InteractionTarget }, + params: ResolveInteractionTargetParams, +): Promise { + if (options.target.kind !== 'ref') { + const resolved = await resolveInteractionTarget(runtime, options, params); + return { resolved, rect: resolvedRect(resolved), measured: true }; + } + 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), + }; +} + +function resolvedRect(resolved: ResolvedInteractionTarget): Rect | undefined { + return 'node' in resolved ? resolved.node?.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. + */ +function motionRefusal( + resolved: Exclude, + disagreement: { firstRect: Rect; secondRect: Rect | undefined } | undefined, + waitedMs: number, +): Error { + const selector = resolved.target.kind === 'selector' ? resolved.target.selector : undefined; + const ref = resolved.target.kind === 'ref' ? resolved.target.ref : undefined; + return discloseDispatch( + new AppError( + 'COMMAND_FAILED', + selector + ? `Selector ${selector} is still moving, so no resting position could be confirmed to press` + : `Ref ${ref} is still moving, so no resting position could be confirmed to press`, + { + reason: TARGET_STILL_MOVING_REASON, + ...(selector !== undefined ? { selector } : { ref }), + ...(resolved.node?.rect ? { rect: resolved.node.rect } : {}), + ...(disagreement?.secondRect ? { movedToRect: disagreement.secondRect } : {}), + waitedMs, + hint: targetStillMovingHint({ selector }), + }, + ), + 'no', + ); +} From 113ae5e104c686d2a7af1568b35b96520736ec29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 12 Oct 2026 18:51:55 +0200 Subject: [PATCH 2/3] chore(gates): export the moving-target decision module (#3471) --- packages/contracts/package.json | 4 ++++ scripts/layering/contracts-exports.snapshot.json | 1 + 2 files changed, 5 insertions(+) diff --git a/packages/contracts/package.json b/packages/contracts/package.json index b4f956b462..123e828a63 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -316,6 +316,10 @@ "types": "./src/managed-web-backend.ts", "default": "./src/managed-web-backend.ts" }, + "./moving-target-gate": { + "types": "./src/moving-target-gate.ts", + "default": "./src/moving-target-gate.ts" + }, "./navigation": { "types": "./src/navigation.ts", "default": "./src/navigation.ts" diff --git a/scripts/layering/contracts-exports.snapshot.json b/scripts/layering/contracts-exports.snapshot.json index 7fb3fb7798..c0313775b4 100644 --- a/scripts/layering/contracts-exports.snapshot.json +++ b/scripts/layering/contracts-exports.snapshot.json @@ -76,6 +76,7 @@ "@agent-device/contracts/local-interactor-operation-set", "@agent-device/contracts/logs-runtime-plan", "@agent-device/contracts/managed-web-backend", + "@agent-device/contracts/moving-target-gate", "@agent-device/contracts/navigation", "@agent-device/contracts/network-runtime", "@agent-device/contracts/network-runtime-plan", From c4d517f60be5fa6f215f47b2d242bc78bafa69fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 12 Oct 2026 22:20:41 +0200 Subject: [PATCH 3/3] fix(selectors): keep the moving-target reason in the vocabulary the adapters read (#3471) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The eager-closure gate (ADR 0027, `scripts/__tests__/eager-closure-budgets.test.ts`) holds every façade that reads an interaction reason to "no more modules than the merge-base evaluated". Importing `TARGET_STILL_MOVING_REASON` from `@agent-device/contracts/moving-target-gate` pulled that module and its kernel dependencies into six closures, `packages/maestro/src/daemon-port/daemon-runtime-port.ts` and `packages/replay-port/src/daemon-port/session-replay-maestro-runtime.ts` among them. The reason is a literal in `INTERACTION_ERROR_REASONS` again, the one vocabulary every adapter already reads, so the leaf imports nothing; the hint stays in contracts, which is on no façade path. --- .../contracts/src/moving-target-gate.test.ts | 5 ----- packages/contracts/src/moving-target-gate.ts | 11 +++++----- packages/selectors/src/interaction-error.ts | 21 ++++++++++++------- .../interaction/runtime/moving-target-gate.ts | 4 ++-- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/packages/contracts/src/moving-target-gate.test.ts b/packages/contracts/src/moving-target-gate.test.ts index 1edee0a161..3f635181a1 100644 --- a/packages/contracts/src/moving-target-gate.test.ts +++ b/packages/contracts/src/moving-target-gate.test.ts @@ -4,7 +4,6 @@ import { MOVING_TARGET_GATE_RECT_TOLERANCE_PX, MOVING_TARGET_GATE_SAMPLE_GAP_MS, MOVING_TARGET_GATE_WINDOW_MS, - TARGET_STILL_MOVING_REASON, movingTargetGateDecision, movingTargetRectsStable, targetStillMovingHint, @@ -163,10 +162,6 @@ test('the stability comparison is symmetric', () => { // --- the refusal's published recovery --- -test('the reason is the one shared constant, not a per-producer literal', () => { - assert.equal(TARGET_STILL_MOVING_REASON, 'target_still_moving'); -}); - test('a refused selector press names the wait that recovers it', () => { const hint = targetStillMovingHint({ selector: 'label="Slide target"' }); diff --git a/packages/contracts/src/moving-target-gate.ts b/packages/contracts/src/moving-target-gate.ts index 51bed8829d..055053d847 100644 --- a/packages/contracts/src/moving-target-gate.ts +++ b/packages/contracts/src/moving-target-gate.ts @@ -112,12 +112,13 @@ export function movingTargetRectsStable( ); } -/** The one reason a press refuses because it cannot name a position its target is at rest in. */ -export const TARGET_STILL_MOVING_REASON = 'target_still_moving'; - /** - * The hint published beside every {@link TARGET_STILL_MOVING_REASON} refusal — the recovery that - * works, owned here so no producer can publish the reason without it. + * 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 diff --git a/packages/selectors/src/interaction-error.ts b/packages/selectors/src/interaction-error.ts index b862b31767..3e73b71a90 100644 --- a/packages/selectors/src/interaction-error.ts +++ b/packages/selectors/src/interaction-error.ts @@ -1,7 +1,13 @@ -import { TARGET_STILL_MOVING_REASON } from '@agent-device/contracts/moving-target-gate'; import type { Rect, SnapshotCoveringNode, SnapshotNode } from '@agent-device/kernel/snapshot'; -/** Machine-readable `error.details.reason` values shared by interaction producers and adapters. */ +/** + * Machine-readable `error.details.reason` values shared by interaction producers and adapters. + * + * Every value is a literal, and this module imports nothing: these codes are the vocabulary the + * Maestro, replay, and host adapters read, and the eager-closure gate (`scripts/__tests__/ + * eager-closure-budgets.test.ts`) holds this leaf at the size it has — a shared constant imported + * from a heavier package makes every façade that reads a reason evaluate that package. + */ export const INTERACTION_ERROR_REASONS = { selectorNotFound: 'selector_not_found', predicateFailed: 'predicate_failed', @@ -26,12 +32,13 @@ export const INTERACTION_ERROR_REASONS = { /** * 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 it was resolved and `details.movedToRect` where the 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 `moving-target-gate.ts` in this - * package, which raises the reason beside the hint that names that recovery. + * 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 + * `src/commands/interaction/runtime/moving-target-gate.ts`, which raises the reason beside the + * hint that names that recovery. */ - targetStillMoving: TARGET_STILL_MOVING_REASON, + targetStillMoving: 'target_still_moving', } as const; /** The part of a covered node a refusal reports. */ diff --git a/src/commands/interaction/runtime/moving-target-gate.ts b/src/commands/interaction/runtime/moving-target-gate.ts index 2f079a4055..e57cbafb36 100644 --- a/src/commands/interaction/runtime/moving-target-gate.ts +++ b/src/commands/interaction/runtime/moving-target-gate.ts @@ -1,11 +1,11 @@ import { MOVING_TARGET_GATE_SAMPLE_GAP_MS, - TARGET_STILL_MOVING_REASON, movingTargetGateDecision, movingTargetRectsStable, targetStillMovingHint, 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 { Rect, SnapshotNode } from '@agent-device/kernel/snapshot'; import { AppError, discloseDispatch } from '@agent-device/kernel/errors'; @@ -208,7 +208,7 @@ function motionRefusal( ? `Selector ${selector} is still moving, so no resting position could be confirmed to press` : `Ref ${ref} is still moving, so no resting position could be confirmed to press`, { - reason: TARGET_STILL_MOVING_REASON, + reason: INTERACTION_ERROR_REASONS.targetStillMoving, ...(selector !== undefined ? { selector } : { ref }), ...(resolved.node?.rect ? { rect: resolved.node.rect } : {}), ...(disagreement?.secondRect ? { movedToRect: disagreement.secondRect } : {}),