Skip to content

Commit 2062bff

Browse files
feat: parametrise .ad replay scripts (#433)
* feat: parametrise .ad replay scripts Support ${VAR} substitution with env header directives and CLI -e overrides so flows can be reused across app variants, environments, and devices without duplicating the script. Precedence (high->low): CLI -e > AD_* shell env > file-local env > built-ins (AD_PLATFORM, AD_SESSION, AD_FILENAME, AD_DEVICE, AD_ARTIFACTS). Supports ${VAR:-default} fallback, \${ escape, and fails with file:line on unresolved vars. * refactor: harden .ad replay parametrisation Tighten the parametrisation surface introduced in the feat commit after an internal review pass: - Reserve the AD_* namespace for built-ins. User env (file env, CLI -e, shell AD_VAR_*) can no longer define AD_* keys, which closes a built-in shadowing vector (e.g. AD_VAR_AD_SESSION). - Change the shell-env prefix from AD_* to AD_VAR_* so unrelated CI secrets that happen to start with AD_ (AD_TOKEN, AD_SECRET_KEY) are not auto-imported into replay scripts. - Extend the replay -u guard to also reject scripts with \${VAR} substitutions in any action, not just those with env directives, so the writer never silently drops substitutions on heal-rewrite. - Reword DX-unfriendly regex errors ("must match /^[A-Z_]...$/" -> "must be uppercase letters, digits, and underscores, e.g. APP_ID"). - Docs rewrite with precedence table, three recipes, fallback/escape examples, and a Notes block covering replay -u limitation, remote daemon caveat, no nested fallback, and loud typo behaviour. - Additional unit tests: namespace reservation on every path, shell prefix migration, \${VAR} round-trip preservation through writeReplayScript, green-path integration test with a fake invoke. * fix: collect .ad replay shell env on the CLI, not the daemon Review feedback (#433 (comment)): the daemon was reading AD_VAR_* from its own process.env, which meant "AD_VAR_K=V agent-device replay" only worked if V was set when the daemon started, and never worked for remote daemons. The client now filters process.env for AD_VAR_* at request time and ships the result as replayShellEnv on the DaemonRequest flags. The daemon prefers the request value when present and falls back to its own process.env for direct-daemon callers (internal tests). Adds two integration tests pinning both paths and updates the docs Notes block to reflect the new behaviour. * fix: thread per-attempt artifacts dir into AD_ARTIFACTS under test Review feedback (#433 (comment)): AD_ARTIFACTS is documented as available under "agent-device test", but buildReplayBuiltinVars was only reading the raw flags.artifactsDir. Under the default artifacts layout the flag is unset and \${AD_ARTIFACTS} failed; when set with --artifacts-dir it pointed at the suite root, not the resolved per-attempt directory the test runner actually writes to. Plumb the attempt-level artifacts dir from session-test.ts through the runReplay callback (via runReplayTestAttempt) down into the nested replay request's flags.artifactsDir. The daemon side is unchanged - buildReplayBuiltinVars just now sees the right value. Extracts the nested-request flag merge into a testable helper (buildNestedReplayFlags) to close the coverage gap between the test harness and the replay runtime. * refactor: DRY .ad replay parametrisation internals - Share VAR_KEY_RE between session-replay-vars and session-replay-script instead of re-declaring the same /^[A-Z_][A-Z0-9_]*$/ in each. - Fold resolveReplayFlags + resolveReplayRuntime into a single generic resolveStringProps<T>; the two were near-identical object-walkers. - Un-export parseReplayEnvLine (was only used inside its own module). - Table-drive the four "reject AD_* namespace" tests with test.each instead of four near-duplicate test blocks. - Extract runReplayFixture helper for runReplayScriptFile integration tests; each test is now ~10 lines instead of ~30. No behaviour change. 812 tests still green. * docs: clarify replay built-in variables * test: cover replay env serialization --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
1 parent 22936fd commit 2062bff

20 files changed

Lines changed: 1147 additions & 29 deletions

src/__tests__/client.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,42 @@ test('client throws AppError for daemon failures', async () => {
371371
);
372372
});
373373

374+
test('replay.run serializes client-collected AD_VAR shell env into daemon request', async () => {
375+
const previousAppId = process.env.AD_VAR_APP_ID;
376+
const previousWaitMs = process.env.AD_VAR_WAIT_MS;
377+
const previousLegacy = process.env.AD_APP_ID;
378+
process.env.AD_VAR_APP_ID = 'com.example.debug';
379+
process.env.AD_VAR_WAIT_MS = '750';
380+
process.env.AD_APP_ID = 'legacy-prefix-ignored';
381+
try {
382+
const setup = createTransport(async () => ({ ok: true, data: {} }));
383+
const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
384+
385+
await client.replay.run({
386+
path: './flows/login.ad',
387+
env: ['APP_ID=cli-override'],
388+
});
389+
390+
assert.equal(setup.calls.length, 1);
391+
assert.equal(setup.calls[0]?.command, 'replay');
392+
assert.deepEqual(setup.calls[0]?.positionals, ['./flows/login.ad']);
393+
assert.deepEqual(setup.calls[0]?.flags?.replayEnv, ['APP_ID=cli-override']);
394+
const replayShellEnv = setup.calls[0]?.flags?.replayShellEnv as
395+
| Record<string, string>
396+
| undefined;
397+
assert.equal(replayShellEnv?.AD_VAR_APP_ID, 'com.example.debug');
398+
assert.equal(replayShellEnv?.AD_VAR_WAIT_MS, '750');
399+
assert.equal(Object.prototype.hasOwnProperty.call(replayShellEnv ?? {}, 'AD_APP_ID'), false);
400+
} finally {
401+
if (previousAppId === undefined) delete process.env.AD_VAR_APP_ID;
402+
else process.env.AD_VAR_APP_ID = previousAppId;
403+
if (previousWaitMs === undefined) delete process.env.AD_VAR_WAIT_MS;
404+
else process.env.AD_VAR_WAIT_MS = previousWaitMs;
405+
if (previousLegacy === undefined) delete process.env.AD_APP_ID;
406+
else process.env.AD_APP_ID = previousLegacy;
407+
}
408+
});
409+
374410
test('client.command.wait prepares selector options and rejects invalid selectors', async () => {
375411
const setup = createTransport(async () => ({
376412
ok: true,

src/cli/commands/generic.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ export const genericClientCommandHandlers = {
7373
...buildSelectionOptions(flags),
7474
path: required(positionals[0], 'replay requires path'),
7575
update: flags.replayUpdate,
76+
env: flags.replayEnv,
7677
}),
7778
),
7879
[CLIENT_COMMANDS.test]: createGenericClientCommandHandler(
@@ -83,6 +84,7 @@ export const genericClientCommandHandlers = {
8384
...buildSelectionOptions(flags),
8485
paths: positionals,
8586
update: flags.replayUpdate,
87+
env: flags.replayEnv,
8688
failFast: flags.failFast,
8789
timeoutMs: flags.timeoutMs,
8890
retries: flags.retries,

src/cli/commands/router.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ import { snapshotCommand } from './snapshot.ts';
1313
import { screenshotCommand, diffCommand } from './screenshot.ts';
1414
import { clientCommandMethodHandlers } from './client-command.ts';
1515
import { genericClientCommandHandlers } from './generic.ts';
16-
import type {
17-
ClientCommandHandler,
18-
ClientCommandHandlerMap,
19-
} from './router-types.ts';
16+
import type { ClientCommandHandler, ClientCommandHandlerMap } from './router-types.ts';
2017

2118
export type {
2219
ClientCommandHandler,

src/client-normalizers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,8 @@ export function buildFlags(options: InternalRequestOptions): CommandFlags {
289289
headless: options.headless,
290290
restart: options.restart,
291291
replayUpdate: options.replayUpdate,
292+
replayEnv: options.replayEnv,
293+
replayShellEnv: options.replayShellEnv,
292294
failFast: options.failFast,
293295
timeoutMs: options.timeoutMs,
294296
retries: options.retries,

src/client-types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,12 +611,14 @@ export type FindOptions =
611611
export type ReplayRunOptions = AgentDeviceRequestOverrides & {
612612
path: string;
613613
update?: boolean;
614+
env?: string[];
614615
};
615616

616617
export type ReplayTestOptions = AgentDeviceRequestOverrides &
617618
AgentDeviceSelectionOptions & {
618619
paths: string[];
619620
update?: boolean;
621+
env?: string[];
620622
failFast?: boolean;
621623
timeoutMs?: number;
622624
retries?: number;
@@ -738,6 +740,8 @@ type CommandExecutionOptions = {
738740
headless?: boolean;
739741
restart?: boolean;
740742
replayUpdate?: boolean;
743+
replayEnv?: string[];
744+
replayShellEnv?: Record<string, string>;
741745
failFast?: boolean;
742746
timeoutMs?: number;
743747
retries?: number;

src/client.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,11 +407,15 @@ export function createAgentDeviceClient(
407407
await executeCommandRequest(CLIENT_COMMANDS.replay, [options.path], {
408408
...options,
409409
replayUpdate: options.update,
410+
replayEnv: options.env,
411+
replayShellEnv: collectReplayClientShellEnv(process.env),
410412
}),
411413
test: async (options) =>
412414
await executeCommandRequest(CLIENT_COMMANDS.test, options.paths, {
413415
...options,
414416
replayUpdate: options.update,
417+
replayEnv: options.env,
418+
replayShellEnv: collectReplayClientShellEnv(process.env),
415419
}),
416420
},
417421
batch: {
@@ -521,6 +525,18 @@ function optionalNumber(value: number | undefined): string[] {
521525
return value === undefined ? [] : [String(value)];
522526
}
523527

528+
const REPLAY_SHELL_ENV_PREFIX = 'AD_VAR_';
529+
530+
function collectReplayClientShellEnv(env: NodeJS.ProcessEnv): Record<string, string> {
531+
const result: Record<string, string> = {};
532+
for (const [key, value] of Object.entries(env)) {
533+
if (typeof value === 'string' && key.startsWith(REPLAY_SHELL_ENV_PREFIX)) {
534+
result[key] = value;
535+
}
536+
}
537+
return result;
538+
}
539+
524540
function mergeClientOptions(
525541
config: AgentDeviceClientConfig,
526542
options: InternalRequestOptions,

src/commands/selector-read-shared.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import type { AgentDeviceRuntime, CommandContext, CommandSessionRecord } from '../runtime-contract.ts';
1+
import type {
2+
AgentDeviceRuntime,
3+
CommandContext,
4+
CommandSessionRecord,
5+
} from '../runtime-contract.ts';
26
import { AppError } from '../utils/errors.ts';
37
import type { SnapshotNode, SnapshotState } from '../utils/snapshot.ts';
48
import { findNodeByRef, normalizeRef } from '../utils/snapshot.ts';

src/core/interactors.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,7 @@ import {
5555
import { readLinuxClipboard, writeLinuxClipboard } from '../platforms/linux/clipboard.ts';
5656
import type { Interactor, RunnerContext } from './interactor-types.ts';
5757

58-
export type {
59-
BackMode,
60-
Interactor,
61-
RunnerContext,
62-
ScreenshotOptions,
63-
} from './interactor-types.ts';
58+
export type { BackMode, Interactor, RunnerContext, ScreenshotOptions } from './interactor-types.ts';
6459

6560
export function getInteractor(device: DeviceInfo, runnerContext: RunnerContext): Interactor {
6661
switch (device.platform) {

src/daemon/handlers/__tests__/session-replay-script.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,3 +259,57 @@ test('readReplayScriptMetadata rejects conflicting metadata keys in context head
259259
/Conflicting replay test metadata "timeoutMs"/.test(error.message),
260260
);
261261
});
262+
263+
test('writeReplayScript round-trips ${VAR} tokens byte-for-byte across positionals and flags', () => {
264+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-script-vars-'));
265+
const replayPath = path.join(root, 'flow.ad');
266+
const actions: SessionAction[] = [
267+
{
268+
ts: Date.now(),
269+
command: 'open',
270+
positionals: ['${APP_ID}'],
271+
runtime: {
272+
platform: 'android',
273+
metroHost: '${HOST}',
274+
},
275+
flags: { relaunch: true },
276+
},
277+
{
278+
ts: Date.now(),
279+
command: 'click',
280+
positionals: ['label=Wait || ${EXTRA}'],
281+
flags: {},
282+
},
283+
{
284+
ts: Date.now(),
285+
command: 'snapshot',
286+
positionals: [],
287+
flags: { snapshotScope: '${SNAPSHOT_SCOPE:-app}' },
288+
},
289+
{
290+
ts: Date.now(),
291+
command: 'fill',
292+
positionals: ['@e2', 'value-${SUFFIX}'],
293+
flags: {},
294+
},
295+
];
296+
297+
writeReplayScript(replayPath, actions, makeSession());
298+
const script = fs.readFileSync(replayPath, 'utf8');
299+
// Each raw ${...} token must be preserved on disk.
300+
assert.ok(script.includes('${APP_ID}'), `missing \${APP_ID} in:\n${script}`);
301+
assert.ok(script.includes('${HOST}'), `missing \${HOST} in:\n${script}`);
302+
assert.ok(script.includes('label=Wait || ${EXTRA}'), `missing \${EXTRA} in:\n${script}`);
303+
assert.ok(
304+
script.includes('${SNAPSHOT_SCOPE:-app}'),
305+
`missing \${SNAPSHOT_SCOPE:-app} in:\n${script}`,
306+
);
307+
assert.ok(script.includes('value-${SUFFIX}'), `missing \${SUFFIX} in:\n${script}`);
308+
309+
const parsed = parseReplayScript(script);
310+
assert.deepEqual(parsed[0]?.positionals, ['${APP_ID}']);
311+
assert.equal(parsed[0]?.runtime?.metroHost, '${HOST}');
312+
assert.deepEqual(parsed[1]?.positionals, ['label=Wait || ${EXTRA}']);
313+
assert.equal(parsed[2]?.flags.snapshotScope, '${SNAPSHOT_SCOPE:-app}');
314+
assert.deepEqual(parsed[3]?.positionals, ['@e2', 'value-${SUFFIX}']);
315+
});

0 commit comments

Comments
 (0)