fix(ios): avoid replaying alert mutations

Author: billsbooth

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift

@@ -0,0 +1,31 @@
+import Foundation
+import XCTest
+
+#if AGENT_DEVICE_RUNNER_UNIT_TESTS
+private struct AlertCommandTraitsFixture: Decodable {
+  let name: String
+  let command: Command
+  let readOnly: Bool
+}
+
+extension RunnerTests {
+  func testAlertReadOnlyClassificationMatchesGoldenTable() throws {
+    let fixtureURL = URL(fileURLWithPath: #filePath)
+      .deletingLastPathComponent()
+      .deletingLastPathComponent()
+      .deletingLastPathComponent()
+      .deletingLastPathComponent()
+      .deletingLastPathComponent()
+      .deletingLastPathComponent()
+      .appendingPathComponent("contracts/fixtures/alert-command-traits.json")
+    let cases = try JSONDecoder().decode(
+      [AlertCommandTraitsFixture].self,
+      from: Data(contentsOf: fixtureURL)
+    )
+    XCTAssertEqual(cases.map { $0.command.action }, [nil, "get", "accept", "dismiss"])
+    for fixture in cases {
+      XCTAssertEqual(isReadOnlyCommand(fixture.command), fixture.readOnly, fixture.name)
+    }
+  }
+}
+#endif

contracts/fixtures/alert-command-traits.json

@@ -0,0 +1,18 @@
+[
+  { "name": "default query", "command": { "command": "alert" }, "readOnly": true },
+  {
+    "name": "explicit query",
+    "command": { "command": "alert", "action": "get" },
+    "readOnly": true
+  },
+  {
+    "name": "accept mutates",
+    "command": { "command": "alert", "action": "accept" },
+    "readOnly": false
+  },
+  {
+    "name": "dismiss mutates",
+    "command": { "command": "alert", "action": "dismiss" },
+    "readOnly": false
+  }
+]

packages/platform-apple/src/runner/__tests__/runner-client.test.ts

@@ -375,14 +375,14 @@ test('withRunnerCommandId preserves existing command ids', () => {
 test('scroll is a mutating, command-id-tracked runner command', () => {
   // Runner command traits classify fused scroll as mutating, routing it through single-send
   // (no transport retry), command-id tracking, and status recovery.
-  assert.equal(isReadOnlyRunnerCommand('scroll'), false);
+  assert.equal(isReadOnlyRunnerCommand({ command: 'scroll' }), false);
 
   const command = withRunnerCommandId({ command: 'scroll', direction: 'down', pixels: 120 });
   assert.match(command.commandId ?? '', /^runner-/);
 });
 
 test('desktopScroll is a mutating, command-id-tracked runner command', () => {
-  assert.equal(isReadOnlyRunnerCommand('desktopScroll'), false);
+  assert.equal(isReadOnlyRunnerCommand({ command: 'desktopScroll' }), false);
 
   const command = withRunnerCommandId({
     command: 'desktopScroll',

packages/platform-apple/src/runner/__tests__/runner-command-traits.test.ts

@@ -1,4 +1,5 @@
 import assert from 'node:assert/strict';
+import fs from 'node:fs';
 import { test } from 'vitest';
 import type { RunnerCommand } from '../runner-contract.ts';
 import {
@@ -22,7 +23,7 @@ test('runner command traits are derived from the runner command manifest', () =>
   for (const [command, expectedTraits] of Object.entries(EXPECTED_RUNNER_COMMAND_TRAITS) as Array<
     [RunnerCommand['command'], RunnerCommandTraits]
   >) {
-    assert.deepEqual(readRunnerCommandTraits(command), expectedTraits, command);
+    assert.deepEqual(readRunnerCommandTraits({ command }), expectedTraits, command);
   }
 });
 
@@ -38,14 +39,14 @@ test('runner command manifest pins lifecycle-sensitive command groups', () => {
     'tap',
   ]);
   assert.deepEqual(commandsForClass('readOnly'), [
-    'alert',
     'findText',
     'gestureViewport',
     'querySelector',
     'readText',
     'screenshot',
     'snapshot',
   ]);
+  assert.deepEqual(commandsForClass('alertAction'), ['alert']);
   assert.deepEqual(commandsForClass('readOnlyReadinessProbe'), ['status', 'uptime']);
   assert.deepEqual(commandsForClass('readinessPreflightExemptMutation'), [
     'activate',
@@ -59,21 +60,38 @@ test('runner command trait helpers read from the shared trait table', () => {
     RunnerCommand['command']
   >) {
     const traits = EXPECTED_RUNNER_COMMAND_TRAITS[command];
-    assert.equal(isReadOnlyRunnerCommand(command), traits.readOnly, command);
-    assert.equal(isRunnerReadinessProbeCommand(command), traits.readinessProbe, command);
+    assert.equal(isReadOnlyRunnerCommand({ command }), traits.readOnly, command);
+    assert.equal(isRunnerReadinessProbeCommand({ command }), traits.readinessProbe, command);
     assert.equal(
-      isRunnerReadinessPreflightExempt(command),
+      isRunnerReadinessPreflightExempt({ command }),
       traits.readinessPreflightExempt,
       command,
     );
     assert.equal(
-      canSkipRunnerReadinessPreflightAfterHealthyMutation(command),
+      canSkipRunnerReadinessPreflightAfterHealthyMutation({ command }),
       traits.readinessPreflightSkipEligibleAfterHealthyMutation,
       command,
     );
   }
 });
 
+test('alert actions match the native read-only golden table', () => {
+  const cases = JSON.parse(
+    fs.readFileSync(
+      new URL('../../../../../contracts/fixtures/alert-command-traits.json', import.meta.url),
+      'utf8',
+    ),
+  ) as Array<{ name: string; command: RunnerCommand; readOnly: boolean }>;
+  assert.deepEqual(
+    cases.map(({ command }) => command.action),
+    [undefined, 'get', 'accept', 'dismiss'],
+  );
+  for (const { name, command, readOnly } of cases) {
+    assert.deepEqual(readRunnerCommandTraits(command), { ...defaults(), readOnly }, name);
+    assert.equal(isReadOnlyRunnerCommand(command), readOnly, name);
+  }
+});
+
 function commandsForClass(
   traitClass: (typeof RUNNER_COMMAND_TRAIT_MANIFEST)[RunnerCommand['command']],
 ): RunnerCommand['command'][] {
@@ -92,6 +110,7 @@ function expectedTraitsForClass(
     case 'readinessPreflightExemptMutation':
       return preflightExemptMutation();
     case 'readOnly':
+    case 'alertAction':
       return readOnly();
     case 'readOnlyReadinessProbe':
       return readOnlyReadinessProbe();

packages/platform-apple/src/runner/__tests__/runner-recovery-wiring.test.ts

@@ -1,9 +1,12 @@
 import assert from 'node:assert/strict';
-import { afterEach, expect, test, vi } from 'vitest';
+import { afterEach, beforeEach, expect, test, vi } from 'vitest';
 import { AppError } from '@agent-device/kernel/errors';
 import { IOS_SIMULATOR } from './device-fixtures.ts';
 import type { ExecResult } from '../host.ts';
 import type { RunnerSession } from '../runner-session.ts';
+import { appleRunnerTestHost } from '../test-host.ts';
+import { withAppleRunnerProvider } from '../runner-provider.ts';
+import type { RunnerCommand } from '../runner-contract.ts';
 import { startFakeRunnerServer, type FakeRunnerServer } from './fake-runner-server.ts';
 
 /**
@@ -45,6 +48,14 @@ vi.mock('../runner-session.ts', async (importOriginal) => {
 
 const { runAppleRunnerCommand } = await import('../runner-client.ts');
 
+beforeEach(() => {
+  const { retryWithPolicy } = appleRunnerTestHost.defaults();
+  appleRunnerTestHost.update({
+    retryWithPolicy: (task, policy, options) =>
+      retryWithPolicy(task, { ...policy, baseDelayMs: 1, maxDelayMs: 1, jitter: 0 }, options),
+  });
+});
+
 type LostResponseAcceptanceCommand = 'press' | 'fill';
 
 const LOST_RESPONSE_MUTATION_ROWS = {
@@ -214,3 +225,95 @@ test('an exact-session command never dispatches to a replacement runner', async
   ).rejects.toThrow('runner session ownership changed');
   assert.deepEqual(server.requests, []);
 });
+
+test.each(
+  (['accept', 'dismiss'] as const).flatMap((action) =>
+    (['accepted', 'started', 'completed'] as const).map((lifecycleState) => ({
+      action,
+      lifecycleState,
+      recovery:
+        lifecycleState === 'completed'
+          ? 'completed_without_retained_response'
+          : 'command_still_in_flight',
+    })),
+  ),
+)(
+  'alert $action with lost response and $lifecycleState status is not replayed',
+  async ({ action, lifecycleState, recovery }) => {
+    server = await startFakeRunnerServer({
+      alert: [{ kind: 'hangUp' }, { kind: 'ok', data: { replayed: true } }],
+      status: [{ kind: 'ok', data: { lifecycleState } }],
+    });
+    seedSession(server.port);
+
+    await expect(
+      runAppleRunnerCommand(IOS_SIMULATOR, { command: 'alert', action }),
+    ).rejects.toMatchObject({ details: { lifecycleState, recovery } });
+
+    const actions = server.requests.filter((request) => request.command === 'alert');
+    const probes = server.requests.filter((request) => request.command === 'status');
+    assert.equal(actions.length, 1);
+    assert.equal(actions[0]?.body.action, action);
+    assert.equal(probes.length, 1);
+    assert.equal(probes[0]?.body.statusCommandId, actions[0]?.body.commandId);
+    assert.equal(invalidateRunnerSessionMock.mock.calls.length, 0);
+  },
+);
+
+test.each([undefined, 'get'] as const)(
+  'alert query action %s remains retryable after a transport failure',
+  async (action) => {
+    server = await startFakeRunnerServer({
+      alert: [{ kind: 'hangUp' }, { kind: 'ok', data: { present: true } }],
+    });
+    seedSession(server.port);
+
+    const result = await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'alert', action });
+
+    assert.deepEqual(result, { present: true });
+    const queries = server.requests.filter((request) => request.command === 'alert');
+    assert.equal(queries.length, 2);
+    assert.equal(queries[0]?.body.commandId, queries[1]?.body.commandId);
+  },
+);
+
+test.each([undefined, 'get', 'accept', 'dismiss'] as const)(
+  'alert action %s selects startup readiness by mutation semantics',
+  async (action) => {
+    server = await startFakeRunnerServer({ alert: [{ kind: 'ok', data: {} }] });
+    seedSession(server.port).ready = false;
+
+    await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'alert', action });
+
+    assert.deepEqual(
+      server.requests.map((request) => request.command),
+      action === 'accept' || action === 'dismiss' ? ['uptime', 'alert'] : ['alert'],
+    );
+  },
+);
+
+test.each([undefined, 'get', 'accept', 'dismiss'] as const)(
+  'alert action %s selects provider retries by mutation semantics',
+  async (action) => {
+    const commands: RunnerCommand[] = [];
+    const failure = new AppError('COMMAND_FAILED', 'response unavailable', { retriable: true });
+    const result = withAppleRunnerProvider(
+      async (_device, command) => {
+        commands.push(command);
+        if (commands.length === 1) throw failure;
+        return { present: true };
+      },
+      { deviceId: IOS_SIMULATOR.id },
+      () => runAppleRunnerCommand(IOS_SIMULATOR, { command: 'alert', action }),
+    );
+
+    if (action === 'accept' || action === 'dismiss') {
+      await assert.rejects(result, (error: unknown) => error === failure);
+      assert.equal(commands.length, 1);
+    } else {
+      assert.deepEqual(await result, { present: true });
+      assert.equal(commands.length, 2);
+      assert.equal(commands[0]?.commandId, commands[1]?.commandId);
+    }
+  },
+);

packages/platform-apple/src/runner/runner-client.ts

@@ -39,7 +39,7 @@ export async function runAppleRunnerCommand(
   assertRunnerRequestActive(options.requestId);
   const runnerCommand = withRunnerCommandId(command);
   const provider = resolveAppleRunnerRuntime(device, options);
-  if (isReadOnlyRunnerCommand(runnerCommand.command)) {
+  if (isReadOnlyRunnerCommand(runnerCommand)) {
     return retryWithPolicy(
       () => {
         assertRunnerRequestActive(options.requestId);

packages/platform-apple/src/runner/runner-command-manifest.ts

@@ -4,6 +4,7 @@ export type RunnerCommandTraitClass =
   | 'default'
   | 'readinessPreflightExemptMutation'
   | 'readOnly'
+  | 'alertAction'
   | 'readOnlyReadinessProbe'
   | 'preflightSkippableTouchMutation';
 
@@ -32,7 +33,7 @@ export const RUNNER_COMMAND_TRAIT_MANIFEST = {
   appSwitcher: 'default',
   keyboardDismiss: 'default',
   keyboardReturn: 'default',
-  alert: 'readOnly',
+  alert: 'alertAction',
   sequence: 'preflightSkippableTouchMutation',
   recordStart: 'default',
   recordStop: 'default',

packages/platform-apple/src/runner/runner-command-recovery.ts

@@ -242,7 +242,7 @@ function handleCompletedRunnerStatus(
       lifecycleState: 'completed',
     };
   }
-  if (isReadOnlyRunnerCommand(command.command)) {
+  if (isReadOnlyRunnerCommand(command)) {
     return {
       type: 'skipInvalidation',
       error: transportError,
@@ -311,7 +311,7 @@ function runnerStatusInFlightError(
   transportError: AppError,
   options: AppleRunnerCommandOptions,
 ): AppError {
-  if (isReadOnlyRunnerCommand(command.command)) {
+  if (isReadOnlyRunnerCommand(command)) {
     return transportError;
   }
   const readinessPreflight = readReadinessPreflightRecoveryDetails(transportError);

packages/platform-apple/src/runner/runner-command-traits.ts

@@ -1,8 +1,5 @@
 import type { RunnerCommand } from './runner-contract.ts';
-import {
-  RUNNER_COMMAND_TRAIT_MANIFEST,
-  type RunnerCommandTraitClass,
-} from './runner-command-manifest.ts';
+import { RUNNER_COMMAND_TRAIT_MANIFEST } from './runner-command-manifest.ts';
 
 export type RunnerCommandTraits = Readonly<{
   readOnly: boolean;
@@ -44,43 +41,34 @@ const PREFLIGHT_SKIPPABLE_TOUCH_MUTATION_TRAITS: RunnerCommandTraits = {
   readinessPreflightSkipEligibleAfterHealthyMutation: true,
 };
 
-const RUNNER_COMMAND_TRAITS = Object.fromEntries(
-  Object.entries(RUNNER_COMMAND_TRAIT_MANIFEST).map(([command, traitClass]) => [
-    command,
-    traitsForClass(traitClass),
-  ]),
-) as Record<RunnerCommand['command'], RunnerCommandTraits>;
-
-export function readRunnerCommandTraits(command: RunnerCommand['command']): RunnerCommandTraits {
-  return RUNNER_COMMAND_TRAITS[command];
-}
-
-export function isReadOnlyRunnerCommand(command: RunnerCommand['command']): boolean {
+export function isReadOnlyRunnerCommand(command: RunnerCommand): boolean {
   return readRunnerCommandTraits(command).readOnly;
 }
 
-export function isRunnerReadinessProbeCommand(command: RunnerCommand['command']): boolean {
+export function isRunnerReadinessProbeCommand(command: RunnerCommand): boolean {
   return readRunnerCommandTraits(command).readinessProbe;
 }
 
-export function isRunnerReadinessPreflightExempt(command: RunnerCommand['command']): boolean {
+export function isRunnerReadinessPreflightExempt(command: RunnerCommand): boolean {
   return readRunnerCommandTraits(command).readinessPreflightExempt;
 }
 
 export function canSkipRunnerReadinessPreflightAfterHealthyMutation(
-  command: RunnerCommand['command'],
+  command: RunnerCommand,
 ): boolean {
   return readRunnerCommandTraits(command).readinessPreflightSkipEligibleAfterHealthyMutation;
 }
 
-function traitsForClass(traitClass: RunnerCommandTraitClass): RunnerCommandTraits {
-  switch (traitClass) {
+export function readRunnerCommandTraits(command: RunnerCommand): RunnerCommandTraits {
+  switch (RUNNER_COMMAND_TRAIT_MANIFEST[command.command]) {
     case 'default':
       return DEFAULT_TRAITS;
     case 'readinessPreflightExemptMutation':
       return READINESS_PREFLIGHT_EXEMPT_MUTATION_TRAITS;
     case 'readOnly':
       return READ_ONLY_TRAITS;
+    case 'alertAction':
+      return (command.action ?? 'get').toLowerCase() === 'get' ? READ_ONLY_TRAITS : DEFAULT_TRAITS;
     case 'readOnlyReadinessProbe':
       return READ_ONLY_READINESS_PROBE_TRAITS;
     case 'preflightSkippableTouchMutation':