fix(ui-automation): Tighten runtime snapshot review issues

cameroncooke · codex · cameroncooke · commit 89358faf7c13 · 2026-05-19T01:34:46.000+01:00
Keep next-step guidance available for unchanged runtime snapshots, cap
compact runtime snapshot rows, and simplify runtime snapshot rendering
branches called out by review.

Add direct semantic tap helper coverage for selector choice, duplicate
selector fallback, switch touch batching, and recoverable AXe fallback.

Co-Authored-By: Codex &lt;noreply@openai.com&gt;
diff --git a/src/mcp/tools/ui-automation/__tests__/semantic-tap.test.ts b/src/mcp/tools/ui-automation/__tests__/semantic-tap.test.ts
@@ -0,0 +1,138 @@
+import { describe, expect, it } from 'vitest';
+import { mockProcess } from '../../../../test-utils/mock-executors.ts';
+import type { CommandExecutor } from '../../../../utils/execution/index.ts';
+import { createRuntimeSnapshotRecord } from '../shared/runtime-snapshot.ts';
+import {
+  createSemanticTapBatchSteps,
+  createSemanticTapCommand,
+  executeSemanticTapWithAmbiguityFallback,
+  isRecoverableAxeSelectorError,
+} from '../shared/semantic-tap.ts';
+import {
+  createMockAxeHelpers,
+  createNode,
+  createSequencedExecutor,
+  simulatorId,
+} from './ui-action-test-helpers.ts';
+
+function createElements(nodes = [createNode()]) {
+  return createRuntimeSnapshotRecord({ simulatorId, uiHierarchy: nodes, nowMs: 1_000 }).elements;
+}
+
+describe('semantic tap helpers', () => {
+  it('recognizes recoverable AXe selector failures', () => {
+    expect(
+      isRecoverableAxeSelectorError(
+        new Error('Multiple (2) accessibility elements matched selector'),
+      ),
+    ).toBe(true);
+    expect(
+      isRecoverableAxeSelectorError({
+        axeOutput: 'No accessibility element matched --label Continue',
+      }),
+    ).toBe(true);
+    expect(isRecoverableAxeSelectorError(new Error('Simulator is not booted'))).toBe(false);
+  });
+
+  it('uses a unique semantic selector before coordinates', () => {
+    const [element] = createElements([
+      createNode({ AXUniqueId: 'continue.button', AXLabel: 'Continue' }),
+    ]);
+
+    const command = createSemanticTapCommand(element!, 'e1', ['--duration', '0.1'], [element!]);
+
+    expect(command.selectorArgs).toEqual([
+      'tap',
+      '--id',
+      'continue.button',
+      '--element-type',
+      'Button',
+      '--duration',
+      '0.1',
+    ]);
+    expect(command.primaryArgs).toBe(command.selectorArgs);
+    expect(command.usedSelector).toBe(true);
+  });
+
+  it('falls back to coordinates when semantic selectors are duplicated', () => {
+    const elements = createElements([
+      createNode({ AXUniqueId: 'duplicate.button', AXLabel: 'Duplicate' }),
+      createNode({
+        AXUniqueId: 'duplicate.button',
+        AXLabel: 'Duplicate',
+        frame: { x: 20, y: 80, width: 100, height: 40 },
+      }),
+    ]);
+
+    const command = createSemanticTapCommand(elements[0]!, 'e1', [], elements);
+
+    expect(command.selectorArgs).toBeNull();
+    expect(command.primaryArgs).toEqual(['tap', '-x', '60', '-y', '40']);
+    expect(command.usedSelector).toBe(false);
+  });
+
+  it('represents switch taps as down/up touch batch steps', () => {
+    const [element] = createElements([
+      createNode({
+        type: 'Switch',
+        role: 'AXSwitch',
+        AXLabel: 'Alerts',
+        frame: { x: 10, y: 20, width: 200, height: 40 },
+      }),
+    ]);
+
+    const command = createSemanticTapCommand(element!, 'e1');
+
+    expect(command.selectorArgs).toBeNull();
+    expect(command.coordinateArgs).toEqual(['touch', '-x', '158', '-y', '40', '--down', '--up']);
+    expect(createSemanticTapBatchSteps(command)).toEqual([
+      'touch -x 158 -y 40 --down',
+      'touch -x 158 -y 40 --up',
+    ]);
+  });
+
+  it('retries recoverable selector failures with coordinates', async () => {
+    const [element] = createElements([
+      createNode({ AXUniqueId: 'continue.button', AXLabel: 'Continue' }),
+    ]);
+    const command = createSemanticTapCommand(element!, 'e1', [], [element!]);
+    const { calls, executor } = createSequencedExecutor([
+      { success: false, error: 'Multiple (2) accessibility elements matched selector' },
+      { success: true, output: 'ok' },
+    ]);
+
+    await executeSemanticTapWithAmbiguityFallback({
+      command,
+      simulatorId,
+      executor,
+      axeHelpers: createMockAxeHelpers(),
+    });
+
+    expect(calls.map((call) => call.command.slice(1, -2))).toEqual([
+      ['tap', '--id', 'continue.button', '--element-type', 'Button'],
+      ['tap', '-x', '60', '-y', '40'],
+    ]);
+  });
+
+  it('does not retry unrecoverable selector failures', async () => {
+    const [element] = createElements([
+      createNode({ AXUniqueId: 'continue.button', AXLabel: 'Continue' }),
+    ]);
+    const command = createSemanticTapCommand(element!, 'e1', [], [element!]);
+    const calls: string[][] = [];
+    const executor: CommandExecutor = async (commandArgs) => {
+      calls.push(commandArgs);
+      return { success: false, output: '', error: 'Simulator is not booted', process: mockProcess };
+    };
+
+    await expect(
+      executeSemanticTapWithAmbiguityFallback({
+        command,
+        simulatorId,
+        executor,
+        axeHelpers: createMockAxeHelpers(),
+      }),
+    ).rejects.toThrow("axe command 'tap' failed.");
+    expect(calls).toHaveLength(1);
+  });
+});
diff --git a/src/mcp/tools/ui-automation/__tests__/snapshot_ui.test.ts b/src/mcp/tools/ui-automation/__tests__/snapshot_ui.test.ts
@@ -239,7 +239,29 @@ describe('Snapshot UI Plugin', () => {
         seq: 2,
       });
       expect(getRuntimeSnapshot('12345678-1234-4234-8234-123456789012')?.seq).toBe(2);
-      expect(second.ctx.nextSteps?.find((step) => step.tool === 'tap')).toBeUndefined();
+      expect(second.ctx.nextSteps).toEqual([
+        {
+          label: 'Refresh after layout changes',
+          tool: 'snapshot_ui',
+          params: { simulatorId: '12345678-1234-4234-8234-123456789012' },
+        },
+        {
+          label: 'Wait for UI to settle',
+          tool: 'wait_for_ui',
+          params: {
+            simulatorId: '12345678-1234-4234-8234-123456789012',
+            predicate: 'settled',
+          },
+        },
+        {
+          label: 'Tap an elementRef',
+          tool: 'tap',
+          params: {
+            simulatorId: '12345678-1234-4234-8234-123456789012',
+            elementRef: 'e1',
+          },
+        },
+      ]);
     });
 
     it('should return full runtime snapshot when sinceScreenHash differs from the current screen hash', async () => {
diff --git a/src/mcp/tools/ui-automation/snapshot_ui.ts b/src/mcp/tools/ui-automation/snapshot_ui.ts
@@ -11,11 +11,12 @@ import {
   getHandlerContext,
   toInternalSchema,
 } from '../../../utils/typed-tool-factory.ts';
-import { recordRuntimeSnapshot } from './shared/snapshot-ui-state.ts';
+import { getRuntimeSnapshot, recordRuntimeSnapshot } from './shared/snapshot-ui-state.ts';
 import { executeAxeCommand, defaultAxeHelpers } from './shared/axe-command.ts';
 import type { AxeHelpers } from './shared/axe-command.ts';
 import type { CaptureResultDomainResult } from '../../../types/domain-results.ts';
 import type { NonStreamingExecutor } from '../../../types/tool-execution.ts';
+import type { RuntimeSnapshotV1 } from '../../../types/ui-snapshot.ts';
 import { createRuntimeSnapshotNextSteps } from './shared/runtime-next-steps.ts';
 import {
   createCaptureFailureResult,
@@ -137,17 +138,21 @@ export async function snapshot_uiLogic(
 
   setCaptureStructuredOutput(ctx, result);
 
-  if (
-    !result.didError &&
-    result.capture &&
-    'type' in result.capture &&
-    result.capture.type === 'runtime-snapshot'
-  ) {
-    ctx.nextSteps = createRuntimeSnapshotNextSteps({
-      simulatorId: params.simulatorId,
-      runtimeSnapshot: result.capture,
-      includeRefreshAndWait: true,
-    });
+  if (!result.didError && result.capture && 'type' in result.capture) {
+    let runtimeSnapshot: RuntimeSnapshotV1 | undefined;
+    if (result.capture.type === 'runtime-snapshot') {
+      runtimeSnapshot = result.capture;
+    } else if (result.capture.type === 'runtime-snapshot-unchanged') {
+      runtimeSnapshot = getRuntimeSnapshot(params.simulatorId)?.payload;
+    }
+
+    if (runtimeSnapshot) {
+      ctx.nextSteps = createRuntimeSnapshotNextSteps({
+        simulatorId: params.simulatorId,
+        runtimeSnapshot,
+        includeRefreshAndWait: true,
+      });
+    }
   }
 }
 
diff --git a/src/utils/__tests__/structured-output-envelope.test.ts b/src/utils/__tests__/structured-output-envelope.test.ts
@@ -138,6 +138,58 @@ describe('toStructuredEnvelope', () => {
     });
   });
 
+  it('caps compact runtime snapshot rows by category', () => {
+    const targets = Array.from({ length: 80 }, (_, index) => ({
+      ref: `e${index + 1}`,
+      role: 'button' as const,
+      label: `Target ${index + 1}`,
+      frame: { x: 0, y: index, width: 100, height: 40 },
+      actions: ['tap' as const],
+    }));
+    const scroll = Array.from({ length: 40 }, (_, index) => ({
+      ref: `e${index + 81}`,
+      role: 'scroll-view' as const,
+      label: `Scroll ${index + 1}`,
+      frame: { x: 0, y: index, width: 390, height: 600 },
+      actions: ['swipeWithin' as const],
+    }));
+    const text = Array.from({ length: 70 }, (_, index) => ({
+      ref: `e${index + 121}`,
+      role: 'text' as const,
+      label: `Text ${index + 1}`,
+      frame: { x: 0, y: index, width: 100, height: 20 },
+      state: { visible: true },
+      actions: ['touch' as const],
+    }));
+    const result: CaptureResultDomainResult = {
+      kind: 'capture-result',
+      didError: false,
+      error: null,
+      summary: { status: 'SUCCEEDED' },
+      artifacts: { simulatorId: 'SIMULATOR-1' },
+      capture: {
+        type: 'runtime-snapshot',
+        protocol: 'rs/1',
+        simulatorId: 'SIMULATOR-1',
+        screenHash: 'large-screen',
+        seq: 4,
+        capturedAtMs: 1_000,
+        expiresAtMs: 61_000,
+        elements: [...targets, ...scroll, ...text],
+        actions: [],
+      },
+    };
+
+    const envelope = toStructuredEnvelope(result, 'xcodebuildmcp.output.capture-result', '2');
+    const data = envelope.data as {
+      capture: { targets: string[]; scroll: string[]; text?: string[] };
+    };
+
+    expect(data.capture.targets).toHaveLength(64);
+    expect(data.capture.scroll).toHaveLength(32);
+    expect(data.capture.text).toHaveLength(64);
+  });
+
   it('compacts unchanged runtime snapshot captures by default', () => {
     const result: CaptureResultDomainResult = {
       kind: 'capture-result',
diff --git a/src/utils/renderers/domain-result-text.ts b/src/utils/renderers/domain-result-text.ts
@@ -1249,16 +1249,24 @@ function sortRuntimeTargetsForDisplay(elements: RuntimeElementV1[]): RuntimeElem
     .map(({ element }) => element);
 }
 
+function getPrimaryRuntimeElementAction(element: RuntimeElementV1, action?: string): string {
+  if (action) {
+    return action;
+  }
+  if (element.actions.includes('typeText')) {
+    return 'typeText';
+  }
+  if (element.actions.includes('tap')) {
+    return 'tap';
+  }
+  if (element.actions.includes('swipeWithin')) {
+    return 'swipe';
+  }
+  return 'none';
+}
+
 function formatRuntimeElementLine(element: RuntimeElementV1, action?: string): string {
-  const primaryAction =
-    action ??
-    (element.actions.includes('typeText')
-      ? 'typeText'
-      : element.actions.includes('tap')
-        ? 'tap'
-        : element.actions.includes('swipeWithin')
-          ? 'swipe'
-          : 'none');
+  const primaryAction = getPrimaryRuntimeElementAction(element, action);
   return [
     element.ref,
     primaryAction,
@@ -1466,17 +1474,14 @@ function createCaptureResultItems(
   if (result.didError) {
     items.push(...createStandardDiagnosticSections(result.diagnostics));
     items.push(...createUiErrorItems(result.uiError));
-    items.push(
-      createStatus(
-        'error',
-        result.error ??
-          (isUiHierarchy
-            ? isRuntimeSnapshot
-              ? 'Failed to get runtime UI snapshot.'
-              : 'Failed to get accessibility hierarchy.'
-            : 'Failed to capture screenshot.'),
-      ),
-    );
+    let fallbackError = 'Failed to capture screenshot.';
+    if (isRuntimeSnapshot) {
+      fallbackError = 'Failed to get runtime UI snapshot.';
+    } else if (isUiHierarchy) {
+      fallbackError = 'Failed to get accessibility hierarchy.';
+    }
+
+    items.push(createStatus('error', result.error ?? fallbackError));
     return items;
   }
 
diff --git a/src/utils/structured-output-envelope.ts b/src/utils/structured-output-envelope.ts
