diff --git a/docs/maestro-compat-debt-map.md b/docs/maestro-compat-debt-map.md index 097bf142e..ea5fd6a09 100644 --- a/docs/maestro-compat-debt-map.md +++ b/docs/maestro-compat-debt-map.md @@ -1,15 +1,15 @@ # Maestro Compatibility Debt Map -This map summarizes the Maestro compatibility surface after the lane 6 audit. LOC values are approximate and come from `wc -l` on the owning files; rows that share a file use an estimated slice. The current compat implementation is about 4.2k LOC under `src/compat/maestro`, with about 1.8k LOC of focused compat unit tests plus daemon/provider integration guards. +This map summarizes the Maestro compatibility surface after the lane 6 audit. LOC values are approximate and come from `wc -l` on the owning files; rows that share a file use an estimated slice. The current compat implementation is about 4.1k LOC under `src/compat/maestro`, with about 2.1k LOC of focused compat unit tests plus daemon/provider integration guards. | Area | Owning files | Approx LOC/code size | Custom handling summary | Native overlap and dependencies | Reliability or faster convergence opportunity | Risk | Test guard status | Recommendation | Suggested PR lane | | --- | --- | ---: | --- | --- | --- | --- | --- | --- | --- | -| Parser and command mapping | `src/compat/maestro/replay-flow.ts`, `command-mapper.ts`, `support.ts`, `types.ts`, plus parse-side slices of `interactions.ts`, `device-actions.ts`, `flow-control.ts`, `run-script.ts` | ~1.5k LOC | Parses multi-document YAML, splits config/commands, resolves env, tracks rough top-level line numbers, flattens hooks, maps commands to `SessionAction`, rejects unsupported commands/fields loudly, and emits `__maestro*` runtime commands for semantics that native replay does not model. | Depends on `yaml`, replay `SessionAction`, `AppError`, and native commands such as `open`, `click`, `fill`, `type`, `wait`, `is`, `scroll`, `swipe`, `keyboard`, and `screenshot`. | Common replay parsing helpers could own env precedence, source-path handling, and line diagnostics. Native command builders could reduce stringly `SessionAction` construction for commands that already match `.ad`. | Line mapping is intentionally approximate for nested lists; parse-time expansion can hide runtime structure; unsupported syntax must keep failing loudly. | Strong: `replay-flow.test.ts` covers supported subset, env, hooks, runFlow, repeat, retry, launch reset, unsupported fields, fixture parsing; `replay-input.test.ts` covers backend routing/env precedence. | Keep Maestro YAML grammar local. Converge shared replay env/source/line utilities and typed action construction where native commands already exist. | Lane 7: parser contract hardening | +| Parser and command mapping | `src/compat/maestro/replay-flow.ts`, `command-mapper.ts`, `support.ts`, `types.ts`, plus parse-side slices of `interactions.ts`, `device-actions.ts`, `flow-control.ts`, `run-script.ts` | ~1.5k LOC | Parses multi-document YAML, splits config/commands, resolves env, tracks rough top-level line numbers, flattens hooks, maps commands to `SessionAction`, rejects unsupported commands/fields loudly, emits `replayControl` for flow blocks/retry, and emits `__maestro*` runtime commands for remaining semantics that native replay does not model. | Depends on `yaml`, replay `SessionAction`, `AppError`, and native commands such as `open`, `click`, `fill`, `type`, `wait`, `is`, `scroll`, `swipe`, `keyboard`, and `screenshot`. | Common replay parsing helpers could own env precedence, source-path handling, and line diagnostics. Native command builders could reduce stringly `SessionAction` construction for commands that already match `.ad`. | Line mapping is intentionally approximate for nested lists; parse-time expansion can hide runtime structure; unsupported syntax must keep failing loudly. | Strong: `replay-flow.test.ts` covers supported subset, env, hooks, runFlow, repeat, retry, launch reset, unsupported fields, fixture parsing; `replay-input.test.ts` covers backend routing/env precedence. | Keep Maestro YAML grammar local. Converge shared replay env/source/line utilities and typed action construction where native commands already exist. | Lane 7: parser contract hardening | | Runtime target matching | `src/compat/maestro/runtime-targets.ts` | 926 LOC | Implements Maestro-specific selector resolution over raw snapshots: id/label/text/value equality-or-regex matching, visible text fuzzy fallback, `childOf`, `index`, visible-only filtering, React Native overlay blocking, foreground duplicate preference, Android rectless hidden navigation handling, tab-strip slot inference, localized breadcrumb selection, and tap-target ancestor promotion. | Uses native selector parsing/matching (`parseSelectorChain`, `matchesSelector`), native visible predicates, snapshot text normalization, and React Native overlay detection. | Extract a reusable snapshot target resolver with policy hooks for ranking, visibility, overlay filtering, and promotion. Native `click`/`wait` could then opt into the generic parts without inheriting Maestro ranking. | Highest debt concentration. Heuristics encode real platform quirks and can regress seemingly unrelated flows, especially Android duplicate/hidden nodes and broad containers. | Strong: `runtime-targets.test.ts` is large and focused; PR #620 adds provider-level Android fresh-snapshot guard through Maestro replay. | Split generic snapshot traversal/filtering from Maestro ranking policy. Keep fuzzy/regex/read-order compatibility local. | Lane 7: target resolver extraction | | Input and focus | `replay-flow.ts` input coalescing, `interactions.ts` input/erase/paste/pressKey slices, `runtime.ts`, daemon Maestro fallback in `interaction-touch.ts` | ~350 LOC plus shared daemon flags | Coalesces `tapOn` + `inputText` + `pressKey Enter` into `wait`/`fill`/enter for likely text-entry selectors, leaves focused-field input as `type`, maps `eraseText` to backspaces, falls back from keyboard enter to newline typing, and allows Maestro non-hittable coordinate fallback for tap selectors. | Uses native `fill`, `type`, `keyboard`, `click`, replay variable substitution, and daemon touch fallback flags. | A native "focus then fill" replay primitive and focused-field clear/erase operation would remove most parser heuristics while improving `.ad` flows too. | Text-entry detection is name-based; focused-field commands depend on existing device focus; non-hittable coordinate fallback is intentionally compatibility-only and can mask poor selectors. | Moderate: parser coalescing tests in `replay-flow.test.ts`; daemon fallback covered in `interaction.test.ts`; no broad provider guard for focused erase/paste. | Converge on native focus/fill/clear semantics. Keep Maestro's optional/non-hittable quirks as compat flags. | Lane 8: native input primitive | | Assertions and waits | `runtime-assertions.ts`, wait/assert slices of `interactions.ts`, `runtime-flow.ts` visible condition path | ~450 LOC | Polls raw snapshots for `assertVisible`, adds a terminal grace capture, treats `assertNotVisible` as stable hidden after repeated samples or timeout, computes animation stability from snapshot signatures, maps `extendedWaitUntil`, and waits briefly for `runFlow.when.visible` while keeping `notVisible` point-in-time. | Uses native `snapshot`, `wait`, `is`, visible predicates, reference frames, replay blocks, and timeout helpers. | A shared waiter/poller utility with explicit grace, stable-hidden, raw-snapshot, and timeout policies would let native `wait` and Maestro share mechanics without sharing defaults. | Timeout and stability semantics are subtle; raw full snapshots are expensive; making `notVisible` wait would change cleanup-flow behavior. | Good: `runtime-assertions.test.ts` covers deadline/hidden edge cases; `runtime-flow.test.ts` covers visible wait and immediate notVisible. | Extract generic polling/stability helpers; keep Maestro timing constants and condition semantics local. | Lane 8: waiter convergence | | Scroll, swipe, and geometry | `points.ts`, `runtime-geometry.ts`, geometry/scroll/swipe slices of `interactions.ts` and `runtime-interactions.ts` | ~500 LOC | Parses absolute and percentage points, converts percentage taps/swipes through raw snapshot reference frames, caches reference frame in replay scope, biases tap points for large text containers and bottom tabs, loops `scrollUntilVisible` with `wait`/`find` probes, maps `swipe.label`, and uses an Android horizontal content-lane adjustment. | Uses native `click`, `scroll`, `swipe`, `find`, `wait`, touch reference frames, and raw snapshots. | A native gesture planner for percent coordinates, target-derived swipes, and frame caching would speed up convergence. Platform-specific lane policies can remain pluggable. | Geometry heuristics are device/frame sensitive; stale or missing raw snapshot frames break percent gestures; Android horizontal swipes depend on app layout assumptions. | Good: `runtime-geometry.test.ts`, `runtime-interactions.test.ts`; PR #620 provider guard asserts fresh snapshots and Android content-lane swipe coordinates. | Extract generic percent/target geometry and frame caching. Keep Maestro Android content-lane and tap bias policies local until broader demand exists. | Lane 8 or 9: gesture planner | -| Flow control, `runFlow`, retry, and hooks | `flow-control.ts`, `runtime-flow.ts`, hook flattening in `replay-flow.ts` | ~700 LOC | Handles `onFlowStart`/`onFlowComplete`, file and inline `runFlow`, per-block env, static platform gates, limited `when.true` boolean/platform expressions, runtime visible/notVisible gates via batch steps, parse-expanded `repeat.times`, and runtime `retry` via replay retry blocks. | Depends on replay block runtime (`invokeReplayActionBlock`, `invokeReplayRetryBlock`), replay vars/env, batch step projection, and native snapshot visibility resolution. | A native replay block AST for conditional blocks, deterministic repeats, and retry would remove parse-time flattening and make line/step reporting more faithful. | `repeat.times` materializes actions with a guardrail; nested line numbers are lossy; expression support is intentionally tiny; visible conditions are snapshot-dependent. | Strong: `replay-flow.test.ts` covers hooks/runFlow/env/repeat/retry/platform gates/expressions; `runtime-flow.test.ts` covers runtime condition behavior; PR #620 covers provider retry/runFlow path. | Converge on native replay control-flow primitives, but keep Maestro expression grammar and unsupported `repeat.while` local until native runtime has loop semantics. | Lane 7 or 9: replay block AST | +| Flow control, `runFlow`, retry, and hooks | `flow-control.ts`, `runtime-flow.ts`, hook flattening in `replay-flow.ts` | ~700 LOC | Handles `onFlowStart`/`onFlowComplete`, file and inline `runFlow`, per-block env, static platform gates, limited `when.true` boolean/platform expressions, runtime visible/notVisible gates through `SessionAction.replayControl`, parse-expanded `repeat.times`, and runtime `retry` via replay retry blocks. | Depends on replay block runtime (`invokeReplayActionBlock`, `invokeReplayRetryBlock`), replay vars/env, typed `SessionReplayControl`, and native snapshot visibility resolution. | Flow block execution is now mostly converged. A native replay block AST would only be worth it if native `.ad` syntax also needs conditional blocks, deterministic repeats, or faithful nested line reporting. | `repeat.times` materializes actions with a guardrail; nested line numbers are lossy; expression support is intentionally tiny; visible conditions are snapshot-dependent; compat flow controls are intentionally in-memory and rejected for `replay -u`. | Strong: `replay-flow.test.ts` covers hooks/runFlow/env/repeat/retry/platform gates/expressions; `runtime-flow.test.ts` covers runtime condition behavior; daemon replay tests guard execution and `replay -u` rejection; PR #620 covers provider retry/runFlow path. | Keep `replayControl` as the shared runtime boundary. Do not add a native block AST unless native replay gains its own block syntax; keep Maestro expression grammar and unsupported `repeat.while` local until native runtime has loop semantics. | Lane 7 complete / revisit only with native block syntax | | `runScript` | `run-script.ts`, `runtime.ts` runScript branch | 229 LOC plus router branch | Executes trusted flow-local JavaScript with `node:vm`, exposes env values, `output`, `json`, and synchronous-looking `http.post` through a timeout-bounded child Node process; exports output as `output.` replay variables. | Uses replay variable scope, `runCmdSync`, and `AppError`; there is no native `.ad` command equivalent. | Shared env/output variable plumbing could converge, but script execution should not become native without a separate security model and product decision. | High security and determinism risk: `node:vm` is not a sandbox, scripts can make network requests, async support is intentionally narrow, and output key rules are compatibility-specific. | Partial: parser/order/env/path behavior covered in `replay-flow.test.ts`; docs describe trust/security limits; no focused execution tests for `http.post`, `json`, or output validation. | Keep compat-local. Add focused execution tests before expanding helpers; do not expose as native command until sandboxing and trust model are explicit. | Lane 9: runScript guard tests | | Suite discovery and test artifacts | `src/daemon/handlers/session-test-discovery.ts`, `session-test-suite.ts`, `session-test-artifacts.ts`, replay grammar/backend plumbing | ~120 Maestro-specific LOC in daemon/test path | When `--maestro`/backend is selected, discovers `.ad`, `.yaml`, and `.yml`, allows untyped YAML through platform filtering, runs through native replay test suite, and preserves original Maestro flow filenames in artifacts. | Native test suite runner, replay backend selection, session lifecycle, artifact materialization, and CLI `--maestro` flag. | Mostly converged already. The remaining improvement is making replay backend extension/filter policy data-driven so future backends avoid daemon conditionals. | Low. Main risk is accidentally including non-Maestro YAML when backend is set, or changing platform-filter behavior for untyped YAML. | Good: `session-test-discovery.test.ts`, `session-test-suite.test.ts`, `session-test-artifacts.test.ts`; PR #620 provider suite runs a Maestro YAML test. | Keep small daemon hook for now; extract backend discovery policy only if another replay backend appears. | Lane 6 complete / no follow-up unless backend count grows | | Docs and support matrix | `support-matrix.ts`, `website/docs/docs/replay-e2e.md`, CLI flag/help tests, issue tracker references | 42 LOC source matrix plus docs | Maintains supported/unsupported capability lists, formats CLI help copy, links tracker/new issue URLs, and keeps replay docs synced with the source matrix. | CLI flag definitions/help rendering and website docs. | Generate or embed the support list from one source in docs/help to remove manual prose drift. | Medium user-facing risk: stale docs can imply unsupported parity or hide known gaps. | Good: `support-matrix.test.ts` asserts CLI help and docs stay synced with the shared matrix. | Keep `support-matrix.ts` as source of truth; update it with every behavior change and mirror only explanatory context in docs. | Lane 6 complete / ongoing docs hygiene | @@ -17,6 +17,6 @@ This map summarizes the Maestro compatibility surface after the lane 6 audit. LO ## Convergence Priority 1. Target matching is the largest and riskiest debt. Extract reusable snapshot traversal/filtering first, then keep Maestro ranking rules as a policy. -2. Replay control-flow should converge before adding more Maestro flow syntax. A native block AST would make `runFlow`, `retry`, and future loop support less brittle. +2. Replay control-flow is partly converged through `SessionAction.replayControl`; avoid a broader native block AST until native replay has its own block syntax or needs faithful nested line reporting. 3. Input/focus and wait semantics are good candidates for native primitives because they improve `.ad` replay as well as Maestro compatibility. 4. `runScript` should remain compatibility-local unless a separate secure native script story is designed. diff --git a/src/compat/maestro/__tests__/replay-flow.test.ts b/src/compat/maestro/__tests__/replay-flow.test.ts index b4fc59063..327d6a46a 100644 --- a/src/compat/maestro/__tests__/replay-flow.test.ts +++ b/src/compat/maestro/__tests__/replay-flow.test.ts @@ -580,18 +580,26 @@ test('parseMaestroReplayFlow keeps visible-gated runFlow commands for runtime ev { platform: 'ios' }, ); - assert.equal(parsed.actions[0]?.command, '__maestroRunFlowWhen'); + assert.equal(parsed.actions[0]?.command, 'runFlow.when'); assert.deepEqual(parsed.actions[0]?.positionals, [ 'visible', 'label="Continue" || text="Continue" || id="Continue"', ]); - assert.deepEqual(parsed.actions[0]?.flags.batchSteps, [ - { - command: '__maestroTapOn', - positionals: ['label="Continue" || text="Continue" || id="Continue"'], - flags: { maestro: { allowNonHittableCoordinateFallback: true } }, - }, - ]); + const control = parsed.actions[0]?.replayControl; + assert.equal(control?.kind, 'maestroRunFlowWhen'); + if (control?.kind !== 'maestroRunFlowWhen') throw new Error('expected runFlow.when control'); + assert.equal(control.mode, 'visible'); + assert.equal(control.selector, 'label="Continue" || text="Continue" || id="Continue"'); + assert.deepEqual( + control.actions.map((entry) => [entry.command, entry.positionals, entry.flags]), + [ + [ + '__maestroTapOn', + ['label="Continue" || text="Continue" || id="Continue"'], + { maestro: { allowNonHittableCoordinateFallback: true } }, + ], + ], + ); }); test('parseMaestroReplayFlow keeps retry commands for runtime evaluation', () => { @@ -608,20 +616,19 @@ test('parseMaestroReplayFlow keeps retry commands for runtime evaluation', () => { env: { APP_SCHEME: 'example://' } }, ); - assert.equal(parsed.actions[0]?.command, '__maestroRetry'); + assert.equal(parsed.actions[0]?.command, 'retry'); assert.deepEqual(parsed.actions[0]?.positionals, ['3']); - assert.deepEqual(parsed.actions[0]?.flags.batchSteps, [ - { - command: 'open', - positionals: ['example://details'], - flags: {}, - }, - { - command: '__maestroAssertVisible', - positionals: ['label="Article" || text="Article" || id="Article"', '5000'], - flags: {}, - }, - ]); + const control = parsed.actions[0]?.replayControl; + assert.equal(control?.kind, 'retry'); + if (control?.kind !== 'retry') throw new Error('expected retry control'); + assert.equal(control.maxRetries, 3); + assert.deepEqual( + control.actions.map((entry) => [entry.command, entry.positionals, entry.flags]), + [ + ['open', ['example://details'], {}], + ['__maestroAssertVisible', ['label="Article" || text="Article" || id="Article"', '5000'], {}], + ], + ); }); test('parseMaestroReplayFlow accepts launchApp reset options', () => { diff --git a/src/compat/maestro/__tests__/runtime-flow.test.ts b/src/compat/maestro/__tests__/runtime-flow.test.ts index 96e2fe2c3..2754147a5 100644 --- a/src/compat/maestro/__tests__/runtime-flow.test.ts +++ b/src/compat/maestro/__tests__/runtime-flow.test.ts @@ -1,24 +1,27 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; -import type { CommandFlags } from '../../../core/dispatch.ts'; import type { DaemonRequest, DaemonResponse, SessionAction } from '../../../daemon/types.ts'; -import { invokeMaestroRunFlowWhen } from '../runtime-flow.ts'; +import { invokeMaestroRunFlowWhenControl } from '../runtime-flow.ts'; -test('invokeMaestroRunFlowWhen waits briefly for visible conditions', async () => { +test('invokeMaestroRunFlowWhenControl waits briefly for visible conditions', async () => { let snapshots = 0; const invokedActions: SessionAction[] = []; - const batchSteps: CommandFlags['batchSteps'] = [ - { command: 'click', positionals: ['label="Dismiss"'] }, + const actions: SessionAction[] = [ + { ts: Date.now(), command: 'click', positionals: ['label="Dismiss"'], flags: {} }, ]; - const response = await invokeMaestroRunFlowWhen({ + const response = await invokeMaestroRunFlowWhenControl({ baseReq: { token: 't', session: 's', flags: { platform: 'android' }, }, - positionals: ['visible', 'label="Dismiss" || text="Dismiss" || id="Dismiss"'], - batchSteps, + control: { + kind: 'maestroRunFlowWhen', + mode: 'visible', + selector: 'label="Dismiss" || text="Dismiss" || id="Dismiss"', + actions, + }, line: 12, step: 4, invoke: async (req: DaemonRequest): Promise => { @@ -61,16 +64,20 @@ test('invokeMaestroRunFlowWhen waits briefly for visible conditions', async () = } }); -test('invokeMaestroRunFlowWhen keeps notVisible conditions immediate', async () => { +test('invokeMaestroRunFlowWhenControl keeps notVisible conditions immediate', async () => { let snapshots = 0; - const response = await invokeMaestroRunFlowWhen({ + const response = await invokeMaestroRunFlowWhenControl({ baseReq: { token: 't', session: 's', flags: { platform: 'android' }, }, - positionals: ['notVisible', 'label="Loading" || text="Loading" || id="Loading"'], - batchSteps: [{ command: 'click', positionals: ['label="Continue"'] }], + control: { + kind: 'maestroRunFlowWhen', + mode: 'notVisible', + selector: 'label="Loading" || text="Loading" || id="Loading"', + actions: [{ ts: Date.now(), command: 'click', positionals: ['label="Continue"'], flags: {} }], + }, line: 14, step: 7, invoke: async (): Promise => { diff --git a/src/compat/maestro/flow-control.ts b/src/compat/maestro/flow-control.ts index 0c526cb93..dfa0496b4 100644 --- a/src/compat/maestro/flow-control.ts +++ b/src/compat/maestro/flow-control.ts @@ -1,8 +1,6 @@ -import type { CommandFlags } from '../../core/dispatch.ts'; import type { SessionAction } from '../../daemon/types.ts'; import { AppError } from '../../utils/errors.ts'; import { maestroSelector } from './interactions.ts'; -import { MAESTRO_RUNTIME_COMMAND } from './runtime-commands.ts'; import { action, assertOnlyKeys, @@ -107,8 +105,10 @@ export function convertRetry( const commands = normalizeCommandList(value.commands); const actions = convertCommandList(commands, config, context, deps); return [ - action(MAESTRO_RUNTIME_COMMAND.retry, [String(maxRetries)], { - batchSteps: actions.map(sessionActionToBatchStep), + replayControlAction('retry', [String(maxRetries)], { + kind: 'retry', + maxRetries, + actions, }), ]; } @@ -412,31 +412,33 @@ function wrapRunFlowCondition( actions: SessionAction[], condition: RunFlowCondition, ): SessionAction[] { - if (!condition.visibleSelector && !condition.notVisibleSelector) return actions; - if (condition.visibleSelector && condition.notVisibleSelector) { + const { visibleSelector, notVisibleSelector } = condition; + if (!visibleSelector && !notVisibleSelector) return actions; + if (visibleSelector && notVisibleSelector) { throw unsupportedMaestroSyntax( 'Maestro runFlow.when cannot combine visible and notVisible yet.', ); } + const mode = visibleSelector ? 'visible' : 'notVisible'; + const selector = visibleSelector ?? notVisibleSelector ?? ''; return [ - action( - MAESTRO_RUNTIME_COMMAND.runFlowWhen, - condition.visibleSelector - ? ['visible', condition.visibleSelector] - : ['notVisible', condition.notVisibleSelector ?? ''], - { batchSteps: actions.map(sessionActionToBatchStep) }, - ), + replayControlAction('runFlow.when', [mode, selector], { + kind: 'maestroRunFlowWhen', + mode, + selector, + actions, + }), ]; } -function sessionActionToBatchStep( - entry: SessionAction, -): NonNullable[number] { +function replayControlAction( + command: string, + positionals: string[], + replayControl: NonNullable, +): SessionAction { return { - command: entry.command, - positionals: entry.positionals, - flags: entry.flags, - ...(entry.runtime !== undefined ? { runtime: entry.runtime } : {}), + ...action(command, positionals), + replayControl, }; } diff --git a/src/compat/maestro/runtime-commands.ts b/src/compat/maestro/runtime-commands.ts index d15292434..ad328ee4b 100644 --- a/src/compat/maestro/runtime-commands.ts +++ b/src/compat/maestro/runtime-commands.ts @@ -1,6 +1,4 @@ export const MAESTRO_RUNTIME_COMMAND = { - runFlowWhen: '__maestroRunFlowWhen', - retry: '__maestroRetry', runScript: '__maestroRunScript', assertVisible: '__maestroAssertVisible', assertNotVisible: '__maestroAssertNotVisible', diff --git a/src/compat/maestro/runtime-flow.ts b/src/compat/maestro/runtime-flow.ts index 40f062307..87948d865 100644 --- a/src/compat/maestro/runtime-flow.ts +++ b/src/compat/maestro/runtime-flow.ts @@ -1,11 +1,6 @@ -import { type CommandFlags } from '../../core/dispatch.ts'; -import type { DaemonRequest, DaemonResponse } from '../../daemon/types.ts'; +import type { DaemonRequest, DaemonResponse, SessionReplayControl } from '../../daemon/types.ts'; import { getSnapshotReferenceFrame } from '../../daemon/touch-reference-frame.ts'; -import { - batchStepsToSessionActions, - invokeReplayActionBlock, - invokeReplayRetryBlock, -} from '../../replay/control-flow-runtime.ts'; +import { invokeReplayActionBlock } from '../../replay/control-flow-runtime.ts'; import { captureMaestroRawSnapshot, errorResponse, @@ -24,70 +19,25 @@ const MAESTRO_RUN_FLOW_WHEN_POLICY = { visiblePollMs: 250, } as const; -type MaestroRunFlowWhenCondition = - | { ok: true; mode: string; selector: string } - | { ok: false; response: DaemonResponse }; +type MaestroRunFlowWhenControl = Extract; -export async function invokeMaestroRunFlowWhen(params: { +export async function invokeMaestroRunFlowWhenControl(params: { baseReq: ReplayBaseRequest; - positionals: string[]; - batchSteps: CommandFlags['batchSteps'] | undefined; + control: MaestroRunFlowWhenControl; line: number; step: number; invoke: (req: DaemonRequest) => Promise; invokeReplayAction: MaestroReplayInvoker; }): Promise { - const condition = readMaestroRunFlowWhenCondition(params.positionals); - if (!condition.ok) return condition.response; - const conditionResult = await evaluateMaestroRunFlowWhenCondition(params, condition); + const conditionResult = await evaluateMaestroRunFlowWhenCondition(params, params.control); if (!conditionResult.ok) return conditionResult.response; if (!conditionResult.matched) { return { ok: true, - data: { skipped: true, condition: condition.mode, selector: condition.selector }, + data: { skipped: true, condition: params.control.mode, selector: params.control.selector }, }; } - return await invokeMaestroRunFlowWhenSteps(params, condition); -} - -export async function invokeMaestroRetry(params: { - positionals: string[]; - batchSteps: CommandFlags['batchSteps'] | undefined; - line: number; - step: number; - invokeReplayAction: MaestroReplayInvoker; -}): Promise { - const [maxRetriesValue = '1'] = params.positionals; - const maxRetries = Number(maxRetriesValue); - if (!Number.isInteger(maxRetries) || maxRetries < 0) { - return errorResponse('INVALID_ARGS', 'retry.maxRetries must be a non-negative integer.'); - } - - return await invokeReplayRetryBlock({ - actions: batchStepsToSessionActions(params.batchSteps), - maxRetries, - line: params.line, - step: params.step, - invokeReplayAction: params.invokeReplayAction, - }); -} - -function readMaestroRunFlowWhenCondition(positionals: string[]): MaestroRunFlowWhenCondition { - const [mode, selector] = positionals; - if ((mode !== 'visible' && mode !== 'notVisible') || !selector) { - return { - ok: false, - response: errorResponse( - 'INVALID_ARGS', - 'runFlow.when requires visible/notVisible and a selector.', - ), - }; - } - return { - ok: true, - mode, - selector, - }; + return await invokeMaestroRunFlowWhenSteps(params); } async function evaluateMaestroRunFlowWhenCondition( @@ -95,7 +45,7 @@ async function evaluateMaestroRunFlowWhenCondition( baseReq: ReplayBaseRequest; invoke: (req: DaemonRequest) => Promise; }, - condition: Extract, + condition: MaestroRunFlowWhenControl, ): Promise<{ ok: true; matched: boolean } | { ok: false; response: DaemonResponse }> { if (condition.mode === 'visible') { return await waitForMaestroRunFlowVisibleCondition(params, condition); @@ -118,7 +68,7 @@ async function waitForMaestroRunFlowVisibleCondition( baseReq: ReplayBaseRequest; invoke: (req: DaemonRequest) => Promise; }, - condition: Extract, + condition: MaestroRunFlowWhenControl, ): Promise<{ ok: true; matched: boolean } | { ok: false; response: DaemonResponse }> { // Maestro conditionals commonly guard UI that appears immediately after the // previous command. Keep this bounded and only for visible; notVisible stays @@ -160,17 +110,14 @@ function readMaestroRunFlowVisibleCondition( return { ok: true, matched }; } -async function invokeMaestroRunFlowWhenSteps( - params: { - batchSteps: CommandFlags['batchSteps'] | undefined; - line: number; - step: number; - invokeReplayAction: MaestroReplayInvoker; - }, - condition: Extract, -): Promise { +async function invokeMaestroRunFlowWhenSteps(params: { + control: MaestroRunFlowWhenControl; + line: number; + step: number; + invokeReplayAction: MaestroReplayInvoker; +}): Promise { const response = await invokeReplayActionBlock({ - actions: batchStepsToSessionActions(params.batchSteps), + actions: params.control.actions, line: params.line, step: params.step, invokeReplayAction: params.invokeReplayAction, @@ -179,6 +126,10 @@ async function invokeMaestroRunFlowWhenSteps( return { ok: true, - data: { ran: response.data?.ran, condition: condition.mode, selector: condition.selector }, + data: { + ran: response.data?.ran, + condition: params.control.mode, + selector: params.control.selector, + }, }; } diff --git a/src/compat/maestro/runtime.ts b/src/compat/maestro/runtime.ts index 42cd0950f..bacbac298 100644 --- a/src/compat/maestro/runtime.ts +++ b/src/compat/maestro/runtime.ts @@ -1,4 +1,3 @@ -import { type CommandFlags } from '../../core/dispatch.ts'; import { asAppError } from '../../utils/errors.ts'; import type { ReplayVarScope } from '../../replay/vars.ts'; import type { DaemonRequest, DaemonResponse } from '../../daemon/types.ts'; @@ -9,7 +8,6 @@ import { invokeMaestroAssertVisible, invokeMaestroWaitForAnimationToEnd, } from './runtime-assertions.ts'; -import { invokeMaestroRetry, invokeMaestroRunFlowWhen } from './runtime-flow.ts'; import { errorResponse, type MaestroReplayInvoker, @@ -28,7 +26,6 @@ export async function invokeMaestroRuntimeCommand(params: { command: string; baseReq: ReplayBaseRequest; positionals: string[]; - batchSteps: CommandFlags['batchSteps'] | undefined; scope: ReplayVarScope; line: number; step: number; @@ -40,8 +37,6 @@ export async function invokeMaestroRuntimeCommand(params: { return await invokeMaestroAssertVisible(params); case MAESTRO_RUNTIME_COMMAND.assertNotVisible: return await invokeMaestroAssertNotVisible(params); - case MAESTRO_RUNTIME_COMMAND.retry: - return await invokeMaestroRetry(params); case MAESTRO_RUNTIME_COMMAND.pressEnter: return await invokeMaestroPressEnter(params); case MAESTRO_RUNTIME_COMMAND.waitForAnimationToEnd: @@ -56,8 +51,6 @@ export async function invokeMaestroRuntimeCommand(params: { return await invokeMaestroTapOn(params); case MAESTRO_RUNTIME_COMMAND.tapPointPercent: return await invokeMaestroTapPointPercent(params); - case MAESTRO_RUNTIME_COMMAND.runFlowWhen: - return await invokeMaestroRunFlowWhen(params); case MAESTRO_RUNTIME_COMMAND.runScript: return invokeMaestroRunScript(params); default: diff --git a/src/daemon/handlers/__tests__/session-replay-vars.test.ts b/src/daemon/handlers/__tests__/session-replay-vars.test.ts index de04036ef..4fb54c72a 100644 --- a/src/daemon/handlers/__tests__/session-replay-vars.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-vars.test.ts @@ -224,6 +224,38 @@ test('resolveReplayAction walks runtime hints', () => { assert.equal(resolved.runtime?.metroHost, '10.0.0.1'); }); +test('resolveReplayAction resolves replay control conditions without pre-resolving nested actions', () => { + const action: SessionAction = { + ts: 0, + command: 'runFlow.when', + positionals: ['visible', '${VISIBLE}'], + flags: {}, + replayControl: { + kind: 'maestroRunFlowWhen', + mode: 'visible', + selector: '${VISIBLE}', + actions: [ + { + ts: 0, + command: 'tap', + positionals: ['${TARGET}'], + flags: {}, + }, + ], + }, + }; + const scope = buildReplayVarScope({ + fileEnv: { VISIBLE: 'Feed', TARGET: '${NEXT}', NEXT: 'Done' }, + }); + const resolved = resolveReplayAction(action, scope, LOC); + assert.equal(resolved.replayControl?.kind, 'maestroRunFlowWhen'); + if (resolved.replayControl?.kind !== 'maestroRunFlowWhen') { + throw new Error('expected runFlow.when control'); + } + assert.equal(resolved.replayControl.selector, 'Feed'); + assert.deepEqual(resolved.replayControl.actions[0]?.positionals, ['${TARGET}']); +}); + test('parseReplayScriptDetailed tracks line numbers', () => { const script = [ '# comment', @@ -305,6 +337,33 @@ test('runReplayScriptFile rejects replay -u on scripts with env directives', asy } }); +test('runReplayScriptFile rejects replay -u for Maestro compat flow controls before serialization', async () => { + const { response } = await runReplayFixture({ + label: 'maestro-replay-update-flow-control', + script: [ + 'appId: demo.app', + '---', + '- runFlow:', + ' when:', + ' visible: Feed', + ' commands:', + ' - tapOn: Continue', + '- retry:', + ' maxRetries: 1', + ' commands:', + ' - assertVisible: Feed', + '', + ].join('\n'), + flags: { replayBackend: 'maestro', replayUpdate: true }, + }); + + assert.equal(response.ok, false); + if (!response.ok) { + assert.equal(response.error.code, 'INVALID_ARGS'); + assert.match(response.error.message, /replay -u is not supported for compat flow input/); + } +}); + test('resolveReplayAction produces dispatch-ready literals for a realistic fixture', () => { const script = [ 'context platform=android', @@ -1854,6 +1913,72 @@ test('runReplayScriptFile runs nested Maestro runtime commands inside runFlow.wh ); }); +test('runReplayScriptFile resolves nested Maestro runFlow.when command variables once at execution', async () => { + const calls: CapturedInvocation[] = []; + const { response } = await runReplayFixture({ + label: 'maestro-run-flow-when-nested-vars', + script: [ + 'appId: demo.app', + 'env:', + ' TARGET_LABEL: ${NEXT_LABEL}', + ' NEXT_LABEL: ${FINAL_LABEL}', + ' FINAL_LABEL: Done', + '---', + '- runFlow:', + ' when:', + ' visible: Feed', + ' commands:', + ' - tapOn: ${TARGET_LABEL}', + '', + ].join('\n'), + flags: { replayBackend: 'maestro' }, + invoke: async (req) => { + calls.push({ command: req.command, positionals: req.positionals, flags: req.flags }); + if (req.command === 'snapshot') { + return { + ok: true, + data: { + nodes: [ + { + index: 0, + type: 'application', + rect: { x: 0, y: 0, width: 390, height: 844 }, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'statictext', + label: 'Feed', + rect: { x: 16, y: 100, width: 120, height: 24 }, + }, + { + index: 2, + depth: 1, + parentIndex: 0, + type: 'button', + label: '${FINAL_LABEL}', + rect: { x: 100, y: 300, width: 80, height: 40 }, + }, + ], + }, + }; + } + return { ok: true, data: {} }; + }, + }); + + assert.equal(response.ok, true); + assert.deepEqual( + calls.map((call) => [call.command, call.positionals]), + [ + ['snapshot', []], + ['snapshot', []], + ['click', ['140', '320']], + ], + ); +}); + test('runReplayScriptFile reads shell env from request (client-collected), not daemon process.env', async () => { // Ensure the daemon's own process.env does NOT contain AD_VAR_APP. assert.equal(process.env.AD_VAR_APP, undefined); diff --git a/src/daemon/handlers/session-replay-action-runtime.ts b/src/daemon/handlers/session-replay-action-runtime.ts index 3ab0868df..52ba171c7 100644 --- a/src/daemon/handlers/session-replay-action-runtime.ts +++ b/src/daemon/handlers/session-replay-action-runtime.ts @@ -8,6 +8,8 @@ import { import type { DaemonRequest, DaemonResponse, SessionAction } from '../types.ts'; import { mergeParentFlags } from './handler-utils.ts'; import { invokeMaestroRuntimeCommand } from '../../compat/maestro/runtime.ts'; +import { invokeMaestroRunFlowWhenControl } from '../../compat/maestro/runtime-flow.ts'; +import { invokeReplayRetryBlock } from '../../replay/control-flow-runtime.ts'; type ReplayBaseRequest = Omit; @@ -99,11 +101,18 @@ async function invokeResolvedReplayAction(params: { meta: req.meta, }; const response = + (await invokeReplayControl({ + control: resolved.replayControl, + baseReq, + line, + step, + invoke, + invokeReplayAction, + })) ?? (await invokeMaestroRuntimeCommand({ command: resolved.command, baseReq, positionals: resolved.positionals ?? [], - batchSteps: resolved.flags?.batchSteps, scope, line, step, @@ -122,6 +131,39 @@ async function invokeResolvedReplayAction(params: { return response; } +async function invokeReplayControl(params: { + control: SessionAction['replayControl'] | undefined; + baseReq: ReplayBaseRequest; + line: number; + step: number; + invoke: (req: DaemonRequest) => Promise; + invokeReplayAction: ReplayActionInvoker; +}): Promise { + const { control, baseReq, line, step, invoke, invokeReplayAction } = params; + if (!control) return undefined; + switch (control.kind) { + case 'retry': + return await invokeReplayRetryBlock({ + actions: control.actions, + maxRetries: control.maxRetries, + line, + step, + invokeReplayAction, + }); + case 'maestroRunFlowWhen': + return await invokeMaestroRunFlowWhenControl({ + baseReq, + control, + line, + step, + invoke, + invokeReplayAction, + }); + } + const _exhaustive: never = control; + return _exhaustive; +} + function readReplayOutputEnv(data: unknown): Record | null { if (!data || typeof data !== 'object') return null; const raw = (data as { outputEnv?: unknown }).outputEnv; diff --git a/src/daemon/types.ts b/src/daemon/types.ts index b943f6832..afb2494da 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -229,11 +229,25 @@ export type SessionState = { }; }; +export type SessionReplayControl = + | { + kind: 'maestroRunFlowWhen'; + mode: 'visible' | 'notVisible'; + selector: string; + actions: SessionAction[]; + } + | { + kind: 'retry'; + maxRetries: number; + actions: SessionAction[]; + }; + export type SessionAction = { ts: number; command: string; positionals: string[]; runtime?: SessionRuntimeHints; + replayControl?: SessionReplayControl; flags: Partial & { snapshotInteractiveOnly?: boolean; snapshotCompact?: boolean; diff --git a/src/replay/control-flow-runtime.ts b/src/replay/control-flow-runtime.ts index d7daa1125..f5b5bf217 100644 --- a/src/replay/control-flow-runtime.ts +++ b/src/replay/control-flow-runtime.ts @@ -1,4 +1,3 @@ -import type { CommandFlags } from '../core/dispatch.ts'; import type { DaemonResponse, SessionAction } from '../daemon/types.ts'; export type ReplayActionBlockInvoker = (params: { @@ -7,27 +6,6 @@ export type ReplayActionBlockInvoker = (params: { step: number; }) => Promise; -function batchStepToSessionAction( - step: NonNullable[number], -): SessionAction { - const action: SessionAction = { - ts: Date.now(), - command: step.command, - positionals: step.positionals ?? [], - flags: step.flags ?? {}, - }; - if (step.runtime && typeof step.runtime === 'object') { - action.runtime = step.runtime as SessionAction['runtime']; - } - return action; -} - -export function batchStepsToSessionActions( - batchSteps: CommandFlags['batchSteps'] | undefined, -): SessionAction[] { - return (batchSteps ?? []).map(batchStepToSessionAction); -} - export async function invokeReplayActionBlock(params: { actions: SessionAction[]; line: number; diff --git a/src/replay/vars.ts b/src/replay/vars.ts index f702d27cc..71d204bd6 100644 --- a/src/replay/vars.ts +++ b/src/replay/vars.ts @@ -154,9 +154,29 @@ export function resolveReplayAction( positionals: (action.positionals ?? []).map((token) => resolveReplayString(token, scope, loc)), flags: resolveStringProps(action.flags, scope, loc) ?? {}, runtime: resolveStringProps(action.runtime, scope, loc), + replayControl: resolveReplayControl(action.replayControl, scope, loc), }; } +function resolveReplayControl( + control: SessionAction['replayControl'] | undefined, + scope: ReplayVarScope, + loc: { file: string; line: number }, +): SessionAction['replayControl'] | undefined { + if (!control) return control; + switch (control.kind) { + case 'maestroRunFlowWhen': + return { + ...control, + selector: resolveReplayString(control.selector, scope, loc), + }; + case 'retry': + return control; + } + const _exhaustive: never = control; + return _exhaustive; +} + function resolveStringProps( obj: T | undefined, scope: ReplayVarScope,