diff --git a/packages/ad-script/src/index.ts b/packages/ad-script/src/index.ts index e498c751c3..ac520cc657 100644 --- a/packages/ad-script/src/index.ts +++ b/packages/ad-script/src/index.ts @@ -10,6 +10,8 @@ export { formatScriptStringLiteral, isClickLikeCommand, isTouchTargetCommand, + SCRIPT_FLAG_COMMANDS, + scriptFlagEntries, stripRecordedRefGeneration, } from './internal/script-utils.ts'; diff --git a/packages/ad-script/src/internal/__tests__/script.test.ts b/packages/ad-script/src/internal/__tests__/script.test.ts index dd18be09b0..d96ab76bac 100644 --- a/packages/ad-script/src/internal/__tests__/script.test.ts +++ b/packages/ad-script/src/internal/__tests__/script.test.ts @@ -205,6 +205,190 @@ test('snapshot replay script writes interactive refresh flags', () => { assert.match(script, /snapshot -i -d 2 -s @e1/); }); +// #3197: reaching an off-screen element used to be CLI-only. `scroll --until` was +// invisible to the script grammar, so its tokens fell through as positionals and the +// daemon read `--until` as the scroll amount ("scroll amount must be a number"). +test('scroll replay script parses the --until stop condition as a flag, not an amount', () => { + const parsed = parseReplayScriptDetailed( + String.raw`scroll down --until "id=\"far-button\""` + '\n', + ).actions; + + assert.deepEqual(parsed[0]?.positionals, ['down']); + assert.equal(parsed[0]?.flags.until, 'id="far-button"'); +}); + +test('scroll replay script keeps an amount positional beside the stop condition', () => { + const parsed = parseReplayScriptDetailed('scroll down 0.8 --until label=Email\n').actions; + + assert.deepEqual(parsed[0]?.positionals, ['down', '0.8']); + assert.equal(parsed[0]?.flags.until, 'label=Email'); +}); + +test('a scroll --until value with spaces survives as one selector when quoted', () => { + const parsed = parseReplayScriptDetailed( + String.raw`scroll down --until "label=\"Sign in\" || label=\"Log in\""`, + ).actions; + + assert.equal(parsed[0]?.flags.until, 'label="Sign in" || label="Log in"'); +}); + +test('scroll replay script writes its stop condition back where the parser reads it', () => { + const actions: SessionAction[] = [ + { + ts: Date.now(), + command: 'scroll', + positionals: ['down', '0.8'], + flags: { until: 'label="Sign in"' }, + }, + ]; + + const script = formatReplayScriptForTest(actions); + + // The generic writer quotes a non-`@` positional as a JSON literal (pre-existing + // for every generic line); the parser reads either spelling back. + assert.match(script, /scroll "down" 0\.8 --until "label=\\"Sign in\\""/); + const reparsed = parseReplayScriptDetailed(script).actions[0]; + assert.deepEqual(reparsed?.positionals, ['down', '0.8']); + assert.deepEqual(reparsed?.flags, { until: 'label="Sign in"' }); +}); + +// The other half of #3197: `--raw`, `--depth`, and `--scope` are declared on `wait` +// (`SELECTOR_SNAPSHOT_FLAGS`) and recorded, but a script line put them INSIDE the +// positional list, where the wait parser refused the line as selector-shaped text. +test('wait replay script parses its capture-scope flags out of the positionals', () => { + const parsed = parseReplayScriptDetailed( + [ + 'wait id="x" --raw', + 'wait --raw label=Email', + String.raw`wait "label=\"Sign in\"" --scope "@e3" --depth 2`, + 'wait "label=Checkout" 5000 --raw', + ].join('\n') + '\n', + ).actions; + + assert.deepEqual(parsed[0]?.positionals, ['id="x"']); + assert.equal(parsed[0]?.flags.snapshotRaw, true); + assert.deepEqual(parsed[1]?.positionals, ['label=Email']); + assert.equal(parsed[1]?.flags.snapshotRaw, true); + assert.deepEqual(parsed[2]?.positionals, ['label="Sign in"']); + assert.equal(parsed[2]?.flags.snapshotScope, '@e3'); + assert.equal(parsed[2]?.flags.snapshotDepth, 2); + // The budget positional and a capture flag compose in either written order. + assert.deepEqual(parsed[3]?.positionals, ['label=Checkout', '5000']); + assert.equal(parsed[3]?.flags.snapshotRaw, true); +}); + +// The `-d`/`-s` CLI aliases stay OUT of the script grammar: pre-existing lines +// like `wait text -s so funny` meant the literal text, and a grammar that +// reclassified them would silently change a passing script's oracle. Recordings +// only ever write the long spelling. +test('wait keeps the -d/-s CLI aliases out of the script grammar', () => { + const parsed = parseReplayScriptDetailed('wait text -d 2 hello world\n').actions; + + assert.deepEqual(parsed[0]?.positionals, ['text', '-d', '2', 'hello', 'world']); + assert.equal(parsed[0]?.flags.snapshotDepth, undefined); +}); + +test('wait replay script writes its capture-scope flags back', () => { + const actions: SessionAction[] = [ + { + ts: Date.now(), + command: 'wait', + positionals: ['label=Email', '2000'], + flags: { snapshotRaw: true, snapshotDepth: 3 }, + }, + ]; + + const script = formatReplayScriptForTest(actions); + + assert.match(script, /wait "label=Email" 2000 --raw --depth 3/); + const reparsed = parseReplayScriptDetailed(script).actions[0]; + assert.deepEqual(reparsed?.positionals, ['label=Email', '2000']); + assert.equal(reparsed?.flags.snapshotRaw, true); + assert.equal(reparsed?.flags.snapshotDepth, 3); +}); + +// The CLI hands a selector through the shell, whose single quotes strip to one +// argument; the same text in a `.ad` line split into fragments (#3197). +test('a single-quoted script token is one argument, with its double quotes intact', () => { + const parsed = parseReplayScriptDetailed( + ['press \'id="far-button"\'', 'wait \'label="Sign in"\' 2000'].join('\n') + '\n', + ).actions; + + assert.deepEqual(parsed[0]?.positionals, ['id="far-button"']); + assert.deepEqual(parsed[1]?.positionals, ['label="Sign in"', '2000']); +}); + +test('a single-quoted script token carries a --until selector with spaces', () => { + const parsed = parseReplayScriptDetailed('scroll down --until \'label="Sign in"\'\n').actions; + + assert.equal(parsed[0]?.flags.until, 'label="Sign in"'); +}); + +test('an apostrophe inside a bare token keeps its old meaning: no quote, no error', () => { + // Only a token LEADING with `'` is a quoting candidate, so a value that merely + // contains an apostrophe still parses as one bare token, as it always did. + const parsed = parseReplayScriptDetailed("wait text it's fine\n").actions; + + assert.deepEqual(parsed[0]?.positionals, ['text', "it's", 'fine']); +}); + +test("a quote that stops mid-word stays the apostrophe it was, not the shell's split", () => { + // The shell reads `'a b'c` as one glued argument. A script line has no second + // reader for that reading, and re-tokenizing would change what a previously-valid + // line means, so a closing quote that does not end the word keeps the old bare + // split (`'a` + `b'c`) rather than inventing a third meaning. + const parsed = parseReplayScriptDetailed("wait text 'a b'c\n").actions; + + assert.deepEqual(parsed[0]?.positionals, ['text', "'a", "b'c"]); +}); + +test('an unclosed single quote never turns a previously valid line into an error', () => { + // A value with one stray apostrophe is not a quoted token; it parses as bare + // tokens exactly as it did before single quotes were quoting characters. + const parsed = parseReplayScriptDetailed("wait text don't\n").actions; + + assert.deepEqual(parsed[0]?.positionals, ['text', "don't"]); +}); + +test("single quotes carry an apostrophe through ', and a backslash stays itself", () => { + // Shell parity: `agent-device wait 'label="don\'t"'` hands over the backslash- + // apostrophe pair, so the script has to read the same selector. A shell keeps a + // bare `\` inside single quotes, and so does the script line — including a `\\` + // pair, which the superseded decoder collapsed to one backslash; this assertion + // is what that regression would fail on. + const parsed = parseReplayScriptDetailed( + [ + String.raw`wait 'label="don\'t"'`, + String.raw`snapshot --scope 'a\\b'`, + String.raw`snapshot --scope 'root\.section'`, + ].join('\n') + '\n', + ).actions; + + assert.deepEqual(parsed[0]?.positionals, ['label="don\'t"']); + assert.equal(parsed[1]?.flags.snapshotScope, String.raw`a\\b`); + assert.equal(parsed[2]?.flags.snapshotScope, String.raw`root\.section`); +}); + +test('a quoted value ending in an even backslash run still closes', () => { + // `wait 'C:\\temp\\'` is one path with literal backslashes, not an unclosed + // quote: only an ODD run pairs with the quote as the apostrophe escape. The + // consequence is that a value ending in ONE literal backslash does not close in + // single quotes under this grammar (the `\'` escape owns that position); a + // double-quoted JSON string is the spelling for that one value. + const parsed = parseReplayScriptDetailed(String.raw`wait 'C:\\temp\\'` + '\n').actions; + + assert.deepEqual(parsed[0]?.positionals, [String.raw`C:\\temp\\`]); +}); + +test('apostrophes that survive decoding keep the old bare reading', () => { + // The shell reads `'don't do this'` as three arguments. A script line has no + // second reader to hand it to, so re-tokenizing would change what a + // previously-valid line means; it stays one bare token run, as it always was. + const parsed = parseReplayScriptDetailed("wait text 'don't do this'\n").actions; + + assert.deepEqual(parsed[0]?.positionals, ['text', "'don't", 'do', "this'"]); +}); + test('a pre-removal gesture line fails the whole script instead of step N', () => { assert.throws( () => diff --git a/packages/ad-script/src/internal/script-utils.ts b/packages/ad-script/src/internal/script-utils.ts index b30f3c6bd7..dd45aa73b5 100644 --- a/packages/ad-script/src/internal/script-utils.ts +++ b/packages/ad-script/src/internal/script-utils.ts @@ -32,7 +32,10 @@ export function stripRecordedRefGeneration(token: string): string { } const NUMERIC_ARG_RE = /^-?\d+(\.\d+)?$/; -const BARE_SCRIPT_TOKEN_RE = /^[^\s"\\]+$/; +// A token may not start with `'`: the tokenizer reads a leading `'` as a +// single-quoted literal (#3197), so the writer must quote such values or the +// re-parse would strip the apostrophe. +const BARE_SCRIPT_TOKEN_RE = /^[^\s"'\\][^\s"\\]*$/; const CLICK_LIKE_NUMERIC_FLAG_MAP = new Map( [ @@ -53,6 +56,123 @@ const GESTURE_NUMERIC_FLAG_MAP = new Map([ const TYPING_NUMERIC_FLAG_MAP = new Map([['--delay-ms', 'delayMs']]); +/** + * `scroll`'s stop condition, in the script grammar beside its `recorded: true` + * declaration (#3197): the hunt for an off-screen element IS the step, so a + * recorded `scroll down --until ` carries it and a hand-written script + * can say the same. Without this the tokens fall through as positionals and the + * daemon reads `--until` as the scroll amount. Distance stays a positional + * (`scroll down 0.8`): `--pixels` and `--duration-ms` are `recorded: false`, and a + * script grammar that accepted a flag the recorder cannot carry would write a line + * the recording path could never reproduce. + */ +const SCROLL_SCRIPT_FLAG_MAP = new Map([ + ['--until', { key: 'until', kind: 'string' }], +]); + +/** + * `wait`'s capture-scope flags (#3197): the command declares them + * (`SELECTOR_SNAPSHOT_FLAGS`) and they are recorded, so the script grammar + * recognizes them too. Otherwise they land inside the positional list and the + * wait parser refuses the line as selector-shaped text. Long spellings only: + * this is the spelling the writer emits for `wait`, so nothing the recorder can + * write needs an alias, and matching `-d`/`-s` would reclassify realistic + * waited text (`wait text -s so funny`) to buy almost nothing — the only line + * losing its old reading is one whose whole token is a literal long flag word, + * which a hand-written script can spell with the selector wrapped instead. + */ +const WAIT_SCRIPT_FLAG_MAP = new Map([ + ['--raw', { key: 'snapshotRaw', kind: 'boolean' }], + ['--depth', { key: 'snapshotDepth', kind: 'int' }], + ['--scope', { key: 'snapshotScope', kind: 'string' }], +]); + +/** How one script flag token carries its value. */ +type ScriptFlagEntry = { + key: 'until' | 'snapshotRaw' | 'snapshotDepth' | 'snapshotScope'; + kind: 'boolean' | 'int' | 'string'; +}; + +/** The commands whose script line carries flags (`scroll`, `wait`). */ +export type ScriptFlagCommand = 'scroll' | 'wait'; + +/** Which script flag tokens each flag-carrying command reads. */ +const SCRIPT_FLAG_MAPS: Record> = { + scroll: SCROLL_SCRIPT_FLAG_MAP, + wait: WAIT_SCRIPT_FLAG_MAP, +}; + +/** The commands whose script line carries flags, derived from the parse tables. */ +export const SCRIPT_FLAG_COMMANDS = Object.keys(SCRIPT_FLAG_MAPS) as readonly ScriptFlagCommand[]; + +/** + * The script flag tokens one command's line reads, with their value kinds and flag keys + * (#3197). Exported for the root admission test + * (`src/commands/replay/script-flag-admission.test.ts`), which proves the tables and the + * flag declarations admit the same keys in both directions — the invariant that keeps the + * script grammar and the flag declarations from diverging the way `--until` and + * `wait --raw` did. + */ +export function scriptFlagEntries( + command: string, +): ReadonlyArray<{ token: string } & ScriptFlagEntry> { + const flagMap = scriptFlagMapFor(command); + if (!flagMap) return []; + return [...flagMap].map(([token, entry]) => ({ token, ...entry })); +} + +function scriptFlagMapFor(command: string): Map | undefined { + return isScriptFlagCommand(command) ? SCRIPT_FLAG_MAPS[command] : undefined; +} + +// Membership comes from the tables' own keys, so a third command cannot compile into the +// type while the guard silently refuses to read its flags. +function isScriptFlagCommand(command: string): command is ScriptFlagCommand { + return (SCRIPT_FLAG_COMMANDS as readonly string[]).includes(command); +} + +/** + * Splits a `scroll` or `wait` script line into positionals and the command's own + * flags (#3197). A token is a flag only when it names one of the command's + * declared script flags and, for a value kind, a value token follows; anything + * else stays positional, so a hand-written target or text value is untouched. + */ +export function parseReplayCommandFlags( + command: ScriptFlagCommand, + args: string[], +): { positionals: string[]; flags: SessionAction['flags'] } { + const positionals: string[] = []; + const flags: SessionAction['flags'] = {}; + const flagMap = SCRIPT_FLAG_MAPS[command]; + + for (let index = 0; index < args.length; index += 1) { + const token = args[index]!; + const entry = flagMap.get(token); + const nextArg = args[index + 1]; + if (entry === undefined || (entry.kind !== 'boolean' && nextArg === undefined)) { + positionals.push(token); + continue; + } + if (entry.kind === 'boolean') { + Object.assign(flags, { [entry.key]: true }); + continue; + } + if (entry.kind === 'int') { + const parsed = parseNonNegativeIntToken(nextArg); + if (parsed === null) { + positionals.push(token); + continue; + } + Object.assign(flags, { [entry.key]: parsed }); + } else { + Object.assign(flags, { [entry.key]: nextArg }); + } + index += 1; + } + + return { positionals, flags }; +} + export function isClickLikeCommand(command: string): command is 'click' | 'press' { return command === 'click' || command === 'press'; } @@ -264,9 +384,34 @@ export function appendGenericActionScriptArgs(parts: string[], action: SessionAc if (action.command === 'fold' && action.flags?.keyframes !== undefined) { parts.push('--keyframes', formatScriptArg(action.flags.keyframes)); } + // #3197: `scroll`'s stop condition is part of the step's meaning, so the writer + // emits it beside the parser that reads it back. Only `--until` is declared + // recorded, so only `--until` can arrive here on a recorded action. + if (action.command === 'scroll' && typeof action.flags?.until === 'string') { + parts.push('--until', formatScriptArg(action.flags.until)); + } + if (action.command === 'wait') { + appendWaitSnapshotScriptFlags(parts, action.flags); + } appendScriptSeriesFlags(parts, action); } +/** + * `wait`'s capture-scope flags, written back in the long spelling its script + * parser reads (`SELECTOR_SNAPSHOT_FLAGS`, all declared recorded). + */ +function appendWaitSnapshotScriptFlags( + parts: string[], + flags: SessionAction['flags'] | undefined, +): void { + if (!flags) return; + if (flags.snapshotRaw === true) parts.push('--raw'); + if (typeof flags.snapshotDepth === 'number') parts.push('--depth', String(flags.snapshotDepth)); + if (typeof flags.snapshotScope === 'string') { + parts.push('--scope', formatScriptArg(flags.snapshotScope)); + } +} + // fallow-ignore-next-line complexity export function parseReplaySeriesFlags( command: string, diff --git a/packages/ad-script/src/internal/script.ts b/packages/ad-script/src/internal/script.ts index c5f6970d04..b18210b7b6 100644 --- a/packages/ad-script/src/internal/script.ts +++ b/packages/ad-script/src/internal/script.ts @@ -11,6 +11,7 @@ import { parseReplayOpenFlags } from './open-script.ts'; import type { SessionAction } from '@agent-device/contracts/session'; import { isClickLikeCommand, + parseReplayCommandFlags, parseReplaySeriesFlags, parseReplayRuntimeFlags, stripRecordedRefGeneration, @@ -462,11 +463,21 @@ function parseReplayScriptLine(line: string): SessionAction | null { return action; } - // wait @ref [timeout], longpress @ref [durationMs], and hover @ref flow - // through this generic branch: strip recorded generation pins like the - // branches above. + if (command === 'scroll' || command === 'wait') { + // #3197: these two commands carry their own flags in the script grammar, so + // a script can hunt for an off-screen target (`scroll down --until `) + // and scope a capture (`wait --raw`) the same way the CLI does. + const parsed = parseReplayCommandFlags(command, args); + + Object.assign(action.flags, parsed.flags); + action.positionals = parsed.positionals.map((token) => stripRecordedRefGeneration(token)); + return action; + } + + // wait @ref, longpress @ref, and hover @ref flow through the generic branch: + // strip recorded generation pins like the branches above. action.positionals = - command === 'wait' || command === 'longpress' || command === 'hover' + command === 'longpress' || command === 'hover' ? args.map((token) => stripRecordedRefGeneration(token)) : args; return action; @@ -490,7 +501,7 @@ function tokenizeReplayLine(line: string): string[] { const parsed = line[cursor] === '"' ? readQuotedReplayToken(line, cursor) - : readBareReplayToken(line, cursor); + : (readSingleQuotedReplayToken(line, cursor) ?? readBareReplayToken(line, cursor)); tokens.push(parsed.value); cursor = parsed.nextCursor; } @@ -542,6 +553,66 @@ function readQuotedReplayToken( return { value: value as string, nextCursor: end + 1 }; } +/** + * A single-quoted token: `'id="far-button"'` or `'label="Sign in"'` (#3197). The + * shell's single quotes strip to one argument and keep `"` literal; a hand-written + * `.ad` line is the same text, so the tokenizer must agree or the identical command + * parses at the CLI and fails in a script. The guarantee is scoped: a run the shell + * reads as one quoted argument NOW reads as one argument (`'Sign in'` was two bare + * tokens before, and taking the shell reading is the point), while a run it would + * not — an unclosed quote or a quote that stops mid-word (`wait text it's fine`, + * `'don't do this'`) — keeps its old bare meaning, so no line relied on before this + * change is re-tokenized. Inside the quotes only `\'` escapes, as in the shell, where + * a `\` keeps its own character; `\'` itself is the one deliberate extension, because + * a shell's single quotes carry no apostrophe at all and a selector like + * `label="don't"` has to be writable in a script without JSON double quotes. The + * escape only consumes a quote an ODD run of backslashes precedes, so a value ending + * in literal backslashes still closes; that also makes a value ending in one literal + * backslash unwritable in single quotes, and double quotes are the spelling for it. + */ +function readSingleQuotedReplayToken( + line: string, + cursor: number, +): { value: string; nextCursor: number } | null { + if (line[cursor] !== "'") return null; + const end = findSingleQuotedTokenEnd(line, cursor + 1); + if (end === -1 || !isReplayTokenBoundary(line, end + 1)) return null; + return { + value: decodeSingleQuotedReplayLiteral(line.slice(cursor + 1, end)), + nextCursor: end + 1, + }; +} + +/** The closing `'` index, or -1 when the quote never closes at this run. */ +function findSingleQuotedTokenEnd(line: string, from: number): number { + let index = from; + while (index < line.length) { + const quote = line.indexOf("'", index); + if (quote === -1) return -1; + if (!isBackslashEscaped(line, quote)) return quote; + index = quote + 1; + } + return -1; +} + +/** Whether the character at `index` follows an odd run of backslashes: the `\'` escape. */ +function isBackslashEscaped(line: string, index: number): boolean { + let backslashes = 0; + while (index > 0 && line.charAt(index - 1) === '\\') { + backslashes += 1; + index -= 1; + } + return backslashes % 2 === 1; +} + +function isReplayTokenBoundary(line: string, index: number): boolean { + return index >= line.length || /\s/.test(line.charAt(index)); +} + +function decodeSingleQuotedReplayLiteral(value: string): string { + return value.replaceAll(String.raw`\'`, "'"); +} + function readBareReplayToken(line: string, cursor: number): { value: string; nextCursor: number } { let end = cursor; while (end < line.length && !/\s/.test(line.charAt(end))) { diff --git a/packages/command-registry/src/flag-definitions-action.ts b/packages/command-registry/src/flag-definitions-action.ts index cd697ca8c3..fb8a32ab62 100644 --- a/packages/command-registry/src/flag-definitions-action.ts +++ b/packages/command-registry/src/flag-definitions-action.ts @@ -158,7 +158,11 @@ export const ACTION_FLAG_DEFINITIONS: readonly FlagDefinition[] = [ usageLabel: '--until ', usageDescription: 'Scroll: repeat passes until the selector is visible on screen', projectConfig: true, - recorded: false, + // #3197: the stop condition IS the step. A recorded scroll that hunted for + // an off-screen target has to replay as the same hunt; recorded as a bare + // `scroll down` it degrades to a fixed gesture that passes on one viewport + // and fails on another. + recorded: true, }, { key: 'doubleTap', diff --git a/packages/maestro/src/internal/export-flow.ts b/packages/maestro/src/internal/export-flow.ts index f50bd617e7..df611f2db7 100644 --- a/packages/maestro/src/internal/export-flow.ts +++ b/packages/maestro/src/internal/export-flow.ts @@ -341,8 +341,19 @@ function convertScreenshotAction(action: SessionAction): ConvertedAction { function convertScrollAction(action: SessionAction): ConvertedAction { const [direction] = action.positionals; - if (!direction || direction === 'down') return { kind: 'commands', commands: ['scroll'] }; - return { kind: 'unsupported', message: `scroll ${direction} is not exported yet` }; + if (direction && direction !== 'down') { + return { kind: 'unsupported', message: `scroll ${direction} is not exported yet` }; + } + // #3197: `--until` is now part of a scroll action, so the export says when it + // cannot carry the stop condition rather than exporting a bare page scroll and + // letting the flow lose the step that made it land on the target. + const until = typeof action.flags?.until === 'string' ? action.flags.until : undefined; + return { + kind: 'commands', + commands: ['scroll'], + warnings: + until === undefined ? [] : [`scroll --until ${until} is not represented by Maestro scroll`], + }; } function convertSwipeAction(action: SessionAction): ConvertedAction { diff --git a/packages/replay-test/src/internal/__tests__/session-test-discovery.test.ts b/packages/replay-test/src/internal/__tests__/session-test-discovery.test.ts index 9abad17d51..2f620df7d5 100644 --- a/packages/replay-test/src/internal/__tests__/session-test-discovery.test.ts +++ b/packages/replay-test/src/internal/__tests__/session-test-discovery.test.ts @@ -74,7 +74,62 @@ test('a suite that matched nothing after filtering is rejected', () => { platformFilter: 'android', discoverSources: sourcesOf({ path: '01-ios.ad', manifest: declared('ios') }), }), - (error: unknown) => error instanceof AppError && /No replay tests matched/.test(error.message), + (error: unknown) => + error instanceof AppError && + // #3197: the skip reason used to live only in the internal entry, so the + // caller saw a bare "no tests matched" for a declaration mismatch. + /1 declaring another platform/.test(error.message) && + /Run a source that declares android/.test(error.message), + ); +}); + +test('a suite whose only source declares no platform is told how to declare one', () => { + // The reported case: header-less files plus `--platform android` on a serial that + // already pins the device. + assert.throws( + () => + discoverReplayTestEntries({ + platformFilter: 'android', + discoverSources: sourcesOf({ path: '01-untyped.ad', manifest: unspecified }), + }), + (error: unknown) => + error instanceof AppError && + error.message === + 'No replay tests matched for --platform android: 1 without a platform declaration. ' + + 'Add "context platform=android" to the first line of a script that has none, ' + + 'or drop --platform when the device is already selected.', + ); +}); + +test('a filter value no script can declare is not answered with declaration advice', () => { + // `web` is excluded from ReplayTestPlatform (#1900): telling the caller to add + // `context platform=web` would send them editing a header the parser drops. + assert.throws( + () => + discoverReplayTestEntries({ + platformFilter: 'web', + discoverSources: sourcesOf({ path: '01-untyped.ad', manifest: unspecified }), + }), + (error: unknown) => + error instanceof AppError && + /No script can declare web as its platform; run this suite without --platform\./.test( + error.message, + ), + ); +}); + +test('a filtered suite with no sources at all keeps the plain sentence: nothing was skipped', () => { + // The reasons the message now names come from the filter visiting a source. With + // no source visited, the old sentence is the true one and stays byte-identical. + assert.throws( + () => + discoverReplayTestEntries({ + platformFilter: 'android', + discoverSources: sourcesOf(), + }), + (error: unknown) => + error instanceof AppError && + error.message === 'No replay tests matched for --platform android.', ); }); diff --git a/packages/replay-test/src/internal/session-test-discovery.ts b/packages/replay-test/src/internal/session-test-discovery.ts index 630c8e2fb8..57e77bd7b5 100644 --- a/packages/replay-test/src/internal/session-test-discovery.ts +++ b/packages/replay-test/src/internal/session-test-discovery.ts @@ -41,6 +41,9 @@ export function discoverReplayTestEntries(params: { const sources = discoverSources(); const entries: ReplayTestDiscoveryEntry[] = []; + // Both counts of why the filter matched nothing, accumulated together so the + // no-match message never re-derives one from an entry shape it does not own. + const filteredOut = { undeclared: 0, declaredOther: 0 }; for (const source of sources) { const { path: filePath, manifest } = source; const run = { kind: 'run', path: filePath, title: manifest.title, manifest } as const; @@ -57,6 +60,7 @@ export function discoverReplayTestEntries(params: { continue; } if (declared.kind === 'unspecified') { + filteredOut.undeclared += 1; entries.push({ kind: 'skip', path: filePath, @@ -66,6 +70,7 @@ export function discoverReplayTestEntries(params: { continue; } if (!matchesPlatformFilter(platformFilter, declared.value)) { + filteredOut.declaredOther += 1; continue; } entries.push(run); @@ -73,13 +78,78 @@ export function discoverReplayTestEntries(params: { const runnableCount = entries.filter((entry) => entry.kind === 'run').length; if (runnableCount === 0) { - const suffix = platformFilter ? ` for --platform ${platformFilter}` : ''; - throw new AppError('INVALID_ARGS', `No replay tests matched${suffix}.`); + throw new AppError( + 'INVALID_ARGS', + noReplayTestsMatchedMessage({ platformFilter, ...filteredOut }), + ); } return entries; } +/** Why a `--platform` filter found nothing, as the two counts the filter itself produced. */ +type NoReplayTestsMatchedReasons = { + platformFilter: PlatformSelector | undefined; + /** Sources with no platform declaration, which the filter skipped rather than dropped. */ + undeclared: number; + /** Sources declaring some other platform, which the filter dropped entirely. */ + declaredOther: number; +}; + +/** The selectors a script cannot declare in its `context platform=` header. */ +type NonDeclarablePlatform = Exclude; + +// An exhaustive table, not a list: a platform that joins the gap between the filter +// vocabulary and `ReplayTestPlatform` fails here as a missing key, and one that leaves it +// fails as an excess key — so the predicate can never silently send the remedy toward a +// header the parser drops (`web` is excluded for #1900). +const NON_DECLARABLE_PLATFORMS: Record = { web: true }; + +/** + * Whether a filter value is one a script can name in its `context platform=` header: the + * declarable vocabulary is `ReplayTestPlatform`, and the table above is enforced to be + * exactly its gap from `PlatformSelector`, so the predicate cannot silently fall behind. + */ +function isDeclarableReplayPlatform(filter: PlatformSelector): filter is ReplayTestPlatform { + return !Object.hasOwn(NON_DECLARABLE_PLATFORMS, filter); +} + +/** + * What "matched nothing" cost the caller (#3197). The two reasons read differently and the old + * bare sentence hid the one the user needed: a header-less file is not a platform mismatch, and + * the fix — a `context platform=` header, or dropping the filter — lived only in an internal skip + * record. The suite's failure projection carries a code and a message and no hint, so both the + * reason and the remedy belong in this one sentence. + */ +function noReplayTestsMatchedMessage(reasons: NoReplayTestsMatchedReasons): string { + const { platformFilter, undeclared, declaredOther } = reasons; + const suffix = platformFilter ? ` for --platform ${platformFilter}` : ''; + // Both counts come from the filter, so no filter (or no source at all) leaves the plain + // sentence exactly as it read before: there is no skip or drop to explain. + if (platformFilter === undefined || (undeclared === 0 && declaredOther === 0)) { + return `No replay tests matched${suffix}.`; + } + const found = [ + ...(undeclared > 0 ? [`${undeclared} without a platform declaration`] : []), + ...(declaredOther > 0 ? [`${declaredOther} declaring another platform`] : []), + ].join(', '); + return `No replay tests matched${suffix}: ${found}. ${noReplayTestsMatchedRemedy(platformFilter, undeclared)}`; +} + +/** + * The way out, stated only for a platform a source can actually name. `web` is excluded from + * `ReplayTestPlatform`, so telling a caller to declare it would send them editing a header that + * `readReplayScriptMetadata` then drops (#1900). + */ +function noReplayTestsMatchedRemedy(platformFilter: PlatformSelector, undeclared: number): string { + if (!isDeclarableReplayPlatform(platformFilter)) { + return `No script can declare ${platformFilter} as its platform; run this suite without --platform.`; + } + return undeclared > 0 + ? `Add "context platform=${platformFilter}" to the first line of a script that has none, or drop --platform when the device is already selected.` + : `Run a source that declares ${platformFilter}, or drop --platform.`; +} + export function buildReplayTestSessionName( sessionName: string, suiteInvocationId: string, diff --git a/src/__tests__/test-utils/property-arbitraries.ts b/src/__tests__/test-utils/property-arbitraries.ts index 4767f11f81..264551c65d 100644 --- a/src/__tests__/test-utils/property-arbitraries.ts +++ b/src/__tests__/test-utils/property-arbitraries.ts @@ -187,10 +187,32 @@ const scriptTextArb: fc.Arbitrary = fc.oneof( const scriptTargetArb: fc.Arbitrary = fc.oneof( refArb.map(formatRef), scriptTextArb.map((text) => JSON.stringify(`label=${text}`)), + // #3197: a hand-written line quotes a selector with single quotes the way the + // shell does. The formatter always rewrites to the JSON spelling, so this tests + // the tokenizer's reading without expecting the single-quote form back. + fc.constantFrom('Sign in', 'Log in', 'Submit').map((label) => `'label=${JSON.stringify(label)}'`), ); const coordinateArb = fc.integer({ min: 0, max: 1200 }).map(String); +/** + * The `wait` line tails the script grammar accepts after the target: its budget + * positional, and the capture-scope flags it declares (#3197). Both may appear on + * one line, positional before flags, which is the order the writer emits them in. + */ +const WAIT_TIMEOUT_ARB: fc.Arbitrary = fc + .integer({ min: 100, max: 5000 }) + .map((timeout) => ` ${timeout}`); + +const WAIT_CAPTURE_FLAG_ARB: fc.Arbitrary = fc.oneof( + fc.constant(' --raw'), + fc.integer({ min: 0, max: 6 }).map((depth) => ` --depth ${depth}`), + scriptTextArb.map((scope) => ` --scope ${JSON.stringify(scope)}`), +); + +/** The `scroll` flags the script grammar carries (#3197): the stop condition. */ +const SCROLL_FLAG_ARB: fc.Arbitrary = scriptTargetArb.map((target) => ` --until ${target}`); + /** * A command is either generated from a line template or explicitly waived with * the reason it needs none. `Record` makes the @@ -234,8 +256,12 @@ const REPLAY_SCRIPT_LINE_PLANS = { longpress: scriptTargetArb.map((target) => `longpress ${target}`), hover: scriptTargetArb.map((target) => `hover ${target}`), wait: fc - .tuple(scriptTargetArb, fc.integer({ min: 100, max: 5000 })) - .map(([target, timeout]) => `wait ${target} ${timeout}`), + .tuple( + scriptTargetArb, + fc.option(WAIT_TIMEOUT_ARB, { nil: '' }), + fc.option(WAIT_CAPTURE_FLAG_ARB, { nil: '' }), + ) + .map(([target, timeout, capture]) => `wait ${target}${timeout}${capture}`), fill: fc .tuple(scriptTargetArb, scriptTextArb) .map(([target, text]) => `fill ${target} ${JSON.stringify(text)}`), @@ -313,7 +339,16 @@ const REPLAY_SCRIPT_LINE_PLANS = { 'react-native': GENERIC_REPLAY_LINE, reinstall: GENERIC_REPLAY_LINE, replay: GENERIC_REPLAY_LINE, - scroll: GENERIC_REPLAY_LINE, + scroll: fc + .tuple( + fc.constantFrom(...SCROLL_DIRECTIONS), + fc.option(fc.constantFrom('0.5', '0.8'), { nil: undefined }), + fc.option(SCROLL_FLAG_ARB, { nil: undefined }), + ) + .map( + ([direction, amount, flags]) => + `scroll ${direction}${amount === undefined ? '' : ` ${amount}`}${flags ?? ''}`, + ), settings: GENERIC_REPLAY_LINE, shutdown: GENERIC_REPLAY_LINE, test: GENERIC_REPLAY_LINE, diff --git a/src/cli/commands/__tests__/replay-maestro-export.test.ts b/src/cli/commands/__tests__/replay-maestro-export.test.ts index 7826174c5d..774aaa5ec1 100644 --- a/src/cli/commands/__tests__/replay-maestro-export.test.ts +++ b/src/cli/commands/__tests__/replay-maestro-export.test.ts @@ -156,6 +156,27 @@ wait 500 ]); }); + // #3197: `scroll --until` became part of a scroll action, so the export names + // the stop condition it cannot carry instead of exporting a bare page scroll. + test('warns when scroll --until exports as a bare Maestro scroll', () => { + const result = exportReplayScriptToMaestro(`open com.example.app +scroll down --until 'id="far-button"' +`); + + expect(parseYamlDocs(result.yaml)).toEqual([ + { appId: 'com.example.app' }, + [{ launchApp: { appId: 'com.example.app' } }, 'scroll'], + ]); + expect(result.warnings).toEqual([ + { + line: 2, + // The action label names positionals only, so the flag shows in the message. + action: 'scroll down', + message: 'scroll --until id="far-button" is not represented by Maestro scroll', + }, + ]); + }); + test('warns when explicit long-press durations export to Maestro defaults', () => { const result = exportReplayScriptToMaestro(`open com.example.app longpress "label=\\"Last message\\"" 800 diff --git a/src/commands/replay/ad-script-round-trip.test.ts b/src/commands/replay/ad-script-round-trip.test.ts index 3d97f73903..b290855c9b 100644 --- a/src/commands/replay/ad-script-round-trip.test.ts +++ b/src/commands/replay/ad-script-round-trip.test.ts @@ -38,11 +38,17 @@ test('serializing a parsed script is a fixed point for generated scripts', () => const canonical = formatReplayScriptForTest(parsed); const reparsed = parseReplayScriptDetailed(canonical).actions; assert.equal(formatReplayScriptForTest(reparsed), canonical); - // The action identity survives the rewrite: same commands, same targets. - assert.deepEqual( - reparsed.map((action) => [action.command, action.positionals]), - parsed.map((action) => [action.command, action.positionals]), - ); + // The action identity survives the rewrite: same commands, same targets, and + // the same flags (#3197 — a dropped flag would silently degrade a hunt or a + // scoped capture into a plain step). One declared normalization aside: + // `--button primary` is the default, so the writer canonically omits it. + const identity = (actions: SessionAction[]) => + actions.map((action) => { + const flags = { ...action.flags }; + if (flags.clickButton === 'primary') delete flags.clickButton; + return [action.command, action.positionals, flags]; + }); + assert.deepEqual(identity(reparsed), identity(parsed)); }), { numRuns: PROPERTY_RUNS }, ); diff --git a/src/commands/replay/script-flag-admission.test.ts b/src/commands/replay/script-flag-admission.test.ts new file mode 100644 index 0000000000..d4c0688a97 --- /dev/null +++ b/src/commands/replay/script-flag-admission.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, test } from 'vitest'; +import { SCRIPT_FLAG_COMMANDS, scriptFlagEntries } from '@agent-device/ad-script'; +import { + COMMON_COMMAND_SUPPORTED_FLAG_KEYS, + DEVICE_SELECTION_FLAG_KEYS, +} from '@agent-device/command-registry/flag-groups'; +import { getCliCommandSchema } from '../schema/command-schema.ts'; +import { + getFlagDefinitionsForKey, + recordedFlagKeys, +} from '@agent-device/command-registry/flag-registry'; + +/** + * #3197 was a divergence, not a bug: `--until` existed as a CLI flag with + * `recorded: false`, and `wait --raw` was declared on the command and recorded, but + * neither was admitted to the `.ad` script grammar — so the CLI form and the script + * line disagreed and the script's own flag became a positional. The commands that + * carry flags in their script line are declared in + * `packages/ad-script/src/internal/script-utils.ts`; this pins admission in BOTH + * directions so neither half can drift again: + * + * - a script token the grammar reads must be a long-spelled flag the command accepts, + * with the same spelling and value kind the declaration gives it, and one the + * recorder may carry (a grammar that accepted a `recorded: false` flag would parse + * a line the recording path can never write); + * - the reverse: a flag the command accepts AND the recorder carries must be in the + * grammar, because that flag is exactly what a recording will one day write, and + * a script line carrying it must parse (the failure `--until` and `wait --raw` hit). + */ +describe.each(SCRIPT_FLAG_COMMANDS)('%s script flags', (command) => { + const entries = scriptFlagEntries(command); + const keys: string[] = [...new Set(entries.map((entry) => entry.key))]; + const acceptedFlagKeys = new Set([ + ...(getCliCommandSchema(command).allowedFlags ?? []), + ...(getCliCommandSchema(command).supportedFlags ?? []), + ]); + + test('the command declares at least one script flag', () => { + expect(entries.length).toBeGreaterThan(0); + }); + + test('every flag the script line carries is one the command itself accepts', () => { + expect(keys.filter((key) => !acceptedFlagKeys.has(key))).toEqual([]); + }); + + test('every flag the script line carries is one the recorder may carry', () => { + expect(keys.filter((key) => !recordedFlagKeys().has(key as never))).toEqual([]); + }); + + test('every script token matches the declaration spelling and kind exactly', () => { + // Long spelling only, and only a spelling the CLI itself declares: a short + // alias invented here would parse a line the recording never writes, and a + // single-letter token would reclassify positional data the grammar used to + // leave alone. + for (const entry of entries) { + expect(entry.token.startsWith('--')).toBe(true); + const definitions = getFlagDefinitionsForKey(entry.key as never); + const definition = definitions.find((candidate) => candidate.names.includes(entry.token)); + expect(definition?.type).toBe(entry.kind); + } + }); + + test('every recorded flag the command owns is in the script grammar', () => { + // The direction #3197 actually broke: a flag a recording can write must be a + // token the parser can read back, or the writer's own line would not replay. + // Scoped to the flags the command owns — the common parser flags (device + // selection, daemon wiring, `--no-record`) are recorded but deliberately not + // part of a step, so they stay out of the grammar by declaration. + const recorded = recordedFlagKeys(); + const missing = [...acceptedFlagKeys].filter( + (key) => + recorded.has(key as never) && !isCommonOrDeviceSelectionFlagKey(key) && !keys.includes(key), + ); + expect(missing).toEqual([]); + }); +}); + +function isCommonOrDeviceSelectionFlagKey(key: string): boolean { + return ( + (COMMON_COMMAND_SUPPORTED_FLAG_KEYS as readonly string[]).includes(key) || + DEVICE_SELECTION_FLAG_KEYS.has(key as never) + ); +} + +test('a command outside the declared set carries no script flags, so its line stays all-positional', () => { + // The generic script branch treats every token as a positional, so a command + // cannot gain a script flag without its own parse branch. `press` is the proof: + // its `--button` handling lives in the click-like branch, not in this grammar. + expect(scriptFlagEntries('press')).toEqual([]); + expect(scriptFlagEntries('snapshot')).toEqual([]); +}); diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 0ac4b4829f..995115e4d2 100644 --- a/src/commands/schema/cli-help.ts +++ b/src/commands/schema/cli-help.ts @@ -163,6 +163,10 @@ Script paths are the caller's: replay and test resolve and read on the machine running the command, then send the script content (Maestro runFlow includes too) with the request. The same flows therefore run against a local daemon and against a remote one (AGENT_DEVICE_DAEMON_BASE_URL) with no copy step, and a missing script fails immediately, naming the path you typed. --save-script writes on the DAEMON host and is rejected against a remote daemon. test --json marks a failed test with infrastructure: true only when the owning runtime classified a device, runner, boot, or transport failure. It remains a failed test; consumers may use the tag to distinguish "the oracle did not run" from a behavioral replay divergence without weakening either gate. +Script line grammar: + A .ad line is [positional...] [flag...]. Whitespace splits tokens; a token quoted with " or ' is one argument, and single quotes keep a double quote literal, exactly as at the shell (press 'id="far"' is one selector). Values in double quotes are JSON strings (escape \\\\, \\", \\t, \\n). Values in single quotes are literal, as at the shell: a backslash keeps its own character and the only escape is \\' for an apostrophe. A script carries only the flags declared for that command and marked recorded, so the script form of a step matches the CLI form: scroll down --until 'id="x"', wait 'label="Sign in"' --raw. CLI-only spellings and per-request options are not part of a step: --settle, --verify, scroll --pixels/--duration-ms, and the device-selection flags (--platform, --serial, --device) are the common ones a script does not carry. + Reaching an off-screen target is viewport-independent in a script the same way it is at the CLI: write scroll down --until , not a fixed scroll amount that passes on one screen size and fails on another. + Reusable open-to-destination scripts: Arm recording on the first open, perform the full journey, verify the destination with a selector-targeted wait, then publish without closing: agent-device open com.example.app --relaunch --save-script=screen-x.ad diff --git a/src/daemon/__tests__/replay-runtime/session-replay-action-runtime.test.ts b/src/daemon/__tests__/replay-runtime/session-replay-action-runtime.test.ts index e50b53df9f..8399af8420 100644 --- a/src/daemon/__tests__/replay-runtime/session-replay-action-runtime.test.ts +++ b/src/daemon/__tests__/replay-runtime/session-replay-action-runtime.test.ts @@ -194,3 +194,35 @@ test('replay never defaults readinessTimeoutMs onto a non-acting step', async () expect(response.ok).toBe(true); expect(dispatchedFlags?.readinessTimeoutMs).toBeUndefined(); }); + +// #3197: a script line now parses `scroll down --until ` into a flag, and the +// dispatch reads the stop condition off the request flags. If the replay dispatch dropped +// action flags for this command, the hunt would degrade to one fixed gesture again. +test('a scroll step carries its parsed --until flag onto the dispatch', async () => { + const action: SessionAction = { + ts: 0, + command: 'scroll', + positionals: ['down'], + flags: { until: 'label="Checkout"' }, + }; + let dispatched: { positionals?: string[]; flags?: Record } | undefined; + const response = await invokeReplayAction({ + req: REPLAY_REQUEST, + sessionName: 'default', + action, + resolved: action, + filePath: 'flow.ad', + line: 1, + step: 1, + resolvedSessionScope: undefined, + dependencies: replayDaemonDependencies, + invoke: async (request) => { + dispatched = request; + return { ok: true, data: {} }; + }, + }); + + expect(response.ok).toBe(true); + expect(dispatched?.positionals).toEqual(['down']); + expect(dispatched?.flags?.until).toBe('label="Checkout"'); +}); diff --git a/src/daemon/__tests__/replay-suite/session-test-runner.test.ts b/src/daemon/__tests__/replay-suite/session-test-runner.test.ts index a8630547f2..9ae03948b9 100644 --- a/src/daemon/__tests__/replay-suite/session-test-runner.test.ts +++ b/src/daemon/__tests__/replay-suite/session-test-runner.test.ts @@ -413,7 +413,10 @@ test('test returns invalid args when no replay scripts match the platform filter invoke: noopInvoke, }); - assertInvalidArgsMessage(response, 'No replay tests matched for --platform android.'); + assertInvalidArgsMessage( + response, + 'No replay tests matched for --platform android: 1 declaring another platform. Run a source that declares android, or drop --platform.', + ); }); test('test rejects duplicate replay test metadata in the context header', async () => { diff --git a/src/daemon/__tests__/replay-suite/session-test-suite.test.ts b/src/daemon/__tests__/replay-suite/session-test-suite.test.ts index 5106db8f23..a3b8b1fc1e 100644 --- a/src/daemon/__tests__/replay-suite/session-test-suite.test.ts +++ b/src/daemon/__tests__/replay-suite/session-test-suite.test.ts @@ -903,6 +903,10 @@ test('test sharding does not require devices when every entry is skipped', async expect(response?.ok).toBe(false); if (response?.ok !== false) throw new Error('Expected failed daemon response.'); expect(response.error.code).toBe('INVALID_ARGS'); - expect(response.error.message).toBe('No replay tests matched for --platform android.'); + expect(response.error.message).toBe( + 'No replay tests matched for --platform android: 1 without a platform declaration. ' + + 'Add "context platform=android" to the first line of a script that has none, ' + + 'or drop --platform when the device is already selected.', + ); expect(inventoryResolved).toBe(false); }); diff --git a/src/daemon/__tests__/session-script-writer.test.ts b/src/daemon/__tests__/session-script-writer.test.ts index 2748d2f125..a32a2e392e 100644 --- a/src/daemon/__tests__/session-script-writer.test.ts +++ b/src/daemon/__tests__/session-script-writer.test.ts @@ -81,6 +81,44 @@ test('write() round-trips wait absent positionals without adding an annotation', expect(parsed.actions[0]?.positionals).toEqual(['absent', 'label="Removed"', '2500']); }); +// #3197: the recording half of `scroll --until`. A recorded hunt for an +// off-screen element has to replay as the same hunt, so the flag must survive the +// recorder's declared-key filter AND the writer's line, then parse back as a flag. +test('write() carries a recorded scroll --until through to the script and back', () => { + const root = mkdtempForTestSync('agent-device-script-writer-scroll-until-'); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const session = makeAuthoringSession('default'); + recordActionEntry(session, { + command: 'scroll', + positionals: ['down'], + flags: { until: 'label="Sign in"', settle: true }, + result: { direction: 'down' }, + }); + + const { parsed } = writeAndParse(writer, session); + expect(parsed.actions[0]?.command).toBe('scroll'); + expect(parsed.actions[0]?.positionals).toEqual(['down']); + expect(parsed.actions[0]?.flags.until).toBe('label="Sign in"'); + // `--settle` is a per-request observation, not part of the step. + expect(parsed.actions[0]?.flags.settle).toBeUndefined(); +}); + +test('write() carries a recorded wait --raw through to the script and back', () => { + const root = mkdtempForTestSync('agent-device-script-writer-wait-raw-'); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const session = makeAuthoringSession('default'); + recordActionEntry(session, { + command: 'wait', + positionals: ['label="Sign in"', '2000'], + flags: { snapshotRaw: true }, + result: {}, + }); + + const { parsed } = writeAndParse(writer, session); + expect(parsed.actions[0]?.positionals).toEqual(['label="Sign in"', '2000']); + expect(parsed.actions[0]?.flags.snapshotRaw).toBe(true); +}); + test('a boundary-sliced script still strips diagnostic snapshot actions', () => { const root = mkdtempForTestSync('agent-device-script-writer-snapshot-strip-'); const writer = new SessionScriptWriter(path.join(root, 'sessions')); diff --git a/src/daemon/handlers/__tests__/session-command-replay.test.ts b/src/daemon/handlers/__tests__/session-command-replay.test.ts index 56d2929ae4..53cd598574 100644 --- a/src/daemon/handlers/__tests__/session-command-replay.test.ts +++ b/src/daemon/handlers/__tests__/session-command-replay.test.ts @@ -122,6 +122,48 @@ test('replay parses inline open runtime flags and replays open with runtime payl }); }); +// #3197 end to end through the handler: a script line that hunts for an off-screen +// target and scopes a capture must arrive at the dispatched step as flags, not as +// poisoned positionals. Parse alone passing here would hide a drop in the replay +// dispatch, which is exactly how the CLI form and the script form diverged. +test('replay delivers scroll --until and wait --raw flags to the dispatched steps', async () => { + const sessionStore = makeSessionStore(); + const replayRoot = mkdtempForTestSync('agent-device-replay-scroll-until-'); + const replayPath = path.join(replayRoot, 'hunt.ad'); + fs.writeFileSync( + replayPath, + 'scroll down --until \'label="General"\'\nwait \'label="Order summary"\' 5000 --raw\n', + ); + + const invoked: DaemonRequest[] = []; + const response = await handleSessionCommands({ + req: { + token: 't', + session: 'default', + command: 'replay', + positionals: [replayPath], + flags: { replayScriptSource: replayScriptSourceBundleFor(replayPath) }, + meta: { cwd: replayRoot }, + }, + sessionName: 'default', + logPath: path.join(mkdtempForTestSync('daemon'), 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req); + return { ok: true, data: {} }; + }, + }); + + expect(response?.ok).toBe(true); + expect(invoked.length).toBe(2); + expect(invoked[0]?.command).toBe('scroll'); + expect(invoked[0]?.positionals).toEqual(['down']); + expect(invoked[0]?.flags?.until).toBe('label="General"'); + expect(invoked[1]?.command).toBe('wait'); + expect(invoked[1]?.positionals).toEqual(['label="Order summary"', '5000']); + expect(invoked[1]?.flags?.snapshotRaw).toBe(true); +}); + test('replay inherits parent device selectors for each invoked step', async () => { const sessionStore = makeSessionStore(); const replayRoot = mkdtempForTestSync('agent-device-replay-parent-selectors-'); @@ -231,6 +273,10 @@ test('test --platform web reports no matching scripts, typed or untyped, because expect(response?.ok).toBe(false); if (response && !response.ok) { expect(response.error.code).toBe('INVALID_ARGS'); - expect(response.error.message).toBe('No replay tests matched for --platform web.'); + expect(response.error.message).toBe( + 'No replay tests matched for --platform web: 1 without a platform declaration, ' + + '1 declaring another platform. No script can declare web as its platform; ' + + 'run this suite without --platform.', + ); } }); diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 0c8b66c295..f8d4b69c64 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -547,8 +547,10 @@ Target-authored drag is supported on Android touch devices and iOS/iPadOS. Backe On iOS simulators it uses private XCTest synthesis for a continuous two-finger pan/scale/rotation path, so verify app-level metrics instead of assuming the requested values map exactly to recognizer output. On Android, `gesture transform` injects a geometric two-finger path. App recognizers may report non-exact pan, scale, and rotation values, so verify qualitative state such as `pan changed yes`, `pinch changed yes`, and `rotate changed yes` unless the app explicitly promises exact centroid metrics. If exact app-state values matter, prefer isolated `gesture pan`, `gesture pinch`, or `gesture rotate` commands. `scroll` accepts either a relative amount (`0.5` means a finger path spanning half of the viewport on that axis) or `--pixels ` for a fixed-distance gesture. Directional scrolls decelerate through the drag on Android to reduce release momentum within the requested duration; `scroll top` and `scroll bottom` retain inertial release for edge traversal. Reduced momentum does not guarantee an exact content offset, especially for very short gestures: apps apply pan-recognition thresholds, collapsing headers, bounds, and their own scroll physics. Large distances are clamped to the usable drag band so the gesture stays reliable across Android, iOS, and macOS. +A recorded `scroll ... --until ` step keeps its stop condition in the `.ad` script it writes, so the replay repeats the same hunt instead of one fixed gesture. A script line carries only what the flag declaration marks recorded, so distance stays a positional there (`scroll down 0.8`); `--pixels`, `--duration-ms`, and `--settle` belong to the CLI invocation, not to a step. A directional scroll places its swipe across the middle of the viewport, so a focused field and its keyboard would put the swipe under the keys: the gesture would land on the keyboard, the surface would not move, and the scroll would read as stuck. On iOS and Android the scroll instead keeps the whole swipe in the band above the keyboard, reporting `keyboardAvoided` and `keyboardMinY` alongside a `referenceHeight` and `pixels` measured against that shorter band. It never dismisses the keyboard, because dismissing drops focus and breaks a `fill`/`scroll`/`fill` loop; run `keyboard dismiss` yourself when you want that. When the keyboard leaves too little room to swipe, the command refuses with the `scroll_keyboard_occludes_surface` reason rather than swiping into the keys, so a scroll that cannot work says so instead of appearing stuck. A directional scroll also reports what it *saw*, as `movement`, because the reported distance describes the swipe that was dispatched rather than content that moved. `moved` means the content inside the scroller the swipe ran in differs from the tree the session held immediately before the gesture. `at-edge` means it did not change and the resolved container reported no hidden content left in that direction, and `unchanged` is the same measurement in a direction that has no end-of-content signal to read. `unobserved` means the pair could not back a claim in either direction — no stored tree, a stored tree the session no longer stands behind or that was captured differently, a surface that never came to rest, or a difference sitting entirely outside the scroller that was swiped, which a changing Android status bar does — so the distance rests on the gesture plan alone, and it is answered honestly rather than dressed up as a confirmation. When the surface is provably unchanged while the container the gesture ran inside still reports hidden content in that direction, the command refuses with the `scroll_no_progress` reason instead of repeating the requested distance: the gesture never reached that list, and the hint names the three ways it usually goes missing (a focused keyboard, a nested scroller, a list that ignores synthesized scrolls and needs a raw `swipe`). A scroll on a runtime that cannot read a screen carries no `movement` field at all, and a Maestro replay or a `--settle` caller is not charged a second observation of a fact its own flags already own. +`scroll --until ` is how you reach an off-screen target without guessing a distance: it repeats the gesture and re-reads the tree between passes, stopping the moment the selector is on screen rather than sailing past it, and it fails when the content runs out first. The amount is optional beside it (`scroll down 0.8 --until `) to widen each pass. The same form works inside a `.ad` script, so a flow brings a target on screen in a viewport-independent way instead of a fixed amount that passes on one screen size and fails on another. Default snapshot text output is visible-first, so off-screen interactive content is summarized instead of shown as tappable refs. When a target only appears in an off-screen summary, use `scroll --settle`: the response waits for the UI to go quiet and returns the diff against the tree you last observed, with fresh refs on the added lines, so no follow-up `snapshot -i` is needed. `back --settle` does the same for navigation. Both are best-effort and never fail the action. For repeated checks without settle, a small shell loop is enough: diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 13486bf252..7b6f384f67 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -40,6 +40,36 @@ agent-device open Settings --platform ios --session e2e --save-script ./workflow - Parent directories are created automatically when they do not exist. - For ambiguous bare values, use `--save-script=workflow.ad` or a path-like value such as `./workflow.ad`. +## `.ad` line grammar + +A `.ad` line is the CLI spelling of one command: ` [positional ...] [flag ...]`. Whitespace separates tokens, so a value with a space needs quotes. + +```sh +open "com.example.app" --relaunch +scroll down --until 'id="far-button"' +press id="far-button" +wait 'label="Order summary"' 5000 +close +``` + +- A token quoted with `"` or `'` is one argument. Single quotes keep a `"` literal, so `'id="far-button"'` and `"id=\"far-button\""` are the same selector — write whichever matches how you typed the command at the shell. +- Values in double quotes are JSON strings, so escape `\\`, `\"`, `\t`, and `\n`. Values in single quotes are literal, as at the shell: a backslash keeps its own character, and the only escape is `\'` for an apostrophe. +- A script carries only the flags declared for that command and marked recorded; CLI-only spellings and per-request options are not part of a step. `--settle`, `--verify`, `scroll --pixels`/`--duration-ms`, and the device-selection flags (`--platform`, `--serial`, `--device`) are the common ones a script does not carry. +- `help ` prints the flags each command accepts. `help scripting` prints this grammar. + +Reaching an off-screen element is viewport-independent in a script exactly as it is at the CLI. Prefer the stop condition over a fixed amount, which passes on one screen size and fails on another: + +```sh +# repeats until the element is on screen +scroll down --until 'id="checkout-submit"' +# one gesture, viewport-relative +scroll down 0.8 +# run to the end of the content +scroll bottom +``` + +A `#` only starts a comment at the beginning of a line. A scroll line carries `--until` and keeps its distance as a positional (`scroll down 0.8 --until `). `wait` carries `--raw`, `--depth `, and `--scope ` (long spellings; the `-d`/`-s` CLI aliases stay out of scripts so a hand-written line like `wait text -s so funny` keeps meaning its literal text) to choose the capture its target is read from. + ## Run replay ```bash @@ -130,7 +160,7 @@ agent-device test ./workflows --reporter default --reporter junit:./tmp/junit.xm - `test` discovers `.ad` files from files, directories, or globs and runs them serially. - Quote relative globs to expand them on the caller from its working directory, including when the directory name contains glob characters such as `[` or `{`. A missing file input without glob characters reports an error. - `context platform=...` inside each `.ad` file is the target source of truth for suite execution. -- `--platform` is a filter for suite discovery; files without platform metadata are skipped when a filter is present. +- `--platform` is a filter for suite discovery; files without platform metadata are skipped when a filter is present. When filtering leaves no runnable sources, the no-match error reports how many sources were skipped for having no `context platform=` header versus how many declared another platform. Add the header to run a file under a filter, or omit `--platform` and let the selected device decide. - `context timeout=...` and `context retries=...` can be declared per script; CLI flags override metadata. Retries are capped at `3`, and duplicate keys in the context header fail fast instead of silently overriding each other. - By default, suite artifacts are written under `.agent-device/test-artifacts//...`. Each attempt writes `replay.ad`, `result.txt`, and `replay-timing.ndjson`. Failed attempts also keep copied logs and artifact files when the replay produced them. - Copied diagnostic artifacts receive numbered filenames when their names collide with another artifact, a replay source, timing trace, or attempt manifest. `result.txt` lists the retained names in `copiedArtifacts`. @@ -396,7 +426,11 @@ Passing `--plan-digest` that no longer matches the current script — because yo - Repeated re-runs are slow or the app is stateful, but the script is still correct: - Leave the replay plan unchanged, repair app state so the reported failed step can be retried, then use its `--from`/`--plan-digest`. Resume starts at `--from`; it does not skip that step. - Replay file parse error: - - Validate quoting in `.ad` lines (unclosed quotes are rejected). + - Validate quoting in `.ad` lines (unclosed double quotes are rejected). A selector with a space + needs quotes, and `help scripting` states how each quote form decodes. +- A replay passes on one device size and fails on another because a target was off screen: + - The script used a fixed `scroll` amount. Replace it with the stop condition, `scroll down --until `, + which repeats until the element is actually on screen. - A `press` or `click` step fails because its target was not found, but the element is on the screenshot: - A `selector-miss` divergence, or `error.details.readiness.end: expired`, means the element was