From 1d0daddfbeba8d19ac343180c219c3906af7509b Mon Sep 17 00:00:00 2001 From: Umair Ahmed Date: Sun, 4 Oct 2026 22:32:33 +0100 Subject: [PATCH 1/6] feat(maestro): support repeat.while conditions Parse and execute repeat.while with platform, visible, notVisible, and JavaScript true conditions, optionally bounded by times. Script results use Maestro string truthiness, and while.true is refused for untrusted remote flows like evalScript. --- .../__tests__/engine-truthiness.test.ts | 18 +++- .../src/internal/__tests__/engine.test.ts | 96 +++++++++++++++++++ .../__tests__/program-ir-parser.test.ts | 58 +++++++++++ .../src/internal/engine-eval-script.ts | 24 ++++- packages/maestro/src/internal/engine-flow.ts | 14 +++ .../maestro/src/internal/engine-truthiness.ts | 8 ++ packages/maestro/src/internal/engine-types.ts | 4 +- .../src/internal/program-ir-flow-parser.ts | 59 ++++++++---- packages/maestro/src/internal/program-ir.ts | 8 +- .../internal/replay-plan-step-execution.ts | 57 +++++++++-- .../maestro/src/internal/replay-plan-steps.ts | 7 +- 11 files changed, 320 insertions(+), 33 deletions(-) diff --git a/packages/maestro/src/internal/__tests__/engine-truthiness.test.ts b/packages/maestro/src/internal/__tests__/engine-truthiness.test.ts index 9439b00716..1cb62e0b70 100644 --- a/packages/maestro/src/internal/__tests__/engine-truthiness.test.ts +++ b/packages/maestro/src/internal/__tests__/engine-truthiness.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest'; -import { isMaestroConditionTruthy } from '../engine-truthiness.ts'; +import { isMaestroConditionTruthy, isMaestroScriptResultTruthy } from '../engine-truthiness.ts'; // assertTrue phase 1 (#1295): pin the string-value truthiness table explicitly // rather than relying on native JS coercion (which would treat every non-empty @@ -47,3 +47,19 @@ describe('isMaestroConditionTruthy', () => { expect(isMaestroConditionTruthy(123)).toBe(true); }); }); + +describe('isMaestroScriptResultTruthy', () => { + test.each([ + ['false', false], + ['0', false], + [false, false], + [0, false], + [null, false], + [undefined, false], + ['true', true], + [1, true], + [{}, true], + ])('%j is %s', (value, expected) => { + expect(isMaestroScriptResultTruthy(value)).toBe(expected); + }); +}); diff --git a/packages/maestro/src/internal/__tests__/engine.test.ts b/packages/maestro/src/internal/__tests__/engine.test.ts index 13212df367..61be8e4656 100644 --- a/packages/maestro/src/internal/__tests__/engine.test.ts +++ b/packages/maestro/src/internal/__tests__/engine.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test, vi } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; import { maestroTestFailure } from '../compatibility-errors.ts'; +import { MAESTRO_COMPATIBILITY_PRESETS } from '../compatibility-policy.ts'; import { parseMaestroProgram } from '../program-ir-parser.ts'; import type { MaestroObservation, @@ -679,6 +680,101 @@ describe('executeMaestroProgram', () => { ).toHaveLength(3); }); + test('repeat while evaluates JavaScript expressions and retains output updates', async () => { + const texts: string[] = []; + const port = makePort({ + execute: vi.fn(async (request) => { + if (request.command.kind === 'inputText') texts.push(request.command.text); + request.invalidateObservation(); + return {}; + }), + }); + const program = parseMaestroProgram( + [ + '---', + '- evalScript: ${output.counter = 0}', + '- repeat:', + ' while:', + ' true: "${output.counter < 3}"', + ' commands:', + ' - inputText: loop', + ' - evalScript: ${output.counter++}', + ].join('\n'), + ); + + await executeMaestroProgram(program, port); + + expect(texts).toEqual(['loop', 'loop', 'loop']); + }); + + test('repeat while observes selectors once per iteration and stops when false', async () => { + let checks = 0; + const observe = vi.fn(async ({ generation }: Parameters[0]) => ({ + generation, + matched: ++checks < 3, + })); + const port = makePort({ observe }); + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' times: 10', + ' while:', + ' platform: Android', + ' notVisible: ValueX', + ' commands:', + ' - inputText: loop', + ].join('\n'), + ); + + await executeMaestroProgram(program, port, { platform: 'android' }); + + expect(port.execute).toHaveBeenCalledTimes(2); + expect(observe).toHaveBeenCalledTimes(3); + expect(observe).toHaveBeenCalledWith( + expect.objectContaining({ + condition: { kind: 'notVisible', selector: { text: 'ValueX' } }, + timeoutMs: MAESTRO_COMPATIBILITY_PRESETS.command.optionalTargetLookupTimeoutMs, + }), + ); + }); + + test('repeat while stops at the times limit even while its condition remains true', async () => { + const port = makePort(); + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' times: 2', + ' while:', + ' true: true', + ' commands:', + ' - inputText: loop', + ].join('\n'), + ); + + await executeMaestroProgram(program, port); + + expect(port.execute).toHaveBeenCalledTimes(2); + }); + + test('repeat while expressions are refused for remote untrusted flows', async () => { + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' while:', + ' true: "${output.counter < 3}"', + ' commands:', + ' - inputText: loop', + ].join('\n'), + ); + + await expect( + executeMaestroProgram(program, makePort(), { trustedScripts: false }), + ).rejects.toMatchObject({ code: 'UNAUTHORIZED' }); + }); + test('evalScript replaces output namespace so shrunken arrays drop stale leaves', async () => { const texts: string[] = []; const port = makePort({ diff --git a/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts b/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts index 52dd72b9bb..a802935c71 100644 --- a/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts +++ b/packages/maestro/src/internal/__tests__/program-ir-parser.test.ts @@ -75,6 +75,64 @@ describe('parseMaestroProgram', () => { assert.equal(retry.commands[0]?.source.line, 27); }); + test('parses repeat while conditions with an optional times limit', () => { + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' while:', + ' platform: Android', + ' notVisible: ValueX', + ' commands:', + ' - tapOn: Button', + '- repeat:', + ' times: 4', + ' while:', + ' true: "${output.counter < 3}"', + ' commands:', + ' - evalScript: ${output.counter++}', + ].join('\n'), + ); + + assert.deepEqual(program.commands[0], { + kind: 'repeat', + source: { line: 2 }, + while: { platform: 'android', notVisible: { text: 'ValueX' } }, + commands: [ + { + kind: 'tapOn', + source: { line: 7 }, + target: { space: 'target', selector: { text: 'Button' } }, + }, + ], + }); + assert.deepEqual(program.commands[1], { + kind: 'repeat', + source: { line: 8 }, + times: 4, + while: { true: '${output.counter < 3}' }, + commands: [{ kind: 'evalScript', source: { line: 13 }, script: '${output.counter++}' }], + }); + }); + + test('requires repeat times or a non-empty while condition', () => { + assert.throws( + () => parseMaestroProgram('---\n- repeat:\n commands: []\n'), + /repeat requires times or while.*line 2/i, + ); + assert.throws( + () => parseMaestroProgram('---\n- repeat:\n while: {}\n commands: []\n'), + /repeat\.while cannot be empty.*line 3/i, + ); + assert.throws( + () => + parseMaestroProgram( + '---\n- repeat:\n while:\n unsupported: Android\n commands: []\n', + ), + /repeat\.while field "unsupported" is not supported.*line 4/i, + ); + }); + test('parses flow tags as typed metadata and validates each tag', () => { const program = parseMaestroProgram( ['name: Pager', 'tags: [smoke, pager]', '---', '- launchApp'].join('\n'), diff --git a/packages/maestro/src/internal/engine-eval-script.ts b/packages/maestro/src/internal/engine-eval-script.ts index 66eae5bc82..a71835746e 100644 --- a/packages/maestro/src/internal/engine-eval-script.ts +++ b/packages/maestro/src/internal/engine-eval-script.ts @@ -13,14 +13,30 @@ export async function evaluateMaestroEvalScript( script: string, values: Readonly>, ): Promise> { + return (await executeMaestroJavaScript(script, values, 'evalScript')).outputEnv; +} + +export async function evaluateMaestroEvalScriptCondition( + script: string, + values: Readonly>, +): Promise<{ value: unknown; outputEnv: Record }> { + return await executeMaestroJavaScript(script, values, 'repeat.while.true'); +} + +async function executeMaestroJavaScript( + script: string, + values: Readonly>, + name: 'evalScript' | 'repeat.while.true', +): Promise<{ value: unknown; outputEnv: Record }> { const output = seedMaestroOutput(values); const expression = unwrapMaestroEvalScriptExpression(script); // ponytail: function-scoped import keeps node:vm out of the maestro eager closure. const { default: vm } = await import('node:vm'); const sandbox: Record = { ...values, output }; + let value: unknown; try { - vm.runInNewContext(expression, sandbox, { - filename: 'evalScript', + value = vm.runInNewContext(expression, sandbox, { + filename: name, timeout: MAESTRO_EVAL_SCRIPT_TIMEOUT_MS, }); } catch (error) { @@ -28,7 +44,7 @@ export async function evaluateMaestroEvalScript( // instances; read the message directly rather than through normalizeError. throw new AppError( 'COMMAND_FAILED', - `Maestro evalScript failed: ${errorMessage(error)}`, + `Maestro ${name} failed: ${errorMessage(error)}`, undefined, error instanceof Error ? error : undefined, ); @@ -36,7 +52,7 @@ export async function evaluateMaestroEvalScript( // The script can replace the `output` binding (`output = { x: 1 }`), which // leaves the seeded host object stale; read the binding back from the vm // global instead of flattening the original reference. - return flattenMaestroOutput(sandbox['output']); + return { value, outputEnv: flattenMaestroOutput(sandbox['output']) }; } function unwrapMaestroEvalScriptExpression(script: string): string { diff --git a/packages/maestro/src/internal/engine-flow.ts b/packages/maestro/src/internal/engine-flow.ts index 6de04600a8..49314e3ed7 100644 --- a/packages/maestro/src/internal/engine-flow.ts +++ b/packages/maestro/src/internal/engine-flow.ts @@ -8,6 +8,7 @@ import { import type { MaestroCommand, MaestroProgram, + MaestroRepeatCondition, MaestroRunFlowCommand, MaestroRunFlowCondition, } from './program-ir.ts'; @@ -19,6 +20,19 @@ export function resolveCommand + parsePlatform(entry, context, `${name}.platform`), + ); + const visible = readOptionalEntry(entries, 'visible', (entry) => + parseMaestroSelector(entry, `${name}.visible`, context), + ); + const notVisible = readOptionalEntry(entries, 'notVisible', (entry) => + parseMaestroSelector(entry, `${name}.notVisible`, context), + ); + const truth = readOptionalEntry(entries, 'true', (entry) => + readConditionTruth(entry, context, `${name}.true`), + ); + return stripUndefined({ platform, visible, notVisible, true: truth }); +} + function readConditionTruth( node: Node | null | undefined, context: MaestroProgramParseContext, + name = 'runFlow.when.true', ): boolean | string { - const value = readScalarValue(node, 'runFlow.when.true', context); + const value = readScalarValue(node, name, context); if (typeof value === 'boolean' || typeof value === 'string') return value; - invalidAt('Maestro runFlow.when.true expects a boolean or expression string.', node, context); + invalidAt(`Maestro ${name} expects a boolean or expression string.`, node, context); } function parsePlatform( node: Node | null | undefined, context: MaestroProgramParseContext, + name = 'runFlow.when.platform', ): MaestroPlatform { - const value = readRequiredString(node, 'runFlow.when.platform', context).toLowerCase(); + const value = readRequiredString(node, name, context).toLowerCase(); if (value === 'android' || value === 'ios' || value === 'web') return value; - invalidAt('Maestro runFlow.when.platform expects Android, iOS, or Web.', node, context); + invalidAt(`Maestro ${name} expects Android, iOS, or Web.`, node, context); } diff --git a/packages/maestro/src/internal/program-ir.ts b/packages/maestro/src/internal/program-ir.ts index d8020097ee..900fd0b88e 100644 --- a/packages/maestro/src/internal/program-ir.ts +++ b/packages/maestro/src/internal/program-ir.ts @@ -242,6 +242,11 @@ export type MaestroRunFlowCondition = { true?: boolean | string; }; +export type MaestroRepeatCondition = Pick< + MaestroRunFlowCondition, + 'platform' | 'visible' | 'notVisible' | 'true' +>; + export type MaestroRunFlowCommand = { kind: 'runFlow'; source: MaestroSourceLocation; @@ -254,7 +259,8 @@ export type MaestroRunFlowCommand = { export type MaestroRepeatCommand = { kind: 'repeat'; source: MaestroSourceLocation; - times: number | string; + times?: number | string; + while?: MaestroRepeatCondition; commands: MaestroCommand[]; }; diff --git a/packages/maestro/src/internal/replay-plan-step-execution.ts b/packages/maestro/src/internal/replay-plan-step-execution.ts index 2a3c02bbec..0b2852ff6a 100644 --- a/packages/maestro/src/internal/replay-plan-step-execution.ts +++ b/packages/maestro/src/internal/replay-plan-step-execution.ts @@ -1,7 +1,7 @@ import { AppError } from '@agent-device/kernel/errors'; import { stripUndefined } from './shared.ts'; import { isMaestroTestFailure, maestroTestFailure } from './compatibility-errors.ts'; -import { isMaestroConditionTruthy } from './engine-truthiness.ts'; +import { isMaestroConditionTruthy, isMaestroScriptResultTruthy } from './engine-truthiness.ts'; import { MAESTRO_COMPATIBILITY_PRESETS, type MaestroCompatibilityTimingPolicy, @@ -15,7 +15,7 @@ import { resolveNumeric, staticConditionMatches, } from './engine-flow.ts'; -import type { MaestroRunFlowCondition } from './program-ir.ts'; +import type { MaestroRepeatCondition, MaestroRunFlowCondition } from './program-ir.ts'; import type { MaestroEngineOptions, MaestroEngineEvent, @@ -207,15 +207,10 @@ async function executeOpaqueStep( state.executed += 1; await executeNestedSteps(step.body, state); return; - case 'repeat': { - const times = readIterationCount(command.times, 0, state.context, 'repeat.times'); + case 'repeat': state.executed += 1; - for (let iteration = 0; iteration < times; iteration += 1) { - checkpointMaestroCancellation(state.options.signal); - await executeNestedSteps(step.body, state); - } + await executeRepeat(step.body, command.times, command.while, state); return; - } case 'retry': { const retries = Math.min( readIterationCount(command.maxRetries, 1, state.context, 'retry.maxRetries'), @@ -237,6 +232,23 @@ async function executeNestedSteps( } } +async function executeRepeat( + steps: readonly MaestroReplayPlanStep[], + times: number | string | undefined, + condition: MaestroRepeatCondition | undefined, + state: MaestroReplayPlanExecutionState, +): Promise { + const maxIterations = + times === undefined && condition + ? Number.POSITIVE_INFINITY + : readIterationCount(times, 0, state.context, 'repeat.times'); + for (let iteration = 0; iteration < maxIterations; iteration += 1) { + checkpointMaestroCancellation(state.options.signal); + if (condition && !(await repeatConditionMatches(condition, state))) return; + await executeNestedSteps(steps, state); + } +} + async function executeRetry( steps: readonly MaestroReplayPlanStep[], maxRetries: number, @@ -272,6 +284,33 @@ async function flowConditionMatches( return true; } +async function repeatConditionMatches( + condition: MaestroRepeatCondition, + state: MaestroReplayPlanExecutionState, +): Promise { + const { true: script, ...rest } = condition; + if (typeof script !== 'string') return await flowConditionMatches(condition, state); + if (!(await scriptConditionMatches(script, state))) return false; + return await flowConditionMatches(rest, state); +} + +async function scriptConditionMatches( + script: string, + state: MaestroReplayPlanExecutionState, +): Promise { + if (state.options.trustedScripts === false) { + throw new AppError( + 'UNAUTHORIZED', + 'Maestro repeat.while.true is not permitted for flows received over the remote daemon surface: ' + + 'node:vm is not a security sandbox, so an untrusted expression can escape to the host.', + ); + } + const { evaluateMaestroEvalScriptCondition } = await import('./engine-eval-script.ts'); + const result = await evaluateMaestroEvalScriptCondition(script, state.context.values); + state.context.replaceOutput(result.outputEnv); + return isMaestroScriptResultTruthy(result.value); +} + async function requireObservation( condition: MaestroObservationCondition, timeoutMs: number, diff --git a/packages/maestro/src/internal/replay-plan-steps.ts b/packages/maestro/src/internal/replay-plan-steps.ts index 685bc313b6..759c52191e 100644 --- a/packages/maestro/src/internal/replay-plan-steps.ts +++ b/packages/maestro/src/internal/replay-plan-steps.ts @@ -277,7 +277,12 @@ function opaqueControlCommand( ...(command.include.kind === 'file' ? { includePath: command.include.path } : {}), }); case 'repeat': - return { kind: command.kind, source: command.source, times: command.times }; + return stripUndefined({ + kind: command.kind, + source: command.source, + times: command.times, + while: command.while, + }); case 'retry': return stripUndefined({ kind: command.kind, From 200030070cf7fefcdcd77e512142b22f8e8084d3 Mon Sep 17 00:00:00 2001 From: Umair Ahmed Date: Sun, 4 Oct 2026 22:32:41 +0100 Subject: [PATCH 2/6] test(maestro): add repeat.while conformance coverage --- .../conformance-selector-projection.test.ts | 53 +++++ .../src/internal/conformance-normalize.ts | 57 ++++- .../maestro-conformance/build-manifest.mjs | 2 + .../corpus/authored/repeat-while.yaml | 15 ++ .../maestro-conformance/corpus/manifest.json | 8 + .../fixtures/layer1-parser.json | 213 +++++++++++++++++- 6 files changed, 343 insertions(+), 5 deletions(-) create mode 100644 scripts/maestro-conformance/corpus/authored/repeat-while.yaml diff --git a/packages/maestro/src/internal/__tests__/conformance-selector-projection.test.ts b/packages/maestro/src/internal/__tests__/conformance-selector-projection.test.ts index 55259ac3b2..d1613ccce1 100644 --- a/packages/maestro/src/internal/__tests__/conformance-selector-projection.test.ts +++ b/packages/maestro/src/internal/__tests__/conformance-selector-projection.test.ts @@ -75,3 +75,56 @@ test('canonical selector projection preserves recursive tree relations', () => { containsDescendants: [{ text: 'Save', childOf: { id: 'body' } }], }); }); + +test('canonicalizes repeat conditions from agent and upstream command shapes', () => { + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' while:', + ' platform: Android', + ' notVisible: Ready', + ' commands: []', + '- repeat:', + ' times: 4', + ' while:', + ' true: "${output.counter < 3}"', + ' commands: []', + ].join('\n'), + ); + + expect(canonicalizeAgentCommands(program)).toEqual([ + { kind: 'repeat', while: { platform: 'android', notVisible: { text: 'Ready' } } }, + { kind: 'repeat', times: 4, while: { true: '${output.counter < 3}' } }, + ]); + expect( + canonicalizeUpstreamFlow([ + { + type: 'RepeatCommand', + fields: { + times: null, + condition: { + platform: 'Android', + visible: null, + notVisible: { textRegex: 'Ready' }, + scriptCondition: null, + }, + }, + }, + { + type: 'RepeatCommand', + fields: { + times: '4', + condition: { + visible: null, + notVisible: null, + scriptCondition: '${output.counter < 3}', + }, + }, + }, + ]), + ).toEqual([ + { kind: 'repeat', while: { platform: 'android', notVisible: { text: 'Ready' } } }, + { kind: 'repeat', times: 4, while: { true: '${output.counter < 3}' } }, + ]); +}); diff --git a/packages/maestro/src/internal/conformance-normalize.ts b/packages/maestro/src/internal/conformance-normalize.ts index 6d61c13be4..cdfca1f26b 100644 --- a/packages/maestro/src/internal/conformance-normalize.ts +++ b/packages/maestro/src/internal/conformance-normalize.ts @@ -6,7 +6,12 @@ // differences (regex-vs-literal selector storage, runScript path-vs-content, // nested runFlow expansion) that the two IR designs express differently. -import type { MaestroCommand, MaestroProgram, MaestroSwipeGesture } from './program-ir.ts'; +import type { + MaestroCommand, + MaestroProgram, + MaestroRepeatCondition, + MaestroSwipeGesture, +} from './program-ir.ts'; import { MAESTRO_COMPATIBILITY_PRESETS } from './compatibility-policy.ts'; import { asRecord, bool, dropUndefined, numLike, str } from './conformance-value-coercion.ts'; import { @@ -36,6 +41,13 @@ export type CanonicalGesture = } | { mode: 'element'; from: CanonicalSelector; direction?: string; duration?: number | string }; +type CanonicalRepeatCondition = { + platform?: string; + visible?: CanonicalSelector; + notVisible?: CanonicalSelector; + true?: boolean | string; +}; + export type CanonicalCommand = | { kind: 'launchApp'; @@ -85,7 +97,11 @@ export type CanonicalCommand = | { kind: 'stopApp' } | { kind: 'setPermissions'; appId?: string; permissions?: Record } | { kind: 'clearState'; appId?: string } - | { kind: 'repeat'; times: string | number } + | { + kind: 'repeat'; + times?: string | number; + while?: CanonicalRepeatCondition; + } | { kind: 'retry'; maxRetries?: string | number } | { kind: 'runFlow'; label?: string; source: 'file' | 'commands' } | { kind: 'runScript' } @@ -232,7 +248,11 @@ function canonicalizeUpstreamCommand(command: UpstreamCommand): CanonicalCommand permissions: permissionsRecord(f.permissions), }); case 'RepeatCommand': - return { kind: 'repeat', times: numLike(f.times) ?? str(f.times) ?? '' }; + return dropUndefined({ + kind: 'repeat' as const, + times: numLike(f.times) ?? str(f.times), + while: canonicalizeUpstreamRepeatCondition(f.condition), + }); case 'RetryCommand': return dropUndefined({ kind: 'retry', @@ -251,6 +271,19 @@ function canonicalizeUpstreamCommand(command: UpstreamCommand): CanonicalCommand } } +function canonicalizeUpstreamRepeatCondition(value: unknown): CanonicalRepeatCondition | undefined { + const condition = asRecord(value); + if (!condition) return undefined; + return dropUndefined({ + platform: lower(str(condition.platform)), + visible: + condition.visible == null ? undefined : canonicalizeUpstreamSelector(condition.visible), + notVisible: + condition.notVisible == null ? undefined : canonicalizeUpstreamSelector(condition.notVisible), + true: str(condition.scriptCondition), + }); +} + /** * Build a canonical tap from the effective repeat semantics. `delay` only means * anything for a repeated tap, so it is dropped for a single tap to keep the two @@ -526,7 +559,11 @@ function canonicalizeAgentCommand( permissions: command.permissions, }); case 'repeat': - return { kind: 'repeat', times: numLike(command.times) ?? str(command.times) ?? '' }; + return dropUndefined({ + kind: 'repeat' as const, + times: numLike(command.times) ?? str(command.times), + while: canonicalizeAgentRepeatCondition(command.while), + }); case 'retry': return dropUndefined({ kind: 'retry', @@ -547,6 +584,18 @@ function canonicalizeAgentCommand( } } +function canonicalizeAgentRepeatCondition( + condition: MaestroRepeatCondition | undefined, +): CanonicalRepeatCondition | undefined { + if (!condition) return undefined; + return dropUndefined({ + platform: condition.platform, + visible: condition.visible && canonicalizeAgentSelector(condition.visible), + notVisible: condition.notVisible && canonicalizeAgentSelector(condition.notVisible), + true: condition.true, + }); +} + function agentGesture(gesture: MaestroSwipeGesture): CanonicalGesture { const duration = numLike(gesture.duration) ?? AGENT_SWIPE_DEFAULT_DURATION; switch (gesture.kind) { diff --git a/scripts/maestro-conformance/build-manifest.mjs b/scripts/maestro-conformance/build-manifest.mjs index c49791ba74..8b38693d94 100644 --- a/scripts/maestro-conformance/build-manifest.mjs +++ b/scripts/maestro-conformance/build-manifest.mjs @@ -36,6 +36,8 @@ const NOTES = { 'Coverage: extendedWaitUntil (upstream 042 interpolates ${TIMEOUT} from a flow env block).', 'authored/repeat': 'Coverage: repeat.times with ${output.list.length} (upstream 053 exercises the same evalScript-to-repeat shape).', + 'authored/repeat-while': + 'Coverage: repeat.while selector and JavaScript conditions, with and without a times limit.', 'authored/presskey': 'Coverage: pressKey supported keys (upstream 034 exercises many unsupported keycodes).', 'authored/numeric-variable-tap': diff --git a/scripts/maestro-conformance/corpus/authored/repeat-while.yaml b/scripts/maestro-conformance/corpus/authored/repeat-while.yaml new file mode 100644 index 0000000000..d65fa3dbf8 --- /dev/null +++ b/scripts/maestro-conformance/corpus/authored/repeat-while.yaml @@ -0,0 +1,15 @@ +appId: com.example.app +--- +- repeat: + while: + platform: Android + notVisible: "Continue" + commands: + - tapOn: "Continue" +- evalScript: ${output.counter = 0} +- repeat: + times: 4 + while: + true: ${output.counter < 3} + commands: + - evalScript: ${output.counter++} diff --git a/scripts/maestro-conformance/corpus/manifest.json b/scripts/maestro-conformance/corpus/manifest.json index 24da051b02..471a8cf542 100644 --- a/scripts/maestro-conformance/corpus/manifest.json +++ b/scripts/maestro-conformance/corpus/manifest.json @@ -560,6 +560,14 @@ "note": "Coverage: pressKey supported keys (upstream 034 exercises many unsupported keycodes)." } }, + { + "id": "authored/repeat-while", + "file": "authored/repeat-while.yaml", + "origin": { + "kind": "authored", + "note": "Coverage: repeat.while selector and JavaScript conditions, with and without a times limit." + } + }, { "id": "authored/repeat", "file": "authored/repeat.yaml", diff --git a/scripts/maestro-conformance/fixtures/layer1-parser.json b/scripts/maestro-conformance/fixtures/layer1-parser.json index 8b3edb6fbe..08cf8d5177 100644 --- a/scripts/maestro-conformance/fixtures/layer1-parser.json +++ b/scripts/maestro-conformance/fixtures/layer1-parser.json @@ -4342,6 +4342,217 @@ } ] }, + { + "id": "authored/repeat-while", + "file": "authored/repeat-while.yaml", + "status": "parsed", + "commands": [ + { + "type": "ApplyConfigurationCommand", + "fields": { + "config": { + "appId": "com.example.app", + "name": null, + "tags": [], + "ext": {}, + "onFlowStart": null, + "onFlowComplete": null, + "properties": {} + }, + "label": null, + "optional": false + } + }, + { + "type": "RepeatCommand", + "fields": { + "times": null, + "condition": { + "platform": "ANDROID", + "visible": null, + "notVisible": { + "textRegex": "Continue", + "idRegex": null, + "size": null, + "below": null, + "above": null, + "leftOf": null, + "rightOf": null, + "containsChild": null, + "containsDescendants": null, + "traits": null, + "index": null, + "enabled": null, + "optional": false, + "selected": null, + "checked": null, + "focused": null, + "childOf": null, + "css": null + }, + "scriptCondition": null, + "label": null + }, + "commands": [ + { + "tapOnElement": { + "selector": { + "textRegex": "Continue", + "idRegex": null, + "size": null, + "below": null, + "above": null, + "leftOf": null, + "rightOf": null, + "containsChild": null, + "containsDescendants": null, + "traits": null, + "index": null, + "enabled": null, + "optional": false, + "selected": null, + "checked": null, + "focused": null, + "childOf": null, + "css": null + }, + "retryIfNoChange": false, + "waitUntilVisible": false, + "longPress": false, + "repeat": null, + "waitToSettleTimeoutMs": null, + "relativePoint": null, + "label": null, + "optional": false + }, + "tapOnPoint": null, + "tapOnPointV2Command": null, + "scrollCommand": null, + "swipeCommand": null, + "backPressCommand": null, + "assertCommand": null, + "assertConditionCommand": null, + "assertScreenshotCommand": null, + "assertNoDefectsWithAICommand": null, + "assertWithAICommand": null, + "extractTextWithAICommand": null, + "inputTextCommand": null, + "inputRandomTextCommand": null, + "launchAppCommand": null, + "setPermissionsCommand": null, + "applyConfigurationCommand": null, + "openLinkCommand": null, + "pressKeyCommand": null, + "eraseTextCommand": null, + "hideKeyboardCommand": null, + "takeScreenshotCommand": null, + "stopAppCommand": null, + "killAppCommand": null, + "clearStateCommand": null, + "clearKeychainCommand": null, + "runFlowCommand": null, + "setLocationCommand": null, + "setOrientationCommand": null, + "repeatCommand": null, + "copyTextCommand": null, + "setClipboardCommand": null, + "pasteTextCommand": null, + "defineVariablesCommand": null, + "runScriptCommand": null, + "waitForAnimationToEndCommand": null, + "evalScriptCommand": null, + "scrollUntilVisible": null, + "travelCommand": null, + "startRecordingCommand": null, + "stopRecordingCommand": null, + "addMediaCommand": null, + "setAirplaneModeCommand": null, + "toggleAirplaneModeCommand": null, + "retryCommand": null + } + ], + "label": null, + "optional": false + } + }, + { + "type": "EvalScriptCommand", + "fields": { + "scriptString": "${output.counter = 0}", + "label": null, + "optional": false + } + }, + { + "type": "RepeatCommand", + "fields": { + "times": "4", + "condition": { + "platform": null, + "visible": null, + "notVisible": null, + "scriptCondition": "${output.counter < 3}", + "label": null + }, + "commands": [ + { + "tapOnElement": null, + "tapOnPoint": null, + "tapOnPointV2Command": null, + "scrollCommand": null, + "swipeCommand": null, + "backPressCommand": null, + "assertCommand": null, + "assertConditionCommand": null, + "assertScreenshotCommand": null, + "assertNoDefectsWithAICommand": null, + "assertWithAICommand": null, + "extractTextWithAICommand": null, + "inputTextCommand": null, + "inputRandomTextCommand": null, + "launchAppCommand": null, + "setPermissionsCommand": null, + "applyConfigurationCommand": null, + "openLinkCommand": null, + "pressKeyCommand": null, + "eraseTextCommand": null, + "hideKeyboardCommand": null, + "takeScreenshotCommand": null, + "stopAppCommand": null, + "killAppCommand": null, + "clearStateCommand": null, + "clearKeychainCommand": null, + "runFlowCommand": null, + "setLocationCommand": null, + "setOrientationCommand": null, + "repeatCommand": null, + "copyTextCommand": null, + "setClipboardCommand": null, + "pasteTextCommand": null, + "defineVariablesCommand": null, + "runScriptCommand": null, + "waitForAnimationToEndCommand": null, + "evalScriptCommand": { + "scriptString": "${output.counter++}", + "label": null, + "optional": false + }, + "scrollUntilVisible": null, + "travelCommand": null, + "startRecordingCommand": null, + "stopRecordingCommand": null, + "addMediaCommand": null, + "setAirplaneModeCommand": null, + "toggleAirplaneModeCommand": null, + "retryCommand": null + } + ], + "label": null, + "optional": false + } + } + ] + }, { "id": "authored/repeat", "file": "authored/repeat.yaml", @@ -5015,5 +5226,5 @@ ] } ], - "contentHash": "3fab6cb48d4ac7ec2640faa223b86a1dd58439d1e181ce442a76a5f754efc725" + "contentHash": "81820a1bb02d5b9ee5381b379b3e22bd055de959f1d552e72b207c9047ce667f" } From e4dc41a2463e5257e4d1ae0d257f79a4eb33d6a8 Mon Sep 17 00:00:00 2001 From: Umair Ahmed Date: Sun, 4 Oct 2026 22:32:41 +0100 Subject: [PATCH 3/6] docs(maestro): document repeat.while support --- packages/maestro/src/internal/support-matrix.ts | 6 +++--- website/docs/docs/replay-e2e.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/maestro/src/internal/support-matrix.ts b/packages/maestro/src/internal/support-matrix.ts index a58f2bcff7..8a95e10f85 100644 --- a/packages/maestro/src/internal/support-matrix.ts +++ b/packages/maestro/src/internal/support-matrix.ts @@ -1,5 +1,5 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ - 'Flows: launchApp (with clearState, permissions, and Apple-only launch arguments; permissions apply after state clearing but before launch, and a launchApp without permissions touches nothing — there is no silent all: allow default); setPermissions (mid-flow permission grants, denials, and resets; all resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specific entries overriding after it); runFlow file/inline with platform, visibility, and limited boolean conditions; onFlowStart/onFlowComplete; repeat.times and retry.', + 'Flows: launchApp (with clearState, permissions, and Apple-only launch arguments; permissions apply after state clearing but before launch, and a launchApp without permissions touches nothing — there is no silent all: allow default); setPermissions (mid-flow permission grants, denials, and resets; all resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specific entries overriding after it); runFlow file/inline with platform, visibility, and limited boolean conditions; onFlowStart/onFlowComplete; repeat.times/while with platform, visibility, or JavaScript conditions and retry.', 'Interactions: tapOn, doubleTapOn, longPressOn, inputText on the focused element, eraseText, openLink, hideKeyboard, basic pressKey, and back; selector targets poll until available and support recursive index, childOf, above, below, leftOf, rightOf, containsChild, containsDescendants, points, and optional; outer command labels are metadata, not target selectors.', 'Assertions and navigation: assertVisible, assertNotVisible, assertTrue (literal values and ${VAR} lookups only; "", "false", "0", "null", and "undefined" are falsy, everything else is truthy), extendedWaitUntil, scroll, scrollUntilVisible, absolute/percentage/target swipe, takeScreenshot, waitForAnimationToEnd, clearState, and stopApp.', 'Scripts: ordered runScript file/env scripts with http.post, json, and output variables; evalScript inline expressions run flow-scoped JavaScript and write output.* leaves for later steps.', @@ -8,10 +8,10 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ export const MAESTRO_COMPAT_LIMITATIONS = [ 'Permissions: every entry is one settings permission call, applied in order with all first; the step stops at the first entry the selected platform refuses, earlier entries stay applied, and the error names what landed. Android’s only allow level is while-in-use, so location: inuse and location: never mean allow and deny there, while location: always and photos: limited are Apple-only and fail. On iOS, which service a runtime changes is simctl privacy’s own verdict: current runtimes refuse a targeted notifications change and leave notifications untouched under all.', 'Runtime: iOS and Android only; launchApp.clearState and standalone clearState support Android and iOS simulators, launch arguments are Apple-only, and other standalone device utility/state commands are unsupported.', - 'Expressions: evalScript is the only command whose payload is evaluated as JavaScript (flow env and prior output leaves are string-typed); with that exception, fields stay literal or ${VAR} lookup-only — assertTrue supports literals and bare lookups, repeat.while is unsupported, and other expression-shaped payloads fail loud.', + 'Expressions: evalScript and repeat.while.true are evaluated as JavaScript (flow env and prior output leaves are string-typed); with those exceptions, fields stay literal or ${VAR} lookup-only — assertTrue supports literals and bare lookups, and other expression-shaped payloads fail loud.', 'Environment: flow env is the default, AD_VAR_* overrides it, and CLI -e KEY=VALUE wins over both.', 'Failure diagnostics: resolved targets and runFlow paths are rendered, while inputText payloads remain hidden; do not place secrets in diagnostic identifiers.', - 'Trust: runScript and evalScript execute flow scripts in-process via node:vm, which is not a security sandbox; runScript may make http.post network requests and its output keys cannot contain a dot. evalScript is refused outright for a flow accepted over the daemon’s remote HTTP surface, since that context can escape to the host.', + 'Trust: runScript, evalScript, and repeat.while.true execute flow scripts in-process via node:vm, which is not a security sandbox; runScript may make http.post network requests and its output keys cannot contain a dot. evalScript and repeat.while.true expressions are refused outright for a flow accepted over the daemon’s remote HTTP surface, since that context can escape to the host.', 'Errors and tracking: unsupported commands and fields fail with source context when available; open a focused issue only when implementation work is planned.', ] as const; diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 13486bf252..9b2db69752 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -83,7 +83,7 @@ agent-device test ./maestro-flows --maestro --platform android --artifacts-dir . Supported subset: -- Flows: `launchApp` (with `clearState`, `permissions`, and Apple-only launch arguments; `permissions` apply after state clearing but before launch, and a `launchApp` without `permissions` touches nothing — there is no silent `all: allow` default); `setPermissions` (mid-flow permission grants, denials, and resets; `all` resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specific entries overriding after it); `runFlow` file/inline with platform, visibility, and limited boolean conditions; `onFlowStart`/`onFlowComplete`; `repeat.times` and retry. +- Flows: `launchApp` (with `clearState`, `permissions`, and Apple-only launch arguments; `permissions` apply after state clearing but before launch, and a `launchApp` without `permissions` touches nothing — there is no silent `all: allow` default); `setPermissions` (mid-flow permission grants, denials, and resets; `all` resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specific entries overriding after it); `runFlow` file/inline with platform, visibility, and limited boolean conditions; `onFlowStart`/`onFlowComplete`; `repeat.times`/`while` with platform, visibility, or JavaScript conditions and retry. - Interactions: `tapOn`, `doubleTapOn`, `longPressOn`, `inputText` on the focused element, `eraseText`, `openLink`, `hideKeyboard`, basic `pressKey`, and `back`; selector targets poll until available and support recursive `index`, `childOf`, `above`, `below`, `leftOf`, `rightOf`, `containsChild`, `containsDescendants`, points, and `optional`; outer command labels are metadata, not target selectors. - Assertions and navigation: `assertVisible`, `assertNotVisible`, `assertTrue` (literal values and `${VAR}` lookups only; `""`, `"false"`, `"0"`, `"null"`, and `"undefined"` are falsy, everything else is truthy), `extendedWaitUntil`, `scroll`, `scrollUntilVisible`, absolute/percentage/target `swipe`, `takeScreenshot`, `waitForAnimationToEnd`, `clearState`, and `stopApp`. - Scripts: ordered `runScript` file/env scripts with `http.post`, `json`, and `output` variables; `evalScript` inline expressions run flow-scoped JavaScript and write `output.*` leaves for later steps. @@ -92,10 +92,10 @@ Boundaries: - Permissions: every entry is one `settings permission` call, applied in order with `all` first; the step stops at the first entry the selected platform refuses, earlier entries stay applied, and the error names what landed. Android’s only allow level is while-in-use, so `location: inuse` and `location: never` mean `allow` and `deny` there, while `location: always` and `photos: limited` are Apple-only and fail. On iOS, which service a runtime changes is `simctl privacy`’s own verdict: current runtimes refuse a targeted `notifications` change and leave notifications untouched under `all`. - Runtime: iOS and Android only; `launchApp.clearState` and standalone `clearState` support Android and iOS simulators, launch arguments are Apple-only, and other standalone device utility/state commands are unsupported. -- Expressions: `evalScript` is the only command whose payload is evaluated as JavaScript (flow `env` and prior `output` leaves are string-typed); with that exception, fields stay literal or `${VAR}` lookup-only — `assertTrue` supports literals and bare lookups, `repeat.while` is unsupported, and other expression-shaped payloads fail loud. +- Expressions: `evalScript` and `repeat.while.true` are evaluated as JavaScript (flow `env` and prior `output` leaves are string-typed); with those exceptions, fields stay literal or `${VAR}` lookup-only — `assertTrue` supports literals and bare lookups, and other expression-shaped payloads fail loud. - Environment: flow `env` is the default, `AD_VAR_*` overrides it, and CLI `-e KEY=VALUE` wins over both. - Failure diagnostics: resolved targets and `runFlow` paths are rendered, while `inputText` payloads remain hidden; do not place secrets in diagnostic identifiers. -- Trust: `runScript` and `evalScript` execute flow scripts in-process via `node:vm`, which is not a security sandbox; `runScript` may make `http.post` network requests and its output keys cannot contain a dot. `evalScript` is refused outright for a flow accepted over the daemon’s remote HTTP surface, since that context can escape to the host. +- Trust: `runScript`, `evalScript`, and `repeat.while.true` execute flow scripts in-process via `node:vm`, which is not a security sandbox; `runScript` may make `http.post` network requests and its output keys cannot contain a dot. `evalScript` and `repeat.while.true` expressions are refused outright for a flow accepted over the daemon’s remote HTTP surface, since that context can escape to the host. - Errors and tracking: unsupported commands and fields fail with source context when available; open a focused issue only when implementation work is planned. - Session takeover: `--keep-session` is a native `.ad` replay option and is rejected for Maestro YAML. From 42af17e2a656a2e13908fe58ed46112ac8c9c594 Mon Sep 17 00:00:00 2001 From: Umair Ahmed Date: Mon, 5 Oct 2026 07:14:30 +0100 Subject: [PATCH 4/6] fix(maestro): treat bigint zero as falsy and canonicalize boolean conditions Upstream stores a literal true: condition as a string scriptCondition, so the agent projection stringifies booleans too. --- .../conformance-selector-projection.test.ts | 14 ++++++++++++++ .../internal/__tests__/engine-truthiness.test.ts | 5 ++++- .../maestro/src/internal/conformance-normalize.ts | 8 ++++---- packages/maestro/src/internal/engine-truthiness.ts | 1 + 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/maestro/src/internal/__tests__/conformance-selector-projection.test.ts b/packages/maestro/src/internal/__tests__/conformance-selector-projection.test.ts index d1613ccce1..33e9932714 100644 --- a/packages/maestro/src/internal/__tests__/conformance-selector-projection.test.ts +++ b/packages/maestro/src/internal/__tests__/conformance-selector-projection.test.ts @@ -90,12 +90,18 @@ test('canonicalizes repeat conditions from agent and upstream command shapes', ( ' while:', ' true: "${output.counter < 3}"', ' commands: []', + '- repeat:', + ' times: 2', + ' while:', + ' true: true', + ' commands: []', ].join('\n'), ); expect(canonicalizeAgentCommands(program)).toEqual([ { kind: 'repeat', while: { platform: 'android', notVisible: { text: 'Ready' } } }, { kind: 'repeat', times: 4, while: { true: '${output.counter < 3}' } }, + { kind: 'repeat', times: 2, while: { true: 'true' } }, ]); expect( canonicalizeUpstreamFlow([ @@ -122,9 +128,17 @@ test('canonicalizes repeat conditions from agent and upstream command shapes', ( }, }, }, + { + type: 'RepeatCommand', + fields: { + times: '2', + condition: { visible: null, notVisible: null, scriptCondition: 'true' }, + }, + }, ]), ).toEqual([ { kind: 'repeat', while: { platform: 'android', notVisible: { text: 'Ready' } } }, { kind: 'repeat', times: 4, while: { true: '${output.counter < 3}' } }, + { kind: 'repeat', times: 2, while: { true: 'true' } }, ]); }); diff --git a/packages/maestro/src/internal/__tests__/engine-truthiness.test.ts b/packages/maestro/src/internal/__tests__/engine-truthiness.test.ts index 1cb62e0b70..9081772bbb 100644 --- a/packages/maestro/src/internal/__tests__/engine-truthiness.test.ts +++ b/packages/maestro/src/internal/__tests__/engine-truthiness.test.ts @@ -50,16 +50,19 @@ describe('isMaestroConditionTruthy', () => { describe('isMaestroScriptResultTruthy', () => { test.each([ + ['', false], ['false', false], ['0', false], [false, false], [0, false], + [0n, false], [null, false], [undefined, false], ['true', true], [1, true], + [1n, true], [{}, true], - ])('%j is %s', (value, expected) => { + ])('%o is %s', (value, expected) => { expect(isMaestroScriptResultTruthy(value)).toBe(expected); }); }); diff --git a/packages/maestro/src/internal/conformance-normalize.ts b/packages/maestro/src/internal/conformance-normalize.ts index cdfca1f26b..6719e1fc87 100644 --- a/packages/maestro/src/internal/conformance-normalize.ts +++ b/packages/maestro/src/internal/conformance-normalize.ts @@ -9,7 +9,7 @@ import type { MaestroCommand, MaestroProgram, - MaestroRepeatCondition, + MaestroRunFlowCondition, MaestroSwipeGesture, } from './program-ir.ts'; import { MAESTRO_COMPATIBILITY_PRESETS } from './compatibility-policy.ts'; @@ -45,7 +45,7 @@ type CanonicalRepeatCondition = { platform?: string; visible?: CanonicalSelector; notVisible?: CanonicalSelector; - true?: boolean | string; + true?: string; }; export type CanonicalCommand = @@ -585,14 +585,14 @@ function canonicalizeAgentCommand( } function canonicalizeAgentRepeatCondition( - condition: MaestroRepeatCondition | undefined, + condition: MaestroRunFlowCondition | undefined, ): CanonicalRepeatCondition | undefined { if (!condition) return undefined; return dropUndefined({ platform: condition.platform, visible: condition.visible && canonicalizeAgentSelector(condition.visible), notVisible: condition.notVisible && canonicalizeAgentSelector(condition.notVisible), - true: condition.true, + true: condition.true === undefined ? undefined : String(condition.true), }); } diff --git a/packages/maestro/src/internal/engine-truthiness.ts b/packages/maestro/src/internal/engine-truthiness.ts index d10002a912..7855e31f0a 100644 --- a/packages/maestro/src/internal/engine-truthiness.ts +++ b/packages/maestro/src/internal/engine-truthiness.ts @@ -28,5 +28,6 @@ export function isMaestroScriptResultTruthy(value: unknown): boolean { if (typeof value === 'boolean' || typeof value === 'number' || typeof value === 'string') { return isMaestroConditionTruthy(value); } + if (typeof value === 'bigint') return value !== 0n; return value !== null && value !== undefined; } From e3f1ecf95db84ef87fc05ae865dcbd673f6bab6c Mon Sep 17 00:00:00 2001 From: Umair Ahmed Date: Mon, 5 Oct 2026 07:14:31 +0100 Subject: [PATCH 5/6] fix(maestro): evaluate runFlow.when and repeat.while through one condition owner Both conditions share one parser, one IR type, and one true: evaluator. Booleans, maestro.platform comparisons, and literal text after ${VAR} lookups need no JavaScript; other expressions run in node:vm with a maestro.platform binding and are refused for untrusted remote flows through a shared trust check. Condition selectors are resolved at every check, and repeat yields to the event loop each iteration so cancellation timers can fire. --- .../src/internal/__tests__/engine.test.ts | 103 ++++++++++++++++++ .../src/internal/engine-eval-script.ts | 13 ++- .../maestro/src/internal/engine-expression.ts | 22 ++-- packages/maestro/src/internal/engine-flow.ts | 73 +++++++++---- packages/maestro/src/internal/engine-types.ts | 3 +- .../src/internal/program-ir-flow-parser.ts | 34 ++---- packages/maestro/src/internal/program-ir.ts | 7 +- .../internal/replay-plan-step-execution.ts | 64 ++++------- .../maestro/src/internal/replay-plan-steps.ts | 8 +- .../maestro/src/internal/support-matrix.ts | 6 +- website/docs/docs/replay-e2e.md | 6 +- 11 files changed, 211 insertions(+), 128 deletions(-) diff --git a/packages/maestro/src/internal/__tests__/engine.test.ts b/packages/maestro/src/internal/__tests__/engine.test.ts index 61be8e4656..4a6c10100b 100644 --- a/packages/maestro/src/internal/__tests__/engine.test.ts +++ b/packages/maestro/src/internal/__tests__/engine.test.ts @@ -775,6 +775,109 @@ describe('executeMaestroProgram', () => { ).rejects.toMatchObject({ code: 'UNAUTHORIZED' }); }); + test('repeat while true binds maestro.platform in JavaScript expressions', async () => { + const port = makePort(); + const program = parseMaestroProgram( + [ + '---', + '- evalScript: ${output.n = 0}', + '- repeat:', + ' while:', + ` true: "\${maestro.platform === 'ios' && output.n < 2}"`, + ' commands:', + ' - inputText: loop', + ' - evalScript: ${output.n++}', + ].join('\n'), + ); + + await executeMaestroProgram(program, port, { platform: 'ios' }); + + expect(port.execute).toHaveBeenCalledTimes(2); + }); + + test('repeat while true accepts plain literals and platform comparisons without scripts', async () => { + const port = makePort(); + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' times: 2', + ' while:', + ' true: yes', + ' commands:', + ' - inputText: literal', + '- repeat:', + ' times: 2', + ' while:', + ` true: "\${maestro.platform == 'android'}"`, + ' commands:', + ' - inputText: platform', + ].join('\n'), + ); + + await executeMaestroProgram(program, port, { platform: 'ios', trustedScripts: false }); + + expect(port.execute).toHaveBeenCalledTimes(2); + }); + + test('runFlow when true shares the repeat while JavaScript evaluator', async () => { + const port = makePort(); + const program = parseMaestroProgram( + [ + '---', + '- evalScript: ${output.n = 2}', + '- runFlow:', + ' when:', + ' true: "${output.n > 1}"', + ' commands:', + ' - inputText: ran', + ].join('\n'), + ); + + await executeMaestroProgram(program, port); + + expect(port.execute).toHaveBeenCalledTimes(1); + }); + + test('repeat while re-resolves condition selectors before every check', async () => { + const observe = vi.fn(async ({ generation }: Parameters[0]) => ({ + generation, + matched: true, + })); + const port = makePort({ observe }); + const program = parseMaestroProgram( + [ + '---', + '- evalScript: ${output.target = "first"}', + '- repeat:', + ' times: 2', + ' while:', + ' visible: ${output.target}', + ' commands:', + ' - evalScript: ${output.target = "second"}', + ].join('\n'), + ); + + await executeMaestroProgram(program, port); + + expect(observe.mock.calls.map(([request]) => request.condition)).toEqual([ + { kind: 'visible', selector: { text: 'first' } }, + { kind: 'visible', selector: { text: 'second' } }, + ]); + }); + + test('an unbounded synchronous repeat still yields to cancellation timers', async () => { + const controller = new AbortController(); + const program = parseMaestroProgram( + ['---', '- repeat:', ' while:', ' true: true', ' commands: []'].join('\n'), + ); + setTimeout(() => controller.abort(), 10); + + await expect( + executeMaestroProgram(program, makePort(), { signal: controller.signal }), + ).rejects.toMatchObject({ details: { reason: 'request_canceled' } }); + }); + test('evalScript replaces output namespace so shrunken arrays drop stale leaves', async () => { const texts: string[] = []; const port = makePort({ diff --git a/packages/maestro/src/internal/engine-eval-script.ts b/packages/maestro/src/internal/engine-eval-script.ts index a71835746e..e6b8b62aa3 100644 --- a/packages/maestro/src/internal/engine-eval-script.ts +++ b/packages/maestro/src/internal/engine-eval-script.ts @@ -13,26 +13,29 @@ export async function evaluateMaestroEvalScript( script: string, values: Readonly>, ): Promise> { - return (await executeMaestroJavaScript(script, values, 'evalScript')).outputEnv; + return (await executeMaestroJavaScript(script, values, 'evalScript', {})).outputEnv; } -export async function evaluateMaestroEvalScriptCondition( +export async function evaluateMaestroConditionScript( script: string, values: Readonly>, + platform: string | undefined, + field: string, ): Promise<{ value: unknown; outputEnv: Record }> { - return await executeMaestroJavaScript(script, values, 'repeat.while.true'); + return await executeMaestroJavaScript(script, values, field, { maestro: { platform } }); } async function executeMaestroJavaScript( script: string, values: Readonly>, - name: 'evalScript' | 'repeat.while.true', + name: string, + bindings: Record, ): Promise<{ value: unknown; outputEnv: Record }> { const output = seedMaestroOutput(values); const expression = unwrapMaestroEvalScriptExpression(script); // ponytail: function-scoped import keeps node:vm out of the maestro eager closure. const { default: vm } = await import('node:vm'); - const sandbox: Record = { ...values, output }; + const sandbox: Record = { ...values, ...bindings, output }; let value: unknown; try { value = vm.runInNewContext(expression, sandbox, { diff --git a/packages/maestro/src/internal/engine-expression.ts b/packages/maestro/src/internal/engine-expression.ts index c82964e246..8624db6af7 100644 --- a/packages/maestro/src/internal/engine-expression.ts +++ b/packages/maestro/src/internal/engine-expression.ts @@ -1,17 +1,19 @@ import { AppError } from '@agent-device/kernel/errors'; -import type { MaestroExecutionContext } from './engine-context.ts'; import type { MaestroPlatform } from './program-ir.ts'; -export function evaluateMaestroBooleanExpression( - value: string, - context: MaestroExecutionContext, +export function tryEvaluateMaestroBooleanExpression( + resolved: string, platform: MaestroPlatform | undefined, -): boolean { - const resolved = unwrapMaestroExpression(context.resolve(value)); - return new MaestroBooleanExpressionParser( - tokenizeMaestroBooleanExpression(resolved), - platform, - ).parse(); +): boolean | undefined { + try { + return new MaestroBooleanExpressionParser( + tokenizeMaestroBooleanExpression(unwrapMaestroExpression(resolved)), + platform, + ).parse(); + } catch (error) { + if (error instanceof AppError && error.code === 'INVALID_ARGS') return undefined; + throw error; + } } function unwrapMaestroExpression(value: string): string { diff --git a/packages/maestro/src/internal/engine-flow.ts b/packages/maestro/src/internal/engine-flow.ts index 49314e3ed7..6fa0fabedc 100644 --- a/packages/maestro/src/internal/engine-flow.ts +++ b/packages/maestro/src/internal/engine-flow.ts @@ -8,31 +8,18 @@ import { import type { MaestroCommand, MaestroProgram, - MaestroRepeatCondition, MaestroRunFlowCommand, MaestroRunFlowCondition, } from './program-ir.ts'; import type { MaestroExecutionContext } from './engine-context.ts'; -import { evaluateMaestroBooleanExpression } from './engine-expression.ts'; +import { tryEvaluateMaestroBooleanExpression } from './engine-expression.ts'; +import { isMaestroConditionTruthy, isMaestroScriptResultTruthy } from './engine-truthiness.ts'; import { type MaestroEngineOptions, type MaestroObservationCondition } from './engine-types.ts'; export function resolveCommand( command: T, context: MaestroExecutionContext, ): T { - if ('kind' in command && command.kind === 'repeat' && 'while' in command && command.while) { - const { while: conditionValue, ...unresolved } = command; - const condition = conditionValue as MaestroRepeatCondition; - const { true: truth, ...selectors } = condition; - return { - ...resolveValue(unresolved, context), - while: { - ...resolveValue(selectors, context), - ...(truth === undefined ? {} : { true: truth }), - }, - source: command.source, - } as T; - } return { ...resolveValue(command, context), source: command.source, @@ -153,24 +140,62 @@ export function registerIncludedProgramPaths( return includedPaths; } -export function staticConditionMatches( - condition: MaestroRunFlowCondition, +export function assertMaestroScriptsTrusted(options: MaestroEngineOptions, field: string): void { + if (options.trustedScripts !== false) return; + throw new AppError( + 'UNAUTHORIZED', + `Maestro ${field} is not permitted for flows received over the remote daemon surface: ` + + 'node:vm is not a security sandbox, so an untrusted expression can escape to the host.', + ); +} + +// Literals, maestro.platform comparisons, and ${VAR} lookups run without JavaScript, so remote flows keep them. +export async function conditionTruthMatches( + truth: boolean | string | undefined, + field: string, context: MaestroExecutionContext, options: MaestroEngineOptions, -): boolean { - if (condition.platform && condition.platform !== options.platform) return false; - if (condition.true === undefined) return true; - if (typeof condition.true === 'boolean') return condition.true; - return evaluateMaestroBooleanExpression(condition.true, context, options.platform); +): Promise { + if (truth === undefined) return true; + if (typeof truth === 'boolean') return truth; + const resolved = resolveWithoutScript(truth, context); + if (resolved !== undefined) { + const restricted = tryEvaluateMaestroBooleanExpression(resolved, options.platform); + if (restricted !== undefined) return restricted; + if (!resolved.includes('${')) return isMaestroConditionTruthy(resolved); + } + assertMaestroScriptsTrusted(options, field); + // ponytail: function-scoped import keeps engine-eval-script (and node:vm) out of the maestro eager closure. + const { evaluateMaestroConditionScript } = await import('./engine-eval-script.ts'); + const result = await evaluateMaestroConditionScript( + truth, + context.values, + options.platform, + field, + ); + context.replaceOutput(result.outputEnv); + return isMaestroScriptResultTruthy(result.value); +} + +function resolveWithoutScript(value: string, context: MaestroExecutionContext): string | undefined { + try { + return context.resolve(value); + } catch (error) { + if (error instanceof AppError && error.code === 'INVALID_ARGS') return undefined; + throw error; + } } export function observationConditions( condition: MaestroRunFlowCondition, + context: MaestroExecutionContext, ): MaestroObservationCondition[] { return [ - ...(condition.visible ? [{ kind: 'visible' as const, selector: condition.visible }] : []), + ...(condition.visible + ? [{ kind: 'visible' as const, selector: resolveValue(condition.visible, context) }] + : []), ...(condition.notVisible - ? [{ kind: 'notVisible' as const, selector: condition.notVisible }] + ? [{ kind: 'notVisible' as const, selector: resolveValue(condition.notVisible, context) }] : []), ]; } diff --git a/packages/maestro/src/internal/engine-types.ts b/packages/maestro/src/internal/engine-types.ts index 9233943ff6..0beebdcdd9 100644 --- a/packages/maestro/src/internal/engine-types.ts +++ b/packages/maestro/src/internal/engine-types.ts @@ -2,7 +2,6 @@ import type { MaestroCommand, MaestroPlatform, MaestroProgram, - MaestroRepeatCondition, MaestroSelector, MaestroSourceLocation, } from './program-ir.ts'; @@ -25,7 +24,7 @@ export type MaestroControlCommandDescriptor = readonly kind: 'repeat'; readonly source: MaestroSourceLocation; readonly times?: number | string; - readonly while?: MaestroRepeatCondition; + readonly while?: Extract['while']; } | { readonly kind: 'retry'; diff --git a/packages/maestro/src/internal/program-ir-flow-parser.ts b/packages/maestro/src/internal/program-ir-flow-parser.ts index 625473cfd2..5ab8e23ccc 100644 --- a/packages/maestro/src/internal/program-ir-flow-parser.ts +++ b/packages/maestro/src/internal/program-ir-flow-parser.ts @@ -3,7 +3,6 @@ import type { MaestroCommand, MaestroPlatform, MaestroRepeatCommand, - MaestroRepeatCondition, MaestroRetryCommand, MaestroRunFlowCommand, MaestroRunFlowCondition, @@ -100,7 +99,7 @@ export function parseMaestroRunFlowCommand( ? readScalarMap(entryValue(entries, 'env'), 'runFlow.env', context) : undefined; const when = hasEntry(entries, 'when') - ? parseMaestroRunFlowCondition(entryValue(entries, 'when'), context) + ? parseMaestroCondition(entryValue(entries, 'when'), 'runFlow.when', context) : undefined; const label = readMaestroCommandLabel(entries, 'runFlow', context); return stripUndefined({ @@ -123,7 +122,7 @@ export function parseMaestroRepeatCommand( const entries = readMapEntries(value, 'repeat', context); assertOnlyKeys(entries, 'repeat', ['times', 'commands', 'while'], context); const whileCondition = hasEntry(entries, 'while') - ? parseMaestroRepeatCondition(entryValue(entries, 'while'), context) + ? parseMaestroCondition(entryValue(entries, 'while'), 'repeat.while', context) : undefined; if (!hasEntry(entries, 'times') && !whileCondition) invalidAt('Maestro repeat requires times or while.', commandNode, context); @@ -163,33 +162,14 @@ export function parseMaestroRetryCommand( }); } -function parseMaestroRunFlowCondition( +function parseMaestroCondition( node: Node | null | undefined, + name: 'runFlow.when' | 'repeat.while', context: MaestroProgramParseContext, ): MaestroRunFlowCondition { - const entries = readMapEntries(node, 'runFlow.when', context); - assertOnlyKeys(entries, 'runFlow.when', ['platform', 'visible', 'notVisible', 'true'], context); - if (entries.length === 0) invalidAt('Maestro runFlow.when cannot be empty.', node, context); - - const platform = readOptionalEntry(entries, 'platform', (entry) => parsePlatform(entry, context)); - const visible = readOptionalEntry(entries, 'visible', (entry) => - parseMaestroSelector(entry, 'runFlow.when.visible', context), - ); - const notVisible = readOptionalEntry(entries, 'notVisible', (entry) => - parseMaestroSelector(entry, 'runFlow.when.notVisible', context), - ); - const truth = readOptionalEntry(entries, 'true', (entry) => readConditionTruth(entry, context)); - return stripUndefined({ platform, visible, notVisible, true: truth }); -} - -function parseMaestroRepeatCondition( - node: Node | null | undefined, - context: MaestroProgramParseContext, -): MaestroRepeatCondition { - const name = 'repeat.while'; const entries = readMapEntries(node, name, context); assertOnlyKeys(entries, name, ['platform', 'visible', 'notVisible', 'true'], context); - if (entries.length === 0) invalidAt('Maestro repeat.while cannot be empty.', node, context); + if (entries.length === 0) invalidAt(`Maestro ${name} cannot be empty.`, node, context); const platform = readOptionalEntry(entries, 'platform', (entry) => parsePlatform(entry, context, `${name}.platform`), @@ -209,7 +189,7 @@ function parseMaestroRepeatCondition( function readConditionTruth( node: Node | null | undefined, context: MaestroProgramParseContext, - name = 'runFlow.when.true', + name: string, ): boolean | string { const value = readScalarValue(node, name, context); if (typeof value === 'boolean' || typeof value === 'string') return value; @@ -219,7 +199,7 @@ function readConditionTruth( function parsePlatform( node: Node | null | undefined, context: MaestroProgramParseContext, - name = 'runFlow.when.platform', + name: string, ): MaestroPlatform { const value = readRequiredString(node, name, context).toLowerCase(); if (value === 'android' || value === 'ios' || value === 'web') return value; diff --git a/packages/maestro/src/internal/program-ir.ts b/packages/maestro/src/internal/program-ir.ts index 900fd0b88e..783008e47a 100644 --- a/packages/maestro/src/internal/program-ir.ts +++ b/packages/maestro/src/internal/program-ir.ts @@ -242,11 +242,6 @@ export type MaestroRunFlowCondition = { true?: boolean | string; }; -export type MaestroRepeatCondition = Pick< - MaestroRunFlowCondition, - 'platform' | 'visible' | 'notVisible' | 'true' ->; - export type MaestroRunFlowCommand = { kind: 'runFlow'; source: MaestroSourceLocation; @@ -260,7 +255,7 @@ export type MaestroRepeatCommand = { kind: 'repeat'; source: MaestroSourceLocation; times?: number | string; - while?: MaestroRepeatCondition; + while?: MaestroRunFlowCondition; commands: MaestroCommand[]; }; diff --git a/packages/maestro/src/internal/replay-plan-step-execution.ts b/packages/maestro/src/internal/replay-plan-step-execution.ts index 0b2852ff6a..c43bb2adc6 100644 --- a/packages/maestro/src/internal/replay-plan-step-execution.ts +++ b/packages/maestro/src/internal/replay-plan-step-execution.ts @@ -1,21 +1,22 @@ import { AppError } from '@agent-device/kernel/errors'; import { stripUndefined } from './shared.ts'; import { isMaestroTestFailure, maestroTestFailure } from './compatibility-errors.ts'; -import { isMaestroConditionTruthy, isMaestroScriptResultTruthy } from './engine-truthiness.ts'; +import { isMaestroConditionTruthy } from './engine-truthiness.ts'; import { MAESTRO_COMPATIBILITY_PRESETS, type MaestroCompatibilityTimingPolicy, } from './compatibility-policy.ts'; import type { MaestroExecutionContext } from './engine-context.ts'; import { + assertMaestroScriptsTrusted, checkpointMaestroCancellation, + conditionTruthMatches, observationConditions, readIterationCount, resolveCommand, resolveNumeric, - staticConditionMatches, } from './engine-flow.ts'; -import type { MaestroRepeatCondition, MaestroRunFlowCondition } from './program-ir.ts'; +import type { MaestroRunFlowCondition } from './program-ir.ts'; import type { MaestroEngineOptions, MaestroEngineEvent, @@ -113,13 +114,7 @@ async function executeEvalScript( ): Promise { // ponytail: function-scoped import keeps engine-eval-script (and node:vm) out of the maestro eager closure. const { evaluateMaestroEvalScript } = await import('./engine-eval-script.ts'); - if (state.options.trustedScripts === false) { - throw new AppError( - 'UNAUTHORIZED', - 'Maestro evalScript is not permitted for flows received over the remote daemon surface: ' + - 'node:vm is not a security sandbox, so an untrusted expression can escape to the host.', - ); - } + assertMaestroScriptsTrusted(state.options, 'evalScript'); const outputEnv = await evaluateMaestroEvalScript(command.script, state.context.values); state.context.replaceOutput(outputEnv); state.executed += 1; @@ -197,10 +192,10 @@ async function executeOpaqueStep( step: MaestroReplayPlanOpaqueStep, state: MaestroReplayPlanExecutionState, ): Promise { - const command = resolveCommand(step.command, state.context); + const command = step.command; switch (command.kind) { case 'runFlow': - if (command.when && !(await flowConditionMatches(command.when, state))) { + if (command.when && !(await conditionMatches(command.when, 'runFlow.when', state))) { state.skipped += 1; return; } @@ -235,7 +230,7 @@ async function executeNestedSteps( async function executeRepeat( steps: readonly MaestroReplayPlanStep[], times: number | string | undefined, - condition: MaestroRepeatCondition | undefined, + condition: MaestroRunFlowCondition | undefined, state: MaestroReplayPlanExecutionState, ): Promise { const maxIterations = @@ -243,8 +238,10 @@ async function executeRepeat( ? Number.POSITIVE_INFINITY : readIterationCount(times, 0, state.context, 'repeat.times'); for (let iteration = 0; iteration < maxIterations; iteration += 1) { + // A synchronous body (e.g. only evalScript) would otherwise starve cancellation timers. + await new Promise((resolve) => setImmediate(resolve)); checkpointMaestroCancellation(state.options.signal); - if (condition && !(await repeatConditionMatches(condition, state))) return; + if (condition && !(await conditionMatches(condition, 'repeat.while', state))) return; await executeNestedSteps(steps, state); } } @@ -270,12 +267,18 @@ async function executeRetry( throw new AppError('COMMAND_FAILED', 'Maestro retry commands failed.'); } -async function flowConditionMatches( +async function conditionMatches( condition: MaestroRunFlowCondition, + field: 'runFlow.when' | 'repeat.while', state: MaestroReplayPlanExecutionState, ): Promise { - if (!staticConditionMatches(condition, state.context, state.options)) return false; - for (const observation of observationConditions(condition)) { + if (condition.platform && condition.platform !== state.options.platform) return false; + if ( + !(await conditionTruthMatches(condition.true, `${field}.true`, state.context, state.options)) + ) { + return false; + } + for (const observation of observationConditions(condition, state.context)) { checkpointMaestroCancellation(state.options.signal); if (!(await observe(observation, state.timing.runFlowConditionTimeoutMs, state)).matched) { return false; @@ -284,33 +287,6 @@ async function flowConditionMatches( return true; } -async function repeatConditionMatches( - condition: MaestroRepeatCondition, - state: MaestroReplayPlanExecutionState, -): Promise { - const { true: script, ...rest } = condition; - if (typeof script !== 'string') return await flowConditionMatches(condition, state); - if (!(await scriptConditionMatches(script, state))) return false; - return await flowConditionMatches(rest, state); -} - -async function scriptConditionMatches( - script: string, - state: MaestroReplayPlanExecutionState, -): Promise { - if (state.options.trustedScripts === false) { - throw new AppError( - 'UNAUTHORIZED', - 'Maestro repeat.while.true is not permitted for flows received over the remote daemon surface: ' + - 'node:vm is not a security sandbox, so an untrusted expression can escape to the host.', - ); - } - const { evaluateMaestroEvalScriptCondition } = await import('./engine-eval-script.ts'); - const result = await evaluateMaestroEvalScriptCondition(script, state.context.values); - state.context.replaceOutput(result.outputEnv); - return isMaestroScriptResultTruthy(result.value); -} - async function requireObservation( condition: MaestroObservationCondition, timeoutMs: number, diff --git a/packages/maestro/src/internal/replay-plan-steps.ts b/packages/maestro/src/internal/replay-plan-steps.ts index 759c52191e..7207b8acb1 100644 --- a/packages/maestro/src/internal/replay-plan-steps.ts +++ b/packages/maestro/src/internal/replay-plan-steps.ts @@ -7,7 +7,7 @@ import { registerIncludedProgramPaths, sourcePathKey, } from './engine-flow.ts'; -import { evaluateMaestroBooleanExpression } from './engine-expression.ts'; +import { tryEvaluateMaestroBooleanExpression } from './engine-expression.ts'; import type { MaestroRuntimeCommand } from './engine-types.ts'; import type { MaestroCommand, MaestroProgram, MaestroRunFlowCommand } from './program-ir.ts'; import type { @@ -214,9 +214,9 @@ function staticBooleanConditionDecision( if (typeof condition !== 'string') return undefined; const resolved = state.context.resolveDeferred(condition); if (hasUnresolvedVariable(resolved)) return 'opaque'; - return evaluateMaestroBooleanExpression(condition, state.context, state.options.platform) - ? undefined - : 'omit'; + const decision = tryEvaluateMaestroBooleanExpression(resolved, state.options.platform); + if (decision === undefined) return 'opaque'; + return decision ? undefined : 'omit'; } function resolveRunFlowInclude( diff --git a/packages/maestro/src/internal/support-matrix.ts b/packages/maestro/src/internal/support-matrix.ts index 8a95e10f85..df98349fad 100644 --- a/packages/maestro/src/internal/support-matrix.ts +++ b/packages/maestro/src/internal/support-matrix.ts @@ -1,5 +1,5 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ - 'Flows: launchApp (with clearState, permissions, and Apple-only launch arguments; permissions apply after state clearing but before launch, and a launchApp without permissions touches nothing — there is no silent all: allow default); setPermissions (mid-flow permission grants, denials, and resets; all resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specific entries overriding after it); runFlow file/inline with platform, visibility, and limited boolean conditions; onFlowStart/onFlowComplete; repeat.times/while with platform, visibility, or JavaScript conditions and retry.', + 'Flows: launchApp (with clearState, permissions, and Apple-only launch arguments; permissions apply after state clearing but before launch, and a launchApp without permissions touches nothing — there is no silent all: allow default); setPermissions (mid-flow permission grants, denials, and resets; all resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specific entries overriding after it); runFlow file/inline; runFlow.when and repeat.while conditions (platform, visible, notVisible, and true, all re-evaluated before every repeat iteration); onFlowStart/onFlowComplete; repeat with times, while, or both; and retry.', 'Interactions: tapOn, doubleTapOn, longPressOn, inputText on the focused element, eraseText, openLink, hideKeyboard, basic pressKey, and back; selector targets poll until available and support recursive index, childOf, above, below, leftOf, rightOf, containsChild, containsDescendants, points, and optional; outer command labels are metadata, not target selectors.', 'Assertions and navigation: assertVisible, assertNotVisible, assertTrue (literal values and ${VAR} lookups only; "", "false", "0", "null", and "undefined" are falsy, everything else is truthy), extendedWaitUntil, scroll, scrollUntilVisible, absolute/percentage/target swipe, takeScreenshot, waitForAnimationToEnd, clearState, and stopApp.', 'Scripts: ordered runScript file/env scripts with http.post, json, and output variables; evalScript inline expressions run flow-scoped JavaScript and write output.* leaves for later steps.', @@ -8,10 +8,10 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ export const MAESTRO_COMPAT_LIMITATIONS = [ 'Permissions: every entry is one settings permission call, applied in order with all first; the step stops at the first entry the selected platform refuses, earlier entries stay applied, and the error names what landed. Android’s only allow level is while-in-use, so location: inuse and location: never mean allow and deny there, while location: always and photos: limited are Apple-only and fail. On iOS, which service a runtime changes is simctl privacy’s own verdict: current runtimes refuse a targeted notifications change and leave notifications untouched under all.', 'Runtime: iOS and Android only; launchApp.clearState and standalone clearState support Android and iOS simulators, launch arguments are Apple-only, and other standalone device utility/state commands are unsupported.', - 'Expressions: evalScript and repeat.while.true are evaluated as JavaScript (flow env and prior output leaves are string-typed); with those exceptions, fields stay literal or ${VAR} lookup-only — assertTrue supports literals and bare lookups, and other expression-shaped payloads fail loud.', + 'Expressions: evalScript and condition true: fields (runFlow.when and repeat.while share one evaluator) are evaluated as JavaScript (flow env and prior output leaves are string-typed); a true: field that is a boolean, a maestro.platform comparison, or plain literal text after ${VAR} lookups is decided without JavaScript, with the assertTrue falsy table for literal text. Other fields stay literal or ${VAR} lookup-only — assertTrue supports literals and bare lookups, and other expression-shaped payloads fail loud.', 'Environment: flow env is the default, AD_VAR_* overrides it, and CLI -e KEY=VALUE wins over both.', 'Failure diagnostics: resolved targets and runFlow paths are rendered, while inputText payloads remain hidden; do not place secrets in diagnostic identifiers.', - 'Trust: runScript, evalScript, and repeat.while.true execute flow scripts in-process via node:vm, which is not a security sandbox; runScript may make http.post network requests and its output keys cannot contain a dot. evalScript and repeat.while.true expressions are refused outright for a flow accepted over the daemon’s remote HTTP surface, since that context can escape to the host.', + 'Trust: runScript, evalScript, and JavaScript condition true: fields execute flow scripts in-process via node:vm, which is not a security sandbox; runScript may make http.post network requests and its output keys cannot contain a dot. evalScript and true: fields that need JavaScript are refused outright for a flow accepted over the daemon’s remote HTTP surface, since that context can escape to the host.', 'Errors and tracking: unsupported commands and fields fail with source context when available; open a focused issue only when implementation work is planned.', ] as const; diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 9b2db69752..bd9a22272c 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -83,7 +83,7 @@ agent-device test ./maestro-flows --maestro --platform android --artifacts-dir . Supported subset: -- Flows: `launchApp` (with `clearState`, `permissions`, and Apple-only launch arguments; `permissions` apply after state clearing but before launch, and a `launchApp` without `permissions` touches nothing — there is no silent `all: allow` default); `setPermissions` (mid-flow permission grants, denials, and resets; `all` resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specific entries overriding after it); `runFlow` file/inline with platform, visibility, and limited boolean conditions; `onFlowStart`/`onFlowComplete`; `repeat.times`/`while` with platform, visibility, or JavaScript conditions and retry. +- Flows: `launchApp` (with `clearState`, `permissions`, and Apple-only launch arguments; `permissions` apply after state clearing but before launch, and a `launchApp` without `permissions` touches nothing — there is no silent `all: allow` default); `setPermissions` (mid-flow permission grants, denials, and resets; `all` resolves in the backend — one simctl call on iOS, the declared permissions on Android — with specific entries overriding after it); `runFlow` file/inline; `runFlow.when` and `repeat.while` conditions (`platform`, `visible`, `notVisible`, and `true`, all re-evaluated before every `repeat` iteration); `onFlowStart`/`onFlowComplete`; `repeat` with `times`, `while`, or both; and retry. - Interactions: `tapOn`, `doubleTapOn`, `longPressOn`, `inputText` on the focused element, `eraseText`, `openLink`, `hideKeyboard`, basic `pressKey`, and `back`; selector targets poll until available and support recursive `index`, `childOf`, `above`, `below`, `leftOf`, `rightOf`, `containsChild`, `containsDescendants`, points, and `optional`; outer command labels are metadata, not target selectors. - Assertions and navigation: `assertVisible`, `assertNotVisible`, `assertTrue` (literal values and `${VAR}` lookups only; `""`, `"false"`, `"0"`, `"null"`, and `"undefined"` are falsy, everything else is truthy), `extendedWaitUntil`, `scroll`, `scrollUntilVisible`, absolute/percentage/target `swipe`, `takeScreenshot`, `waitForAnimationToEnd`, `clearState`, and `stopApp`. - Scripts: ordered `runScript` file/env scripts with `http.post`, `json`, and `output` variables; `evalScript` inline expressions run flow-scoped JavaScript and write `output.*` leaves for later steps. @@ -92,10 +92,10 @@ Boundaries: - Permissions: every entry is one `settings permission` call, applied in order with `all` first; the step stops at the first entry the selected platform refuses, earlier entries stay applied, and the error names what landed. Android’s only allow level is while-in-use, so `location: inuse` and `location: never` mean `allow` and `deny` there, while `location: always` and `photos: limited` are Apple-only and fail. On iOS, which service a runtime changes is `simctl privacy`’s own verdict: current runtimes refuse a targeted `notifications` change and leave notifications untouched under `all`. - Runtime: iOS and Android only; `launchApp.clearState` and standalone `clearState` support Android and iOS simulators, launch arguments are Apple-only, and other standalone device utility/state commands are unsupported. -- Expressions: `evalScript` and `repeat.while.true` are evaluated as JavaScript (flow `env` and prior `output` leaves are string-typed); with those exceptions, fields stay literal or `${VAR}` lookup-only — `assertTrue` supports literals and bare lookups, and other expression-shaped payloads fail loud. +- Expressions: `evalScript` and condition `true:` fields (`runFlow.when` and `repeat.while` share one evaluator) are evaluated as JavaScript (flow `env` and prior `output` leaves are string-typed); a `true:` field that is a boolean, a `maestro.platform` comparison, or plain literal text after `${VAR}` lookups is decided without JavaScript, with the `assertTrue` falsy table for literal text. Other fields stay literal or `${VAR}` lookup-only — `assertTrue` supports literals and bare lookups, and other expression-shaped payloads fail loud. - Environment: flow `env` is the default, `AD_VAR_*` overrides it, and CLI `-e KEY=VALUE` wins over both. - Failure diagnostics: resolved targets and `runFlow` paths are rendered, while `inputText` payloads remain hidden; do not place secrets in diagnostic identifiers. -- Trust: `runScript`, `evalScript`, and `repeat.while.true` execute flow scripts in-process via `node:vm`, which is not a security sandbox; `runScript` may make `http.post` network requests and its output keys cannot contain a dot. `evalScript` and `repeat.while.true` expressions are refused outright for a flow accepted over the daemon’s remote HTTP surface, since that context can escape to the host. +- Trust: `runScript`, `evalScript`, and JavaScript condition `true:` fields execute flow scripts in-process via `node:vm`, which is not a security sandbox; `runScript` may make `http.post` network requests and its output keys cannot contain a dot. `evalScript` and `true:` fields that need JavaScript are refused outright for a flow accepted over the daemon’s remote HTTP surface, since that context can escape to the host. - Errors and tracking: unsupported commands and fields fail with source context when available; open a focused issue only when implementation work is planned. - Session takeover: `--keep-session` is a native `.ad` replay option and is rejected for Maestro YAML. From ecdc3986a571f80c9768940b5165113eb8988a78 Mon Sep 17 00:00:00 2001 From: Umair Ahmed Date: Mon, 5 Oct 2026 09:51:14 +0100 Subject: [PATCH 6/6] test(maestro): move repeat and condition execution cases to a sibling module Keeps engine.test.ts under the 1,000-line test file limit; makePort moves to the shared runtime-port fixtures. --- .../src/internal/__tests__/engine.test.ts | 220 +----------------- .../replay-plan-step-execution.test.ts | 205 ++++++++++++++++ .../__tests__/runtime-port-fixtures.ts | 20 ++ 3 files changed, 226 insertions(+), 219 deletions(-) create mode 100644 packages/maestro/src/internal/__tests__/replay-plan-step-execution.test.ts diff --git a/packages/maestro/src/internal/__tests__/engine.test.ts b/packages/maestro/src/internal/__tests__/engine.test.ts index 4a6c10100b..813666e6a3 100644 --- a/packages/maestro/src/internal/__tests__/engine.test.ts +++ b/packages/maestro/src/internal/__tests__/engine.test.ts @@ -1,15 +1,13 @@ import { describe, expect, test, vi } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; import { maestroTestFailure } from '../compatibility-errors.ts'; -import { MAESTRO_COMPATIBILITY_PRESETS } from '../compatibility-policy.ts'; import { parseMaestroProgram } from '../program-ir-parser.ts'; import type { MaestroObservation, MaestroRuntimePort, MaestroRuntimeRequest, - MaestroRuntimeResult, } from '../engine-types.ts'; -import { executeMaestroProgram } from './runtime-port-fixtures.ts'; +import { executeMaestroProgram, makePort } from './runtime-port-fixtures.ts'; describe('executeMaestroProgram', () => { test('preserves authored percentage swipe intent without observing', async () => { @@ -680,204 +678,6 @@ describe('executeMaestroProgram', () => { ).toHaveLength(3); }); - test('repeat while evaluates JavaScript expressions and retains output updates', async () => { - const texts: string[] = []; - const port = makePort({ - execute: vi.fn(async (request) => { - if (request.command.kind === 'inputText') texts.push(request.command.text); - request.invalidateObservation(); - return {}; - }), - }); - const program = parseMaestroProgram( - [ - '---', - '- evalScript: ${output.counter = 0}', - '- repeat:', - ' while:', - ' true: "${output.counter < 3}"', - ' commands:', - ' - inputText: loop', - ' - evalScript: ${output.counter++}', - ].join('\n'), - ); - - await executeMaestroProgram(program, port); - - expect(texts).toEqual(['loop', 'loop', 'loop']); - }); - - test('repeat while observes selectors once per iteration and stops when false', async () => { - let checks = 0; - const observe = vi.fn(async ({ generation }: Parameters[0]) => ({ - generation, - matched: ++checks < 3, - })); - const port = makePort({ observe }); - const program = parseMaestroProgram( - [ - '---', - '- repeat:', - ' times: 10', - ' while:', - ' platform: Android', - ' notVisible: ValueX', - ' commands:', - ' - inputText: loop', - ].join('\n'), - ); - - await executeMaestroProgram(program, port, { platform: 'android' }); - - expect(port.execute).toHaveBeenCalledTimes(2); - expect(observe).toHaveBeenCalledTimes(3); - expect(observe).toHaveBeenCalledWith( - expect.objectContaining({ - condition: { kind: 'notVisible', selector: { text: 'ValueX' } }, - timeoutMs: MAESTRO_COMPATIBILITY_PRESETS.command.optionalTargetLookupTimeoutMs, - }), - ); - }); - - test('repeat while stops at the times limit even while its condition remains true', async () => { - const port = makePort(); - const program = parseMaestroProgram( - [ - '---', - '- repeat:', - ' times: 2', - ' while:', - ' true: true', - ' commands:', - ' - inputText: loop', - ].join('\n'), - ); - - await executeMaestroProgram(program, port); - - expect(port.execute).toHaveBeenCalledTimes(2); - }); - - test('repeat while expressions are refused for remote untrusted flows', async () => { - const program = parseMaestroProgram( - [ - '---', - '- repeat:', - ' while:', - ' true: "${output.counter < 3}"', - ' commands:', - ' - inputText: loop', - ].join('\n'), - ); - - await expect( - executeMaestroProgram(program, makePort(), { trustedScripts: false }), - ).rejects.toMatchObject({ code: 'UNAUTHORIZED' }); - }); - - test('repeat while true binds maestro.platform in JavaScript expressions', async () => { - const port = makePort(); - const program = parseMaestroProgram( - [ - '---', - '- evalScript: ${output.n = 0}', - '- repeat:', - ' while:', - ` true: "\${maestro.platform === 'ios' && output.n < 2}"`, - ' commands:', - ' - inputText: loop', - ' - evalScript: ${output.n++}', - ].join('\n'), - ); - - await executeMaestroProgram(program, port, { platform: 'ios' }); - - expect(port.execute).toHaveBeenCalledTimes(2); - }); - - test('repeat while true accepts plain literals and platform comparisons without scripts', async () => { - const port = makePort(); - const program = parseMaestroProgram( - [ - '---', - '- repeat:', - ' times: 2', - ' while:', - ' true: yes', - ' commands:', - ' - inputText: literal', - '- repeat:', - ' times: 2', - ' while:', - ` true: "\${maestro.platform == 'android'}"`, - ' commands:', - ' - inputText: platform', - ].join('\n'), - ); - - await executeMaestroProgram(program, port, { platform: 'ios', trustedScripts: false }); - - expect(port.execute).toHaveBeenCalledTimes(2); - }); - - test('runFlow when true shares the repeat while JavaScript evaluator', async () => { - const port = makePort(); - const program = parseMaestroProgram( - [ - '---', - '- evalScript: ${output.n = 2}', - '- runFlow:', - ' when:', - ' true: "${output.n > 1}"', - ' commands:', - ' - inputText: ran', - ].join('\n'), - ); - - await executeMaestroProgram(program, port); - - expect(port.execute).toHaveBeenCalledTimes(1); - }); - - test('repeat while re-resolves condition selectors before every check', async () => { - const observe = vi.fn(async ({ generation }: Parameters[0]) => ({ - generation, - matched: true, - })); - const port = makePort({ observe }); - const program = parseMaestroProgram( - [ - '---', - '- evalScript: ${output.target = "first"}', - '- repeat:', - ' times: 2', - ' while:', - ' visible: ${output.target}', - ' commands:', - ' - evalScript: ${output.target = "second"}', - ].join('\n'), - ); - - await executeMaestroProgram(program, port); - - expect(observe.mock.calls.map(([request]) => request.condition)).toEqual([ - { kind: 'visible', selector: { text: 'first' } }, - { kind: 'visible', selector: { text: 'second' } }, - ]); - }); - - test('an unbounded synchronous repeat still yields to cancellation timers', async () => { - const controller = new AbortController(); - const program = parseMaestroProgram( - ['---', '- repeat:', ' while:', ' true: true', ' commands: []'].join('\n'), - ); - setTimeout(() => controller.abort(), 10); - - await expect( - executeMaestroProgram(program, makePort(), { signal: controller.signal }), - ).rejects.toMatchObject({ details: { reason: 'request_canceled' } }); - }); - test('evalScript replaces output namespace so shrunken arrays drop stale leaves', async () => { const texts: string[] = []; const port = makePort({ @@ -1056,21 +856,3 @@ describe('executeMaestroProgram', () => { expect(execute.mock.calls[0]?.[0].signal).toBe(controller.signal); }); }); - -function makePort(overrides: Partial = {}): MaestroRuntimePort { - return { - execute: vi.fn(async (request): Promise => { - const { command } = request; - if ( - command.kind !== 'takeScreenshot' && - command.kind !== 'runScript' && - command.kind !== 'waitForAnimationToEnd' - ) { - request.invalidateObservation(); - } - return command.kind === 'takeScreenshot' ? { artifactPaths: [command.path] } : {}; - }), - observe: vi.fn(async ({ generation }) => ({ generation, matched: true })), - ...overrides, - }; -} diff --git a/packages/maestro/src/internal/__tests__/replay-plan-step-execution.test.ts b/packages/maestro/src/internal/__tests__/replay-plan-step-execution.test.ts new file mode 100644 index 0000000000..4b1648a179 --- /dev/null +++ b/packages/maestro/src/internal/__tests__/replay-plan-step-execution.test.ts @@ -0,0 +1,205 @@ +import { describe, expect, test, vi } from 'vitest'; +import { MAESTRO_COMPATIBILITY_PRESETS } from '../compatibility-policy.ts'; +import { parseMaestroProgram } from '../program-ir-parser.ts'; +import type { MaestroRuntimePort } from '../engine-types.ts'; +import { executeMaestroProgram, makePort } from './runtime-port-fixtures.ts'; + +describe('repeat and condition execution', () => { + test('repeat while evaluates JavaScript expressions and retains output updates', async () => { + const texts: string[] = []; + const port = makePort({ + execute: vi.fn(async (request) => { + if (request.command.kind === 'inputText') texts.push(request.command.text); + request.invalidateObservation(); + return {}; + }), + }); + const program = parseMaestroProgram( + [ + '---', + '- evalScript: ${output.counter = 0}', + '- repeat:', + ' while:', + ' true: "${output.counter < 3}"', + ' commands:', + ' - inputText: loop', + ' - evalScript: ${output.counter++}', + ].join('\n'), + ); + + await executeMaestroProgram(program, port); + + expect(texts).toEqual(['loop', 'loop', 'loop']); + }); + + test('repeat while observes selectors once per iteration and stops when false', async () => { + let checks = 0; + const observe = vi.fn(async ({ generation }: Parameters[0]) => ({ + generation, + matched: ++checks < 3, + })); + const port = makePort({ observe }); + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' times: 10', + ' while:', + ' platform: Android', + ' notVisible: ValueX', + ' commands:', + ' - inputText: loop', + ].join('\n'), + ); + + await executeMaestroProgram(program, port, { platform: 'android' }); + + expect(port.execute).toHaveBeenCalledTimes(2); + expect(observe).toHaveBeenCalledTimes(3); + expect(observe).toHaveBeenCalledWith( + expect.objectContaining({ + condition: { kind: 'notVisible', selector: { text: 'ValueX' } }, + timeoutMs: MAESTRO_COMPATIBILITY_PRESETS.command.optionalTargetLookupTimeoutMs, + }), + ); + }); + + test('repeat while stops at the times limit even while its condition remains true', async () => { + const port = makePort(); + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' times: 2', + ' while:', + ' true: true', + ' commands:', + ' - inputText: loop', + ].join('\n'), + ); + + await executeMaestroProgram(program, port); + + expect(port.execute).toHaveBeenCalledTimes(2); + }); + + test('repeat while expressions are refused for remote untrusted flows', async () => { + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' while:', + ' true: "${output.counter < 3}"', + ' commands:', + ' - inputText: loop', + ].join('\n'), + ); + + await expect( + executeMaestroProgram(program, makePort(), { trustedScripts: false }), + ).rejects.toMatchObject({ code: 'UNAUTHORIZED' }); + }); + + test('repeat while true binds maestro.platform in JavaScript expressions', async () => { + const port = makePort(); + const program = parseMaestroProgram( + [ + '---', + '- evalScript: ${output.n = 0}', + '- repeat:', + ' while:', + ` true: "\${maestro.platform === 'ios' && output.n < 2}"`, + ' commands:', + ' - inputText: loop', + ' - evalScript: ${output.n++}', + ].join('\n'), + ); + + await executeMaestroProgram(program, port, { platform: 'ios' }); + + expect(port.execute).toHaveBeenCalledTimes(2); + }); + + test('repeat while true accepts plain literals and platform comparisons without scripts', async () => { + const port = makePort(); + const program = parseMaestroProgram( + [ + '---', + '- repeat:', + ' times: 2', + ' while:', + ' true: yes', + ' commands:', + ' - inputText: literal', + '- repeat:', + ' times: 2', + ' while:', + ` true: "\${maestro.platform == 'android'}"`, + ' commands:', + ' - inputText: platform', + ].join('\n'), + ); + + await executeMaestroProgram(program, port, { platform: 'ios', trustedScripts: false }); + + expect(port.execute).toHaveBeenCalledTimes(2); + }); + + test('runFlow when true shares the repeat while JavaScript evaluator', async () => { + const port = makePort(); + const program = parseMaestroProgram( + [ + '---', + '- evalScript: ${output.n = 2}', + '- runFlow:', + ' when:', + ' true: "${output.n > 1}"', + ' commands:', + ' - inputText: ran', + ].join('\n'), + ); + + await executeMaestroProgram(program, port); + + expect(port.execute).toHaveBeenCalledTimes(1); + }); + + test('repeat while re-resolves condition selectors before every check', async () => { + const observe = vi.fn(async ({ generation }: Parameters[0]) => ({ + generation, + matched: true, + })); + const port = makePort({ observe }); + const program = parseMaestroProgram( + [ + '---', + '- evalScript: ${output.target = "first"}', + '- repeat:', + ' times: 2', + ' while:', + ' visible: ${output.target}', + ' commands:', + ' - evalScript: ${output.target = "second"}', + ].join('\n'), + ); + + await executeMaestroProgram(program, port); + + expect(observe.mock.calls.map(([request]) => request.condition)).toEqual([ + { kind: 'visible', selector: { text: 'first' } }, + { kind: 'visible', selector: { text: 'second' } }, + ]); + }); + + test('an unbounded synchronous repeat still yields to cancellation timers', async () => { + const controller = new AbortController(); + const program = parseMaestroProgram( + ['---', '- repeat:', ' while:', ' true: true', ' commands: []'].join('\n'), + ); + setTimeout(() => controller.abort(), 10); + + await expect( + executeMaestroProgram(program, makePort(), { signal: controller.signal }), + ).rejects.toMatchObject({ details: { reason: 'request_canceled' } }); + }); +}); diff --git a/packages/maestro/src/internal/__tests__/runtime-port-fixtures.ts b/packages/maestro/src/internal/__tests__/runtime-port-fixtures.ts index 765892c9b2..8bc902a47a 100644 --- a/packages/maestro/src/internal/__tests__/runtime-port-fixtures.ts +++ b/packages/maestro/src/internal/__tests__/runtime-port-fixtures.ts @@ -1,3 +1,4 @@ +import { vi } from 'vitest'; import type { MaestroRuntimeOperationContext, MaestroRuntimeOperations, @@ -7,6 +8,7 @@ import type { MaestroEngineOptions, MaestroEngineResult, MaestroRuntimePort, + MaestroRuntimeResult, } from '../engine-types.ts'; import type { MaestroProgram } from '../program-ir.ts'; import { compileMaestroReplayPlan } from '../replay-plan.ts'; @@ -35,6 +37,24 @@ export function createMaestroRuntimePort(operations: MaestroRuntimeOperations): }; } +export function makePort(overrides: Partial = {}): MaestroRuntimePort { + return { + execute: vi.fn(async (request): Promise => { + const { command } = request; + if ( + command.kind !== 'takeScreenshot' && + command.kind !== 'runScript' && + command.kind !== 'waitForAnimationToEnd' + ) { + request.invalidateObservation(); + } + return command.kind === 'takeScreenshot' ? { artifactPaths: [command.path] } : {}; + }), + observe: vi.fn(async ({ generation }) => ({ generation, matched: true })), + ...overrides, + }; +} + export function makeOperations( overrides: Partial = {}, ): MaestroRuntimeOperations {