diff --git a/packages/contracts/src/interaction-guarantees.ts b/packages/contracts/src/interaction-guarantees.ts index 16e8ece06c..59da90fb15 100644 --- a/packages/contracts/src/interaction-guarantees.ts +++ b/packages/contracts/src/interaction-guarantees.ts @@ -83,6 +83,14 @@ export const INTERACTION_GUARANTEES = [ // a settled diff). Distinct from the opt-in verifyEvidence/settleObservation features; this is the // path's baseline, always-on claim. 'outcomeObservation', + // The position the path dispatches at is one the target is confirmed to be at REST in (#3471). + // Distinct from every guard above: those judge a tree, this judges the tree against the previous + // action's tree. A capture is one instant's photograph of a surface that may still be rearranging + // itself — the ADB-backed Android helper is measured reporting frames in flight — so a press + // issued right after an action that animated the UI can resolve a point the target has already + // vacated and report success on the vacancy. The gate is backend-neutral: it re-observes past the + // window and refuses rather than dispatching at a position two observations disagree about. + 'targetStability', ] as const; export type InteractionGuarantee = (typeof INTERACTION_GUARANTEES)[number]; @@ -151,6 +159,21 @@ export type InteractionPathContract = { const GAPS_UMBRELLA_ISSUE = 'https://github.com/callstack/agent-device/issues/1081'; const TAP_OUTCOME_NOT_OBSERVED_GAP_ISSUE = 'https://github.com/callstack/agent-device/issues/3335'; +/** + * The moving-target gate's shared cell (#3471). Scoped to the two commands that dispatch a tap the + * daemon aimed at a rect it captured: `fill` aims at a field it expects to be found wherever it + * is, and its own staleness failure mode is a wrong focus rather than a silent miss, which the + * guard's re-resolve would not fix. `longpress` aims the same way and is deliberately left out for + * now: a hold that begins one frame early is a gesture the recognizer still owns, the measured + * failure and the caller-visible recovery are both tap-shaped, and widening this list is one + * declaration plus one scenario per added command. + */ +const RUNTIME_TREE_TARGET_STABILITY: GuaranteeEnforcement = { + kind: 'runtime', + via: 'src/commands/interaction/runtime/moving-target-gate.ts#guardResolvedTargetAgainstMotion', + appliesTo: ['press', 'click'], +}; + // Every path shares the SAME cell by construction: response payloads have one // construction site (ADR 0011 Layer 2), and the hand-rolled-literal guard test // (interaction-response-construction-guard.test.ts) keeps new branches on it. @@ -193,6 +216,16 @@ const DIRECT_IOS_OUTCOME_NOT_OBSERVED_GAP: GuaranteeEnforcement = { trackingIssue: TAP_OUTCOME_NOT_OBSERVED_GAP_ISSUE, }; +/** + * Shares the SAME reasoning as DIRECT_IOS_SINGLE_QUERY_READINESS: the fused request is one XCTest + * round trip, so it also cannot carry a stale daemon-captured position into the tap. + */ +const DIRECT_IOS_TARGET_STABILITY: GuaranteeEnforcement = { + kind: 'waived', + reason: + "Intentional: the fused runner request queries the element and taps it inside one XCTest request, so the runner aims at the element's own live frame at tap time rather than at a rect the daemon captured earlier and carried across a round trip — the capture-to-dispatch distance #3471 closes does not exist on this route. Its hittability semantics stay the runner's, as the offscreen cell above already records.", +}; + // The two runtime tree paths (selector and ref resolution) run the SAME shared // guard/observation implementations; only how the target is found // (disambiguation) and how failures are described (errorTaxonomy) differ. @@ -260,6 +293,7 @@ const RUNTIME_TREE_SHARED_GUARANTEES = { kind: 'runtime', via: 'src/commands/interaction/runtime/settle.ts#settleAfterInteraction', }, + targetStability: RUNTIME_TREE_TARGET_STABILITY, } satisfies Partial>; export const INTERACTION_DISPATCH_PATHS: Record = { @@ -390,6 +424,11 @@ export const INTERACTION_DISPATCH_PATHS: Record; diff --git a/scripts/layering/session-state.ts b/scripts/layering/session-state.ts index 1388abc843..e3d97159d9 100644 --- a/scripts/layering/session-state.ts +++ b/scripts/layering/session-state.ts @@ -51,7 +51,10 @@ export const SESSION_STATE_FIELD_OWNERS: Readonly { diff --git a/src/daemon/__tests__/direct-ios-selector.test.ts b/src/daemon/__tests__/direct-ios-selector.test.ts index 05c4b28433..120ca4b83f 100644 --- a/src/daemon/__tests__/direct-ios-selector.test.ts +++ b/src/daemon/__tests__/direct-ios-selector.test.ts @@ -119,7 +119,7 @@ test('isLocalIosRunnerSession: iOS local sessions are eligible, Android and unde test('isLocalIosRunnerSession: skipPendingPostGestureStabilization:true excludes a pending session (the tap fast path)', () => { const pending = makeSession('ios', { - postGestureStabilization: { action: 'scroll', positionals: [], markedAt: Date.now() }, + deferredSurface: { kind: 'stabilize', action: 'scroll', positionals: [], markedAt: Date.now() }, }); assert.equal( isLocalIosRunnerSession(pending, { skipPendingPostGestureStabilization: true }), @@ -129,7 +129,7 @@ test('isLocalIosRunnerSession: skipPendingPostGestureStabilization:true excludes test('isLocalIosRunnerSession: skipPendingPostGestureStabilization:false keeps a pending session eligible (the offscreen double-check)', () => { const pending = makeSession('ios', { - postGestureStabilization: { action: 'scroll', positionals: [], markedAt: Date.now() }, + deferredSurface: { kind: 'stabilize', action: 'scroll', positionals: [], markedAt: Date.now() }, }); assert.equal( isLocalIosRunnerSession(pending, { skipPendingPostGestureStabilization: false }), diff --git a/src/daemon/__tests__/generic-settle.test.ts b/src/daemon/__tests__/generic-settle.test.ts index c745d57cdf..62e887af35 100644 --- a/src/daemon/__tests__/generic-settle.test.ts +++ b/src/daemon/__tests__/generic-settle.test.ts @@ -40,6 +40,7 @@ vi.mock('../interaction/index.ts', async (importOriginal) => { import { captureSnapshotForSession } from '../interaction/index.ts'; import { dispatchGenericCommand } from '../request-generic-dispatch.ts'; +import { pendingSurfaceStabilization } from '../deferred-interaction-outcome.ts'; const mockCaptureSnapshotForSession = vi.mocked(captureSnapshotForSession); @@ -98,7 +99,7 @@ async function emulateCaptureSnapshotForSession( ) { captureObservations.push({ postGestureStabilizationPending: - sessionStore.requireCurrent(ref).postGestureStabilization !== undefined, + pendingSurfaceStabilization(sessionStore.requireCurrent(ref)) !== undefined, }); const effectiveFlags = { ...(flags ?? {}), snapshotInteractiveOnly: options.interactiveOnly }; // Same rule the real seam applies: a request-bound capture is the tree this request reads, and diff --git a/src/daemon/__tests__/moving-target-baseline.test.ts b/src/daemon/__tests__/moving-target-baseline.test.ts new file mode 100644 index 0000000000..91893b9a1b --- /dev/null +++ b/src/daemon/__tests__/moving-target-baseline.test.ts @@ -0,0 +1,303 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import type { SettleObservation, SurfaceScopedNodes } from '@agent-device/contracts/interaction'; +import { makeSnapshotState } from '@agent-device/selectors/snapshot-geometry-fixtures'; +import { + markDeferredInteractionOutcome, + movingTargetBaseline, + retireMovingTargetBaseline, +} from '../deferred-interaction-outcome.ts'; +import { expireRefFrame } from '../ref-frame.ts'; +import type { SessionState } from '../session-state.ts'; +import { makeSession } from './post-gesture-stabilization-fixtures.ts'; + +// #3471, the `recheck` half of the session's deferred surface: what a finished mutation is allowed +// to record about where things were. Everything here is about refusing to record — the gate's whole +// cost contract rests on a session with no trustworthy history reading as idle rather than as a +// surface that just moved. The capture-side `stabilize` variant is the sibling cluster's subject +// (post-gesture-stabilization.test.ts). + +function appTree(label = 'Slide target') { + return makeSnapshotState([ + { index: 0, type: 'Application', label: 'App', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + label, + rect: { x: 10, y: 240, width: 120, height: 50 }, + hittable: true, + }, + ]); +} + +/** Marks a press (a non-gesture action) that resolved its target against `preActionTree`. */ +function markPress(session: SessionState, preActionTree: SurfaceScopedNodes | undefined): void { + markDeferredInteractionOutcome({ + session, + command: 'press', + positionals: ['label="Slide target"'], + flags: undefined, + preActionTree, + }); +} + +test('a mutation that resolved a target records its tree at the instant it finished', () => { + const session = makeSession('android'); + const preAction = appTree(); + const before = Date.now(); + + markPress(session, { nodes: preAction.nodes }); + + const baseline = movingTargetBaseline(session); + assert.ok(baseline); + assert.equal(baseline.tree.nodes, preAction.nodes); + // Bracketed by the reads around the call rather than by a fixed slack: a CI pause between the mark + // and the assertion would blow a "within one second" bound while the recorded instant stayed right. + assert.ok( + baseline.finishedAt >= before && baseline.finishedAt <= Date.now(), + 'the clock is the mutation END: the gate waits out what is LEFT of the window', + ); +}); + +test('a mutation that resolved no tree leaves the session idle', () => { + const session = makeSession('android'); + session.snapshot = appTree(); + + // The routes that dispatch without a daemon tree (a raw coordinate, a Maestro direct-selector tap, + // `back`, an app open) must not invent one: substituting the stored snapshot would attribute to + // this action a surface it never looked at, and make the next press pay a re-observation for it. + markPress(session, undefined); + + assert.equal(movingTargetBaseline(session), undefined); + assert.equal(session.deferredSurface, undefined); +}); + +test('an empty tree records nothing, because it would make every later target read as newly arrived', () => { + const session = makeSession('android'); + + markPress(session, { nodes: [] }); + + assert.equal(movingTargetBaseline(session), undefined); +}); + +test('a baseline from an earlier action is replaced rather than kept beside the new one', () => { + const session = makeSession('android'); + markPress(session, { nodes: appTree('First').nodes }); + const first = movingTargetBaseline(session); + assert.ok(first); + + markPress(session, { nodes: appTree('Second').nodes }); + + const second = movingTargetBaseline(session); + assert.ok(second); + assert.notEqual(second, first); + assert.notEqual(second.tree.nodes, first.tree.nodes); +}); + +test('the baseline is the tree the action acted through, whatever the session now holds', () => { + // The session snapshot may have been replaced by a post-action capture (a `--settle` observation, + // a `--verify` capture) — the better account of where the surface came to rest — but nothing here + // can tell one from a freshness capture or a `@ref` press's authorized frame, because all three + // arrive as a different node array. Adopting it on array identity would screen the next press + // against positions this action never looked at, which is what this marker exists to prevent. So + // the acted-through tree wins and the settled press simply owes its successor a re-observation. + const session = makeSession('android'); + const actedThrough = appTree('Pre-action'); + session.snapshot = appTree('Newer, provenance unknown'); + + markPress(session, { nodes: actedThrough.nodes }); + + assert.equal(movingTargetBaseline(session)?.tree.nodes, actedThrough.nodes); +}); + +test('a baseline keeps the surface its tree came from', () => { + // #2438: an iOS system-sheet tree must never be screened against app content, so the scoping the + // capture carried has to survive into the value the gate reads. + const session = makeSession('ios'); + const sheet = makeSnapshotState( + [{ index: 0, type: 'Button', label: 'Allow', rect: { x: 10, y: 20, width: 80, height: 40 } }], + { iosSystemSurfaceBundleId: 'com.apple.springboard' }, + ); + session.snapshot = sheet; + + // The scope rides through as-is: the capture side narrows its tree with the same shape + // (`surfaceScopedNodes`), so an iOS system-sheet tree must never be screened against app content. + markPress(session, { nodes: sheet.nodes, surfaceBundleId: 'com.apple.springboard' }); + + assert.equal(movingTargetBaseline(session)?.tree.surfaceBundleId, 'com.apple.springboard'); +}); + +test('a device side effect the session did not resolve a target through retires the baseline', () => { + // ADR 0014's seam: the tree on hand now predates a change nobody measured, so screening a later + // press against it would be positional coincidence rather than evidence. A gate out of evidence + // goes idle instead of reconstructing a baseline from another source. + const session = makeSession('android'); + markPress(session, { nodes: appTree().nodes }); + + expireRefFrame(session); + + assert.equal(movingTargetBaseline(session), undefined); +}); + +test('retiring is idempotent and leaves no stale window behind', () => { + const session = makeSession('android'); + markPress(session, { nodes: appTree().nodes }); + + retireMovingTargetBaseline(session); + retireMovingTargetBaseline(session); + + assert.equal(session.deferredSurface, undefined); +}); + +test('retiring never touches a pending capture-side stabilization', () => { + // The seam fires on the way INTO every device op, including the gesture that just marked a + // stabilization. Clearing both variants there would silently drop the quiet-window loop the + // gesture is owed — a scroll would read the tree mid-flight again. + const session = makeSession('ios'); + markDeferredInteractionOutcome({ + session, + command: 'scroll', + positionals: ['down'], + flags: undefined, + preActionTree: { nodes: appTree().nodes }, + }); + + expireRefFrame(session); + + assert.equal(session.deferredSurface?.kind, 'stabilize'); + assert.equal(movingTargetBaseline(session), undefined); +}); + +test('a press never erases a stabilization the capture side is still owed', () => { + // A `@ref` press resolves against its authorized frame and spends no capture, so a scroll's owed + // quiet-window loop is STILL pending when this press finishes (#1542/#1600). Overwriting it with a + // baseline would hand the next capture a plain read and drop the unsettled-tree check the scroll + // paid for. A selector press discharges the loop with its own resolution capture before reaching + // here, so this guard only ever protects the owed case. + const session = makeSession('ios'); + markDeferredInteractionOutcome({ + session, + command: 'scroll', + positionals: ['down'], + flags: undefined, + preActionTree: { nodes: appTree().nodes }, + }); + + markPress(session, { nodes: appTree('Other').nodes }); + + assert.equal(session.deferredSurface?.kind, 'stabilize'); + assert.equal(movingTargetBaseline(session), undefined); +}); + +test('a gesture takes the surface over from a press, rather than joining it', () => { + // One field, one mechanism per mutation: a press that left the surface mid-animation followed by + // a scroll hands that surface to the capture side, which is the stronger of the two waits. + const session = makeSession('android'); + markPress(session, { nodes: appTree().nodes }); + + markDeferredInteractionOutcome({ + session, + command: 'scroll', + positionals: ['down'], + flags: undefined, + preActionTree: { nodes: appTree().nodes }, + }); + + assert.equal(session.deferredSurface?.kind, 'stabilize'); + assert.equal(movingTargetBaseline(session), undefined); +}); + +/** A converged `--settle`. `stored` is the load-bearing axis: the runtime attaches `diff` only when + * the loop converged AND stored its settled tree. */ +function settleMark(params: { stored: boolean }): SettleObservation { + return { + settled: true, + waitedMs: 120, + captures: 3, + quietMs: 50, + timeoutMs: 1_000, + ...(params.stored + ? { diff: { summary: { additions: 1, removals: 0, unchanged: 4 }, lines: [] } } + : {}), + }; +} + +test('a settle that stored its settled tree records that tree, not the tree the press started from', () => { + // The loop stopped because two reads agreed and that tree became the session snapshot, so it IS + // the resting surface — and the follow-up press is contractually supposed to spend a tap and + // nothing else (provider-scenarios/settle-observation.test.ts). + const session = makeSession('android'); + const settled = appTree('Settled'); + session.snapshot = settled; + + markDeferredInteractionOutcome({ + session, + command: 'press', + positionals: ['label="Slide target"'], + flags: undefined, + preActionTree: { nodes: appTree('Pre-action').nodes }, + settle: settleMark({ stored: true }), + }); + + assert.equal(movingTargetBaseline(session)?.tree.nodes, settled.nodes); +}); + +test('a converged settle that stored nothing keeps the acted-through tree', () => { + // `settled: true` is not the whole claim, and neither is a settle that merely ran: a sparse + // capture is refused by the store and a cross-surface transition (#2438) withholds the diff, + // leaving whatever an earlier capture published, and a loop that ran out its budget without two + // reads agreeing says nothing about rest either. Only the stored settled tree may carry a rest + // claim; recording the newest capture otherwise would turn an unsettled surface into evidence of + // quiet. + const session = makeSession('android'); + session.snapshot = appTree('Published by some earlier capture'); + const actedThrough = appTree('Pre-action'); + + markDeferredInteractionOutcome({ + session, + command: 'press', + positionals: ['label="Slide target"'], + flags: undefined, + preActionTree: { nodes: actedThrough.nodes }, + settle: settleMark({ stored: false }), + }); + + assert.equal(movingTargetBaseline(session)?.tree.nodes, actedThrough.nodes); +}); + +test('the trigger is wider than the guard on purpose: any command that acted through a tree records it', () => { + // The ADR 0011 `targetStability` cells scope which commands the GATE refuses to dispatch blind + // (press, click). This marker answers a different question — did a mutation just leave a surface + // that may still be animating — and a `longpress`, `hover`, or `fill` that resolved its target + // through a captured tree can animate the surface exactly as a press does. Narrowing the writer to + // press/click would un-guard the press that follows a `fill` which opened a keyboard, which is the + // failure the issue describes with a different verb in front of it. + const session = makeSession('android'); + const actedThrough = appTree(); + + markDeferredInteractionOutcome({ + session, + command: 'fill', + positionals: ['label="Field"', 'hello'], + flags: undefined, + preActionTree: { nodes: actedThrough.nodes }, + }); + + assert.equal(movingTargetBaseline(session)?.tree.nodes, actedThrough.nodes); +}); + +test('an ineligible action marks nothing at all, leaving the record untouched', () => { + // Eligibility stays per marker: `type` is neither gesture-shaped nor given a resolved tree here, + // so a session that was idle stays idle rather than owing every later press a re-observation. + const session = makeSession('android'); + + markDeferredInteractionOutcome({ + session, + command: 'type', + positionals: ['hello'], + flags: undefined, + }); + + assert.equal(session.deferredSurface, undefined); +}); diff --git a/src/daemon/__tests__/post-gesture-no-effect-claim.test.ts b/src/daemon/__tests__/post-gesture-no-effect-claim.test.ts index 3d89be8d87..03da844c60 100644 --- a/src/daemon/__tests__/post-gesture-no-effect-claim.test.ts +++ b/src/daemon/__tests__/post-gesture-no-effect-claim.test.ts @@ -15,6 +15,7 @@ import { import type { CommandFlags } from '@agent-device/contracts/command'; import { capturePostGestureStabilizedResult, + pendingSurfaceStabilization, markDeferredInteractionOutcome, } from '../deferred-interaction-outcome.ts'; import { formatPostGestureOutcomeWarning } from '@agent-device/capture-kit/post-gesture-stability'; @@ -197,7 +198,7 @@ test('no pre-gesture snapshot means no baseline, no rebase, and no no-effect cla const session = makeSession('ios'); session.snapshot = undefined; // nothing captured before the gesture markPostGestureStabilization(session, 'scroll', ['up']); - assert.equal(session.postGestureStabilization?.baselineSignature, undefined); + assert.equal(pendingSurfaceStabilization(session)?.baselineSignature, undefined); const { result, rebased, vetoed } = await runStabilization(session, () => makeSnapshotState(pickupSnapshot(500).nodes, { diff --git a/src/daemon/__tests__/post-gesture-stabilization.test.ts b/src/daemon/__tests__/post-gesture-stabilization.test.ts index 81cfceca5d..65ca3e8e2b 100644 --- a/src/daemon/__tests__/post-gesture-stabilization.test.ts +++ b/src/daemon/__tests__/post-gesture-stabilization.test.ts @@ -9,6 +9,7 @@ import { import { buildInteractionSurfaceSignature } from '../interaction-surface-signature.ts'; import { capturePostGestureStabilizedResult, + pendingSurfaceStabilization, markDeferredInteractionOutcome, } from '../deferred-interaction-outcome.ts'; import type { SessionState } from '../session-state.ts'; @@ -51,7 +52,7 @@ test('markPostGestureStabilization marks iOS swipe sessions', () => { markPostGestureStabilization(session, 'swipe'); - assert.equal(session.postGestureStabilization?.action, 'swipe'); + assert.equal(pendingSurfaceStabilization(session)?.action, 'swipe'); }); test('markPostGestureStabilization marks Android swipe sessions', () => { @@ -59,7 +60,7 @@ test('markPostGestureStabilization marks Android swipe sessions', () => { markPostGestureStabilization(session, 'swipe'); - assert.equal(session.postGestureStabilization?.action, 'swipe'); + assert.equal(pendingSurfaceStabilization(session)?.action, 'swipe'); }); test('markPostGestureStabilization marks gesture swipe sessions', () => { @@ -67,7 +68,7 @@ test('markPostGestureStabilization marks gesture swipe sessions', () => { markPostGestureStabilization(session, 'gesture', ['swipe', 'left']); - assert.equal(session.postGestureStabilization?.action, 'gesture'); + assert.equal(pendingSurfaceStabilization(session)?.action, 'gesture'); }); test('markPostGestureStabilization honors an explicit opt-out', () => { @@ -75,7 +76,7 @@ test('markPostGestureStabilization honors an explicit opt-out', () => { markPostGestureStabilization(session, 'swipe', [], { postGestureStabilization: false }); - assert.equal(session.postGestureStabilization, undefined); + assert.equal(pendingSurfaceStabilization(session), undefined); }); test('markPostGestureStabilization ignores non-swipe gesture sessions', () => { @@ -83,7 +84,7 @@ test('markPostGestureStabilization ignores non-swipe gesture sessions', () => { markPostGestureStabilization(session, 'gesture', ['pinch', 'in']); - assert.equal(session.postGestureStabilization, undefined); + assert.equal(pendingSurfaceStabilization(session), undefined); }); test('markPostGestureStabilization captures the pre-gesture baseline signature on iOS', () => { @@ -103,10 +104,10 @@ test('markPostGestureStabilization captures the pre-gesture baseline signature o markPostGestureStabilization(session, 'scroll'); assert.deepEqual( - session.postGestureStabilization?.baselineSignature, + pendingSurfaceStabilization(session)?.baselineSignature, buildInteractionSurfaceSignature(session.snapshot.nodes), ); - assert.ok((session.postGestureStabilization?.baselineSignature?.length ?? 0) > 0); + assert.ok((pendingSurfaceStabilization(session)?.baselineSignature?.length ?? 0) > 0); }); test('markPostGestureStabilization does not compute a baseline signature on Android', () => { @@ -122,7 +123,7 @@ test('markPostGestureStabilization does not compute a baseline signature on Andr markPostGestureStabilization(session, 'scroll'); - assert.equal(session.postGestureStabilization?.baselineSignature, undefined); + assert.equal(pendingSurfaceStabilization(session)?.baselineSignature, undefined); }); test('markPostGestureStabilization tolerates a missing pre-gesture snapshot on iOS', () => { @@ -133,7 +134,7 @@ test('markPostGestureStabilization tolerates a missing pre-gesture snapshot on i // "No usable baseline" has exactly one representation: the field is absent, // so nothing downstream ever has to distinguish `undefined` from `[]` — and // the loop cannot rebase a baseline that was never really there. - assert.equal(session.postGestureStabilization?.baselineSignature, undefined); + assert.equal(pendingSurfaceStabilization(session)?.baselineSignature, undefined); }); // --------------------------------------------------------------------------- @@ -173,7 +174,7 @@ test('capturePostGestureStabilizedResult keeps polling past the normal deadline assert.equal(staleAccepts, 1); assert.equal(settled, 0); - assert.equal(session.postGestureStabilization, undefined); + assert.equal(pendingSurfaceStabilization(session), undefined); // #1600: a stale-accept is the daemon PROVING the gesture moved nothing — // that verdict must reach the caller, not only the diagnostics stream. assert.equal(result.postGestureOutcome?.kind, 'no-effect'); @@ -259,7 +260,7 @@ test('capturePostGestureStabilizedResult trusts an Android baseline match immedi const session = makeSession('android'); session.snapshot = pickupSnapshot(500); markPostGestureStabilization(session, 'scroll'); - assert.equal(session.postGestureStabilization?.baselineSignature, undefined); + assert.equal(pendingSurfaceStabilization(session)?.baselineSignature, undefined); const capture = vi.fn(async () => pickupSnapshot(500)); // identical throughout, like the iOS stale case @@ -379,7 +380,7 @@ test('capturePostGestureStabilizedResult catches a frozen target even when the b session.snapshot = pickupSnapshotWithExtraText(500); markPostGestureStabilization(session, 'scroll'); assert.ok( - (session.postGestureStabilization?.baselineSignature?.length ?? 0) > + (pendingSurfaceStabilization(session)?.baselineSignature?.length ?? 0) > buildInteractionSurfaceSignature(pickupSnapshot(500).nodes).length, 'the baseline must carry the extra text entry the post-gesture reads never see', ); @@ -657,7 +658,7 @@ test('capturePostGestureStabilizedResult clears the pending record when a captur const session = makeSession('ios'); session.snapshot = pickupSnapshot(500); markPostGestureStabilization(session, 'scroll'); - assert.ok(session.postGestureStabilization); + assert.ok(pendingSurfaceStabilization(session)); let captureCount = 0; const capture = vi.fn(async () => { @@ -679,5 +680,5 @@ test('capturePostGestureStabilizedResult clears the pending record when a captur assert.ok(outcome instanceof DOMException && outcome.name === 'TimeoutError'); assert.equal(captureCount, 2); - assert.equal(session.postGestureStabilization, undefined); + assert.equal(pendingSurfaceStabilization(session), undefined); }); diff --git a/src/daemon/__tests__/scroll-movement.test.ts b/src/daemon/__tests__/scroll-movement.test.ts index 93d164b9ff..1e2a400be3 100644 --- a/src/daemon/__tests__/scroll-movement.test.ts +++ b/src/daemon/__tests__/scroll-movement.test.ts @@ -684,7 +684,12 @@ test('a tree the session no longer stands behind is refused as a baseline', () = test('a gesture nobody has read yet may still be moving the stored tree', () => { const session = makeSession('movement-plan-pending', { snapshot: state(screen(0)) }); - session.postGestureStabilization = { action: 'tap', positionals: [], markedAt: Date.now() }; + session.deferredSurface = { + kind: 'stabilize', + action: 'tap', + positionals: [], + markedAt: Date.now(), + }; const plan = planScrollMovement({ device: IOS_SIMULATOR, flags: undefined, session }); assertPlanKind(plan, 'unobservable'); diff --git a/src/daemon/__tests__/scroll-runtime.test.ts b/src/daemon/__tests__/scroll-runtime.test.ts index 99ed55f873..1104f840ce 100644 --- a/src/daemon/__tests__/scroll-runtime.test.ts +++ b/src/daemon/__tests__/scroll-runtime.test.ts @@ -679,7 +679,7 @@ test('bound scroll will not read movement off a tree that predates the last muta test('bound scroll will not bill an unread earlier gesture to the scroll it is dispatching', async () => { const captures = frozenCaptures([automationScreen(-320)]); const session = sessionWithStoredScreen(automationScreen(0), { - postGestureStabilization: { action: 'press', positionals: [], markedAt: Date.now() }, + deferredSurface: { kind: 'stabilize', action: 'press', positionals: [], markedAt: Date.now() }, }); const result = await runScroll( diff --git a/src/daemon/__tests__/snapshot-capture.test.ts b/src/daemon/__tests__/snapshot-capture.test.ts index 9762517b3f..389e469178 100644 --- a/src/daemon/__tests__/snapshot-capture.test.ts +++ b/src/daemon/__tests__/snapshot-capture.test.ts @@ -18,6 +18,7 @@ import { inboxBaselineNodes, makeAndroidFreshnessSession, } from '../handlers/__tests__/snapshot-handler.fixtures.ts'; +import { pendingSurfaceStabilization } from '../deferred-interaction-outcome.ts'; const captureSnapshotWithInteractor = vi.hoisted(() => vi.fn()); vi.mock('../snapshot-interactor-capture.ts', () => ({ captureSnapshotWithInteractor })); @@ -115,7 +116,8 @@ test('captureSnapshot composes post-gesture stabilization with Android freshness androidTextRows(18, (row) => (row === 1 ? 'album-0' : `Album row ${row}`)), ); const session = makeAndroidFreshnessSession(sessionName, 'click', baselineNodes); - session.postGestureStabilization = { + session.deferredSurface = { + kind: 'stabilize', action: 'click', positionals: [], markedAt: Date.now(), @@ -150,5 +152,5 @@ test('captureSnapshot composes post-gesture stabilization with Android freshness 'snapshot', ]); expect(session.androidSnapshotFreshness).toBeUndefined(); - expect(session.postGestureStabilization).toBeUndefined(); + expect(pendingSurfaceStabilization(session)).toBeUndefined(); }); diff --git a/src/daemon/deferred-interaction-outcome.ts b/src/daemon/deferred-interaction-outcome.ts index d7084cee34..d8aded048a 100644 --- a/src/daemon/deferred-interaction-outcome.ts +++ b/src/daemon/deferred-interaction-outcome.ts @@ -1,8 +1,10 @@ import type { CommandFlags } from '@agent-device/contracts/command'; +import type { SettleObservation, SurfaceScopedNodes } from '@agent-device/contracts/interaction'; import type { PostOpenObservation } from '@agent-device/contracts/application-lifecycle-runtime'; import type { SnapshotCaptureAnnotations } from '@agent-device/contracts/capture'; import { isApplePlatform, isMobilePlatform } from '@agent-device/kernel/device'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; +import type { MovingTargetBaseline } from '@agent-device/contracts/moving-target-gate'; import { captureAndroidFreshnessRecoveredAttempt, getActiveAndroidSnapshotFreshness, @@ -24,7 +26,7 @@ import { runPostGestureStabilityLoop, type PostGestureStabilityOutcome, } from '@agent-device/capture-kit/post-gesture-stability'; -import type { SessionState } from './session-state.ts'; +import type { DeferredSurface, SessionState } from './session-state.ts'; /** * The deferred interaction outcome: the daemon's answer to "did that mutation @@ -57,6 +59,17 @@ export type DeferredInteractionOutcomeMark = { positionals: string[]; flags: CommandFlags | undefined; androidFreshnessBaseline?: SnapshotState | undefined; + /** + * The tree this mutation resolved its target against, when it resolved one (#3471). Feeds the + * moving-target baseline; absent for the routes that dispatch without a daemon tree. + */ + preActionTree?: SurfaceScopedNodes | undefined; + /** + * The action's own `--settle` observation, when it ran one (#3471). Its `diff` is the load-bearing + * field: the runtime attaches it only when the loop converged AND stored its settled tree within + * one surface, which is exactly when that published tree may stand as the baseline. + */ + settle?: SettleObservation | undefined; }; export function markDeferredInteractionOutcome( @@ -69,21 +82,45 @@ export function markDeferredInteractionOutcome( positionals, flags, androidFreshnessBaseline, + preActionTree, + settle, } = params; if (isNavigationSensitiveAction(action)) { markAndroidSnapshotFreshness(session, action, androidFreshnessBaseline ?? session.snapshot); } - markPostGestureStabilization(session, action, positionals, flags); + // The two post-action mechanisms are complementary, and this is the seam that keeps them apart. + // A gesture-shaped mutation hands its unsettled surface to the capture side: every following + // capture runs the quiet-window loop before it returns a tree, so the moving-target gate would + // only re-do that wait. A mutation that marks no stabilization — the press whose own effect was + // an animation — is the case nothing else watches, and is exactly what the gate is for. It also + // never displaces a stabilization some earlier mutation still owes: an `@ref` press spends no + // capture, so a scroll's owed loop can still be pending here, and erasing it would drop the + // #1542/#1600 unsettled-tree check the scroll bought. + if (markSurfaceStabilization(session, action, positionals, flags)) return; + if (pendingSurfaceStabilization(session)) return; + markMovingTargetBaseline(session, preActionTree, settle?.diff !== undefined); } -function markPostGestureStabilization( +/** Narrows a published capture to the shape a baseline keeps, so a stored tree never smuggles in more. */ +function movingTargetRestTree(snapshot: SnapshotState | undefined): SurfaceScopedNodes | undefined { + if (!snapshot) return undefined; + return { + nodes: snapshot.nodes, + ...(snapshot.iosSystemSurfaceBundleId + ? { surfaceBundleId: snapshot.iosSystemSurfaceBundleId } + : {}), + }; +} + +/** Whether this mutation left an unsettled surface for the capture side to steady. */ +function markSurfaceStabilization( session: SessionState, action: string, positionals: string[] = [], flags?: CommandFlags, -): void { - if (!supportsPostGestureStabilization(session.device)) return; - if (!isPostGestureStabilizingAction(action, positionals, flags)) return; +): boolean { + if (!supportsPostGestureStabilization(session.device)) return false; + if (!isPostGestureStabilizingAction(action, positionals, flags)) return false; // No extra capture: `session.snapshot` is still whatever was captured // before this gesture dispatched (this call happens post-dispatch, // pre-capture — the last known pre-action snapshot). An empty signature is @@ -93,7 +130,8 @@ function markPostGestureStabilization( const baselineSignature = requiresPostGestureBaselineDistrust(session.device) ? buildInteractionSurfaceSignature(session.snapshot?.nodes ?? []) : undefined; - session.postGestureStabilization = { + session.deferredSurface = { + kind: 'stabilize', action, positionals, markedAt: Date.now(), @@ -106,6 +144,70 @@ function markPostGestureStabilization( } : {}), }; + return true; +} + +/** + * Records the tree a finished mutation acted through, stamped at the instant it finished (#3471) — + * the `recheck` variant of the session's deferred surface, for the mutation that marks no + * stabilization: the press whose own effect was an animation, which nothing else watches. + * + * `preActionTree` is the observation the action resolved its target against, and is required: a + * mutation that resolved nothing has no tree to describe where things were, and substituting an + * older snapshot would attribute to this action a surface it never looked at. A tree that names no + * nodes records nothing either, for the same reason — an empty tree would make every later target + * read as newly arrived, which is a claim about motion no action here ever observed. That is also + * why the routes that dispatch with no tree of their own leave the session idle: a raw coordinate + * unless `--verify`/`--settle` made it capture one, a Maestro direct-selector tap, `back`, an app + * open. The ordinary navigate-then-tap step pays the gate nothing. + * + * `restProved` switches the tree to the session's published one, and is set only when the action's + * own settle loop converged AND stored its settled tree. Everything else keeps the pre-action tree, + * a timed-out loop included: the session's snapshot may hold a freshness capture, a `@ref` press's + * authorized frame, a lone `--verify` capture, or the newest read of a surface that never quieted, + * and none of those proves rest. Preferring it because it merely differs from the tree the action + * resolved against is the same forbidden substitution, from the other side. + */ +function markMovingTargetBaseline( + session: SessionState, + preActionTree: SurfaceScopedNodes | undefined, + restProved: boolean, +): void { + const tree = restProved ? movingTargetRestTree(session.snapshot) : preActionTree; + if (!tree || tree.nodes.length === 0) { + retireMovingTargetBaseline(session); + return; + } + session.deferredSurface = { + kind: 'recheck', + baseline: { finishedAt: Date.now(), tree }, + }; +} + +/** + * Retires the `recheck` variant only, from ADR 0014's device side-effect seam. A gate out of evidence + * goes idle rather than reconstructing a baseline from another source. A pending stabilization is + * left alone: the seam fires on the way INTO every device op, including the gesture that just marked + * one, and the capture side still owes that loop. + */ +export function retireMovingTargetBaseline(session: SessionState): void { + if (session.deferredSurface?.kind !== 'recheck') return; + session.deferredSurface = undefined; +} + +/** + * The baseline the #3471 gate screens a freshly resolved rect against, or nothing when the session + * is idle or owes the capture-side loop instead. The tree is carried to the command layer because + * the gate judges a new rect against the tree the PREVIOUS action acted through — the `snapshot` + * beside it is the newest observation, which is the one thing that cannot show a target having + * moved. + */ +export function movingTargetBaseline( + session: SessionState | undefined, +): MovingTargetBaseline | undefined { + return session?.deferredSurface?.kind === 'recheck' + ? session.deferredSurface.baseline + : undefined; } /** @@ -123,12 +225,28 @@ export function markPostOpenStabilization( if (!isApplePlatform(session.device.platform)) return; if (!supportsPostGestureStabilization(session.device)) return; if (observation === undefined || observation === 'observable') return; - session.postGestureStabilization = { action: 'open', positionals: [], markedAt: Date.now() }; + session.deferredSurface = { + kind: 'stabilize', + action: 'open', + positionals: [], + markedAt: Date.now(), + }; } -function clearPostGestureStabilization(session: SessionState | undefined): void { - if (!session?.postGestureStabilization) return; - session.postGestureStabilization = undefined; +function clearSurfaceStabilization(session: SessionState | undefined): void { + if (session?.deferredSurface?.kind !== 'stabilize') return; + session.deferredSurface = undefined; +} + +/** + * The one read of the pending capture-side stabilization. The boolean below is what callers outside + * this module ask; this is the record itself, for the loop that has to run it and for the tests that + * pin what a marker stored. Nothing else narrows the variant. + */ +export function pendingSurfaceStabilization( + session: SessionState | undefined, +): Extract | undefined { + return session?.deferredSurface?.kind === 'stabilize' ? session.deferredSurface : undefined; } /** @@ -139,7 +257,7 @@ function clearPostGestureStabilization(session: SessionState | undefined): void * implementation. */ export function isPostGestureStabilizationPending(session: SessionState | undefined): boolean { - return Boolean(session?.postGestureStabilization); + return pendingSurfaceStabilization(session) !== undefined; } export type DeferredOutcomeSnapshotAttempt = { @@ -223,7 +341,7 @@ export async function capturePostGestureStabilizedResult(params: { initial?: T; }): Promise> { const { session, capture, readSnapshot } = params; - const pending = session?.postGestureStabilization; + const pending = pendingSurfaceStabilization(session); if (!session || !supportsPostGestureStabilization(session.device) || !pending) { return { value: params.initial ?? (await capture()) }; } @@ -257,7 +375,7 @@ export async function capturePostGestureStabilizedResult(params: { }, }); } finally { - clearPostGestureStabilization(session); + clearSurfaceStabilization(session); } } diff --git a/src/daemon/interaction/internal/__tests__/interaction-ios-tap-outcome.test.ts b/src/daemon/interaction/internal/__tests__/interaction-ios-tap-outcome.test.ts index f58f23989a..eafa297b90 100644 --- a/src/daemon/interaction/internal/__tests__/interaction-ios-tap-outcome.test.ts +++ b/src/daemon/interaction/internal/__tests__/interaction-ios-tap-outcome.test.ts @@ -35,6 +35,7 @@ import { } from '../../../__tests__/interaction-get-runtime-fixture.ts'; import { captureSnapshotWithInteractor } from '../../../snapshot-interactor-capture.ts'; import { corroborateIosTapFailure } from '../interaction-ios-tap-outcome.ts'; +import { movingTargetBaseline } from '../../../deferred-interaction-outcome.ts'; import { mkdtempForTestSync } from '@agent-device/test-kit/tmp-dir'; import { testCreateCommandSurface } from '../../../__tests__/command-surface-fixture.ts'; @@ -136,6 +137,34 @@ test('a changed post-action capture corroborates an iOS tap reported as failed', expect(sessionStore.get(sessionName)?.actions).toHaveLength(1); }); +test('a corroborated tap failure still records the tree it acted through (#3471)', async () => { + // The corroboration is a dispatch that touched the device through a resolved tree, so it owes the + // moving-target baseline like any other mutation: withholding it there would RETIRE the record a + // previous action made and leave the next press unscreened by the gate. + const sessionName = 'ios-tap-corroboration-baseline'; + const sessionStore = makeSessionStore(); + const session = makeIosSession(sessionName, { + appBundleId: 'com.example.app', + snapshot: snapshot(profileNodes), + }); + sessionStore.publish(sessionName, session); + legacyDispatchCapture.mockImplementation(async (_device, command) => { + if (command === 'press') { + throw new AppError( + 'XCTEST_RECORDED_FAILURE', + 'XCTest recorded a failure while executing tap; the action may not have been performed.', + ); + } + if (command === 'snapshot') return snapshotPayload(imageViewerNodes); + return {}; + }); + + const response = await runClick(sessionStore, sessionName); + + expect(response?.ok).toBe(true); + expect(movingTargetBaseline(sessionStore.get(sessionName)!)).toBeDefined(); +}); + test('an unchanged post-action capture keeps a failed iOS tap failed', async () => { const sessionName = 'ios-unchanged-tap-corroboration'; const sessionStore = makeSessionStore(); diff --git a/src/daemon/interaction/internal/__tests__/interaction-touch-direct-ios-eligibility.test.ts b/src/daemon/interaction/internal/__tests__/interaction-touch-direct-ios-eligibility.test.ts index 561eac1279..3b32d89ebc 100644 --- a/src/daemon/interaction/internal/__tests__/interaction-touch-direct-ios-eligibility.test.ts +++ b/src/daemon/interaction/internal/__tests__/interaction-touch-direct-ios-eligibility.test.ts @@ -235,7 +235,12 @@ test('click simple iOS id selector waits for snapshot path after pending gesture const sessionStore = makeSessionStore(); const sessionName = 'ios-direct-selector-after-swipe'; const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); - session.postGestureStabilization = { action: 'swipe', positionals: [], markedAt: Date.now() }; + session.deferredSurface = { + kind: 'stabilize', + action: 'swipe', + positionals: [], + markedAt: Date.now(), + }; sessionStore.publish(sessionName, session); mockCaptureSnapshotForSession.mockResolvedValue({ diff --git a/src/daemon/interaction/internal/interaction-common.ts b/src/daemon/interaction/internal/interaction-common.ts index 520b4e428a..05d1ecccab 100644 --- a/src/daemon/interaction/internal/interaction-common.ts +++ b/src/daemon/interaction/internal/interaction-common.ts @@ -1,4 +1,5 @@ import type { CommandFlags } from '@agent-device/contracts/command'; +import type { SettleObservation, SurfaceScopedNodes } from '@agent-device/contracts/interaction'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; import type { DaemonResponse } from '../../daemon-request.ts'; import { stripInternalInteractionFlags } from '../../deferred-interaction-outcome.ts'; @@ -28,6 +29,13 @@ export function finalizeTouchInteraction(params: { actionStartedAt: number; actionFinishedAt: number; androidFreshnessBaseline?: SnapshotState | undefined; + /** + * The tree this action resolved its target against, plus its own `--settle` observation when it ran + * one — both forwarded to the deferred-outcome marker, which reads the settled-tree signal from the + * second (#3471). Absent for the dispatches that acted without a daemon tree. + */ + preActionTree?: SurfaceScopedNodes | undefined; + settle?: SettleObservation | undefined; }): DaemonResponse { const { operations, @@ -42,6 +50,8 @@ export function finalizeTouchInteraction(params: { actionStartedAt, actionFinishedAt, androidFreshnessBaseline, + preActionTree, + settle, } = params; const actionFlags = stripInternalInteractionFlags(flags); const [parameterizedResult, parameterizedResponseData] = parameterizeFillPayloads({ @@ -73,6 +83,8 @@ export function finalizeTouchInteraction(params: { positionals, flags, androidFreshnessBaseline, + preActionTree, + settle, }); operations.recordGestureVisualization( actionCommand, diff --git a/src/daemon/interaction/internal/interaction-touch-runtime.ts b/src/daemon/interaction/internal/interaction-touch-runtime.ts index feb9f5cc34..b7b7480468 100644 --- a/src/daemon/interaction/internal/interaction-touch-runtime.ts +++ b/src/daemon/interaction/internal/interaction-touch-runtime.ts @@ -125,6 +125,11 @@ export async function dispatchRuntimeInteraction< actionStartedAt, actionFinishedAt, androidFreshnessBaseline: options.androidFreshnessBaseline, + preActionTree: runtimeResult.preAction, + // The action's own settle observation, so the baseline can be the tree that proved rest + // instead of the tree the action started from. Without it the follow-up press re-observes a + // surface the settle already proved quiet — the round trip `--settle` exists to remove. + settle: runtimeResult.settle, }); } catch (error) { const appError = publishAmbiguousMatchCandidateRefs( @@ -198,6 +203,10 @@ async function buildRuntimeIosCorroboratedResponse(params: { actionStartedAt: params.actionStartedAt, actionFinishedAt: Date.now(), androidFreshnessBaseline: params.androidFreshnessBaseline, + // A corroborated failure still acted through a tree, and its own effect may still be animating. + // Withholding it would retire the evidence a previous mutation recorded and leave the next press + // unscreened, which is the opposite of what a dispatch that touched the device should do. + preActionTree: resolvedTarget?.preAction, }); } diff --git a/src/daemon/ref-frame.ts b/src/daemon/ref-frame.ts index b51c332084..c8d4b6ff3e 100644 --- a/src/daemon/ref-frame.ts +++ b/src/daemon/ref-frame.ts @@ -8,6 +8,7 @@ import { type RefFrameState, } from './ref-frame-slot.ts'; import type { SessionState } from './session-state.ts'; +import { retireMovingTargetBaseline } from './deferred-interaction-outcome.ts'; const runtimeRevisions = new WeakMap(); const outdatedObservations = new WeakSet(); @@ -93,6 +94,9 @@ export function expireRefFrame(session: SessionState): void { markSessionSnapshotOutdated(session); session.refFrame = expiredRefFrame(refFrame(session)); session.snapshotScopeSource = undefined; + // #3471: a side effect no resolved target was aimed through leaves a tree predating an unmeasured + // change, so it cannot screen a later press. + retireMovingTargetBaseline(session); } /** diff --git a/src/daemon/runtime-session.ts b/src/daemon/runtime-session.ts index 7685156899..902f731183 100644 --- a/src/daemon/runtime-session.ts +++ b/src/daemon/runtime-session.ts @@ -3,6 +3,7 @@ import type { CommandSessionStore, } from '@agent-device/contracts/runtime-contract'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; +import { movingTargetBaseline } from './deferred-interaction-outcome.ts'; import { refFrameTree } from './ref-frame.ts'; import type { SessionRef, SessionState } from './session-state.ts'; import type { SessionStore } from './session-store.ts'; @@ -26,6 +27,9 @@ function toRuntimeSessionRecord( ): CommandSessionRecord | undefined { if (!session) return undefined; const frameTree = refFrameTree(session); + // #3471: the gate judges a fresh rect against the tree the PREVIOUS action acted through, so that + // tree has to reach the command layer; the `snapshot` beside it cannot show a target having moved. + const baseline = movingTargetBaseline(session); return { name, appBundleId: session.appBundleId, @@ -39,6 +43,7 @@ function toRuntimeSessionRecord( ...(frameTree && options.omitRefFrameSnapshot !== true ? { refFrameSnapshot: frameTree } : {}), + ...(baseline ? { movingTargetBaseline: baseline } : {}), } : {}), metadata: { diff --git a/src/daemon/session-lifecycle/internal/__tests__/session-open-execution-runtime.test.ts b/src/daemon/session-lifecycle/internal/__tests__/session-open-execution-runtime.test.ts index de7620bba1..12ee6c9410 100644 --- a/src/daemon/session-lifecycle/internal/__tests__/session-open-execution-runtime.test.ts +++ b/src/daemon/session-lifecycle/internal/__tests__/session-open-execution-runtime.test.ts @@ -85,6 +85,7 @@ import { } from '../../../../platform-runtime-runtime-hints.ts'; import { resolveAndroidPackageForOpen } from '@agent-device/platform-android/mechanics'; import { dispatchApplicationLifecycleEffect } from '../../../__tests__/application-lifecycle-runtime-fixture.ts'; +import { pendingSurfaceStabilization } from '../../../deferred-interaction-outcome.ts'; import { makeAndroidEmulator, makeAppleSimulator, @@ -520,19 +521,19 @@ async function openReportingObservation( test('a fresh iOS open that could not observe its app leaves the next capture to settle', async () => { const session = await openReportingObservation('unobserved-open', 'unobservable'); - expect(session?.postGestureStabilization?.action).toBe('open'); + expect(pendingSurfaceStabilization(session)?.action).toBe('open'); }); test('a fresh iOS open whose observation probe failed leaves the next capture to settle', async () => { const session = await openReportingObservation('probe-failed-open', 'probe-failed'); - expect(session?.postGestureStabilization?.action).toBe('open'); + expect(pendingSurfaceStabilization(session)?.action).toBe('open'); }); test('a fresh iOS open that observed its app leaves nothing pending', async () => { const session = await openReportingObservation('observed-open', 'observable'); - expect(session?.postGestureStabilization).toBeUndefined(); + expect(pendingSurfaceStabilization(session)).toBeUndefined(); }); function holdNativeOpen() { diff --git a/src/daemon/session-state.ts b/src/daemon/session-state.ts index fcc6a88398..9444108349 100644 --- a/src/daemon/session-state.ts +++ b/src/daemon/session-state.ts @@ -8,6 +8,7 @@ import type { } from '@agent-device/contracts/perf-runtime'; import type { ScreenRecordingLiveHandle } from '@agent-device/contracts/screen-recording-runtime'; import type { SessionAction, SessionScope, SessionSurface } from '@agent-device/contracts/session'; +import type { MovingTargetBaseline as ContractsMovingTargetBaseline } from '@agent-device/contracts/moving-target-gate'; import type { LeaseBackend, SessionRuntimeHints as PublicSessionRuntimeHints, @@ -72,32 +73,59 @@ export type InteractionSurfaceEntry = { discriminating: boolean; }; -export type PostGestureStabilization = { - action: string; - /** The gesture's own positionals — wording input for the #1600 no-effect - * warning; never re-dispatched. Always set by every writer. */ - positionals: string[]; - markedAt: number; - /** - * Pre-gesture interaction-surface signature, captured from the session's - * last-known snapshot before the gesture dispatched (no extra capture — see - * `markPostGestureStabilization`). Populated only when - * `requiresPostGestureBaselineDistrust` is true for the session's device - * (Apple mobile only, #1542 defect 2): a post-gesture quiet-poll match that - * still equals this baseline is a stale-but-internally-consistent AX read, - * not proof the screen settled. Android's persistent helper clears its a11y - * cache before every capture (#1254/#1259) and needs no baseline check. - */ - baselineSignature?: InteractionSurfaceEntry[]; - /** - * Snapshot backend that produced `baselineSignature`. Backends do not return - * comparable views of one screen — on the same iOS screen private AX returns - * the scrolled-away content the tree backend prunes — so a quiet capture from - * a different backend can only be re-baselined against, never concluded from - * (#1569). - */ - baselineBackend?: string; -}; +/** + * What the session still owes because of its last mutation, before its surface can be trusted + * again. Two mechanisms own that question and a mutation leaves room for exactly one of them, so + * the variants are mutually exclusive by construction rather than by agreement: + * + * - `stabilize` — the surface is unsettled and the CAPTURE side owns settling it: the next capture + * runs the quiet-window loop before it returns a tree. Gesture-shaped mutations (swipe, scroll, a + * synthesized gesture) and an open that never read its app mark this. + * - `recheck` — the mutation dispatched through a captured tree and its own effect may still be + * animating (#3471). The next action that resolves a target asks the tree it recorded whether its + * coordinates are still in play; captures stay plain, because nothing here is owed a wait. + * + * `markedAt` is when the mutation FINISHED, so the window waits out only what is left of it, and + * writing either variant retires the other. + */ +export type DeferredSurface = + | { + kind: 'stabilize'; + action: string; + /** The gesture's own positionals — wording input for the #1600 no-effect + * warning; never re-dispatched. Always set by every writer. */ + positionals: string[]; + markedAt: number; + /** + * Pre-gesture interaction-surface signature, captured from the session's + * last-known snapshot before the gesture dispatched (no extra capture — see + * `markSurfaceStabilization`). Populated only when + * `requiresPostGestureBaselineDistrust` is true for the session's device + * (Apple mobile only, #1542 defect 2): a post-gesture quiet-poll match that + * still equals this baseline is a stale-but-internally-consistent AX read, + * not proof the screen settled. Android's persistent helper clears its a11y + * cache before every capture (#1254/#1259) and needs no baseline check. + */ + baselineSignature?: InteractionSurfaceEntry[]; + /** + * Snapshot backend that produced `baselineSignature`. Backends do not return + * comparable views of one screen — on the same iOS screen private AX returns + * the scrolled-away content the tree backend prunes — so a quiet capture from + * a different backend can only be re-baselined against, never concluded from + * (#1569). + */ + baselineBackend?: string; + } + | { + kind: 'recheck'; + /** + * The tree the mutation resolved its target against, beside the instant the mutation + * finished, so the next action can ask whether its own coordinates are still in play + * (#3471). The shape is `MovingTargetBaseline` in + * `@agent-device/contracts/moving-target-gate`, declared beside the gate that reads it. + */ + baseline: ContractsMovingTargetBaseline; + }; /** * A session together with the store key that addresses it: `address` is the exact string @@ -186,7 +214,12 @@ export type SessionState = { /** Last broad snapshot safe for Android route-freshness comparisons after interactive snapshots. */ lastComparisonSafeSnapshot?: SnapshotState; androidSnapshotFreshness?: SnapshotFreshnessWindow; - postGestureStabilization?: PostGestureStabilization; + /** + * The one writer's view of `DeferredSurface`: what this session's last mutation left owed. One + * field because one mutation leaves room for one of the two; the transitions are + * `deferred-interaction-outcome.ts`'s (R7). + */ + deferredSurface?: DeferredSurface; snapshotDiagnostics?: SnapshotDiagnosticsState; trace?: { outPath: string; diff --git a/test/integration/interaction-contract/fixtures.ts b/test/integration/interaction-contract/fixtures.ts index c5a4162470..378505b9bd 100644 --- a/test/integration/interaction-contract/fixtures.ts +++ b/test/integration/interaction-contract/fixtures.ts @@ -241,6 +241,24 @@ export function continueButtonSnapshot(): SnapshotState { ]); } +/** + * The #3471 fixture: one Continue button reported at a caller-chosen rect. A press resolving the + * button at one rect while a later observation reports another is the moving-target exposure, so the + * scenarios vary the rect and nothing else. + */ +export function continueButtonAtRect(x: number, y: number): SnapshotState { + return makeSnapshotState([ + { + index: 0, + depth: 0, + type: 'Button', + label: 'Continue', + rect: { x, y, width: 100, height: 40 }, + hittable: true, + }, + ]); +} + // Same button, reported non-hittable. export function nonHittableButtonSnapshot(): SnapshotState { return makeSnapshotState([ diff --git a/test/integration/interaction-contract/runtime-harness.ts b/test/integration/interaction-contract/runtime-harness.ts index 7e708cf206..50fb30555a 100644 --- a/test/integration/interaction-contract/runtime-harness.ts +++ b/test/integration/interaction-contract/runtime-harness.ts @@ -1,4 +1,5 @@ import type { AgentDeviceBackend } from '@agent-device/contracts/backend'; +import type { CommandClock, CommandSessionRecord } from '@agent-device/contracts/runtime-contract'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { createLocalArtifactAdapter } from '../../../src/io.ts'; import { createAgentDevice } from '../../../src/runtime.ts'; @@ -18,6 +19,16 @@ type ContractBackendOverrides = Partial< > > & { platform?: AgentDeviceBackend['platform']; + /** + * A scenario-owned clock, so a guarantee with a time budget (the moving-target window) can state + * its timing claim without a wall-clock race. Without one the runtime falls back to real time. + */ + clock?: CommandClock; + /** + * Session state beyond the stored snapshot — the #3471 moving-target baseline is the case that + * needs it, because the gate's whole premise is a session that has just finished mutating. + */ + sessionState?: Partial>; }; /** @@ -44,7 +55,28 @@ export function createContractDevice( resolveGestureViewport: overrides.resolveGestureViewport, } satisfies AgentDeviceBackend, artifacts: createLocalArtifactAdapter(), - sessions: createMemorySessionStore([{ name: 'default', snapshot }]), + sessions: createMemorySessionStore([ + { name: 'default', snapshot, ...overrides.sessionState } as CommandSessionRecord, + ]), policy: localCommandPolicy(), + ...(overrides.clock ? { clock: overrides.clock } : {}), }); } + +/** + * A contract-scenario clock: `now()` reads elapsed virtual time and `sleep()` advances it. A + * guarantee with a time budget — the #3471 moving-target window — can then assert what it waited + * without racing the machine it runs on. + */ +export function createContractClock(startMs = 0): { + now(): number; + sleep(ms: number): Promise; +} { + let elapsed = startMs; + return { + now: () => elapsed, + sleep: async (ms: number) => { + elapsed += Math.max(ms, 0); + }, + }; +} diff --git a/test/integration/interaction-contract/runtime-ref.contract.test.ts b/test/integration/interaction-contract/runtime-ref.contract.test.ts index e056c0ea10..07925a0135 100644 --- a/test/integration/interaction-contract/runtime-ref.contract.test.ts +++ b/test/integration/interaction-contract/runtime-ref.contract.test.ts @@ -9,6 +9,7 @@ import { scenarioName, scenarioNames } from './coverage-manifest.ts'; import { RUNTIME_REF_COVERAGE } from './runtime-ref.coverage.ts'; import { closedDrawerSnapshot, + continueButtonAtRect, continueButtonSnapshot, coveredButtonSnapshot, fullyTiledParentSnapshot, @@ -16,8 +17,9 @@ import { nonHittableCellSnapshot, RUNNER_CONTINUE_NODES, settledWelcomeSnapshot, + viewportOnlySnapshot, } from './fixtures.ts'; -import { createContractDevice } from './runtime-harness.ts'; +import { createContractClock, createContractDevice } from './runtime-harness.ts'; import { runnerSnapshotEntry, runnerTapEntry, withIosContractDaemon } from './daemon-harness.ts'; // ADR 0011 Layer 3, runtime-ref path: session snapshot ref lookup, guarded @@ -126,6 +128,68 @@ test(scenario('nonHittable'), async () => { assert.match(result.hint ?? '', /hittable: false/); }); +// #3471 targetStability on the ref path. An `@ref` resolution spends no capture (ADR 0014 binds it +// to its authorized frame), so the gate supplies the fresh observation and lets the frame model's own +// reconciliation adopt coordinates for the same local identity — which is why a ref can be re-observed +// at all without ever being retargeted by positional coincidence. +test(scenarioNames(RUNTIME_REF_COVERAGE, 'targetStability')[0]!, async () => { + const taps: Point[] = []; + let captures = 0; + const positions = [continueButtonAtRect(300, 20), continueButtonAtRect(360, 20)]; + const device = createContractDevice(continueButtonAtRect(240, 20), { + captureSnapshot: async () => ({ snapshot: positions[captures++] ?? positions[1]! }), + tap: async (_context, point) => { + taps.push(point); + }, + clock: createContractClock(), + sessionState: { + movingTargetBaseline: { finishedAt: 0, tree: { nodes: viewportOnlySnapshot().nodes } }, + }, + }); + + await assert.rejects( + () => device.interactions.click(ref('@e1'), { session: 'default' }), + (error: unknown) => { + assert.ok(error instanceof Error); + // One message for both refusal shapes: the confirmation that failed, not motion. + assert.match( + error.message, + /Ref @e1 had no confirmed resting position when the transition window ran out/, + ); + const details = (error as { details?: Record }).details; + assert.equal(details?.reason, 'target_still_moving'); + assert.equal(details?.dispatched, 'no'); + assert.equal(details?.ref, '@e1'); + assert.deepEqual(details?.rect, { x: 240, y: 20, width: 100, height: 40 }); + assert.deepEqual(details?.movedToRect, { x: 360, y: 20, width: 100, height: 40 }); + // An @ref cannot survive the recovery wait, so the hint sends it to a selector. + assert.match(String(details?.hint), /snapshot -i/); + return true; + }, + ); + assert.deepEqual(taps, []); +}); + +test(scenarioNames(RUNTIME_REF_COVERAGE, 'targetStability')[1]!, async () => { + const taps: Point[] = []; + const resting = continueButtonAtRect(10, 20); + const device = createContractDevice(continueButtonAtRect(240, 20), { + captureSnapshot: async () => ({ snapshot: resting }), + tap: async (_context, point) => { + taps.push(point); + }, + clock: createContractClock(), + sessionState: { + movingTargetBaseline: { finishedAt: 0, tree: { nodes: viewportOnlySnapshot().nodes } }, + }, + }); + + const result = await device.interactions.click(ref('@e1'), { session: 'default' }); + + assert.equal(result.kind, 'ref'); + assert.deepEqual(taps, [{ x: 60, y: 40 }]); +}); + test(scenario('verifyEvidence'), async () => { const device = createContractDevice(continueButtonSnapshot(), { tap: async () => ({ ok: true }), diff --git a/test/integration/interaction-contract/runtime-ref.coverage.ts b/test/integration/interaction-contract/runtime-ref.coverage.ts index 4a5ee61f72..50ef4698c6 100644 --- a/test/integration/interaction-contract/runtime-ref.coverage.ts +++ b/test/integration/interaction-contract/runtime-ref.coverage.ts @@ -20,4 +20,8 @@ export const RUNTIME_REF_COVERAGE = definePathCoverage('runtime-ref', { 'runtime-ref resolutionDisclosure: an @ref discloses the exact ref-provenance shape', 'runtime-ref resolutionDisclosure: trailing-label recovery discloses label-fallback, never exact', ], + targetStability: [ + 'runtime-ref targetStability: a ref whose authorized node is still moving is refused with target_still_moving', + 'runtime-ref targetStability: a ref that came to rest is tapped at the re-observed position', + ], }); diff --git a/test/integration/interaction-contract/runtime-selector.contract.test.ts b/test/integration/interaction-contract/runtime-selector.contract.test.ts index 01977e5b46..26574ee8a4 100644 --- a/test/integration/interaction-contract/runtime-selector.contract.test.ts +++ b/test/integration/interaction-contract/runtime-selector.contract.test.ts @@ -10,6 +10,7 @@ import { scenarioName, scenarioNames } from './coverage-manifest.ts'; import { RUNTIME_SELECTOR_COVERAGE } from './runtime-selector.coverage.ts'; import { closedDrawerSnapshot, + continueButtonAtRect, continueButtonSnapshot, coveredButtonSnapshot, keyboardCoveredTabBarSnapshot, @@ -22,7 +23,7 @@ import { settledWelcomeSnapshot, viewportOnlySnapshot, } from './fixtures.ts'; -import { createContractDevice } from './runtime-harness.ts'; +import { createContractClock, createContractDevice } from './runtime-harness.ts'; import { runnerSnapshotEntry, runnerTapEntry, withIosContractDaemon } from './daemon-harness.ts'; // ADR 0011 Layer 3, runtime-selector path: daemon tree capture, selector @@ -235,6 +236,121 @@ test(scenario('targetReadiness'), async () => { assert.deepEqual(taps, [{ x: 60, y: 40 }]); }); +// #3471 targetStability: the session's last action animated the UI, so the rect this press resolved +// may be a position the target has already left. The gate re-observes past the window: two agreeing +// observations are the resting position it taps, and two disagreeing ones are a refusal. +test(scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'targetStability')[0]!, async () => { + const taps: Point[] = []; + let captures = 0; + const resting = continueButtonAtRect(10, 20); + const device = createContractDevice(continueButtonAtRect(240, 20), { + // A slide-in the resolution capture caught mid-flight, at rest by the gate's re-observations. + captureSnapshot: async () => ({ + snapshot: captures++ === 0 ? continueButtonAtRect(240, 20) : resting, + }), + tap: async (_context, point) => { + taps.push(point); + }, + clock: createContractClock(), + sessionState: { + movingTargetBaseline: { finishedAt: 0, tree: { nodes: viewportOnlySnapshot().nodes } }, + }, + }); + + const result = await device.interactions.press(selector('label=Continue'), { + session: 'default', + }); + + assert.equal(result.kind, 'selector'); + // Tapped at the resting centre (10+50, 20+20), not the mid-flight one it resolved. + assert.deepEqual(taps, [{ x: 60, y: 40 }]); + assert.equal(captures, 3, 'one resolution capture plus the gate two re-observations'); +}); + +// The wiring, through the real daemon route rather than an injected session record: press A must +// LEAVE the tree it acted through on the session (#3471) and press B must be screened against it. +// Deleting either half of that — the writer in `dispatchRuntimeInteraction` or the carrier in +// `toRuntimeSessionRecord` — leaves B idle, so it taps the vacated point and this fails. +// +// Flake surface: this route installs no runtime clock (production never does), so the 500 ms window +// runs on the wall clock and B must reach its gate read inside it. A host loaded enough to starve +// this process past the window sends B idle, and B taps — the same `Unexpected provider call: +// ios.runner.tap` a wiring regression gives. Re-run it unloaded before reading it as one. +test( + scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'targetStability')[1]!, + async () => { + // The shared runner tree with the Continue button's frame translated to `y`. + const runnerNodesAtY = (y: number) => + RUNNER_CONTINUE_NODES.map((node) => + node.index === 1 ? { ...node, rect: { ...node.rect, y } } : node, + ); + await withIosContractDaemon( + [ + // A resolves and taps at y=300; B resolves at 360 and its two re-observations disagree. + runnerSnapshotEntry(runnerNodesAtY(300)), + runnerTapEntry({ x: 200, y: 322 }), + runnerSnapshotEntry(runnerNodesAtY(360)), + runnerSnapshotEntry(runnerNodesAtY(420)), + runnerSnapshotEntry(runnerNodesAtY(480)), + ], + async (daemon, transcript) => { + assertRpcOk(await daemon.callCommand('press', ['label=Continue'])); + const refusal = await daemon.callCommand('press', ['label=Continue']); + const data = assertRpcError( + refusal, + 'COMMAND_FAILED', + /label=Continue had no confirmed resting position when the transition window ran out/, + ); + const details = data.details as Record; + assert.equal(details.reason, INTERACTION_ERROR_REASONS.targetStillMoving); + assert.equal(details.dispatched, 'no'); + assert.deepEqual(details.rect, { x: 100, y: 360, width: 200, height: 44 }); + assert.deepEqual(details.movedToRect, { x: 100, y: 480, width: 200, height: 44 }); + // A: one capture. B: its resolution plus the gate's two. Any other count is the harness's + // `assertComplete` failing, which is what makes the unused deletion above visible. + assert.equal( + transcript.calls.filter((call) => call.command === 'ios.runner.snapshot').length, + 4, + ); + assert.equal( + transcript.calls.filter((call) => call.command === 'ios.runner.tap').length, + 1, + ); + }, + ); + }, + PARALLEL_PROVIDER_SCENARIO_TIMEOUT_MS, +); + +// The cost contract, as a scenario: the gate reads a tree the session already holds, so a press +// whose target the previous action left in place spends the one capture it always spent. +test(scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'targetStability')[2]!, async () => { + const taps: Point[] = []; + let captures = 0; + const resting = continueButtonAtRect(10, 20); + const device = createContractDevice(resting, { + captureSnapshot: async () => { + captures += 1; + return { snapshot: resting }; + }, + tap: async (_context, point) => { + taps.push(point); + }, + clock: createContractClock(), + sessionState: { + movingTargetBaseline: { finishedAt: 0, tree: { nodes: resting.nodes } }, + }, + }); + + const result = await device.interactions.press(selector('label=Continue'), { + session: 'default', + }); + + assert.equal(result.kind, 'selector'); + assert.deepEqual(taps, [{ x: 60, y: 40 }]); + assert.equal(captures, 1, 'a stationary target must not pay for a re-observation'); +}); + test(scenario('verifyEvidence'), async () => { const device = createContractDevice(continueButtonSnapshot(), { tap: async () => ({ ok: true }), diff --git a/test/integration/interaction-contract/runtime-selector.coverage.ts b/test/integration/interaction-contract/runtime-selector.coverage.ts index 90855d4dae..6119511b2c 100644 --- a/test/integration/interaction-contract/runtime-selector.coverage.ts +++ b/test/integration/interaction-contract/runtime-selector.coverage.ts @@ -33,4 +33,9 @@ export const RUNTIME_SELECTOR_COVERAGE = definePathCoverage('runtime-selector', ], targetReadiness: 'runtime-selector targetReadiness: a target missing on the first capture resolves once a later poll observes it', + targetStability: [ + 'runtime-selector targetStability: a settled slide-in is tapped where it came to rest', + 'runtime-selector targetStability: a target still moving at the end of the window is refused with target_still_moving', + 'runtime-selector targetStability: a target the previous action left in place costs no extra capture', + ], }); diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 7291f3c47d..7ce60915be 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -422,6 +422,8 @@ agent-device gesture transform 200 420 80 -40 2 35 700 # combined pan, zoom, and A successful `click` or `press` means the action was sent to where the command found the target. It does not confirm that the element received it or that the screen changed. When the next step depends on it, add `--verify` (reports `changedFromBefore`), add `--settle` (returns the diff once the UI goes quiet), or follow with `wait `. Before retrying a failed interaction, see [Retry after a failed command](#retry-after-a-failed-command). +A `click` or `press` whose target the daemon resolves to a point (`@ref` or selector) re-checks that target's position before dispatching when the session's previous action animated the screen, and dispatches where it came to rest. When that check cannot confirm a stable position — the target was still moving, or a re-check could not measure it — the command refuses with `target_still_moving` and `dispatched: no`: nothing was dispatched, the point the command resolved is in `error.details.rect`, and where the target was last seen is in `error.details.movedToRect` when that last look measured one. Run `wait ` on the target and retry the same command, or add `--settle` so the action waits for the UI to go quiet. A refused `@ref` press needs a fresh `snapshot -i` and a selector, because the wait retires the ref. Two dispatch paths hand off before the check: a press at raw coordinates has no resolved target to re-check, and a dispatch the backend performs on the element itself — `click @ref` on a backend with element dispatch, or a Maestro non-hittable coordinate fallback — goes to that backend rather than through a resolved point. + ### Text entry - `fill` clears then types. `type` appends to the focused field and accepts text only: use `fill @ref "text"` to target a field, or `press @ref` then `type "text"`.