Skip to content

Commit 6d08de4

Browse files
authored
feat(scroll): find off-screen targets in one command with --until (#2436)
* refactor(interaction): extract the scroll command runtime out of gestures.ts * feat(scroll): add --until <selector>, report honored travel, fix web amount units * test(scroll): cover --until through the provider-backed integration path * perf(selectors): keep the scroll-until predicate off the eager import path * fix(scroll): refuse an unreadable capture instead of reporting end-of-content * fix(selectors): keep the capture-readability check off the eager import path * test(selectors): use a declared snapshot quality state in the capture fixtures * fix(scroll): read the capture quality verdict under the spelling the backend uses * refactor(scroll): collapse --until onto the one route that runs it * refactor(scroll): drop unexported until types and duplicated guidance prose * test(scroll): fix the climbing fixture and drop duplicated route-level cases * refactor(scroll): delete the dead command-runtime executor and reuse canonical predicates * refactor(interaction): keep requireResolvedPoint local to the gesture runtime
1 parent da76aa4 commit 6d08de4

36 files changed

Lines changed: 1004 additions & 567 deletions

packages/capture-kit/src/snapshot/__tests__/scroll-edge-state-pass-orchestration.test.ts

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,45 +14,67 @@ import { captureThrows, scrollSnapshot, windowRoot } from './scroll-edge-state-f
1414

1515
test('formatScrollEdgeMessage: edge reached with zero passes reports already-at-edge (bottom)', () => {
1616
assert.equal(
17-
formatScrollEdgeMessage('down', 'bottom', 0, undefined, undefined),
17+
formatScrollEdgeMessage({ direction: 'down', edge: 'bottom', passes: 0 }),
1818
'Already at bottom; no hidden content below detected',
1919
);
2020
});
2121

2222
test('formatScrollEdgeMessage: edge reached with zero passes reports already-at-edge (top)', () => {
2323
assert.equal(
24-
formatScrollEdgeMessage('up', 'top', 0, undefined, undefined),
24+
formatScrollEdgeMessage({ direction: 'up', edge: 'top', passes: 0 }),
2525
'Already at top; no hidden content above detected',
2626
);
2727
});
2828

2929
test('formatScrollEdgeMessage: edge reached after N passes', () => {
3030
assert.equal(
31-
formatScrollEdgeMessage('down', 'bottom', 4, undefined, undefined),
31+
formatScrollEdgeMessage({ direction: 'down', edge: 'bottom', passes: 4 }),
3232
'Scrolled to bottom with 4 down passes',
3333
);
3434
});
3535

3636
test('formatScrollEdgeMessage: no edge, pixel amount given', () => {
3737
assert.equal(
38-
formatScrollEdgeMessage('down', undefined, 0, undefined, 250),
38+
formatScrollEdgeMessage({ direction: 'down', passes: 0, pixels: 250 }),
3939
'Scrolled down by 250px',
4040
);
4141
});
4242

4343
test('formatScrollEdgeMessage: no edge, no pixels, symbolic amount given', () => {
44-
assert.equal(formatScrollEdgeMessage('up', undefined, 0, 3, undefined), 'Scrolled up by 3');
44+
assert.equal(
45+
formatScrollEdgeMessage({ direction: 'up', passes: 0, amount: 3 }),
46+
'Scrolled up by 3',
47+
);
4548
});
4649

4750
test('formatScrollEdgeMessage: no edge, no pixels, no amount falls back to bare direction', () => {
51+
assert.equal(formatScrollEdgeMessage({ direction: 'left', passes: 0 }), 'Scrolled left');
52+
});
53+
54+
test('formatScrollEdgeMessage: pixels takes priority over amount when both are set', () => {
4855
assert.equal(
49-
formatScrollEdgeMessage('left', undefined, 0, undefined, undefined),
50-
'Scrolled left',
56+
formatScrollEdgeMessage({ direction: 'down', passes: 0, amount: 3, pixels: 250 }),
57+
'Scrolled down by 250px',
5158
);
5259
});
5360

54-
test('formatScrollEdgeMessage: pixels takes priority over amount when both are set', () => {
55-
assert.equal(formatScrollEdgeMessage('down', undefined, 0, 3, 250), 'Scrolled down by 250px');
61+
/**
62+
* One gesture saturates at the viewport axis minus its edge padding, so a large amount buys less
63+
* travel than it names. The message reports what the planner honored rather than what was asked.
64+
*/
65+
test('an amount-based message names the honored travel when the planner reports it', () => {
66+
assert.equal(
67+
formatScrollEdgeMessage({ direction: 'down', passes: 1, amount: 3, honoredPixels: 640 }),
68+
'Scrolled down by 3 of the viewport (640px)',
69+
);
70+
assert.equal(
71+
formatScrollEdgeMessage({ direction: 'down', passes: 1, amount: 0.65 }),
72+
'Scrolled down by 0.65',
73+
);
74+
assert.equal(
75+
formatScrollEdgeMessage({ direction: 'down', passes: 1, pixels: 5000, honoredPixels: 640 }),
76+
'Scrolled down by 640px',
77+
);
5678
});
5779

5880
// ---------------------------------------------------------------------------
@@ -279,7 +301,7 @@ test('runScrollEdgePasses: throws a COMMAND_FAILED AppError once the pass limit
279301
'scroll bottom reached the safety limit before the snapshot showed the edge',
280302
);
281303
assert.deepEqual(error.details, {
282-
hint: 'The scoped scroll container still reports hidden content. Use a smaller manual scroll + snapshot loop to inspect the current state.',
304+
hint: 'The scoped scroll container still reports hidden content. Run scroll <dir> --until <selector> to stop on the element you are after, or snapshot -i to inspect the current state.',
283305
});
284306
return true;
285307
},

packages/capture-kit/src/snapshot/scroll-edge-state.ts

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,18 @@ export async function captureScrollEdgeState(params: {
3737
}
3838
}
3939

40+
/**
41+
* Is there hidden content left at this edge? The same question `runScrollEdgePasses` loops on,
42+
* exposed for callers with their own stop condition (`scroll --until`) so both read one signal.
43+
*/
44+
export async function canScrollFurtherAtEdge(
45+
nodes: readonly (RawSnapshotNode | SnapshotNode)[],
46+
edge: ScrollEdge,
47+
): Promise<boolean> {
48+
const { analyzeScrollEdgeState } = await import('./scroll-edge-state/selection.ts');
49+
return analyzeScrollEdgeState(nodes, edge).canScroll;
50+
}
51+
4052
export async function runScrollEdgePasses<TResult>(params: {
4153
edge: ScrollEdge;
4254
captureState: (scope?: string) => Promise<ScrollEdgeState>;
@@ -56,7 +68,7 @@ export async function runScrollEdgePasses<TResult>(params: {
5668
'COMMAND_FAILED',
5769
`scroll ${edge} reached the safety limit before the snapshot showed the edge`,
5870
{
59-
hint: 'The scoped scroll container still reports hidden content. Use a smaller manual scroll + snapshot loop to inspect the current state.',
71+
hint: 'The scoped scroll container still reports hidden content. Run scroll <dir> --until <selector> to stop on the element you are after, or snapshot -i to inspect the current state.',
6072
},
6173
);
6274
}
@@ -69,19 +81,31 @@ export async function runScrollEdgePasses<TResult>(params: {
6981
return { passes, result };
7082
}
7183

72-
export function formatScrollEdgeMessage(
73-
direction: ScrollDirection,
74-
edge: ScrollEdge | undefined,
75-
passes: number,
76-
amount: number | undefined,
77-
pixels: number | undefined,
78-
): string {
84+
/**
85+
* `honoredPixels` is the travel the gesture planner actually produced, which is not always the
86+
* travel that was asked for: one gesture cannot cross more than the viewport axis minus its edge
87+
* padding, so a large `amount` saturates. Naming the honored distance is what keeps
88+
* `scroll down 3` from reporting a three-viewport scroll it never performed.
89+
*/
90+
export function formatScrollEdgeMessage(params: {
91+
direction: ScrollDirection;
92+
edge?: ScrollEdge | undefined;
93+
passes: number;
94+
amount?: number | undefined;
95+
pixels?: number | undefined;
96+
honoredPixels?: number | undefined;
97+
}): string {
98+
const { direction, edge, passes, amount, pixels, honoredPixels } = params;
7999
if (edge && passes === 0) {
80100
return `Already at ${edge}; no hidden content ${edge === 'bottom' ? 'below' : 'above'} detected`;
81101
}
82102
if (edge) return `Scrolled to ${edge} with ${passes} ${direction} passes`;
83-
if (pixels !== undefined) return `Scrolled ${direction} by ${pixels}px`;
84-
if (amount !== undefined) return `Scrolled ${direction} by ${amount}`;
103+
if (pixels !== undefined) return `Scrolled ${direction} by ${honoredPixels ?? pixels}px`;
104+
if (amount !== undefined) {
105+
return honoredPixels === undefined
106+
? `Scrolled ${direction} by ${amount}`
107+
: `Scrolled ${direction} by ${amount} of the viewport (${honoredPixels}px)`;
108+
}
85109
return `Scrolled ${direction}`;
86110
}
87111

packages/contracts/src/cli-flags.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ export type CliFlags = CloudProviderProfileFields &
109109
holdMs?: number;
110110
jitterPx?: number;
111111
pixels?: number;
112+
/** Scroll: repeat passes until this selector is visible on screen. */
113+
until?: string;
112114
doubleTap?: boolean;
113115
verify?: boolean;
114116
settle?: boolean;

packages/contracts/src/client-gesture.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,4 +146,6 @@ export type ScrollOptions = DeviceCommandBaseOptions &
146146
amount?: number;
147147
pixels?: number;
148148
durationMs?: number;
149+
/** Repeat scroll passes until this selector is visible on screen, then stop. */
150+
until?: string;
149151
};

packages/contracts/src/client-request.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ export type CommandExecutionOptions = Partial<ScreenshotRequestFlags> &
3939
holdMs?: number;
4040
jitterPx?: number;
4141
pixels?: number;
42+
/** Scroll: repeat passes until this selector is visible on screen. */
43+
until?: string;
4244
doubleTap?: boolean;
4345
verify?: boolean;
4446
settle?: boolean;

packages/contracts/src/platform-runtime-operations.ts

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,14 @@ export const gestureViewportRuntimeUse = defineUse({ required: ['gestureViewport
249249
/** `scroll <direction>` executes one pass and needs nothing else. */
250250
const scrollDirectionUse = defineUse({ required: ['scrollDirection'] });
251251
/**
252-
* `scroll top` / `scroll bottom` verify hidden content between passes, so the capture is part of
253-
* the tier's requirement rather than something discovered mid-run — the retired leaf's
252+
* Every scroll that verifies between passes: `scroll top`/`scroll bottom` read hidden content at
253+
* the edge, and `scroll --until <selector>` re-reads the tree to decide whether the target came
254+
* into view. Both check against that capture rather than a stale session snapshot, so the capture
255+
* is part of the tier's requirement rather than something discovered mid-run — the retired leaf's
254256
* "requires snapshot support to verify hidden content before scrolling" refusal, moved to
255-
* admission.
257+
* admission. One declaration, because the two tiers admit on identical facts.
256258
*/
257-
const scrollEdgeUse = defineUse({ required: ['scrollDirection', 'captureSnapshot'] });
259+
const scrollVerifiedPassUse = defineUse({ required: ['scrollDirection', 'captureSnapshot'] });
258260

259261
const gestureUsesByTier = Object.freeze({
260262
plan: gesturePlanUse,
@@ -275,7 +277,10 @@ export const gestureRuntimePlanUses = Object.freeze([
275277
export const swipeRuntimePlanUses = Object.freeze([gesturePlanUse] as const);
276278

277279
/** Every use `scroll` can select between. */
278-
export const scrollRuntimePlanUses = Object.freeze([scrollDirectionUse, scrollEdgeUse] as const);
280+
export const scrollRuntimePlanUses = Object.freeze([
281+
scrollDirectionUse,
282+
scrollVerifiedPassUse,
283+
] as const);
279284

280285
type GesturePlanFor<Tier extends GestureRuntimeTier> = Readonly<{
281286
tier: Tier;
@@ -337,15 +342,28 @@ function gesturePlan<const Tier extends GestureRuntimeTier>(tier: Tier): Gesture
337342
*/
338343
export type ScrollRuntimePlan =
339344
| Readonly<{ kind: 'direction'; use: typeof scrollDirectionUse }>
340-
| Readonly<{ kind: 'edge'; edge: 'top' | 'bottom'; use: typeof scrollEdgeUse }>;
345+
| Readonly<{ kind: 'edge'; edge: 'top' | 'bottom'; use: typeof scrollVerifiedPassUse }>
346+
| Readonly<{ kind: 'until'; until: string; use: typeof scrollVerifiedPassUse }>;
341347

342-
/** `scroll top`/`scroll bottom` verify between passes; every other scroll executes one pass. */
348+
/**
349+
* `scroll top`/`scroll bottom` verify between passes, `scroll --until` re-reads the tree between
350+
* passes to check its selector, and every other scroll executes one pass. The edge directions carry
351+
* their own stop condition, so pairing them with `--until` names two, which the caller rejects
352+
* before this resolves.
353+
*/
343354
export function resolveScrollRuntimePlan(
344-
input: Readonly<{ edge?: 'top' | 'bottom' }>,
355+
input: Readonly<{ edge?: 'top' | 'bottom'; until?: string }>,
345356
): ScrollRuntimePlan {
357+
if (input.until !== undefined) {
358+
return Object.freeze({
359+
kind: 'until',
360+
until: input.until,
361+
use: scrollVerifiedPassUse,
362+
} as const);
363+
}
346364
return input.edge === undefined
347365
? Object.freeze({ kind: 'direction', use: scrollDirectionUse } as const)
348-
: Object.freeze({ kind: 'edge', edge: input.edge, use: scrollEdgeUse } as const);
366+
: Object.freeze({ kind: 'edge', edge: input.edge, use: scrollVerifiedPassUse } as const);
349367
}
350368
const captureSnapshotWithCustomActionsUse = defineUse({
351369
required: ['captureSnapshot', 'captureSnapshotWithCustomActions'],

packages/contracts/src/scroll-command.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,24 @@ export function assertExclusiveScrollDistanceInputs(
4747
}
4848
}
4949

50+
/**
51+
* `top`/`bottom` are scroll-to-extreme requests that already carry a stop condition, so pairing one
52+
* with `--until` names two and the request has no single meaning. Rejected at the surface rather
53+
* than resolved by precedence, so neither stop condition can silently win.
54+
*/
55+
export function assertScrollUntilCompatible(
56+
input: Readonly<{ edge?: 'top' | 'bottom'; until?: string }>,
57+
): void {
58+
if (input.until === undefined || input.edge === undefined) return;
59+
throw new AppError(
60+
'INVALID_ARGS',
61+
`scroll ${input.edge} already scrolls to the ${input.edge} edge and cannot take --until`,
62+
{
63+
hint: `Use scroll ${input.edge === 'bottom' ? 'down' : 'up'} --until <selector> to stop at the target, or scroll ${input.edge} to reach the edge.`,
64+
},
65+
);
66+
}
67+
5068
export function normalizeScrollDurationMs(
5169
durationMs: number | undefined,
5270
options: { field?: string; invalidMessage?: string; max?: number } = {},
@@ -64,6 +82,13 @@ export function normalizeScrollDurationMs(
6482
return durationMs;
6583
}
6684

85+
/** The travel the planner produced, which saturates below a large requested amount. */
86+
export function honoredScrollPixels(
87+
result: Record<string, unknown> | undefined,
88+
): number | undefined {
89+
return typeof result?.pixels === 'number' ? result.pixels : undefined;
90+
}
91+
6792
export function honoredScrollDurationMs(
6893
result: Record<string, unknown> | undefined,
6994
): number | undefined {
@@ -84,7 +109,9 @@ export type ScrollCommandResult = {
84109
direction: ScrollDirection;
85110
/** Set for `top`/`bottom` requests: the extreme being scrolled to. */
86111
edge?: 'top' | 'bottom';
87-
/** Edge scrolls only: how many scroll-and-check passes ran. */
112+
/** Set for `--until` requests: the selector the passes stopped on. */
113+
until?: string;
114+
/** Edge and until scrolls only: how many scroll-and-check passes ran. */
88115
passes?: number;
89116
amount?: number;
90117
pixels?: number;

packages/contracts/src/scroll-gesture.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,13 @@ export type InPageSwipeGesturePlan = {
8383
referenceHeight: number;
8484
};
8585

86-
const DEFAULT_SCROLL_AMOUNT = 0.6;
86+
/**
87+
* The finger-path fraction of the viewport axis one scroll covers when the caller names no
88+
* distance. Exported because a backend with no viewport to measure against (the browser) scales its
89+
* own default step by the ratio to this, and that ratio is only meaningful while both sides read
90+
* the same number.
91+
*/
92+
export const DEFAULT_SCROLL_AMOUNT = 0.6;
8793
// Scroll gestures never touch the outer 10% of either axis. Modern app windows are edge-to-edge,
8894
// so the viewport includes the system bars: a swipe that starts inside the status bar (5.7% of a
8995
// Pixel 7's height, 6.9% of an iPhone's with a Dynamic Island) pulls the notification shade or

packages/platform-linux/src/input-actions.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { ensureInputTool } from './linux-env.ts';
22
import { resolveLinuxToolProvider, type LinuxPointerButton } from './tool-provider.ts';
33
import { sleep } from '@agent-device/host-kit/retry';
44
import type { ScrollDirection } from '@agent-device/contracts/scroll-gesture';
5+
import { DEFAULT_SCROLL_AMOUNT } from '@agent-device/contracts/scroll-gesture';
56

67
// ── Low-level wrappers ─────────────────────────────────────────────────
78

@@ -227,8 +228,11 @@ export async function scrollLinux(
227228
? Math.max(1, Math.round(options.pixels / 15))
228229
: Math.max(1, Math.round(options.pixels / 40));
229230
} else if (options?.amount != null) {
230-
// amount is a fraction (0–1+) of the viewport; scale relative to default
231-
scrollCount = Math.max(1, Math.round(DEFAULT_SCROLL_CLICKS * (options.amount / 0.6)));
231+
// amount is a fraction (0–1+) of the viewport; scale relative to the shared default
232+
scrollCount = Math.max(
233+
1,
234+
Math.round(DEFAULT_SCROLL_CLICKS * (options.amount / DEFAULT_SCROLL_AMOUNT)),
235+
);
232236
}
233237

234238
// xdotool: button 4=up, 5=down, 6=left, 7=right

packages/platform-web/src/agent-browser-provider.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,3 +851,28 @@ function createAudioProbeScriptPage(): AudioProbeScriptPage {
851851
},
852852
};
853853
}
854+
855+
/**
856+
* #2432: `amount` is a fraction of the viewport axis everywhere else, and the browser scrolls by
857+
* CSS pixels. Passing it through raw made `scroll down 0.5` travel half a pixel.
858+
*/
859+
test('a relative scroll amount reaches agent-browser as pixels, not as the fraction itself', async () => {
860+
await withManagedAgentBrowserProvider({ session: 'web-session' }, async (provider) => {
861+
const calls: AgentBrowserCall[] = [];
862+
863+
await withCommandExecutorOverride(recordingExecutor(calls), async () => {
864+
await provider.scroll('down', { amount: 0.6 });
865+
await provider.scroll('down', { amount: 1.2 });
866+
await provider.scroll('down', undefined);
867+
});
868+
869+
assert.deepEqual(
870+
calls.map((call) => call.args),
871+
[
872+
['scroll', 'down', '300', '--json', '--session', 'web-session'],
873+
['scroll', 'down', '600', '--json', '--session', 'web-session'],
874+
['scroll', 'down', '--json', '--session', 'web-session'],
875+
],
876+
);
877+
});
878+
});

0 commit comments

Comments
 (0)