fix: align maestro tap runtime semantics

Author: thymikee

docs/adr/0015-direct-maestro-engine.md

@@ -87,10 +87,10 @@ matches. It does not adopt the public agent-device command surface's unique-matc
 suppressed by Maestro `optional`; atomic iOS dispatch handles that result by performing fresh Maestro
 resolution, as described above. Cancellation and infrastructure failures are likewise non-optional.
 
-`tapOn` preserves Maestro's two-attempt `retryIfNoChange` default for coordinate injection, including
-the live-bound iOS selector path and non-hittable fallback. After one compatibility polling interval it
-compares the hierarchy with the target-resolution hierarchy and retries once when nothing changed. The
-observed result primes the next command, so the outcome check does not add another hierarchy read there.
+Every `tapOn` waits for hierarchy stability before the next command, matching Maestro's tap boundary.
+`retryTapIfNoChange` defaults to false; when explicitly enabled, the runtime compares the stable hierarchy
+with the target-resolution hierarchy and retries once when nothing changed. The stable result primes the
+next command, so the tap boundary does not add another hierarchy read there.
 
 Successful gestures and scrolls require stabilization before the next command executes. The runtime
 port records that requirement without capturing a hierarchy in the gesture command itself. At the next

src/compat/maestro/__tests__/daemon-runtime-port-observation-input.test.ts

@@ -151,8 +151,5 @@ test('commits Maestro input text before dispatching an immediate tap', async ()
     'snapshot',
     'snapshot',
     'click',
-    'snapshot',
-    'click',
-    'snapshot',
   ]);
 });

src/compat/maestro/__tests__/daemon-runtime-port-target-geometry.test.ts

@@ -131,19 +131,10 @@ test('uses resolved iOS geometry when canonical presentation changes target boun
     invalidateObservation() {},
   });
 
-  expect(requests.map(({ command }) => command)).toEqual([
-    'snapshot',
-    'click',
-    'snapshot',
-    'click',
-    'snapshot',
-  ]);
+  expect(requests.map(({ command }) => command)).toEqual(['snapshot', 'click']);
   expect(
     requests.filter(({ command }) => command === 'click').map(({ positionals }) => positionals),
-  ).toEqual([
-    ['56', '121'],
-    ['56', '121'],
-  ]);
+  ).toEqual([['56', '121']]);
 });
 
 test('atomically dispatches canonical iOS geometry with the same tap point', async () => {
@@ -260,19 +251,10 @@ test('does not collapse distinct nested iOS controls with identical frames', asy
     invalidateObservation() {},
   });
 
-  expect(requests.map(({ command }) => command)).toEqual([
-    'snapshot',
-    'click',
-    'snapshot',
-    'click',
-    'snapshot',
-  ]);
+  expect(requests.map(({ command }) => command)).toEqual(['snapshot', 'click']);
   expect(
     requests.filter(({ command }) => command === 'click').map(({ positionals }) => positionals),
-  ).toEqual([
-    ['106', '322'],
-    ['106', '322'],
-  ]);
+  ).toEqual([['106', '322']]);
 });
 
 test('does not atomically dispatch distinct iOS matches with identical frames', async () => {
@@ -312,17 +294,8 @@ test('does not atomically dispatch distinct iOS matches with identical frames',
     invalidateObservation() {},
   });
 
-  expect(requests.map(({ command }) => command)).toEqual([
-    'snapshot',
-    'click',
-    'snapshot',
-    'click',
-    'snapshot',
-  ]);
+  expect(requests.map(({ command }) => command)).toEqual(['snapshot', 'click']);
   expect(
     requests.filter(({ command }) => command === 'click').map(({ positionals }) => positionals),
-  ).toEqual([
-    ['106', '322'],
-    ['106', '322'],
-  ]);
+  ).toEqual([['106', '322']]);
 });

src/compat/maestro/__tests__/daemon-runtime-port-target-refresh.test.ts

@@ -79,7 +79,6 @@ test('falls back to fresh Maestro geometry when atomic iOS dispatch resolves off
     'click',
     'snapshot',
     'click',
-    'snapshot',
   ]);
   expect(requests.filter(({ command }) => command === 'click').at(-1)?.positionals).toEqual([
     '220',
@@ -149,20 +148,10 @@ test.each(['ios', 'android'] as const)(
       invalidateObservation() {},
     });
 
-    expect(requests.map((request) => request.command)).toEqual([
-      'snapshot',
-      'snapshot',
-      'click',
-      'snapshot',
-      'click',
-      'snapshot',
-    ]);
+    expect(requests.map((request) => request.command)).toEqual(['snapshot', 'snapshot', 'click']);
     expect(
       requests.filter(({ command }) => command === 'click').map(({ positionals }) => positionals),
-    ).toEqual([
-      ['220', '122'],
-      ['220', '122'],
-    ]);
+    ).toEqual([['220', '122']]);
   },
 );
 

src/compat/maestro/__tests__/daemon-runtime-port-targets.test.ts

@@ -59,16 +59,10 @@ test('waits for a delayed input target using fresh snapshots', async () => {
     'snapshot',
     'snapshot',
     'click',
-    'snapshot',
-    'click',
-    'snapshot',
   ]);
   expect(
     requests.filter(({ command }) => command === 'click').map(({ positionals }) => positionals),
-  ).toEqual([
-    ['80', '62'],
-    ['80', '62'],
-  ]);
+  ).toEqual([['80', '62']]);
 });
 
 test('atomically dispatches a unique exact iOS target from same-generation evidence', async () => {
@@ -149,7 +143,7 @@ test('atomically dispatches a unique exact iOS target from same-generation evide
     }),
   ).resolves.toEqual({});
 
-  expect(requests.map((request) => request.command)).toEqual(['snapshot', 'click', 'snapshot']);
+  expect(requests.map((request) => request.command)).toEqual(['snapshot', 'click']);
   expect(observation.identity).toBeDefined();
   const click = requests.find((request) => request.command === 'click');
   expect(click?.positionals).toEqual(['id="continue"']);
@@ -211,6 +205,7 @@ test('retries an iOS non-hittable coordinate fallback when the hierarchy does no
       kind: 'tapOn',
       source: { line: 2 },
       target: { space: 'target', selector: { text: 'Pop to top' } },
+      retryTapIfNoChange: true,
     },
     generation: 0,
     env: {},
@@ -221,8 +216,10 @@ test('retries an iOS non-hittable coordinate fallback when the hierarchy does no
     'snapshot',
     'click',
     'snapshot',
+    'snapshot',
     'click',
     'snapshot',
+    'snapshot',
   ]);
   expect(clicks).toBe(2);
 });

src/compat/maestro/__tests__/program-ir-parser.test.ts

@@ -93,6 +93,7 @@ describe('parseMaestroProgram', () => {
     point: 20%, 30%
 - tapOn:
     id: submit
+    retryTapIfNoChange: true
 - doubleTapOn:
     point: 100,200
 - longPressOn:
@@ -128,6 +129,7 @@ describe('parseMaestroProgram', () => {
 
     assert.deepEqual(percentTapCommand.target, { space: 'percent', x: 20, y: 30 });
     assert.deepEqual(targetTapCommand.target, { space: 'target', selector: { id: 'submit' } });
+    assert.equal(targetTapCommand.retryTapIfNoChange, true);
     assert.deepEqual(absoluteDoubleTapCommand.target, { space: 'absolute', x: 100, y: 200 });
     assert.deepEqual(targetLongPressCommand.target, {
       space: 'target',

src/compat/maestro/compatibility-policy.ts

@@ -12,7 +12,7 @@ export const MAESTRO_COMPATIBILITY_PRESETS = {
     retryMaxRetries: 3,
   },
   command: {
-    tapMaxAttempts: 2,
+    retryTapMaxAttempts: 2,
     targetLookupTimeoutMs: 17_000,
     optionalTargetLookupTimeoutMs: 7_000,
     scrollUntilVisibleTimeoutMs: 20_000,

src/compat/maestro/daemon-runtime-port.ts

@@ -132,9 +132,12 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper
     },
 
     tapOn: async (input, context) =>
-      await tapTargetWithOutcome(options, snapshots, input.target, context, {
-        count: input.repeat,
-        intervalMs: input.delay,
+      await tapTargetAndSettle(options, snapshots, input.target, context, {
+        click: {
+          count: input.repeat,
+          intervalMs: input.delay,
+        },
+        retryIfNoChange: input.retryTapIfNoChange === true,
       }),
     doubleTapOn: async (input) => {
       snapshots.invalidate();
@@ -387,31 +390,42 @@ function createSnapshotSource(
   };
 }
 
-async function tapTargetWithOutcome(
+async function tapTargetAndSettle(
   options: CreateDaemonMaestroRuntimeOperationsOptions,
   snapshots: MaestroSnapshotSource,
   target: Parameters<MaestroRuntimeOperations['tapOn']>[0]['target'],
   context: MaestroRuntimeOperationContext,
-  flags: MaestroClickOptions,
+  policy: { click: MaestroClickOptions; retryIfNoChange: boolean },
 ): Promise<void> {
+  const dispatch = async () =>
+    await dispatchTapTarget(options, snapshots, target, context, policy.click);
+  if (!policy.retryIfNoChange) {
+    await dispatch();
+    snapshots.requireStability();
+    return;
+  }
+
   const baselineSignature =
     target.resolution?.surfaceSignature ??
     maestroSnapshotSignature(await snapshots.capture(context));
-  const dispatch = async () => await dispatchTapTarget(options, snapshots, target, context, flags);
+  const settle = async () =>
+    await waitForTypedSnapshotStability({
+      timeoutMs: MAESTRO_DEFAULT_SETTLE_TIMEOUT_MS,
+      context,
+      snapshot: snapshots.capture,
+      dependencies: options.dependencies,
+    });
 
   await dispatch();
-
-  await options.dependencies.sleep(MAESTRO_OBSERVATION_POLL_MS, context.signal);
-  let observed = await snapshots.capture(context);
+  let observed = await settle();
   let attempts = 1;
   while (
     maestroSnapshotSignature(observed) === baselineSignature &&
-    attempts < MAESTRO_COMPATIBILITY_PRESETS.command.tapMaxAttempts
+    attempts < MAESTRO_COMPATIBILITY_PRESETS.command.retryTapMaxAttempts
   ) {
     await dispatch();
     attempts += 1;
-    await options.dependencies.sleep(MAESTRO_OBSERVATION_POLL_MS, context.signal);
-    observed = await snapshots.capture(context);
+    observed = await settle();
   }
   snapshots.prime(context.generation + 1, observed);
 }

src/compat/maestro/program-ir-gesture-parser.ts

@@ -90,7 +90,12 @@ export function parseMaestroTapOnCommand(
 
   const entries = readMapEntries(value, 'tapOn', context);
   if (hasEntry(entries, 'point')) {
-    assertOnlyKeys(entries, 'tapOn', ['point', 'repeat', 'delay', 'optional', 'label'], context);
+    assertOnlyKeys(
+      entries,
+      'tapOn',
+      ['point', 'retryTapIfNoChange', 'repeat', 'delay', 'optional', 'label'],
+      context,
+    );
     return {
       kind: 'tapOn',
       source,
@@ -102,7 +107,7 @@ export function parseMaestroTapOnCommand(
   assertOnlyKeys(
     entries,
     'tapOn',
-    [...TAP_SELECTOR_KEYS, 'repeat', 'delay', 'optional', 'index', 'childOf'],
+    [...TAP_SELECTOR_KEYS, 'retryTapIfNoChange', 'repeat', 'delay', 'optional', 'index', 'childOf'],
     context,
   );
   const selectorEntries = entries.filter((entry) => isSelectorKey(entry.key, TAP_SELECTOR_KEYS));
@@ -344,7 +349,10 @@ function tapOptions(
   entries: readonly MaestroMapEntry[],
   context: MaestroProgramParseContext,
   includeLabel: boolean,
-): Pick<MaestroTapOnCommand, 'repeat' | 'delay' | 'optional' | 'label'> {
+): Pick<MaestroTapOnCommand, 'retryTapIfNoChange' | 'repeat' | 'delay' | 'optional' | 'label'> {
+  const retryTapIfNoChange = readOptionalEntry(entries, 'retryTapIfNoChange', (entry) =>
+    readOptionalBoolean(entry, 'tapOn.retryTapIfNoChange', context),
+  );
   const repeat = readOptionalEntry(entries, 'repeat', (entry) =>
     readRequiredPositiveInteger(entry, 'tapOn.repeat', context),
   );
@@ -358,6 +366,7 @@ function tapOptions(
       )
     : undefined;
   return {
+    ...(retryTapIfNoChange === undefined ? {} : { retryTapIfNoChange }),
     ...(repeat === undefined ? {} : { repeat }),
     ...(delay === undefined ? {} : { delay }),
     ...(optional === undefined ? {} : { optional }),

src/compat/maestro/program-ir.ts

@@ -54,6 +54,7 @@ export type MaestroTapOnCommand = MaestroOptionalCommand & {
   kind: 'tapOn';
   source: MaestroSourceLocation;
   target: MaestroGestureTarget;
+  retryTapIfNoChange?: boolean;
   repeat?: number;
   delay?: number;
   optional?: boolean;