refactor: converge Maestro assertion waits

Author: thymikee

src/compat/maestro/__tests__/replay-flow.test.ts

@@ -330,6 +330,21 @@ test('parseMaestroReplayFlow preserves selector state and absolute swipe command
   assert.deepEqual(parsed.actionLines, [3, 6]);
 });
 
+test('parseMaestroReplayFlow maps extendedWaitUntil.notVisible through Maestro visibility assertions', () => {
+  const parsed = parseMaestroReplayFlow(`appId: com.callstack.agentdevicelab
+---
+- extendedWaitUntil:
+    notVisible:
+      text: Loading
+    timeout: 1200
+`);
+
+  assert.deepEqual(
+    parsed.actions.map((entry) => [entry.command, entry.positionals]),
+    [['__maestroAssertNotVisible', ['label="Loading" || text="Loading" || id="Loading"', '1200']]],
+  );
+});
+
 test('parseMaestroReplayFlow rejects deferred Maestro utility commands loudly', () => {
   assert.throws(
     () => parseMaestroReplayFlow('---\n- assertTrue: "${READY}"\n'),

src/compat/maestro/__tests__/runtime-assertions.test.ts

@@ -1,6 +1,9 @@
 import assert from 'node:assert/strict';
 import { afterEach, test, vi } from 'vitest';
-import { invokeMaestroAssertNotVisible, invokeMaestroAssertVisible } from '../runtime-assertions.ts';
+import {
+  invokeMaestroAssertNotVisible,
+  invokeMaestroAssertVisible,
+} from '../runtime-assertions.ts';
 import type { DaemonRequest, DaemonResponse } from '../../../daemon/types.ts';
 
 afterEach(() => {
@@ -80,9 +83,10 @@ test('invokeMaestroAssertNotVisible passes after a slow hidden sample exhausts t
   });
 
   assert.equal(response.ok, true);
-  assert.deepEqual(calls.map((call) => [call.command, call.positionals]), [
-    ['snapshot', []],
-  ]);
+  assert.deepEqual(
+    calls.map((call) => [call.command, call.positionals]),
+    [['snapshot', []]],
+  );
   if (response.ok) {
     assert.ok(response.data);
     assert.equal(response.data.stableSamples, 1);
@@ -125,3 +129,30 @@ test('invokeMaestroAssertNotVisible ignores matched nodes without visible rects'
     assert.equal(response.data.stableSamples, 1);
   }
 });
+
+test('invokeMaestroAssertNotVisible accepts timeout overrides for short extended waits', async () => {
+  vi.spyOn(Date, 'now').mockReturnValueOnce(0).mockReturnValueOnce(0).mockReturnValueOnce(300);
+
+  const response = await invokeMaestroAssertNotVisible({
+    baseReq: {
+      token: 't',
+      session: 's',
+      flags: {},
+    },
+    positionals: ['id="toast"', '1'],
+    invoke: async (): Promise<DaemonResponse> => ({
+      ok: true,
+      data: {
+        createdAt: 1,
+        nodes: [],
+      },
+    }),
+  });
+
+  assert.equal(response.ok, true);
+  if (response.ok) {
+    assert.ok(response.data);
+    assert.equal(response.data.stableSamples, 1);
+    assert.equal(response.data.timeoutMs, 1);
+  }
+});

src/compat/maestro/interactions.ts

@@ -142,7 +142,7 @@ export function convertExtendedWaitUntil(
   const selector = maestroSelector(target, 'extendedWaitUntil', [], context);
   const timeoutMs = String(readTimeoutMs(value, 30000));
   if (value.notVisible !== undefined) {
-    return [action('wait', [timeoutMs]), action('is', ['hidden', selector])];
+    return [action(MAESTRO_RUNTIME_COMMAND.assertNotVisible, [selector, timeoutMs])];
   }
   return [action(MAESTRO_RUNTIME_COMMAND.assertVisible, [selector, timeoutMs])];
 }

src/compat/maestro/runtime-assertions.ts

@@ -20,31 +20,48 @@ const MAESTRO_ASSERTION_POLICY = {
   assertVisibleGraceMs: 1000,
   assertVisiblePollMs: 250,
   assertNotVisiblePollMs: 250,
-  assertNotVisibleTimeoutMs: 3000,
+  defaultAssertNotVisibleTimeoutMs: 3000,
 } as const;
 
+type MaestroVisibilitySample =
+  | { visible: true; response: DaemonResponse }
+  | { visible: false; response: DaemonResponse; infrastructureFailure: boolean };
+
 export async function invokeMaestroAssertVisible(params: {
   baseReq: ReplayBaseRequest;
   positionals: string[];
   invoke: MaestroRuntimeInvoke;
   scope?: ReplayVarScope;
 }): Promise<DaemonResponse> {
-  const args = readAssertVisibleArgs(params.positionals);
+  const args = readVisibilityAssertionArgs(params.positionals, {
+    command: 'assertVisible',
+    defaultTimeoutMs: 5000,
+  });
   if (!args.ok) return args.response;
 
+  // Native wait/is cannot replace this loop: wait only proves existence, while
+  // is requires unique resolution and does not apply Maestro overlay filtering.
   const startedAt = Date.now();
   const deadlineMs = args.timeoutMs + MAESTRO_ASSERTION_POLICY.assertVisibleGraceMs;
   let lastResponse: DaemonResponse | undefined;
   let capturedAfterDeadline = false;
   while (true) {
     const captureStartedAt = Date.now();
-    const attempt = await readAssertVisibleAttempt(params, args.selector, startedAt);
-    if (attempt.done) return attempt.response;
-    lastResponse = attempt.response;
+    const sample = await readMaestroVisibilitySample(params, args.selector, 'assertVisible');
+    if (sample.visible) return visibleAssertionResponse(sample.response, args.selector, startedAt);
+    if (!sample.visible && sample.infrastructureFailure) return sample.response;
+    lastResponse = sample.response;
 
     const elapsedMs = Date.now() - startedAt;
     if (elapsedMs >= deadlineMs) {
-      if (shouldCaptureOnceAfterDeadline(capturedAfterDeadline, captureStartedAt, startedAt, deadlineMs)) {
+      if (
+        shouldCaptureOnceAfterDeadline(
+          capturedAfterDeadline,
+          captureStartedAt,
+          startedAt,
+          deadlineMs,
+        )
+      ) {
         capturedAfterDeadline = true;
         continue;
       }
@@ -62,42 +79,47 @@ export async function invokeMaestroAssertVisible(params: {
   );
 }
 
-function readAssertVisibleArgs(
+function readVisibilityAssertionArgs(
   positionals: string[],
-):
-  | { ok: true; selector: string; timeoutMs: number }
-  | { ok: false; response: DaemonResponse } {
-  const [selector, timeoutValue = '5000'] = positionals;
+  options: { command: string; defaultTimeoutMs: number },
+): { ok: true; selector: string; timeoutMs: number } | { ok: false; response: DaemonResponse } {
+  const [selector, timeoutValue = String(options.defaultTimeoutMs)] = positionals;
   if (!selector) {
-    return { ok: false, response: errorResponse('INVALID_ARGS', 'assertVisible requires a selector.') };
+    return {
+      ok: false,
+      response: errorResponse('INVALID_ARGS', `${options.command} requires a selector.`),
+    };
   }
   const timeoutMs = Number(timeoutValue);
   if (!Number.isFinite(timeoutMs) || timeoutMs < 0) {
     return {
       ok: false,
-      response: errorResponse('INVALID_ARGS', 'assertVisible timeout must be a non-negative number.'),
+      response: errorResponse(
+        'INVALID_ARGS',
+        `${options.command} timeout must be a non-negative number.`,
+      ),
     };
   }
   return { ok: true, selector, timeoutMs };
 }
 
-async function readAssertVisibleAttempt(
+async function readMaestroVisibilitySample(
   params: {
     baseReq: ReplayBaseRequest;
-    positionals: string[];
     invoke: MaestroRuntimeInvoke;
     scope?: ReplayVarScope;
   },
   selector: string,
-  startedAt: number,
-): Promise<{ done: true; response: DaemonResponse } | { done: false; response: DaemonResponse }> {
+  command: string,
+): Promise<MaestroVisibilitySample> {
   const response = await captureMaestroRawSnapshot(params);
-  if (!response.ok) return { done: false, response };
+  if (!response.ok) return { visible: false, response, infrastructureFailure: true };
   const snapshot = readSnapshotState(response.data);
   if (!snapshot) {
     return {
-      done: true,
-      response: errorResponse('COMMAND_FAILED', 'Unable to read snapshot data for assertVisible.'),
+      visible: false,
+      response: errorResponse('COMMAND_FAILED', `Unable to read snapshot data for ${command}.`),
+      infrastructureFailure: true,
     };
   }
   const target = resolveVisibleMaestroNodeFromSnapshot(
@@ -107,10 +129,14 @@ async function readAssertVisibleAttempt(
     getSnapshotReferenceFrame(snapshot),
   );
   if (!target.ok) {
-    return { done: false, response: errorResponse('COMMAND_FAILED', target.message, { selector }) };
+    return {
+      visible: false,
+      response: errorResponse('COMMAND_FAILED', target.message, { selector }),
+      infrastructureFailure: false,
+    };
   }
   return {
-    done: true,
+    visible: true,
     response: {
       ok: true,
       data: {
@@ -121,12 +147,27 @@ async function readAssertVisibleAttempt(
         nodeLabel: target.node.label,
         nodeIdentifier: target.node.identifier,
         rect: target.rect,
-        waitedMs: Date.now() - startedAt,
       },
     },
   };
 }
 
+function visibleAssertionResponse(
+  response: DaemonResponse,
+  selector: string,
+  startedAt: number,
+): DaemonResponse {
+  if (!response.ok) return response;
+  return {
+    ok: true,
+    data: {
+      selector,
+      ...response.data,
+      waitedMs: Date.now() - startedAt,
+    },
+  };
+}
+
 function shouldCaptureOnceAfterDeadline(
   capturedAfterDeadline: boolean,
   captureStartedAt: number,
@@ -141,99 +182,59 @@ export async function invokeMaestroAssertNotVisible(params: {
   positionals: string[];
   invoke: MaestroRuntimeInvoke;
 }): Promise<DaemonResponse> {
-  const [selector] = params.positionals;
-  if (!selector) {
-    return errorResponse('INVALID_ARGS', 'assertNotVisible requires a selector.');
-  }
+  const args = readVisibilityAssertionArgs(params.positionals, {
+    command: 'assertNotVisible',
+    defaultTimeoutMs: MAESTRO_ASSERTION_POLICY.defaultAssertNotVisibleTimeoutMs,
+  });
+  if (!args.ok) return args.response;
+
+  // Native is hidden intentionally fails for absent selectors. Maestro
+  // assertNotVisible treats absent and overlay-blocked targets as passing, so
+  // this loop shares the visible resolver instead of delegating to native is.
   const startedAt = Date.now();
   let hiddenSamples = 0;
   let lastVisibleResponse: DaemonResponse | undefined;
-  while (Date.now() - startedAt <= MAESTRO_ASSERTION_POLICY.assertNotVisibleTimeoutMs) {
-    const attempt = await readAssertNotVisibleAttempt(params, selector);
-    if (attempt.visible) {
+  while (Date.now() - startedAt <= args.timeoutMs) {
+    const sample = await readMaestroVisibilitySample(params, args.selector, 'assertNotVisible');
+    if (!sample.visible && sample.infrastructureFailure) return sample.response;
+    if (sample.visible) {
       hiddenSamples = 0;
-      lastVisibleResponse = attempt.response;
-    } else if (attempt.hidden) {
+      lastVisibleResponse = sample.response;
+    } else {
       hiddenSamples += 1;
       const waitedMs = Date.now() - startedAt;
-      if (
-        hiddenSamples >= 2 ||
-        waitedMs >= MAESTRO_ASSERTION_POLICY.assertNotVisibleTimeoutMs
-      ) {
+      if (hiddenSamples >= 2 || waitedMs >= args.timeoutMs) {
         return {
           ok: true,
           data: {
             pass: true,
-            selector,
+            selector: args.selector,
             stableSamples: hiddenSamples,
             waitedMs,
-            timeoutMs: MAESTRO_ASSERTION_POLICY.assertNotVisibleTimeoutMs,
+            timeoutMs: args.timeoutMs,
           },
         };
       }
-    } else {
-      return attempt.response;
     }
     await sleep(MAESTRO_ASSERTION_POLICY.assertNotVisiblePollMs);
   }
-  return errorResponse('COMMAND_FAILED', `Expected not visible but matched: ${selector}`, {
-    selector,
-    timeoutMs: MAESTRO_ASSERTION_POLICY.assertNotVisibleTimeoutMs,
-    lastResponse: lastVisibleResponse,
-  });
-}
-
-async function readAssertNotVisibleAttempt(
-  params: {
-    baseReq: ReplayBaseRequest;
-    positionals: string[];
-    invoke: MaestroRuntimeInvoke;
-  },
-  selector: string,
-): Promise<
-  | { visible: true; hidden: false; response: DaemonResponse }
-  | { visible: false; hidden: true; response: DaemonResponse }
-  | { visible: false; hidden: false; response: DaemonResponse }
-> {
-  const response = await captureMaestroRawSnapshot(params);
-  if (!response.ok) return { visible: false, hidden: false, response };
-  const snapshot = readSnapshotState(response.data);
-  if (!snapshot) {
+  if (hiddenSamples > 0) {
     return {
-      visible: false,
-      hidden: false,
-      response: errorResponse('COMMAND_FAILED', 'Unable to read snapshot data for assertNotVisible.'),
-    };
-  }
-  const target = resolveVisibleMaestroNodeFromSnapshot(
-    snapshot,
-    selector,
-    readMaestroSelectorPlatform(params.baseReq.flags),
-    getSnapshotReferenceFrame(snapshot),
-  );
-  if (!target.ok) {
-    return {
-      visible: false,
-      hidden: true,
-      response: errorResponse('COMMAND_FAILED', target.message, { selector }),
-    };
-  }
-  return {
-    visible: true,
-    hidden: false,
-    response: {
       ok: true,
       data: {
-        selector,
-        matches: target.matches,
-        nodeIndex: target.node.index,
-        nodeType: target.node.type,
-        nodeLabel: target.node.label,
-        nodeIdentifier: target.node.identifier,
-        rect: target.rect,
+        pass: true,
+        selector: args.selector,
+        stableSamples: hiddenSamples,
+        waitedMs: Date.now() - startedAt,
+        timeoutMs: args.timeoutMs,
       },
-    },
-  };
+    };
+  }
+  return errorResponse('COMMAND_FAILED', `Expected not visible but matched: ${args.selector}`, {
+    selector: args.selector,
+    timeoutMs: args.timeoutMs,
+    lastResponse: lastVisibleResponse,
+  });
 }
 
 export async function invokeMaestroWaitForAnimationToEnd(params: {
@@ -245,6 +246,8 @@ export async function invokeMaestroWaitForAnimationToEnd(params: {
   if (!Number.isFinite(timeoutMs) || timeoutMs < 0) {
     return errorResponse('INVALID_ARGS', 'waitForAnimationToEnd timeout must be a number.');
   }
+  // There is no native wait/is equivalent for "animation has ended"; this is
+  // snapshot stability polling by design.
   const startedAt = Date.now();
   let previousSignature: string | undefined;
   let lastResponse: DaemonResponse | undefined;