diff --git a/packages/contracts/src/scroll-command.ts b/packages/contracts/src/scroll-command.ts index 39de1e80cf..52e6f89fcb 100644 --- a/packages/contracts/src/scroll-command.ts +++ b/packages/contracts/src/scroll-command.ts @@ -1,6 +1,7 @@ import { AppError } from '@agent-device/kernel/errors'; import type { SettleObservation } from './interaction.ts'; import type { ScrollDirection } from './scroll-gesture.ts'; +import type { Rect } from '@agent-device/kernel/snapshot'; export const SCROLL_DURATION_MAX_MS = 10_000; export const DEFAULT_MOBILE_SCROLL_DURATION_MS = 300; @@ -38,13 +39,24 @@ export type ScrollTimingOptions = { export type ScrollCommandOptions = ScrollDistanceOptions & ScrollTimingOptions; -export type ScrollExecutionOptions = ScrollCommandOptions & { - releaseBehavior?: ScrollReleaseBehavior; +/** + * The scroll container the caller resolved from the current tree, in the snapshot's absolute space. + * An owner that plans its own swipe keeps both endpoints inside it (#2714); an owner without that + * planner ignores it. + */ +export type ScrollContainerTarget = { + containerRect?: Rect; }; -export type ResolvedScrollExecutionOptions = ScrollCommandOptions & { - releaseBehavior: ScrollReleaseBehavior; -}; +export type ScrollExecutionOptions = ScrollCommandOptions & + ScrollContainerTarget & { + releaseBehavior?: ScrollReleaseBehavior; + }; + +export type ResolvedScrollExecutionOptions = ScrollCommandOptions & + ScrollContainerTarget & { + releaseBehavior: ScrollReleaseBehavior; + }; export function resolveScrollExecutionOptions( options: ScrollCommandOptions, diff --git a/packages/contracts/src/scroll-gesture-container.test.ts b/packages/contracts/src/scroll-gesture-container.test.ts new file mode 100644 index 0000000000..e969e22b8c --- /dev/null +++ b/packages/contracts/src/scroll-gesture-container.test.ts @@ -0,0 +1,94 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import type { Rect } from '@agent-device/kernel/snapshot'; +import { buildScrollGesturePlan, type ScrollDirection } from './scroll-gesture.ts'; + +// The iPhone geometry #2714 was reported on: a 402x874 frame, the default 10% edge padding leaving +// y 87..787 for a swipe. +const FRAME = { referenceWidth: 402, referenceHeight: 874 }; +const PADDED_TOP = 87; +const PADDED_BOTTOM = 787; +const INSET_CONTAINER: Rect = { x: 18, y: 178, width: 366, height: 662 }; +const FITTING_CONTAINER: Rect = { x: 18, y: 62, width: 366, height: 729 }; + +function endpointsInside( + plan: { x1: number; y1: number; x2: number; y2: number }, + rect: Rect, +): boolean { + const inside = (x: number, y: number) => + x >= rect.x && x <= rect.x + rect.width && y >= rect.y && y <= rect.y + rect.height; + return inside(plan.x1, plan.y1) && inside(plan.x2, plan.y2); +} + +test.each(['up', 'down'])( + 'a %s swipe longer than an inset container shrinks to the container and reports both spans', + (direction) => { + const unclamped = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME }); + const plan = buildScrollGesturePlan({ + direction, + amount: 0.75, + ...FRAME, + container: INSET_CONTAINER, + }); + + assert.ok(endpointsInside(plan, INSET_CONTAINER), JSON.stringify(plan)); + assert.deepEqual( + [Math.min(plan.y1, plan.y2), Math.max(plan.y1, plan.y2)], + [INSET_CONTAINER.y, PADDED_BOTTOM], + ); + assert.equal(plan.pixels, PADDED_BOTTOM - INSET_CONTAINER.y); + assert.equal(plan.requestedPixels, unclamped.pixels); + assert.equal(Math.sign(plan.y2 - plan.y1), Math.sign(unclamped.y2 - unclamped.y1)); + }, +); + +test.each(['up', 'down', 'left', 'right'])( + 'a %s swipe already inside its container keeps today’s endpoints and distance', + (direction) => { + const unclamped = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME }); + const plan = buildScrollGesturePlan({ + direction, + amount: 0.75, + ...FRAME, + container: FITTING_CONTAINER, + }); + + assert.deepEqual(plan, unclamped); + assert.equal('requestedPixels' in plan, false); + }, +); + +test('a swipe that fits an offset container moves into it without shortening', () => { + const lowerContainer: Rect = { x: 0, y: 300, width: 402, height: 574 }; + const unclamped = buildScrollGesturePlan({ direction: 'down', amount: 0.5, ...FRAME }); + const plan = buildScrollGesturePlan({ + direction: 'down', + amount: 0.5, + ...FRAME, + container: lowerContainer, + }); + + assert.ok(endpointsInside(plan, lowerContainer), JSON.stringify(plan)); + assert.equal(Math.min(plan.y1, plan.y2), lowerContainer.y); + assert.equal(Math.abs(plan.y1 - plan.y2), Math.abs(unclamped.y1 - unclamped.y2)); + assert.equal(plan.pixels, unclamped.pixels); + assert.equal('requestedPixels' in plan, false); +}); + +test('a vertical swipe beside a narrow container runs down its centre line', () => { + const rightColumn: Rect = { x: 250, y: 0, width: 152, height: 874 }; + const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME }); + const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: rightColumn }); + + assert.equal(plan.x1, rightColumn.x + rightColumn.width / 2); + assert.equal(plan.x2, plan.x1); + assert.deepEqual([plan.y1, plan.y2, plan.pixels], [unclamped.y1, unclamped.y2, unclamped.pixels]); +}); + +test('a container lying wholly in the edge padding leaves the padded swipe as it was', () => { + const statusBand: Rect = { x: 0, y: 0, width: 402, height: PADDED_TOP - 1 }; + const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME }); + const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: statusBand }); + + assert.deepEqual(plan, unclamped); +}); diff --git a/packages/contracts/src/scroll-gesture.ts b/packages/contracts/src/scroll-gesture.ts index 5b8be2fe8f..bf450867d8 100644 --- a/packages/contracts/src/scroll-gesture.ts +++ b/packages/contracts/src/scroll-gesture.ts @@ -49,6 +49,12 @@ export type ScrollGestureOptions = { pixels?: number; referenceWidth: number; referenceHeight: number; + /** + * The scroll container the swipe is meant for, in the reference frame's own zero-origin space. + * Both endpoints stay inside its intersection with the padded frame (#2714): a span that already + * fits is unchanged, one that leaves it is moved in, and one longer than it is shortened. + */ + container?: Rect; }; export type ScrollGesturePlan = { @@ -60,7 +66,10 @@ export type ScrollGesturePlan = { referenceWidth: number; referenceHeight: number; amount?: number; + /** The span dispatched, which is less than `requestedPixels` when the container shortened it. */ pixels: number; + /** Present only when the container shortened the swipe: the span the padded frame would carry. */ + requestedPixels?: number; }; export type SwipePresetGesturePlan = { @@ -125,16 +134,65 @@ export function buildScrollGesturePlan(options: ScrollGestureOptions): ScrollGes pixels: travelPixels, }); - switch (direction) { - case 'up': - return buildPlan(centerX, centerY - halfTravel, centerX, centerY + halfTravel); - case 'down': - return buildPlan(centerX, centerY + halfTravel, centerX, centerY - halfTravel); - case 'left': - return buildPlan(centerX - halfTravel, centerY, centerX + halfTravel, centerY); - case 'right': - return buildPlan(centerX + halfTravel, centerY, centerX - halfTravel, centerY); - } + const vertical = direction === 'up' || direction === 'down'; + const unclamped = { + axisStart: (vertical ? centerY : centerX) - halfTravel, + axisEnd: (vertical ? centerY : centerX) + halfTravel, + cross: vertical ? centerX : centerY, + }; + const lane = options.container + ? keepSwipeInContainer(unclamped, options.container, vertical, { + min: edgePadding, + max: axisLength - edgePadding, + }) + : unclamped; + const shortened = lane.axisEnd - lane.axisStart < unclamped.axisEnd - unclamped.axisStart; + const plan = buildPlan( + vertical ? lane.cross : lane.axisStart, + vertical ? lane.axisStart : lane.cross, + vertical ? lane.cross : lane.axisEnd, + vertical ? lane.axisEnd : lane.cross, + ); + const placed = shortened + ? { ...plan, pixels: lane.axisEnd - lane.axisStart, requestedPixels: travelPixels } + : plan; + // `up`/`left` drag from the low end toward the high end; `down`/`right` drag the other way. + return direction === 'up' || direction === 'left' + ? placed + : { ...placed, x1: placed.x2, y1: placed.y2, x2: placed.x1, y2: placed.y1 }; +} + +type SwipeLane = { axisStart: number; axisEnd: number; cross: number }; + +/** + * Moves a centred swipe lane inside the container's intersection with the padded axis, shortening + * it only when the intersection is shorter than the lane. A container wholly inside the padding + * leaves nothing to swipe in, so the lane stays as the padded frame planned it. + */ +function keepSwipeInContainer( + lane: SwipeLane, + container: Rect, + vertical: boolean, + padded: { min: number; max: number }, +): SwipeLane { + const containerMin = vertical ? container.y : container.x; + const containerSize = vertical ? container.height : container.width; + const bandMin = Math.max(padded.min, Math.round(containerMin)); + const bandMax = Math.min(padded.max, Math.round(containerMin + containerSize)); + if (bandMax <= bandMin) return lane; + + const crossMin = vertical ? container.x : container.y; + const crossSize = vertical ? container.width : container.height; + const cross = + lane.cross >= crossMin && lane.cross <= crossMin + crossSize + ? lane.cross + : Math.round(crossMin + crossSize / 2); + + const length = lane.axisEnd - lane.axisStart; + if (length >= bandMax - bandMin) return { axisStart: bandMin, axisEnd: bandMax, cross }; + const shift = + lane.axisStart < bandMin ? bandMin - lane.axisStart : Math.min(0, bandMax - lane.axisEnd); + return { axisStart: lane.axisStart + shift, axisEnd: lane.axisEnd + shift, cross }; } /** diff --git a/packages/daemon/src/execution/__tests__/scroll-runtime.test.ts b/packages/daemon/src/execution/__tests__/scroll-runtime.test.ts index 0dd7118984..4219f10ba7 100644 --- a/packages/daemon/src/execution/__tests__/scroll-runtime.test.ts +++ b/packages/daemon/src/execution/__tests__/scroll-runtime.test.ts @@ -786,3 +786,70 @@ test('bound scroll will not bill an unread earlier gesture to the scroll it is d assert.equal(result.movement, 'unobserved'); assert.equal(captures.calls(), 0); }); + +/** + * The container a directional swipe is planned inside (#2714): read off the same current tree the + * movement claim compares against, before the gesture, and never off a tree that may no longer be + * the screen. + */ +async function containerHandedToOwner( + positionals: string[], + session: SessionState, + flags?: CommandFlags, +): Promise { + const calls: ScrollCall[] = []; + await runScroll( + positionals, + {}, + { + scroll: async (direction, options) => { + calls.push({ direction, options }); + return { ...OBSERVED_SWIPE }; + }, + }, + { session, ...(flags === undefined ? {} : { flags }) }, + ); + const [call] = calls; + assert.ok(call && calls.length === 1, `expected one scroll, got ${calls.length}`); + return (call.options as { containerRect?: unknown }).containerRect; +} + +test.each<[string, CommandFlags | undefined]>([ + ['an observed scroll', undefined], + ['a --settle scroll', { settle: true }], +])('bound scroll hands %s the container resolved from the current tree', async (_, flags) => { + const containerRect = await containerHandedToOwner( + ['down', '0.75'], + sessionWithStoredScreen(automationScreen(0)), + flags, + ); + + assert.deepEqual(containerRect, SCREEN_CONTAINER); +}); + +test('bound scroll hands no container from a tree that predates the last mutation', async () => { + const session = sessionWithStoredScreen(automationScreen(0)); + expireRefFrame(session); + + assert.equal(await containerHandedToOwner(['down', '0.75'], session), undefined); +}); + +test('bound scroll hands no container for a horizontal swipe the edge analyzer cannot place', async () => { + const containerRect = await containerHandedToOwner( + ['left'], + sessionWithStoredScreen(automationScreen(0)), + ); + + assert.equal(containerRect, undefined); +}); + +test('bound scroll hands no container to a desktop owner that never synthesizes a swipe', async () => { + const session = makeMacOsSession('scroll-desktop'); + session.snapshot = buildSnapshotState( + { nodes: automationScreen(0), backend: 'xctest', producer: 'apple-runner' }, + undefined, + ); + activateCompleteRefFrame(session); + + assert.equal(await containerHandedToOwner(['down', '0.75'], session), undefined); +}); diff --git a/packages/daemon/src/execution/scroll-movement.ts b/packages/daemon/src/execution/scroll-movement.ts index 250c25fb07..0fc1f0454e 100644 --- a/packages/daemon/src/execution/scroll-movement.ts +++ b/packages/daemon/src/execution/scroll-movement.ts @@ -164,17 +164,40 @@ export function readScrollSurfaceBaseline( function freezeScrollSurfaceBaseline(session: SessionState): ScrollMovementPlan { const baseline = readScrollSurfaceBaseline(session.snapshot); if (!baseline) return { kind: 'unobservable', reason: 'stored-surface-unusable' }; - if (refFrameState(session) !== 'active') { - return { kind: 'unobservable', reason: 'stored-surface-not-current' }; - } - if (isPostGestureStabilizationPending(session)) { - // A gesture nobody has read yet may still be moving this tree, which would bill its motion to - // the scroll being dispatched now. - return { kind: 'unobservable', reason: 'prior-gesture-unsettled' }; - } + const staleness = storedSurfaceStaleness(session); + if (staleness) return { kind: 'unobservable', reason: staleness }; return { kind: 'observe', baseline }; } +function storedSurfaceStaleness( + session: SessionState, +): Exclude | undefined { + if (refFrameState(session) !== 'active') return 'stored-surface-not-current'; + // A gesture nobody has read yet may still be moving this tree, which would bill its motion to + // the scroll being dispatched now. + if (isPostGestureStabilizationPending(session)) return 'prior-gesture-unsettled'; + return undefined; +} + +/** + * The scroll container a directional swipe should stay inside (#2714), read before the gesture off + * the same current tree a movement claim would compare against, through the edge verdict's own + * selection. A tree that may no longer be the screen names no container, and neither does a + * horizontal scroll, which the vertical-only edge analyzer cannot place, nor a platform that moves + * content without a synthesized swipe. + */ +export async function readScrollTargetContainer( + device: DeviceInfo, + session: SessionState, + direction: ScrollDirection, +): Promise { + if (!isMobilePlatform(device)) return undefined; + const edge = verticalEdgeFor(direction); + const nodes = session.snapshot?.nodes; + if (!edge || !nodes || nodes.length === 0 || storedSurfaceStaleness(session)) return undefined; + return (await readScrollEdgeState(nodes, edge)).containerRect; +} + /** * Observes one directional scroll and returns the movement its response may claim. * diff --git a/packages/daemon/src/execution/scroll-runtime.ts b/packages/daemon/src/execution/scroll-runtime.ts index 0716eadc0f..e58671749b 100644 --- a/packages/daemon/src/execution/scroll-runtime.ts +++ b/packages/daemon/src/execution/scroll-runtime.ts @@ -47,6 +47,7 @@ import { import { observeScrollMovement, planScrollMovement, + readScrollTargetContainer, reportScrollMovementNotApplicable, reportScrollMovementUnobserved, type ScrollMovementPlan, @@ -145,10 +146,23 @@ export async function resolveBoundScrollRuntime( flags: params.flags, session: params.session, }); + const containerRect = await readScrollTargetContainer( + params.device, + params.session, + target.direction, + ); + const directionOptions = + containerRect === undefined ? options : { ...options, containerRect }; return await resolveBoundGenericRuntime( { ...admission, use: plan.use }, async (runtime, dispatchContext) => - await executeDirectionScroll(runtime, target, options, dispatchContext, movementPlan), + await executeDirectionScroll( + runtime, + target, + directionOptions, + dispatchContext, + movementPlan, + ), ); } case 'edge': { diff --git a/packages/platform-android/src/__tests__/input-actions.test.ts b/packages/platform-android/src/__tests__/input-actions.test.ts index 35261477c9..29906d8337 100644 --- a/packages/platform-android/src/__tests__/input-actions.test.ts +++ b/packages/platform-android/src/__tests__/input-actions.test.ts @@ -134,6 +134,56 @@ test('scrollAndroid swipes the whole window when no IME window is on screen', as }); }); +function swipeSpan(result: Record): [number, number] { + const ys = [Number(result.y1), Number(result.y2)]; + return [Math.min(...ys), Math.max(...ys)]; +} + +test('scrollAndroid keeps both endpoints inside an inset container and reports the shortened span', async () => { + // A window below the status bar, holding a list that starts under a tall header: the unclamped + // 0.8 swipe would start in the header, so the gesture is shortened to the list (#2714). + const offsetWindow = { x: 0, y: 136, width: 1080, height: 2264 }; + const list = { x: 0, y: 436, width: 1080, height: 1700 }; + await withFakeAdb(helperRouteAdb(offsetWindow), async ({ device }) => { + const unclamped = await scrollAndroid(device, 'down', { amount: 0.8 }); + const result = await scrollAndroid(device, 'down', { amount: 0.8, containerRect: list }); + + assert.ok(swipeSpan(unclamped)[0] < list.y, 'the fixture must start outside the list'); + assert.deepEqual(swipeSpan(result), [list.y, list.y + list.height]); + assert.ok(Number(result.y1) > Number(result.y2), 'a down scroll still drags upward'); + assert.equal(result.pixels, list.height); + assert.equal(result.requestedPixels, unclamped.pixels); + }); +}); + +test('scrollAndroid keeps the swipe inside a container that runs under the keyboard', async () => { + // The keyboard clip alone keeps the frame's top, so a long swipe would start on the header above + // the list; the container and the clip together leave only the list above the keys. + const list = { x: 0, y: 400, width: 1080, height: 1880 }; + await withFakeAdb( + helperRouteAdb({ ...PORTRAIT_WINDOW, keyboard: LOWER_HALF_KEYBOARD }), + async ({ device }) => { + const result = await scrollAndroid(device, 'up', { amount: 0.8, containerRect: list }); + const [top, bottom] = swipeSpan(result); + + assert.equal(result.keyboardAvoided, true); + assert.equal(top, list.y); + assert.ok(bottom <= LOWER_HALF_KEYBOARD.y, `swipe endpoint ${bottom} landed on the keyboard`); + assert.equal(result.pixels, bottom - top); + }, + ); +}); + +test('scrollAndroid leaves a swipe that already fits its container unchanged', async () => { + const list = { x: 0, y: 100, width: 1080, height: 2100 }; + await withFakeAdb(helperRouteAdb({ ...PORTRAIT_WINDOW }), async ({ device }) => { + const unclamped = await scrollAndroid(device, 'down', { pixels: 600 }); + const result = await scrollAndroid(device, 'down', { pixels: 600, containerRect: list }); + + assert.deepEqual(result, unclamped); + }); +}); + test('scrollAndroid refuses rather than flinging into a keyboard that owns the window', async () => { // A landscape IME leaves 40px of a 900px window: a swipe there reads as a stuck surface, so the // command refuses with its own typed reason instead of the generic no-progress stop. diff --git a/packages/platform-android/src/input-actions.ts b/packages/platform-android/src/input-actions.ts index 49fbc0fbbc..342294b8f5 100644 --- a/packages/platform-android/src/input-actions.ts +++ b/packages/platform-android/src/input-actions.ts @@ -10,6 +10,7 @@ import { import { buildGesturePlan } from '@agent-device/contracts/gesture-plan'; import { GESTURE_DURATION_MIN_MS } from '@agent-device/contracts/gesture-plan-types'; import { + type ScrollContainerTarget, type ScrollReleaseBehavior, DEFAULT_MOBILE_SCROLL_DURATION_MS, } from '@agent-device/contracts/scroll-command'; @@ -220,7 +221,8 @@ export async function scrollAndroid( pixels?: number; durationMs?: number; releaseBehavior?: ScrollReleaseBehavior; - } & AndroidHelperSessionOptions, + } & ScrollContainerTarget & + AndroidHelperSessionOptions, ): Promise> { // The viewport read and the gesture are two helper calls one command apart: giving the read the // command's session scope keeps both on the same instrumentation. @@ -234,6 +236,9 @@ export async function scrollAndroid( pixels: options?.pixels, referenceWidth: swipeSurface.viewport.width, referenceHeight: swipeSurface.viewport.height, + ...(options?.containerRect === undefined + ? {} + : { container: relativeToViewport(options.containerRect, swipeSurface.viewport) }), }); const scrollPlan = { ...relativePlan, @@ -275,6 +280,11 @@ export async function scrollAndroid( }; } +/** The planner works in the swipe surface's zero-origin frame; snapshot rects are absolute. */ +function relativeToViewport(rect: Rect, viewport: Rect): Rect { + return { ...rect, x: rect.x - viewport.x, y: rect.y - viewport.y }; +} + /** * The band one Android scroll may swipe, refusing when the keyboard owns the window (#2500). *