Skip to content

Commit 0fef406

Browse files
authored
fix(ad-script): let .ad scripts carry scroll --until and wait capture flags (#3197) (#3234)
* fix(ad-script): let .ad scripts carry scroll --until and wait capture flags #3197 reported three failures; two were one grammar divergence and the third was guidance that never arrived. `scroll "<direction>" --until <selector>` and `wait <target> --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\""`. * fix(ad-script): address review — strict shell parity for single quotes, pinned grammar rules 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. * fix(ad-script): address round-3 review — backslash-run quote closing, single derived source - `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<NonDeclarablePlatform, true>` 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. * 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.
1 parent 715c8e7 commit 0fef406

20 files changed

Lines changed: 883 additions & 25 deletions

File tree

‎packages/ad-script/src/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ export {
1010
formatScriptStringLiteral,
1111
isClickLikeCommand,
1212
isTouchTargetCommand,
13+
SCRIPT_FLAG_COMMANDS,
14+
scriptFlagEntries,
1315
stripRecordedRefGeneration,
1416
} from './internal/script-utils.ts';
1517

‎packages/ad-script/src/internal/__tests__/script.test.ts‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,190 @@ test('snapshot replay script writes interactive refresh flags', () => {
205205
assert.match(script, /snapshot -i -d 2 -s @e1/);
206206
});
207207

208+
// #3197: reaching an off-screen element used to be CLI-only. `scroll --until` was
209+
// invisible to the script grammar, so its tokens fell through as positionals and the
210+
// daemon read `--until` as the scroll amount ("scroll amount must be a number").
211+
test('scroll replay script parses the --until stop condition as a flag, not an amount', () => {
212+
const parsed = parseReplayScriptDetailed(
213+
String.raw`scroll down --until "id=\"far-button\""` + '\n',
214+
).actions;
215+
216+
assert.deepEqual(parsed[0]?.positionals, ['down']);
217+
assert.equal(parsed[0]?.flags.until, 'id="far-button"');
218+
});
219+
220+
test('scroll replay script keeps an amount positional beside the stop condition', () => {
221+
const parsed = parseReplayScriptDetailed('scroll down 0.8 --until label=Email\n').actions;
222+
223+
assert.deepEqual(parsed[0]?.positionals, ['down', '0.8']);
224+
assert.equal(parsed[0]?.flags.until, 'label=Email');
225+
});
226+
227+
test('a scroll --until value with spaces survives as one selector when quoted', () => {
228+
const parsed = parseReplayScriptDetailed(
229+
String.raw`scroll down --until "label=\"Sign in\" || label=\"Log in\""`,
230+
).actions;
231+
232+
assert.equal(parsed[0]?.flags.until, 'label="Sign in" || label="Log in"');
233+
});
234+
235+
test('scroll replay script writes its stop condition back where the parser reads it', () => {
236+
const actions: SessionAction[] = [
237+
{
238+
ts: Date.now(),
239+
command: 'scroll',
240+
positionals: ['down', '0.8'],
241+
flags: { until: 'label="Sign in"' },
242+
},
243+
];
244+
245+
const script = formatReplayScriptForTest(actions);
246+
247+
// The generic writer quotes a non-`@` positional as a JSON literal (pre-existing
248+
// for every generic line); the parser reads either spelling back.
249+
assert.match(script, /scroll "down" 0\.8 --until "label=\\"Sign in\\""/);
250+
const reparsed = parseReplayScriptDetailed(script).actions[0];
251+
assert.deepEqual(reparsed?.positionals, ['down', '0.8']);
252+
assert.deepEqual(reparsed?.flags, { until: 'label="Sign in"' });
253+
});
254+
255+
// The other half of #3197: `--raw`, `--depth`, and `--scope` are declared on `wait`
256+
// (`SELECTOR_SNAPSHOT_FLAGS`) and recorded, but a script line put them INSIDE the
257+
// positional list, where the wait parser refused the line as selector-shaped text.
258+
test('wait replay script parses its capture-scope flags out of the positionals', () => {
259+
const parsed = parseReplayScriptDetailed(
260+
[
261+
'wait id="x" --raw',
262+
'wait --raw label=Email',
263+
String.raw`wait "label=\"Sign in\"" --scope "@e3" --depth 2`,
264+
'wait "label=Checkout" 5000 --raw',
265+
].join('\n') + '\n',
266+
).actions;
267+
268+
assert.deepEqual(parsed[0]?.positionals, ['id="x"']);
269+
assert.equal(parsed[0]?.flags.snapshotRaw, true);
270+
assert.deepEqual(parsed[1]?.positionals, ['label=Email']);
271+
assert.equal(parsed[1]?.flags.snapshotRaw, true);
272+
assert.deepEqual(parsed[2]?.positionals, ['label="Sign in"']);
273+
assert.equal(parsed[2]?.flags.snapshotScope, '@e3');
274+
assert.equal(parsed[2]?.flags.snapshotDepth, 2);
275+
// The budget positional and a capture flag compose in either written order.
276+
assert.deepEqual(parsed[3]?.positionals, ['label=Checkout', '5000']);
277+
assert.equal(parsed[3]?.flags.snapshotRaw, true);
278+
});
279+
280+
// The `-d`/`-s` CLI aliases stay OUT of the script grammar: pre-existing lines
281+
// like `wait text -s so funny` meant the literal text, and a grammar that
282+
// reclassified them would silently change a passing script's oracle. Recordings
283+
// only ever write the long spelling.
284+
test('wait keeps the -d/-s CLI aliases out of the script grammar', () => {
285+
const parsed = parseReplayScriptDetailed('wait text -d 2 hello world\n').actions;
286+
287+
assert.deepEqual(parsed[0]?.positionals, ['text', '-d', '2', 'hello', 'world']);
288+
assert.equal(parsed[0]?.flags.snapshotDepth, undefined);
289+
});
290+
291+
test('wait replay script writes its capture-scope flags back', () => {
292+
const actions: SessionAction[] = [
293+
{
294+
ts: Date.now(),
295+
command: 'wait',
296+
positionals: ['label=Email', '2000'],
297+
flags: { snapshotRaw: true, snapshotDepth: 3 },
298+
},
299+
];
300+
301+
const script = formatReplayScriptForTest(actions);
302+
303+
assert.match(script, /wait "label=Email" 2000 --raw --depth 3/);
304+
const reparsed = parseReplayScriptDetailed(script).actions[0];
305+
assert.deepEqual(reparsed?.positionals, ['label=Email', '2000']);
306+
assert.equal(reparsed?.flags.snapshotRaw, true);
307+
assert.equal(reparsed?.flags.snapshotDepth, 3);
308+
});
309+
310+
// The CLI hands a selector through the shell, whose single quotes strip to one
311+
// argument; the same text in a `.ad` line split into fragments (#3197).
312+
test('a single-quoted script token is one argument, with its double quotes intact', () => {
313+
const parsed = parseReplayScriptDetailed(
314+
['press \'id="far-button"\'', 'wait \'label="Sign in"\' 2000'].join('\n') + '\n',
315+
).actions;
316+
317+
assert.deepEqual(parsed[0]?.positionals, ['id="far-button"']);
318+
assert.deepEqual(parsed[1]?.positionals, ['label="Sign in"', '2000']);
319+
});
320+
321+
test('a single-quoted script token carries a --until selector with spaces', () => {
322+
const parsed = parseReplayScriptDetailed('scroll down --until \'label="Sign in"\'\n').actions;
323+
324+
assert.equal(parsed[0]?.flags.until, 'label="Sign in"');
325+
});
326+
327+
test('an apostrophe inside a bare token keeps its old meaning: no quote, no error', () => {
328+
// Only a token LEADING with `'` is a quoting candidate, so a value that merely
329+
// contains an apostrophe still parses as one bare token, as it always did.
330+
const parsed = parseReplayScriptDetailed("wait text it's fine\n").actions;
331+
332+
assert.deepEqual(parsed[0]?.positionals, ['text', "it's", 'fine']);
333+
});
334+
335+
test("a quote that stops mid-word stays the apostrophe it was, not the shell's split", () => {
336+
// The shell reads `'a b'c` as one glued argument. A script line has no second
337+
// reader for that reading, and re-tokenizing would change what a previously-valid
338+
// line means, so a closing quote that does not end the word keeps the old bare
339+
// split (`'a` + `b'c`) rather than inventing a third meaning.
340+
const parsed = parseReplayScriptDetailed("wait text 'a b'c\n").actions;
341+
342+
assert.deepEqual(parsed[0]?.positionals, ['text', "'a", "b'c"]);
343+
});
344+
345+
test('an unclosed single quote never turns a previously valid line into an error', () => {
346+
// A value with one stray apostrophe is not a quoted token; it parses as bare
347+
// tokens exactly as it did before single quotes were quoting characters.
348+
const parsed = parseReplayScriptDetailed("wait text don't\n").actions;
349+
350+
assert.deepEqual(parsed[0]?.positionals, ['text', "don't"]);
351+
});
352+
353+
test("single quotes carry an apostrophe through ', and a backslash stays itself", () => {
354+
// Shell parity: `agent-device wait 'label="don\'t"'` hands over the backslash-
355+
// apostrophe pair, so the script has to read the same selector. A shell keeps a
356+
// bare `\` inside single quotes, and so does the script line — including a `\\`
357+
// pair, which the superseded decoder collapsed to one backslash; this assertion
358+
// is what that regression would fail on.
359+
const parsed = parseReplayScriptDetailed(
360+
[
361+
String.raw`wait 'label="don\'t"'`,
362+
String.raw`snapshot --scope 'a\\b'`,
363+
String.raw`snapshot --scope 'root\.section'`,
364+
].join('\n') + '\n',
365+
).actions;
366+
367+
assert.deepEqual(parsed[0]?.positionals, ['label="don\'t"']);
368+
assert.equal(parsed[1]?.flags.snapshotScope, String.raw`a\\b`);
369+
assert.equal(parsed[2]?.flags.snapshotScope, String.raw`root\.section`);
370+
});
371+
372+
test('a quoted value ending in an even backslash run still closes', () => {
373+
// `wait 'C:\\temp\\'` is one path with literal backslashes, not an unclosed
374+
// quote: only an ODD run pairs with the quote as the apostrophe escape. The
375+
// consequence is that a value ending in ONE literal backslash does not close in
376+
// single quotes under this grammar (the `\'` escape owns that position); a
377+
// double-quoted JSON string is the spelling for that one value.
378+
const parsed = parseReplayScriptDetailed(String.raw`wait 'C:\\temp\\'` + '\n').actions;
379+
380+
assert.deepEqual(parsed[0]?.positionals, [String.raw`C:\\temp\\`]);
381+
});
382+
383+
test('apostrophes that survive decoding keep the old bare reading', () => {
384+
// The shell reads `'don't do this'` as three arguments. A script line has no
385+
// second reader to hand it to, so re-tokenizing would change what a
386+
// previously-valid line means; it stays one bare token run, as it always was.
387+
const parsed = parseReplayScriptDetailed("wait text 'don't do this'\n").actions;
388+
389+
assert.deepEqual(parsed[0]?.positionals, ['text', "'don't", 'do', "this'"]);
390+
});
391+
208392
test('a pre-removal gesture line fails the whole script instead of step N', () => {
209393
assert.throws(
210394
() =>

‎packages/ad-script/src/internal/script-utils.ts‎

Lines changed: 146 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ export function stripRecordedRefGeneration(token: string): string {
3232
}
3333

3434
const NUMERIC_ARG_RE = /^-?\d+(\.\d+)?$/;
35-
const BARE_SCRIPT_TOKEN_RE = /^[^\s"\\]+$/;
35+
// A token may not start with `'`: the tokenizer reads a leading `'` as a
36+
// single-quoted literal (#3197), so the writer must quote such values or the
37+
// re-parse would strip the apostrophe.
38+
const BARE_SCRIPT_TOKEN_RE = /^[^\s"'\\][^\s"\\]*$/;
3639

3740
const CLICK_LIKE_NUMERIC_FLAG_MAP = new Map<string, 'count' | 'intervalMs' | 'holdMs' | 'jitterPx'>(
3841
[
@@ -53,6 +56,123 @@ const GESTURE_NUMERIC_FLAG_MAP = new Map<string, 'pointerCount'>([
5356

5457
const TYPING_NUMERIC_FLAG_MAP = new Map<string, 'delayMs'>([['--delay-ms', 'delayMs']]);
5558

59+
/**
60+
* `scroll`'s stop condition, in the script grammar beside its `recorded: true`
61+
* declaration (#3197): the hunt for an off-screen element IS the step, so a
62+
* recorded `scroll down --until <selector>` carries it and a hand-written script
63+
* can say the same. Without this the tokens fall through as positionals and the
64+
* daemon reads `--until` as the scroll amount. Distance stays a positional
65+
* (`scroll down 0.8`): `--pixels` and `--duration-ms` are `recorded: false`, and a
66+
* script grammar that accepted a flag the recorder cannot carry would write a line
67+
* the recording path could never reproduce.
68+
*/
69+
const SCROLL_SCRIPT_FLAG_MAP = new Map<string, ScriptFlagEntry>([
70+
['--until', { key: 'until', kind: 'string' }],
71+
]);
72+
73+
/**
74+
* `wait`'s capture-scope flags (#3197): the command declares them
75+
* (`SELECTOR_SNAPSHOT_FLAGS`) and they are recorded, so the script grammar
76+
* recognizes them too. Otherwise they land inside the positional list and the
77+
* wait parser refuses the line as selector-shaped text. Long spellings only:
78+
* this is the spelling the writer emits for `wait`, so nothing the recorder can
79+
* write needs an alias, and matching `-d`/`-s` would reclassify realistic
80+
* waited text (`wait text -s so funny`) to buy almost nothing — the only line
81+
* losing its old reading is one whose whole token is a literal long flag word,
82+
* which a hand-written script can spell with the selector wrapped instead.
83+
*/
84+
const WAIT_SCRIPT_FLAG_MAP = new Map<string, ScriptFlagEntry>([
85+
['--raw', { key: 'snapshotRaw', kind: 'boolean' }],
86+
['--depth', { key: 'snapshotDepth', kind: 'int' }],
87+
['--scope', { key: 'snapshotScope', kind: 'string' }],
88+
]);
89+
90+
/** How one script flag token carries its value. */
91+
type ScriptFlagEntry = {
92+
key: 'until' | 'snapshotRaw' | 'snapshotDepth' | 'snapshotScope';
93+
kind: 'boolean' | 'int' | 'string';
94+
};
95+
96+
/** The commands whose script line carries flags (`scroll`, `wait`). */
97+
export type ScriptFlagCommand = 'scroll' | 'wait';
98+
99+
/** Which script flag tokens each flag-carrying command reads. */
100+
const SCRIPT_FLAG_MAPS: Record<ScriptFlagCommand, Map<string, ScriptFlagEntry>> = {
101+
scroll: SCROLL_SCRIPT_FLAG_MAP,
102+
wait: WAIT_SCRIPT_FLAG_MAP,
103+
};
104+
105+
/** The commands whose script line carries flags, derived from the parse tables. */
106+
export const SCRIPT_FLAG_COMMANDS = Object.keys(SCRIPT_FLAG_MAPS) as readonly ScriptFlagCommand[];
107+
108+
/**
109+
* The script flag tokens one command's line reads, with their value kinds and flag keys
110+
* (#3197). Exported for the root admission test
111+
* (`src/commands/replay/script-flag-admission.test.ts`), which proves the tables and the
112+
* flag declarations admit the same keys in both directions — the invariant that keeps the
113+
* script grammar and the flag declarations from diverging the way `--until` and
114+
* `wait --raw` did.
115+
*/
116+
export function scriptFlagEntries(
117+
command: string,
118+
): ReadonlyArray<{ token: string } & ScriptFlagEntry> {
119+
const flagMap = scriptFlagMapFor(command);
120+
if (!flagMap) return [];
121+
return [...flagMap].map(([token, entry]) => ({ token, ...entry }));
122+
}
123+
124+
function scriptFlagMapFor(command: string): Map<string, ScriptFlagEntry> | undefined {
125+
return isScriptFlagCommand(command) ? SCRIPT_FLAG_MAPS[command] : undefined;
126+
}
127+
128+
// Membership comes from the tables' own keys, so a third command cannot compile into the
129+
// type while the guard silently refuses to read its flags.
130+
function isScriptFlagCommand(command: string): command is ScriptFlagCommand {
131+
return (SCRIPT_FLAG_COMMANDS as readonly string[]).includes(command);
132+
}
133+
134+
/**
135+
* Splits a `scroll` or `wait` script line into positionals and the command's own
136+
* flags (#3197). A token is a flag only when it names one of the command's
137+
* declared script flags and, for a value kind, a value token follows; anything
138+
* else stays positional, so a hand-written target or text value is untouched.
139+
*/
140+
export function parseReplayCommandFlags(
141+
command: ScriptFlagCommand,
142+
args: string[],
143+
): { positionals: string[]; flags: SessionAction['flags'] } {
144+
const positionals: string[] = [];
145+
const flags: SessionAction['flags'] = {};
146+
const flagMap = SCRIPT_FLAG_MAPS[command];
147+
148+
for (let index = 0; index < args.length; index += 1) {
149+
const token = args[index]!;
150+
const entry = flagMap.get(token);
151+
const nextArg = args[index + 1];
152+
if (entry === undefined || (entry.kind !== 'boolean' && nextArg === undefined)) {
153+
positionals.push(token);
154+
continue;
155+
}
156+
if (entry.kind === 'boolean') {
157+
Object.assign(flags, { [entry.key]: true });
158+
continue;
159+
}
160+
if (entry.kind === 'int') {
161+
const parsed = parseNonNegativeIntToken(nextArg);
162+
if (parsed === null) {
163+
positionals.push(token);
164+
continue;
165+
}
166+
Object.assign(flags, { [entry.key]: parsed });
167+
} else {
168+
Object.assign(flags, { [entry.key]: nextArg });
169+
}
170+
index += 1;
171+
}
172+
173+
return { positionals, flags };
174+
}
175+
56176
export function isClickLikeCommand(command: string): command is 'click' | 'press' {
57177
return command === 'click' || command === 'press';
58178
}
@@ -264,9 +384,34 @@ export function appendGenericActionScriptArgs(parts: string[], action: SessionAc
264384
if (action.command === 'fold' && action.flags?.keyframes !== undefined) {
265385
parts.push('--keyframes', formatScriptArg(action.flags.keyframes));
266386
}
387+
// #3197: `scroll`'s stop condition is part of the step's meaning, so the writer
388+
// emits it beside the parser that reads it back. Only `--until` is declared
389+
// recorded, so only `--until` can arrive here on a recorded action.
390+
if (action.command === 'scroll' && typeof action.flags?.until === 'string') {
391+
parts.push('--until', formatScriptArg(action.flags.until));
392+
}
393+
if (action.command === 'wait') {
394+
appendWaitSnapshotScriptFlags(parts, action.flags);
395+
}
267396
appendScriptSeriesFlags(parts, action);
268397
}
269398

399+
/**
400+
* `wait`'s capture-scope flags, written back in the long spelling its script
401+
* parser reads (`SELECTOR_SNAPSHOT_FLAGS`, all declared recorded).
402+
*/
403+
function appendWaitSnapshotScriptFlags(
404+
parts: string[],
405+
flags: SessionAction['flags'] | undefined,
406+
): void {
407+
if (!flags) return;
408+
if (flags.snapshotRaw === true) parts.push('--raw');
409+
if (typeof flags.snapshotDepth === 'number') parts.push('--depth', String(flags.snapshotDepth));
410+
if (typeof flags.snapshotScope === 'string') {
411+
parts.push('--scope', formatScriptArg(flags.snapshotScope));
412+
}
413+
}
414+
270415
// fallow-ignore-next-line complexity
271416
export function parseReplaySeriesFlags(
272417
command: string,

0 commit comments

Comments
 (0)