fix: address Maestro replay review feedback

thymikee · thymikee · commit 78be55c2d70b · 2026-05-25T22:29:05.000+02:00
diff --git a/ios-runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift b/ios-runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift
@@ -236,7 +236,10 @@ extension RunnerTests {
       return false
     }
     let appFrame = app.frame
-    return appFrame.isEmpty || appFrame.intersects(frame)
+    if appFrame.isEmpty {
+      return true
+    }
+    return appFrame.contains(CGPoint(x: frame.midX, y: frame.midY))
   }
 
   func queryElement(app: XCUIApplication, selectorKey: String, selectorValue: String) -> Response {
diff --git a/src/compat/maestro/__tests__/replay-flow.test.ts b/src/compat/maestro/__tests__/replay-flow.test.ts
@@ -69,7 +69,7 @@ env:
       ['scroll', ['right']],
       [
         '__maestroScrollUntilVisible',
-        ['label="Discover" || text="Discover" || id="Discover"', '5000', 'down'],
+        ['label="Discover" || text="Discover" || id="Discover"', '5000', 'up'],
       ],
       ['screenshot', ['./screens/form.png']],
       ['keyboard', ['dismiss']],
@@ -128,6 +128,29 @@ output.result = SERVER_PATH + ':' + json(res.body).appviewDid
   );
 });
 
+test('parseMaestroReplayFlow reports runScript http failures with command context', () => {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-maestro-runscript-fail-'));
+  const scriptPath = path.join(root, 'setup.js');
+  const flowPath = path.join(root, 'flow.yml');
+  fs.writeFileSync(scriptPath, `output.result = http.post('http://127.0.0.1:1').body`);
+
+  assert.throws(
+    () =>
+      parseMaestroReplayFlow(
+        `appId: com.callstack.agentdevicelab
+---
+- runScript: ./setup.js
+`,
+        { sourcePath: flowPath },
+      ),
+    (error) =>
+      error instanceof AppError &&
+      error.code === 'COMMAND_FAILED' &&
+      /runScript failed/.test(error.message) &&
+      /http\.post failed/.test(error.message),
+  );
+});
+
 test('parseMaestroReplayFlow rejects unsupported Maestro commands', () => {
   assert.throws(
     () => parseMaestroReplayFlow('---\n- travelThroughTime: Save\n'),
@@ -325,7 +348,7 @@ test('parseMaestroReplayFlow keeps visible-gated runFlow commands for runtime ev
   ]);
 });
 
-test('parseMaestroReplayFlow accepts launchApp reset options without state-reset side effects', () => {
+test('parseMaestroReplayFlow accepts launchApp reset options', () => {
   const parsed = parseMaestroReplayFlow(`appId: com.callstack.agentdevicelab
 ---
 - launchApp:
diff --git a/src/compat/maestro/interactions.ts b/src/compat/maestro/interactions.ts
@@ -142,9 +142,7 @@ export function convertScrollUntilVisible(
   assertOnlyKeys(value, 'scrollUntilVisible', ['element', 'direction', 'timeout']);
   const selector = maestroSelector(value.element, 'scrollUntilVisible.element', [], context);
   const direction =
-    typeof value.direction === 'string'
-      ? readScrollPositionalsFromDirectionSwipe(value.direction)[0]
-      : 'down';
+    typeof value.direction === 'string' ? readScrollUntilVisibleDirection(value.direction) : 'down';
   const timeoutMs = String(readTimeoutMs(value, 5000));
   return [action(MAESTRO_RUNTIME_COMMAND.scrollUntilVisible, [selector, timeoutMs, direction])];
 }
@@ -198,6 +196,20 @@ function readScrollPositionalsFromDirectionSwipe(direction: string): string[] {
   }
 }
 
+function readScrollUntilVisibleDirection(direction: string): string {
+  switch (direction.toLowerCase()) {
+    case 'up':
+    case 'down':
+    case 'left':
+    case 'right':
+      return direction.toLowerCase();
+    default:
+      throw unsupportedMaestroSyntax(
+        'Maestro scrollUntilVisible.direction must be UP, DOWN, LEFT, or RIGHT.',
+      );
+  }
+}
+
 export function convertPressKey(value: unknown): SessionAction {
   const key = requireStringValue('pressKey', value).toLowerCase();
   if (key === 'back') return action('back');
diff --git a/src/compat/maestro/run-script.ts b/src/compat/maestro/run-script.ts
@@ -12,6 +12,8 @@ import {
 } from './support.ts';
 import type { MaestroParseContext } from './types.ts';
 
+const RUN_SCRIPT_TIMEOUT_MS = 30_000;
+
 type HttpResponse = {
   status: number;
   body: string;
@@ -51,7 +53,7 @@ export function executeRunScript(value: unknown, context: MaestroParseContext):
   try {
     vm.runInNewContext(script, buildScriptGlobals(scriptEnv, output), {
       filename: scriptPath,
-      timeout: 30_000,
+      timeout: RUN_SCRIPT_TIMEOUT_MS,
     });
   } catch (error) {
     throw new AppError(
@@ -124,13 +126,41 @@ function runHttpRequestSync(
       headers: options?.headers ?? {},
       body: options?.body ?? '',
     }),
-    timeoutMs: 30_000,
+    timeoutMs: RUN_SCRIPT_TIMEOUT_MS,
+    allowFailure: true,
   });
-  return JSON.parse(result.stdout) as HttpResponse;
+  if (result.exitCode !== 0) {
+    throw new AppError(
+      'COMMAND_FAILED',
+      `Maestro runScript http.${method.toLowerCase()} failed for ${url}: ${trimHttpErrorOutput(result.stderr)}`,
+      {
+        exitCode: result.exitCode,
+        stderr: result.stderr,
+      },
+    );
+  }
+  try {
+    return JSON.parse(result.stdout) as HttpResponse;
+  } catch (error) {
+    throw new AppError(
+      'COMMAND_FAILED',
+      `Maestro runScript http.${method.toLowerCase()} returned invalid JSON for ${url}`,
+      {
+        stdout: result.stdout.slice(0, 1000),
+        stderr: result.stderr.slice(0, 1000),
+      },
+      error instanceof Error ? error : undefined,
+    );
+  }
 }
 
 function stringifyOutputValue(value: unknown): string {
   if (typeof value === 'string') return value;
   if (typeof value === 'number' || typeof value === 'boolean') return String(value);
   return JSON.stringify(value);
 }
+
+function trimHttpErrorOutput(stderr: string): string {
+  const trimmed = stderr.trim();
+  return trimmed.length > 0 ? trimmed.slice(0, 1000) : 'request process exited without stderr';
+}
diff --git a/src/core/__tests__/dispatch-open.test.ts b/src/core/__tests__/dispatch-open.test.ts
@@ -23,3 +23,26 @@ test('dispatch open rejects URL as first argument when second URL is provided',
     },
   );
 });
+
+test('dispatch open rejects Android launch arguments instead of dropping them', async () => {
+  const device: DeviceInfo = {
+    platform: 'android',
+    id: 'emulator-5554',
+    name: 'Pixel',
+    kind: 'emulator',
+    booted: true,
+  };
+
+  await assert.rejects(
+    () =>
+      dispatchCommand(device, 'open', ['com.example.app'], undefined, {
+        launchArgs: ['--fixture', 'demo'],
+      }),
+    (error: unknown) => {
+      assert.equal(error instanceof AppError, true);
+      assert.equal((error as AppError).code, 'UNSUPPORTED_OPERATION');
+      assert.match((error as AppError).message, /Apple platforms/i);
+      return true;
+    },
+  );
+});
diff --git a/src/core/dispatch.ts b/src/core/dispatch.ts
@@ -225,6 +225,12 @@ async function handleOpenCommand(
   if (launchConsole && isDeepLinkTarget(app)) {
     throw new AppError('INVALID_ARGS', LAUNCH_CONSOLE_DIRECT_APP_ONLY_MESSAGE);
   }
+  if (device.platform === 'android' && context?.launchArgs && context.launchArgs.length > 0) {
+    throw new AppError(
+      'UNSUPPORTED_OPERATION',
+      'Launch arguments are currently supported only on Apple platforms.',
+    );
+  }
   if (context?.maestroClearState) {
     if (isDeepLinkTarget(app)) {
       throw new AppError(
diff --git a/src/daemon/handlers/__tests__/session-replay-vars.test.ts b/src/daemon/handlers/__tests__/session-replay-vars.test.ts
@@ -480,10 +480,10 @@ test('runReplayScriptFile retries Maestro scrollUntilVisible with scroll probes'
     [
       ['wait', ['label="Discover" || text="Discover" || id="Discover"', '500']],
       ['find', ['Discover', 'wait', '500']],
-      ['scroll', ['down']],
+      ['scroll', ['up']],
       ['wait', ['label="Discover" || text="Discover" || id="Discover"', '500']],
       ['find', ['Discover', 'wait', '500']],
-      ['scroll', ['down']],
+      ['scroll', ['up']],
       ['wait', ['label="Discover" || text="Discover" || id="Discover"', '200']],
     ],
   );
@@ -728,6 +728,61 @@ test('runReplayScriptFile skips Maestro runFlow.when.visible commands when absen
   );
 });
 
+test('runReplayScriptFile skips Maestro runFlow.when.visible commands on false predicate', async () => {
+  const calls: CapturedInvocation[] = [];
+  const { response } = await runReplayFixture({
+    label: 'maestro-run-flow-when-visible-false-skip',
+    script: [
+      'appId: demo.app',
+      '---',
+      '- runFlow:',
+      '    when:',
+      '      visible: Continue',
+      '    commands:',
+      '      - tapOn: Continue',
+      '',
+    ].join('\n'),
+    flags: { replayBackend: 'maestro' },
+    invoke: async (req) => {
+      calls.push({ command: req.command, positionals: req.positionals, flags: req.flags });
+      return { ok: true, data: { pass: false } };
+    },
+  });
+
+  assert.equal(response.ok, true);
+  assert.deepEqual(
+    calls.map((call) => [call.command, call.positionals]),
+    [['is', ['visible', 'label="Continue" || text="Continue" || id="Continue"']]],
+  );
+});
+
+test('runReplayScriptFile propagates Maestro runFlow.when runtime errors', async () => {
+  const { response } = await runReplayFixture({
+    label: 'maestro-run-flow-when-visible-runtime-error',
+    script: [
+      'appId: demo.app',
+      '---',
+      '- runFlow:',
+      '    when:',
+      '      visible: Continue',
+      '    commands:',
+      '      - tapOn: Continue',
+      '',
+    ].join('\n'),
+    flags: { replayBackend: 'maestro' },
+    invoke: async () => ({
+      ok: false,
+      error: { code: 'UNKNOWN', message: 'fetch failed' },
+    }),
+  });
+
+  assert.equal(response.ok, false);
+  if (!response.ok) {
+    assert.equal(response.error.code, 'UNKNOWN');
+    assert.match(response.error.message, /fetch failed/);
+  }
+});
+
 test('runReplayScriptFile runs Maestro runFlow.when.visible commands when present', async () => {
   const calls: CapturedInvocation[] = [];
   const { response } = await runReplayFixture({
@@ -766,6 +821,42 @@ test('runReplayScriptFile runs Maestro runFlow.when.visible commands when presen
   );
 });
 
+test('runReplayScriptFile runs nested Maestro runtime commands inside runFlow.when', async () => {
+  const calls: CapturedInvocation[] = [];
+  const { response } = await runReplayFixture({
+    label: 'maestro-run-flow-when-nested-runtime',
+    script: [
+      'appId: demo.app',
+      '---',
+      '- runFlow:',
+      '    when:',
+      '      visible: Feed',
+      '    commands:',
+      '      - scrollUntilVisible:',
+      '          element: Done',
+      '          direction: DOWN',
+      '          timeout: 500',
+      '',
+    ].join('\n'),
+    flags: { replayBackend: 'maestro' },
+    invoke: async (req) => {
+      calls.push({ command: req.command, positionals: req.positionals, flags: req.flags });
+      if (req.command === 'is') return { ok: true, data: { pass: true } };
+      if (req.command === 'wait') return { ok: true, data: { found: true } };
+      return { ok: true, data: {} };
+    },
+  });
+
+  assert.equal(response.ok, true);
+  assert.deepEqual(
+    calls.map((call) => [call.command, call.positionals]),
+    [
+      ['is', ['visible', 'label="Feed" || text="Feed" || id="Feed"']],
+      ['wait', ['label="Done" || text="Done" || id="Done"', '500']],
+    ],
+  );
+});
+
 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-maestro-runtime.ts b/src/daemon/handlers/session-replay-maestro-runtime.ts
diff --git a/src/utils/command-schema.ts b/src/utils/command-schema.ts
diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md

Original file line number	Diff line number	Diff line change
`@@ -236,7 +236,10 @@ extension RunnerTests {`
`236`	`236`	`return false`
`237`	`237`	`}`
`238`	`238`	`let appFrame = app.frame`
`239`		`- return appFrame.isEmpty \|\| appFrame.intersects(frame)`
	`239`	`+ if appFrame.isEmpty {`
	`240`	`+ return true`
	`241`	`+ }`
	`242`	`+ return appFrame.contains(CGPoint(x: frame.midX, y: frame.midY))`
`240`	`243`	`}`
`241`	`244`
`242`	`245`	`func queryElement(app: XCUIApplication, selectorKey: String, selectorValue: String) -> Response {`