From 31eb32447a258bf32505d15968795ff7cfe13ffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 5 Oct 2026 16:42:38 +0200 Subject: [PATCH 1/4] fix(ad-script): let .ad scripts carry scroll --until and wait capture flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3197 reported three failures; two were one grammar divergence and the third was guidance that never arrived. `scroll "" --until ` and `wait --raw` worked at the CLI and parse-failed in a `.ad` script: the script grammar had no flag vocabulary for either command, so the flag tokens fell through as positionals and the daemon read `--until` as the scroll amount ("scroll amount must be a number"). The `.ad` parser now reads each command's own declared flags, and the line writer emits them back, so a recorded hunt replays as the same hunt rather than as one fixed gesture. `until` is therefore `recorded: true`: the stop condition IS the step. The grammar stays narrower than the CLI on purpose. A script line carries only what the flag declaration marks recorded, so `--pixels` and `--duration-ms` stay CLI-only, and `wait` reads the long spellings while `-d`/`-s` stay CLI sugar — admitting them would reclassify data that pre-existing lines wait on (`wait text -s so funny` once meant the literal text). An admission test pins both directions of that contract, the one #3197 broke included. A tokenizer gap hid behind the first two: `--until 'label="x"'`, one argument at the shell, split into fragments in a script. A token leading with `'` now reads as the shell reads it. A candidate must close at end of word, so a line that parsed before keeps its meaning; the writer refuses to emit such a value bare, which keeps the round trip a fixed point. The third report, "No replay tests matched for platform ios" with no next step, is a messaging fix: the run now says how many sources had no `context platform=` header versus how many declared another platform, and names the remedy. The suite's failure projection carries a code and a message and no hint, so the remedy belongs in that sentence; `--platform web` is answered without advising a header the parser drops (#1900). Verified on an iPhone 18 Pro simulator: replaying a hand-written `--until` script, the missing-target case failing as a proper divergence rather than a crash, and a recording round-trip that wrote and replayed `scroll "down" --until "label=\"General\""`. --- packages/ad-script/src/index.ts | 3 + .../src/internal/__tests__/script.test.ts | 154 ++++++++++++++++++ .../ad-script/src/internal/script-utils.ts | 148 ++++++++++++++++- packages/ad-script/src/internal/script.ts | 73 ++++++++- .../src/flag-definitions-action.ts | 6 +- packages/maestro/src/internal/export-flow.ts | 13 +- .../__tests__/session-test-discovery.test.ts | 57 ++++++- .../src/internal/session-test-discovery.ts | 66 +++++++- .../test-utils/property-arbitraries.ts | 41 ++++- .../__tests__/replay-maestro-export.test.ts | 21 +++ .../replay/ad-script-round-trip.test.ts | 16 +- .../replay/script-flag-admission.test.ts | 98 +++++++++++ src/commands/schema/cli-help.ts | 4 + .../session-replay-action-runtime.test.ts | 32 ++++ .../replay-suite/session-test-runner.test.ts | 5 +- .../replay-suite/session-test-suite.test.ts | 6 +- .../__tests__/session-script-writer.test.ts | 38 +++++ .../__tests__/session-command-replay.test.ts | 48 +++++- website/docs/docs/commands.md | 2 + website/docs/docs/replay-e2e.md | 38 ++++- 20 files changed, 844 insertions(+), 25 deletions(-) create mode 100644 src/commands/replay/script-flag-admission.test.ts diff --git a/packages/ad-script/src/index.ts b/packages/ad-script/src/index.ts index e498c751c3..cc60a6dece 100644 --- a/packages/ad-script/src/index.ts +++ b/packages/ad-script/src/index.ts @@ -10,6 +10,9 @@ export { formatScriptStringLiteral, isClickLikeCommand, isTouchTargetCommand, + SCRIPT_FLAG_COMMANDS, + scriptFlagEntries, + scriptFlagKeys, 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..71a474233e 100644 --- a/packages/ad-script/src/internal/__tests__/script.test.ts +++ b/packages/ad-script/src/internal/__tests__/script.test.ts @@ -205,6 +205,160 @@ 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("apostrophes that survive decoding keep the bare reading, not the shell's split", () => { + // 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..d9022a8b2c 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,124 @@ 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: + * the `-d`/`-s` CLI aliases would reclassify pre-existing positional data + * (`wait text -s so funny` once meant the text `-s so funny`), and the writer + * only ever emits the long form. + */ +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 flag keys one command's script line can carry (#3197), read back from the parse + * tables. 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 scriptFlagKeys(command: string): readonly string[] { + const flagMap = scriptFlagMapFor(command); + return flagMap ? [...new Set([...flagMap.values()].map((entry) => entry.key))] : []; +} + +/** The script flag tokens one command reads, with their value kinds, for the admission test. */ +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; +} + +function isScriptFlagCommand(command: string): command is ScriptFlagCommand { + return command === 'scroll' || command === 'wait'; +} + +/** + * 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 +385,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..23e755f593 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,58 @@ 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 candidate must close immediately + * before whitespace or end of line: a quote that stops mid-word was punctuation in + * the old bare reading (`wait text it's fine`, `'don't do this'`), so the whole run + * keeps its old meaning and nothing that parsed before changes meaning. Inside the + * quotes only `\'` and `\\` escape. + */ +function readSingleQuotedReplayToken( + line: string, + cursor: number, +): { value: string; nextCursor: number } | null { + if (line[cursor] !== "'") return null; + let end = cursor + 1; + while (end < line.length) { + const char = line.charAt(end); + if (char === '\\') { + end += 2; + continue; + } + if (char === "'") break; + end += 1; + } + if (end >= line.length || !isReplayTokenBoundary(line, end + 1)) return null; + return { + value: decodeSingleQuotedReplayLiteral(line.slice(cursor + 1, end)), + nextCursor: end + 1, + }; +} + +function isReplayTokenBoundary(line: string, index: number): boolean { + return index >= line.length || /\s/.test(line.charAt(index)); +} + +function decodeSingleQuotedReplayLiteral(value: string): string { + let decoded = ''; + let cursor = 0; + while (cursor < value.length) { + const char = value.charAt(cursor); + if (char === '\\' && (value.charAt(cursor + 1) === "'" || value.charAt(cursor + 1) === '\\')) { + decoded += value.charAt(cursor + 1); + cursor += 2; + continue; + } + decoded += char; + cursor += 1; + } + return decoded; +} + 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..93344ac431 100644 --- a/packages/maestro/src/internal/export-flow.ts +++ b/packages/maestro/src/internal/export-flow.ts @@ -341,8 +341,17 @@ 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 warnings = + typeof action.flags?.until === 'string' + ? [`scroll --until ${action.flags.until} is not represented by Maestro scroll`] + : []; + return { kind: 'commands', commands: ['scroll'], ...(warnings.length ? { warnings } : {}) }; } 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..01c61b6e8a 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,70 @@ 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; +}; + +/** + * Whether a filter value is one a script can name in its `context platform=` header: the + * declarable vocabulary is `ReplayTestPlatform`, and `web` is the only `PlatformSelector` it + * excludes. The type predicate fails to compile if a future declarable platform joins the + * exclusion, so this stays pinned to the type rather than restating a list. + */ +function isDeclarableReplayPlatform(filter: PlatformSelector): filter is ReplayTestPlatform { + return filter !== 'web'; +} + +/** + * 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..c131945786 --- /dev/null +++ b/src/commands/replay/script-flag-admission.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, test } from 'vitest'; +import { SCRIPT_FLAG_COMMANDS, scriptFlagEntries, scriptFlagKeys } 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 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 = scriptFlagKeys(command); + + test('the command declares at least one script flag', () => { + expect(keys.length).toBeGreaterThan(0); + }); + + test('every flag the script line carries is one the command itself accepts', () => { + const schema = getCliCommandSchema(command); + const accepted = new Set([ + ...(schema.allowedFlags ?? []), + ...(schema.supportedFlags ?? []), + ]); + const refused = keys.filter((key) => !accepted.has(key)); + expect(refused).toEqual([]); + }); + + test('every flag the script line carries is one the recorder may carry', () => { + const unrecorded = keys.filter((key) => !recordedFlagKeys().has(key as never)); + expect(unrecorded).toEqual([]); + }); + + test('every script token matches the declaration spelling and kind exactly', () => { + // The long spelling only: an alias the grammar invented that the CLI does not + // declare would parse a line `agent-device ` itself would refuse. + for (const entry of entries) { + const definitions = getFlagDefinitionsForKey(entry.key as never); + const declaredNames = new Set(definitions.flatMap((definition) => [...definition.names])); + expect(declaredNames.has(entry.token)).toBe(true); + 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 schema = getCliCommandSchema(command); + const accepted = new Set([ + ...(schema.allowedFlags ?? []), + ...(schema.supportedFlags ?? []), + ]); + const recorded = recordedFlagKeys(); + const missing = [...accepted].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(scriptFlagKeys('press')).toEqual([]); + expect(scriptFlagKeys('snapshot')).toEqual([]); +}); diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 0ac4b4829f..3c805d9ae2 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 except for \\' and \\\\. A command reads only its own declared flags, so the CLI form and the script line are the same text: scroll down --until 'id="x"', wait 'label="Sign in"' --raw, snapshot -i. Recorded lines carry what the flag declaration marks recorded; per-request options (--settle, --verify) are not part of a step and are not written. + 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..50cda00188 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. + +```ad +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 except for `\'` and `\\`. +- A command accepts only its own flags. A flag the CLI accepts is available in a script when it belongs to the step; per-request options are not part of a step. `--settle`, `--verify`, 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: + +```ad +# 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; `--pixels` and `--duration-ms` belong to the CLI invocation, not to a step. `wait` carries `--raw`, `--depth `, and `--scope ` 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, and the run 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 From 9a4b41e0769438e6e11cf87ce081dd9bb255ad22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 5 Oct 2026 18:58:29 +0200 Subject: [PATCH 2/4] =?UTF-8?q?fix(ad-script):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20strict=20shell=20parity=20for=20single=20quotes,=20?= =?UTF-8?q?pinned=20grammar=20rules?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single-quoted script values now decode exactly what the shell hands over: the only escape is `\'` (the one deliberate extension, since a shell's single quotes cannot carry an apostrophe and a `label="don't"` selector has to be writable in a script), and a backslash keeps its own character instead of collapsing. - The tokenizer comment states the scoped guarantee: a whole-word quoted run gains the shell reading (that is the change), a run the shell would not read as one argument keeps its old bare meaning. - The admission test now asserts every script token is a long `--` spelling, so an `-d`/`-s` regression fails there. - The `wait` long-form-only comment carries the real trade: writer parity, and realistic short-flag-waited-text versus an almost-impossible literal that quoting already solves. - Docs: script flags are declared-and-recorded ones (no contradiction with the scroll pixel note), single-quote decode rules match the code, and the discovery counts sentence is scoped to the no-match error as it actually behaves. --- .../src/internal/__tests__/script.test.ts | 15 +++++++++- .../ad-script/src/internal/script-utils.ts | 8 +++-- packages/ad-script/src/internal/script.ts | 30 +++++++------------ .../replay/script-flag-admission.test.ts | 7 +++-- src/commands/schema/cli-help.ts | 2 +- website/docs/docs/replay-e2e.md | 8 ++--- 6 files changed, 40 insertions(+), 30 deletions(-) diff --git a/packages/ad-script/src/internal/__tests__/script.test.ts b/packages/ad-script/src/internal/__tests__/script.test.ts index 71a474233e..0bce50bf57 100644 --- a/packages/ad-script/src/internal/__tests__/script.test.ts +++ b/packages/ad-script/src/internal/__tests__/script.test.ts @@ -350,7 +350,20 @@ test('an unclosed single quote never turns a previously valid line into an error assert.deepEqual(parsed[0]?.positionals, ['text', "don't"]); }); -test("apostrophes that survive decoding keep the bare reading, not the shell's split", () => { +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. + const parsed = parseReplayScriptDetailed( + [String.raw`wait 'label="don\'t"'`, 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`root\.section`); +}); + +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. diff --git a/packages/ad-script/src/internal/script-utils.ts b/packages/ad-script/src/internal/script-utils.ts index d9022a8b2c..199f59a24d 100644 --- a/packages/ad-script/src/internal/script-utils.ts +++ b/packages/ad-script/src/internal/script-utils.ts @@ -75,9 +75,11 @@ const SCROLL_SCRIPT_FLAG_MAP = new Map([ * (`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: - * the `-d`/`-s` CLI aliases would reclassify pre-existing positional data - * (`wait text -s so funny` once meant the text `-s so funny`), and the writer - * only ever emits the long form. + * 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' }], diff --git a/packages/ad-script/src/internal/script.ts b/packages/ad-script/src/internal/script.ts index 23e755f593..c2144a97e7 100644 --- a/packages/ad-script/src/internal/script.ts +++ b/packages/ad-script/src/internal/script.ts @@ -557,11 +557,15 @@ function readQuotedReplayToken( * 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 candidate must close immediately - * before whitespace or end of line: a quote that stops mid-word was punctuation in - * the old bare reading (`wait text it's fine`, `'don't do this'`), so the whole run - * keeps its old meaning and nothing that parsed before changes meaning. Inside the - * quotes only `\'` and `\\` escape. + * 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. */ function readSingleQuotedReplayToken( line: string, @@ -571,7 +575,7 @@ function readSingleQuotedReplayToken( let end = cursor + 1; while (end < line.length) { const char = line.charAt(end); - if (char === '\\') { + if (char === '\\' && line.charAt(end + 1) === "'") { end += 2; continue; } @@ -590,19 +594,7 @@ function isReplayTokenBoundary(line: string, index: number): boolean { } function decodeSingleQuotedReplayLiteral(value: string): string { - let decoded = ''; - let cursor = 0; - while (cursor < value.length) { - const char = value.charAt(cursor); - if (char === '\\' && (value.charAt(cursor + 1) === "'" || value.charAt(cursor + 1) === '\\')) { - decoded += value.charAt(cursor + 1); - cursor += 2; - continue; - } - decoded += char; - cursor += 1; - } - return decoded; + return value.replaceAll(String.raw`\'`, "'"); } function readBareReplayToken(line: string, cursor: number): { value: string; nextCursor: number } { diff --git a/src/commands/replay/script-flag-admission.test.ts b/src/commands/replay/script-flag-admission.test.ts index c131945786..3223857ac4 100644 --- a/src/commands/replay/script-flag-admission.test.ts +++ b/src/commands/replay/script-flag-admission.test.ts @@ -51,9 +51,12 @@ describe.each(SCRIPT_FLAG_COMMANDS)('%s script flags', (command) => { }); test('every script token matches the declaration spelling and kind exactly', () => { - // The long spelling only: an alias the grammar invented that the CLI does not - // declare would parse a line `agent-device ` itself would refuse. + // 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 declaredNames = new Set(definitions.flatMap((definition) => [...definition.names])); expect(declaredNames.has(entry.token)).toBe(true); diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 3c805d9ae2..995115e4d2 100644 --- a/src/commands/schema/cli-help.ts +++ b/src/commands/schema/cli-help.ts @@ -164,7 +164,7 @@ Script paths are the caller's: 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 except for \\' and \\\\. A command reads only its own declared flags, so the CLI form and the script line are the same text: scroll down --until 'id="x"', wait 'label="Sign in"' --raw, snapshot -i. Recorded lines carry what the flag declaration marks recorded; per-request options (--settle, --verify) are not part of a step and are not written. + 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: diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 50cda00188..d6764e7e2d 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -53,8 +53,8 @@ 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 except for `\'` and `\\`. -- A command accepts only its own flags. A flag the CLI accepts is available in a script when it belongs to the step; per-request options are not part of a step. `--settle`, `--verify`, and the device-selection flags (`--platform`, `--serial`, `--device`) are the common ones a script does not carry. +- 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: @@ -68,7 +68,7 @@ scroll down 0.8 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; `--pixels` and `--duration-ms` belong to the CLI invocation, not to a step. `wait` carries `--raw`, `--depth `, and `--scope ` to choose the capture its target is read from. +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 @@ -160,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, and the run 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. +- `--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`. From d9cd2ad6c6fd169b6177b49dd149235770854021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 5 Oct 2026 20:07:03 +0200 Subject: [PATCH 3/4] =?UTF-8?q?fix(ad-script):=20address=20round-3=20revie?= =?UTF-8?q?w=20=E2=80=94=20backslash-run=20quote=20closing,=20single=20der?= =?UTF-8?q?ived=20source?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `readSingleQuotedReplayToken` closed on a backslash-quote only via skip-2, so a value ending in a literal backslash pair (`wait 'C:\\temp\\'`) never closed and fell back to a bare token keeping the quote characters. The scan now counts the backslash run and honors a closing quote only after an even run, matching the documented guarantee; the odd run is the `\'` escape. Split into named helpers (`findSingleQuotedTokenEnd`, `isBackslashEscaped`), which also clears the complexity finding. - The admission test derives flag keys from `scriptFlagEntries` alone; the `scriptFlagKeys` projection it duplicated is gone. Command membership for the parse guard comes from the derived `SCRIPT_FLAG_COMMANDS`, not a parallel literal comparison. - `isDeclarableReplayPlatform` no longer claims a compile pin it does not have: the exclusions are now a `Record` table, which fails to compile in BOTH directions when a platform joins or leaves the gap between `PlatformSelector` and `ReplayTestPlatform` (verified by planting). - Maestro `convertScrollAction` sets `warnings` unconditionally like its sibling converters instead of conditionally spreading it. --- packages/ad-script/src/index.ts | 1 - .../src/internal/__tests__/script.test.ts | 11 +++++ .../ad-script/src/internal/script-utils.ts | 21 ++++------ packages/ad-script/src/internal/script.ts | 40 ++++++++++++------ packages/maestro/src/internal/export-flow.ts | 12 +++--- .../src/internal/session-test-discovery.ts | 16 +++++-- .../replay/script-flag-admission.test.ts | 42 +++++++------------ 7 files changed, 83 insertions(+), 60 deletions(-) diff --git a/packages/ad-script/src/index.ts b/packages/ad-script/src/index.ts index cc60a6dece..ac520cc657 100644 --- a/packages/ad-script/src/index.ts +++ b/packages/ad-script/src/index.ts @@ -12,7 +12,6 @@ export { isTouchTargetCommand, SCRIPT_FLAG_COMMANDS, scriptFlagEntries, - scriptFlagKeys, 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 0bce50bf57..df636a3b30 100644 --- a/packages/ad-script/src/internal/__tests__/script.test.ts +++ b/packages/ad-script/src/internal/__tests__/script.test.ts @@ -363,6 +363,17 @@ test("single quotes carry an apostrophe through ', and a backslash stays itself" assert.equal(parsed[1]?.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 diff --git a/packages/ad-script/src/internal/script-utils.ts b/packages/ad-script/src/internal/script-utils.ts index 199f59a24d..dd45aa73b5 100644 --- a/packages/ad-script/src/internal/script-utils.ts +++ b/packages/ad-script/src/internal/script-utils.ts @@ -106,18 +106,13 @@ const SCRIPT_FLAG_MAPS: Record> export const SCRIPT_FLAG_COMMANDS = Object.keys(SCRIPT_FLAG_MAPS) as readonly ScriptFlagCommand[]; /** - * The flag keys one command's script line can carry (#3197), read back from the parse - * tables. 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. + * 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 scriptFlagKeys(command: string): readonly string[] { - const flagMap = scriptFlagMapFor(command); - return flagMap ? [...new Set([...flagMap.values()].map((entry) => entry.key))] : []; -} - -/** The script flag tokens one command reads, with their value kinds, for the admission test. */ export function scriptFlagEntries( command: string, ): ReadonlyArray<{ token: string } & ScriptFlagEntry> { @@ -130,8 +125,10 @@ function scriptFlagMapFor(command: string): Map | undef 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 command === 'scroll' || command === 'wait'; + return (SCRIPT_FLAG_COMMANDS as readonly string[]).includes(command); } /** diff --git a/packages/ad-script/src/internal/script.ts b/packages/ad-script/src/internal/script.ts index c2144a97e7..b18210b7b6 100644 --- a/packages/ad-script/src/internal/script.ts +++ b/packages/ad-script/src/internal/script.ts @@ -565,30 +565,46 @@ function readQuotedReplayToken( * 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. + * `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; - let end = cursor + 1; - while (end < line.length) { - const char = line.charAt(end); - if (char === '\\' && line.charAt(end + 1) === "'") { - end += 2; - continue; - } - if (char === "'") break; - end += 1; - } - if (end >= line.length || !isReplayTokenBoundary(line, end + 1)) 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)); } diff --git a/packages/maestro/src/internal/export-flow.ts b/packages/maestro/src/internal/export-flow.ts index 93344ac431..df611f2db7 100644 --- a/packages/maestro/src/internal/export-flow.ts +++ b/packages/maestro/src/internal/export-flow.ts @@ -347,11 +347,13 @@ function convertScrollAction(action: SessionAction): ConvertedAction { // #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 warnings = - typeof action.flags?.until === 'string' - ? [`scroll --until ${action.flags.until} is not represented by Maestro scroll`] - : []; - return { kind: 'commands', commands: ['scroll'], ...(warnings.length ? { warnings } : {}) }; + 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/session-test-discovery.ts b/packages/replay-test/src/internal/session-test-discovery.ts index 01c61b6e8a..57e77bd7b5 100644 --- a/packages/replay-test/src/internal/session-test-discovery.ts +++ b/packages/replay-test/src/internal/session-test-discovery.ts @@ -96,14 +96,22 @@ type NoReplayTestsMatchedReasons = { 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 `web` is the only `PlatformSelector` it - * excludes. The type predicate fails to compile if a future declarable platform joins the - * exclusion, so this stays pinned to the type rather than restating a list. + * 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 filter !== 'web'; + return !Object.hasOwn(NON_DECLARABLE_PLATFORMS, filter); } /** diff --git a/src/commands/replay/script-flag-admission.test.ts b/src/commands/replay/script-flag-admission.test.ts index 3223857ac4..d4c0688a97 100644 --- a/src/commands/replay/script-flag-admission.test.ts +++ b/src/commands/replay/script-flag-admission.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest'; -import { SCRIPT_FLAG_COMMANDS, scriptFlagEntries, scriptFlagKeys } from '@agent-device/ad-script'; +import { SCRIPT_FLAG_COMMANDS, scriptFlagEntries } from '@agent-device/ad-script'; import { COMMON_COMMAND_SUPPORTED_FLAG_KEYS, DEVICE_SELECTION_FLAG_KEYS, @@ -19,35 +19,32 @@ import { * `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 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); + * - 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 = scriptFlagKeys(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(keys.length).toBeGreaterThan(0); + expect(entries.length).toBeGreaterThan(0); }); test('every flag the script line carries is one the command itself accepts', () => { - const schema = getCliCommandSchema(command); - const accepted = new Set([ - ...(schema.allowedFlags ?? []), - ...(schema.supportedFlags ?? []), - ]); - const refused = keys.filter((key) => !accepted.has(key)); - expect(refused).toEqual([]); + expect(keys.filter((key) => !acceptedFlagKeys.has(key))).toEqual([]); }); test('every flag the script line carries is one the recorder may carry', () => { - const unrecorded = keys.filter((key) => !recordedFlagKeys().has(key as never)); - expect(unrecorded).toEqual([]); + expect(keys.filter((key) => !recordedFlagKeys().has(key as never))).toEqual([]); }); test('every script token matches the declaration spelling and kind exactly', () => { @@ -58,8 +55,6 @@ describe.each(SCRIPT_FLAG_COMMANDS)('%s script flags', (command) => { for (const entry of entries) { expect(entry.token.startsWith('--')).toBe(true); const definitions = getFlagDefinitionsForKey(entry.key as never); - const declaredNames = new Set(definitions.flatMap((definition) => [...definition.names])); - expect(declaredNames.has(entry.token)).toBe(true); const definition = definitions.find((candidate) => candidate.names.includes(entry.token)); expect(definition?.type).toBe(entry.kind); } @@ -71,13 +66,8 @@ describe.each(SCRIPT_FLAG_COMMANDS)('%s script flags', (command) => { // 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 schema = getCliCommandSchema(command); - const accepted = new Set([ - ...(schema.allowedFlags ?? []), - ...(schema.supportedFlags ?? []), - ]); const recorded = recordedFlagKeys(); - const missing = [...accepted].filter( + const missing = [...acceptedFlagKeys].filter( (key) => recorded.has(key as never) && !isCommonOrDeviceSelectionFlagKey(key) && !keys.includes(key), ); @@ -96,6 +86,6 @@ test('a command outside the declared set carries no script flags, so its line st // 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(scriptFlagKeys('press')).toEqual([]); - expect(scriptFlagKeys('snapshot')).toEqual([]); + expect(scriptFlagEntries('press')).toEqual([]); + expect(scriptFlagEntries('snapshot')).toEqual([]); }); From 9cb45a0c6751f6817688490eace97742a9a7aaad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 5 Oct 2026 20:20:34 +0200 Subject: [PATCH 4/4] docs(replay): fence .ad grammar examples as sh; pin the backslash decode with a real case The Rspress Shiki bundle has no `ad` grammar, so the two new fences failed deploy-preview. The page's existing `.ad` example already renders under `sh`; the grammar section now matches, and no other fence under website/docs names a language the bundle lacks. The apostrophe/backslash test gained the case that actually pins the decode change: `snapshot --scope 'a\\b'` must carry two literal backslashes. Planting the superseded collapsing decoder fails exactly this assertion and the even-run-close test; nothing else in the file noticed. --- .../src/internal/__tests__/script.test.ts | 14 ++++++++++---- website/docs/docs/replay-e2e.md | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/ad-script/src/internal/__tests__/script.test.ts b/packages/ad-script/src/internal/__tests__/script.test.ts index df636a3b30..d96ab76bac 100644 --- a/packages/ad-script/src/internal/__tests__/script.test.ts +++ b/packages/ad-script/src/internal/__tests__/script.test.ts @@ -353,14 +353,20 @@ test('an unclosed single quote never turns a previously valid line into an error 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. + // 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 'root\.section'`].join('\n') + - '\n', + [ + 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`root\.section`); + 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', () => { diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index d6764e7e2d..7b6f384f67 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -44,7 +44,7 @@ agent-device open Settings --platform ios --session e2e --save-script ./workflow A `.ad` line is the CLI spelling of one command: ` [positional ...] [flag ...]`. Whitespace separates tokens, so a value with a space needs quotes. -```ad +```sh open "com.example.app" --relaunch scroll down --until 'id="far-button"' press id="far-button" @@ -59,7 +59,7 @@ close 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: -```ad +```sh # repeats until the element is on screen scroll down --until 'id="checkout-submit"' # one gesture, viewport-relative