fix(ad-script): let .ad scripts carry scroll --until and wait capture flags (#3197)

packages/ad-script/src/index.ts

@@ -10,6 +10,8 @@ export {
   formatScriptStringLiteral,
   isClickLikeCommand,
   isTouchTargetCommand,
+  SCRIPT_FLAG_COMMANDS,
+  scriptFlagEntries,
   stripRecordedRefGeneration,
 } from './internal/script-utils.ts';
 

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

@@ -205,6 +205,190 @@ test('snapshot replay script writes interactive refresh flags', () => {
   assert.match(script, /snapshot -i -d 2 -s @e1/);
 });
 
+// #3197: reaching an off-screen element used to be CLI-only. `scroll --until` was
+// invisible to the script grammar, so its tokens fell through as positionals and the
+// daemon read `--until` as the scroll amount ("scroll amount must be a number").
+test('scroll replay script parses the --until stop condition as a flag, not an amount', () => {
+  const parsed = parseReplayScriptDetailed(
+    String.raw`scroll down --until "id=\"far-button\""` + '\n',
+  ).actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['down']);
+  assert.equal(parsed[0]?.flags.until, 'id="far-button"');
+});
+
+test('scroll replay script keeps an amount positional beside the stop condition', () => {
+  const parsed = parseReplayScriptDetailed('scroll down 0.8 --until label=Email\n').actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['down', '0.8']);
+  assert.equal(parsed[0]?.flags.until, 'label=Email');
+});
+
+test('a scroll --until value with spaces survives as one selector when quoted', () => {
+  const parsed = parseReplayScriptDetailed(
+    String.raw`scroll down --until "label=\"Sign in\" || label=\"Log in\""`,
+  ).actions;
+
+  assert.equal(parsed[0]?.flags.until, 'label="Sign in" || label="Log in"');
+});
+
+test('scroll replay script writes its stop condition back where the parser reads it', () => {
+  const actions: SessionAction[] = [
+    {
+      ts: Date.now(),
+      command: 'scroll',
+      positionals: ['down', '0.8'],
+      flags: { until: 'label="Sign in"' },
+    },
+  ];
+
+  const script = formatReplayScriptForTest(actions);
+
+  // The generic writer quotes a non-`@` positional as a JSON literal (pre-existing
+  // for every generic line); the parser reads either spelling back.
+  assert.match(script, /scroll "down" 0\.8 --until "label=\\"Sign in\\""/);
+  const reparsed = parseReplayScriptDetailed(script).actions[0];
+  assert.deepEqual(reparsed?.positionals, ['down', '0.8']);
+  assert.deepEqual(reparsed?.flags, { until: 'label="Sign in"' });
+});
+
+// The other half of #3197: `--raw`, `--depth`, and `--scope` are declared on `wait`
+// (`SELECTOR_SNAPSHOT_FLAGS`) and recorded, but a script line put them INSIDE the
+// positional list, where the wait parser refused the line as selector-shaped text.
+test('wait replay script parses its capture-scope flags out of the positionals', () => {
+  const parsed = parseReplayScriptDetailed(
+    [
+      'wait id="x" --raw',
+      'wait --raw label=Email',
+      String.raw`wait "label=\"Sign in\"" --scope "@e3" --depth 2`,
+      'wait "label=Checkout" 5000 --raw',
+    ].join('\n') + '\n',
+  ).actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['id="x"']);
+  assert.equal(parsed[0]?.flags.snapshotRaw, true);
+  assert.deepEqual(parsed[1]?.positionals, ['label=Email']);
+  assert.equal(parsed[1]?.flags.snapshotRaw, true);
+  assert.deepEqual(parsed[2]?.positionals, ['label="Sign in"']);
+  assert.equal(parsed[2]?.flags.snapshotScope, '@e3');
+  assert.equal(parsed[2]?.flags.snapshotDepth, 2);
+  // The budget positional and a capture flag compose in either written order.
+  assert.deepEqual(parsed[3]?.positionals, ['label=Checkout', '5000']);
+  assert.equal(parsed[3]?.flags.snapshotRaw, true);
+});
+
+// The `-d`/`-s` CLI aliases stay OUT of the script grammar: pre-existing lines
+// like `wait text -s so funny` meant the literal text, and a grammar that
+// reclassified them would silently change a passing script's oracle. Recordings
+// only ever write the long spelling.
+test('wait keeps the -d/-s CLI aliases out of the script grammar', () => {
+  const parsed = parseReplayScriptDetailed('wait text -d 2 hello world\n').actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['text', '-d', '2', 'hello', 'world']);
+  assert.equal(parsed[0]?.flags.snapshotDepth, undefined);
+});
+
+test('wait replay script writes its capture-scope flags back', () => {
+  const actions: SessionAction[] = [
+    {
+      ts: Date.now(),
+      command: 'wait',
+      positionals: ['label=Email', '2000'],
+      flags: { snapshotRaw: true, snapshotDepth: 3 },
+    },
+  ];
+
+  const script = formatReplayScriptForTest(actions);
+
+  assert.match(script, /wait "label=Email" 2000 --raw --depth 3/);
+  const reparsed = parseReplayScriptDetailed(script).actions[0];
+  assert.deepEqual(reparsed?.positionals, ['label=Email', '2000']);
+  assert.equal(reparsed?.flags.snapshotRaw, true);
+  assert.equal(reparsed?.flags.snapshotDepth, 3);
+});
+
+// The CLI hands a selector through the shell, whose single quotes strip to one
+// argument; the same text in a `.ad` line split into fragments (#3197).
+test('a single-quoted script token is one argument, with its double quotes intact', () => {
+  const parsed = parseReplayScriptDetailed(
+    ['press \'id="far-button"\'', 'wait \'label="Sign in"\' 2000'].join('\n') + '\n',
+  ).actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['id="far-button"']);
+  assert.deepEqual(parsed[1]?.positionals, ['label="Sign in"', '2000']);
+});
+
+test('a single-quoted script token carries a --until selector with spaces', () => {
+  const parsed = parseReplayScriptDetailed('scroll down --until \'label="Sign in"\'\n').actions;
+
+  assert.equal(parsed[0]?.flags.until, 'label="Sign in"');
+});
+
+test('an apostrophe inside a bare token keeps its old meaning: no quote, no error', () => {
+  // Only a token LEADING with `'` is a quoting candidate, so a value that merely
+  // contains an apostrophe still parses as one bare token, as it always did.
+  const parsed = parseReplayScriptDetailed("wait text it's fine\n").actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['text', "it's", 'fine']);
+});
+
+test("a quote that stops mid-word stays the apostrophe it was, not the shell's split", () => {
+  // The shell reads `'a b'c` as one glued argument. A script line has no second
+  // reader for that reading, and re-tokenizing would change what a previously-valid
+  // line means, so a closing quote that does not end the word keeps the old bare
+  // split (`'a` + `b'c`) rather than inventing a third meaning.
+  const parsed = parseReplayScriptDetailed("wait text 'a b'c\n").actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['text', "'a", "b'c"]);
+});
+
+test('an unclosed single quote never turns a previously valid line into an error', () => {
+  // A value with one stray apostrophe is not a quoted token; it parses as bare
+  // tokens exactly as it did before single quotes were quoting characters.
+  const parsed = parseReplayScriptDetailed("wait text don't\n").actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['text', "don't"]);
+});
+
+test("single quotes carry an apostrophe through ', and a backslash stays itself", () => {
+  // Shell parity: `agent-device wait 'label="don\'t"'` hands over the backslash-
+  // apostrophe pair, so the script has to read the same selector. A shell keeps a
+  // bare `\` inside single quotes, and so does the script line — including a `\\`
+  // pair, which the superseded decoder collapsed to one backslash; this assertion
+  // is what that regression would fail on.
+  const parsed = parseReplayScriptDetailed(
+    [
+      String.raw`wait 'label="don\'t"'`,
+      String.raw`snapshot --scope 'a\\b'`,
+      String.raw`snapshot --scope 'root\.section'`,
+    ].join('\n') + '\n',
+  ).actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['label="don\'t"']);
+  assert.equal(parsed[1]?.flags.snapshotScope, String.raw`a\\b`);
+  assert.equal(parsed[2]?.flags.snapshotScope, String.raw`root\.section`);
+});
+
+test('a quoted value ending in an even backslash run still closes', () => {
+  // `wait 'C:\\temp\\'` is one path with literal backslashes, not an unclosed
+  // quote: only an ODD run pairs with the quote as the apostrophe escape. The
+  // consequence is that a value ending in ONE literal backslash does not close in
+  // single quotes under this grammar (the `\'` escape owns that position); a
+  // double-quoted JSON string is the spelling for that one value.
+  const parsed = parseReplayScriptDetailed(String.raw`wait 'C:\\temp\\'` + '\n').actions;
+
+  assert.deepEqual(parsed[0]?.positionals, [String.raw`C:\\temp\\`]);
+});
+
+test('apostrophes that survive decoding keep the old bare reading', () => {
+  // The shell reads `'don't do this'` as three arguments. A script line has no
+  // second reader to hand it to, so re-tokenizing would change what a
+  // previously-valid line means; it stays one bare token run, as it always was.
+  const parsed = parseReplayScriptDetailed("wait text 'don't do this'\n").actions;
+
+  assert.deepEqual(parsed[0]?.positionals, ['text', "'don't", 'do', "this'"]);
+});
+
 test('a pre-removal gesture line fails the whole script instead of step N', () => {
   assert.throws(
     () =>

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<string, 'count' | 'intervalMs' | 'holdMs' | 'jitterPx'>(
   [
@@ -53,6 +56,123 @@ const GESTURE_NUMERIC_FLAG_MAP = new Map<string, 'pointerCount'>([
 
 const TYPING_NUMERIC_FLAG_MAP = new Map<string, 'delayMs'>([['--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 <selector>` 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<string, ScriptFlagEntry>([
+  ['--until', { key: 'until', kind: 'string' }],
+]);
+
+/**
+ * `wait`'s capture-scope flags (#3197): the command declares them
+ * (`SELECTOR_SNAPSHOT_FLAGS`) and they are recorded, so the script grammar
+ * recognizes them too. Otherwise they land inside the positional list and the
+ * wait parser refuses the line as selector-shaped text. Long spellings only:
+ * this is the spelling the writer emits for `wait`, so nothing the recorder can
+ * write needs an alias, and matching `-d`/`-s` would reclassify realistic
+ * waited text (`wait text -s so funny`) to buy almost nothing — the only line
+ * losing its old reading is one whose whole token is a literal long flag word,
+ * which a hand-written script can spell with the selector wrapped instead.
+ */
+const WAIT_SCRIPT_FLAG_MAP = new Map<string, ScriptFlagEntry>([
+  ['--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<ScriptFlagCommand, Map<string, ScriptFlagEntry>> = {
+  scroll: SCROLL_SCRIPT_FLAG_MAP,
+  wait: WAIT_SCRIPT_FLAG_MAP,
+};
+
+/** The commands whose script line carries flags, derived from the parse tables. */
+export const SCRIPT_FLAG_COMMANDS = Object.keys(SCRIPT_FLAG_MAPS) as readonly ScriptFlagCommand[];
+
+/**
+ * The script flag tokens one command's line reads, with their value kinds and flag keys
+ * (#3197). Exported for the root admission test
+ * (`src/commands/replay/script-flag-admission.test.ts`), which proves the tables and the
+ * flag declarations admit the same keys in both directions — the invariant that keeps the
+ * script grammar and the flag declarations from diverging the way `--until` and
+ * `wait --raw` did.
+ */
+export function scriptFlagEntries(
+  command: string,
+): ReadonlyArray<{ token: string } & ScriptFlagEntry> {
+  const flagMap = scriptFlagMapFor(command);
+  if (!flagMap) return [];
+  return [...flagMap].map(([token, entry]) => ({ token, ...entry }));
+}
+
+function scriptFlagMapFor(command: string): Map<string, ScriptFlagEntry> | undefined {
+  return isScriptFlagCommand(command) ? SCRIPT_FLAG_MAPS[command] : undefined;
+}
+
+// Membership comes from the tables' own keys, so a third command cannot compile into the
+// type while the guard silently refuses to read its flags.
+function isScriptFlagCommand(command: string): command is ScriptFlagCommand {
+  return (SCRIPT_FLAG_COMMANDS as readonly string[]).includes(command);
+}
+
+/**
+ * Splits a `scroll` or `wait` script line into positionals and the command's own
+ * flags (#3197). A token is a flag only when it names one of the command's
+ * declared script flags and, for a value kind, a value token follows; anything
+ * else stays positional, so a hand-written target or text value is untouched.
+ */
+export function parseReplayCommandFlags(
+  command: ScriptFlagCommand,
+  args: string[],
+): { positionals: string[]; flags: SessionAction['flags'] } {
+  const positionals: string[] = [];
+  const flags: SessionAction['flags'] = {};
+  const flagMap = SCRIPT_FLAG_MAPS[command];
+
+  for (let index = 0; index < args.length; index += 1) {
+    const token = args[index]!;
+    const entry = flagMap.get(token);
+    const nextArg = args[index + 1];
+    if (entry === undefined || (entry.kind !== 'boolean' && nextArg === undefined)) {
+      positionals.push(token);
+      continue;
+    }
+    if (entry.kind === 'boolean') {
+      Object.assign(flags, { [entry.key]: true });
+      continue;
+    }
+    if (entry.kind === 'int') {
+      const parsed = parseNonNegativeIntToken(nextArg);
+      if (parsed === null) {
+        positionals.push(token);
+        continue;
+      }
+      Object.assign(flags, { [entry.key]: parsed });
+    } else {
+      Object.assign(flags, { [entry.key]: nextArg });
+    }
+    index += 1;
+  }
+
+  return { positionals, flags };
+}
+
 export function isClickLikeCommand(command: string): command is 'click' | 'press' {
   return command === 'click' || command === 'press';
 }
@@ -264,9 +384,34 @@ export function appendGenericActionScriptArgs(parts: string[], action: SessionAc
   if (action.command === 'fold' && action.flags?.keyframes !== undefined) {
     parts.push('--keyframes', formatScriptArg(action.flags.keyframes));
   }
+  // #3197: `scroll`'s stop condition is part of the step's meaning, so the writer
+  // emits it beside the parser that reads it back. Only `--until` is declared
+  // recorded, so only `--until` can arrive here on a recorded action.
+  if (action.command === 'scroll' && typeof action.flags?.until === 'string') {
+    parts.push('--until', formatScriptArg(action.flags.until));
+  }
+  if (action.command === 'wait') {
+    appendWaitSnapshotScriptFlags(parts, action.flags);
+  }
   appendScriptSeriesFlags(parts, action);
 }
 
+/**
+ * `wait`'s capture-scope flags, written back in the long spelling its script
+ * parser reads (`SELECTOR_SNAPSHOT_FLAGS`, all declared recorded).
+ */
+function appendWaitSnapshotScriptFlags(
+  parts: string[],
+  flags: SessionAction['flags'] | undefined,
+): void {
+  if (!flags) return;
+  if (flags.snapshotRaw === true) parts.push('--raw');
+  if (typeof flags.snapshotDepth === 'number') parts.push('--depth', String(flags.snapshotDepth));
+  if (typeof flags.snapshotScope === 'string') {
+    parts.push('--scope', formatScriptArg(flags.snapshotScope));
+  }
+}
+
 // fallow-ignore-next-line complexity
 export function parseReplaySeriesFlags(
   command: string,