fix: address maestro replay control review

Author: thymikee

src/compat/maestro/flow-control.ts

@@ -412,26 +412,25 @@ function wrapRunFlowCondition(
   actions: SessionAction[],
   condition: RunFlowCondition,
 ): SessionAction[] {
-  if (!condition.visibleSelector && !condition.notVisibleSelector) return actions;
-  if (condition.visibleSelector && condition.notVisibleSelector) {
+  const control = buildRunFlowConditionReplayControl(condition, actions);
+  if (!control) return actions;
+  return [replayControlAction('runFlow.when', [control.mode, control.selector], control)];
+}
+
+function buildRunFlowConditionReplayControl(
+  condition: RunFlowCondition,
+  actions: SessionAction[],
+): Extract<NonNullable<SessionAction['replayControl']>, { kind: 'maestroRunFlowWhen' }> | null {
+  const { visibleSelector, notVisibleSelector } = condition;
+  if (!visibleSelector && !notVisibleSelector) return null;
+  if (visibleSelector && notVisibleSelector) {
     throw unsupportedMaestroSyntax(
       'Maestro runFlow.when cannot combine visible and notVisible yet.',
     );
   }
-  return [
-    replayControlAction(
-      'runFlow.when',
-      condition.visibleSelector
-        ? ['visible', condition.visibleSelector]
-        : ['notVisible', condition.notVisibleSelector ?? ''],
-      {
-        kind: 'maestroRunFlowWhen',
-        mode: condition.visibleSelector ? 'visible' : 'notVisible',
-        selector: condition.visibleSelector ?? condition.notVisibleSelector ?? '',
-        actions,
-      },
-    ),
-  ];
+  const mode = visibleSelector ? 'visible' : 'notVisible';
+  const selector = visibleSelector ?? notVisibleSelector ?? '';
+  return { kind: 'maestroRunFlowWhen', mode, selector, actions };
 }
 
 function replayControlAction(

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',
@@ -1824,6 +1856,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);

src/daemon/handlers/session-replay-action-runtime.ts

@@ -160,7 +160,8 @@ async function invokeReplayControl(params: {
         invokeReplayAction,
       });
   }
-  return undefined;
+  const _exhaustive: never = control;
+  return _exhaustive;
 }
 
 function readReplayOutputEnv(data: unknown): Record<string, string> | null {

src/replay/vars.ts

@@ -154,10 +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: resolveStringProps(action.replayControl, 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<T extends object>(
   obj: T | undefined,
   scope: ReplayVarScope,

test/integration/provider-scenarios/android-test-suite.test.ts

@@ -126,60 +126,6 @@ test('Provider-backed integration Android Maestro replay uses fresh selector sna
   );
 });
 
-test('Provider-backed integration Android Maestro executes runFlow conditions and retry batches at runtime', async () => {
-  let snapshots = 0;
-  await withProviderScenarioResource(
-    async () =>
-      await createAndroidSettingsWorld({
-        snapshotXml: () => {
-          snapshots += 1;
-          return androidMaestroReplayXml('[100,300][260,360]');
-        },
-      }),
-    async (world) => {
-      const client = world.daemon.client();
-      const suiteRoot = path.join(world.tempRoot, 'suite-maestro-runtime-flow');
-      fs.mkdirSync(suiteRoot, { recursive: true });
-      const flowPath = path.join(suiteRoot, 'runtime-flow.yaml');
-      fs.writeFileSync(
-        flowPath,
-        [
-          'appId: com.android.settings',
-          '---',
-          '- launchApp',
-          '- runFlow:',
-          '    when:',
-          '      visible: Apps',
-          '    commands:',
-          '      - tapOn: Search',
-          '- retry:',
-          '    maxRetries: 1',
-          '    commands:',
-          '      - assertVisible: Apps',
-          '',
-        ].join('\n'),
-      );
-
-      const suite = await client.replay.test({
-        paths: [flowPath],
-        backend: 'maestro',
-        artifactsDir: path.join(suiteRoot, 'artifacts'),
-        timeoutMs: 30000,
-        ...world.selection,
-      });
-
-      assert.equal(suite.total, 1, JSON.stringify(suite));
-      assert.equal(suite.passed, 1, JSON.stringify(suite));
-      assert.equal(suite.failed, 0, JSON.stringify(suite));
-      assert.deepEqual(
-        world.adbCalls.find((call) => call.slice(0, 3).join(' ') === 'shell input tap'),
-        ['shell', 'input', 'tap', '180', '330'],
-      );
-      assert.equal(snapshots >= 3, true);
-    },
-  );
-});
-
 function androidMaestroReplayXml(searchBounds: string): string {
   return [
     '<?xml version="1.0" encoding="UTF-8"?>',