fix(apple-runner): recovery decisions read typed rules, and a deadline gets a verdict

CHANGELOG.md

@@ -20,6 +20,21 @@
   BrowserStack runs for the session, as `bstack:options.appiumVersion`. Unset, BrowserStack falls
   back to Appium 1.x, which predates the `mobile:` commands the interactor issues (`deepLink`,
   `pressButton`, `activateApp`).
+- Changed (iOS runner): what recovers a stuck runner is decided by the recorded error, not by its
+  wording. A readiness preflight marks the error it gives up with, and that marker is now the whole
+  test for restarting the session and replaying the command — except for a request that was canceled,
+  which that same catch also marks: a command nobody is going to send again has no restart to spend,
+  and the session it would tear down may be one that still works. Two message checks decided it before,
+  and a preflight reaches the caller in whatever shape its connect loop ended with — "Runner did not
+  accept connection", "Runner endpoint probe failed", a killed `simctl` fallback, a post that ran out
+  of its budget — so only some of those restarted and the rest failed the command. The other half is
+  what no longer happens: when a slow
+  boot spends the whole prepare budget, the health check reports "prepare ios-runner timed out", and
+  that no longer wipes a restored `xcodebuild` artifact on the way to a rebuild — the runner session
+  is invalidated and prepare retries with the artifact intact. Only a failure that indicts the
+  artifact rebuilds it, so a runner that refuses the connection or never answers on any route still
+  wipes it and rebuilds, which is what that rule is for.
+
 - Changed (sessions): the implicit session is now keyed by workspace **and platform**, so one checkout
   can drive iOS and Android without inventing a `--session` name for every command (#2580). An
   implicit session was addressed by `cwd:<workspace>:default`, one slot per checkout, and it stayed

packages/kernel/src/errors.ts

@@ -262,10 +262,19 @@ export function createRequestCanceledError(details?: AppErrorDetails, cause?: un
   );
 }
 
+/**
+ * The typed reason of a canceled request, for a caller holding the details rather than the error:
+ * a rule table that matches on details needs the same fact {@link isRequestCanceledError} reads, and
+ * must not restate the literal.
+ */
+export function isRequestCanceledDetails(details: AppErrorDetails | undefined): boolean {
+  return details?.reason === REQUEST_CANCELED_REASON;
+}
+
 export function isRequestCanceledError(error: unknown): boolean {
   if (!(error instanceof AppError)) return false;
   if (error.code !== 'COMMAND_FAILED') return false;
-  if (error.details?.reason === REQUEST_CANCELED_REASON) return true;
+  if (isRequestCanceledDetails(error.details)) return true;
   // Owned debt: canceled errors that crossed a wire without their details keep
   // the message; do not add new message sniffs beside it.
   return error.message === REQUEST_CANCELED_MESSAGE;

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

@@ -342,56 +342,6 @@ test('mutating commands retry startup sessions with stale bundle cleanup', async
   assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession);
 });
 
-test('mutating commands restart stale sessions when readiness preflight fails before command send', async () => {
-  const staleSession = makeRunnerSession({ port: 8100, ready: true });
-  const freshSession = makeRunnerSession({ port: 8101, ready: false });
-
-  mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession);
-  mockExecuteRunnerCommandWithSession
-    .mockRejectedValueOnce(
-      new AppError('COMMAND_FAILED', 'fetch failed', {
-        runnerReadinessPreflightFailed: true,
-      }),
-    )
-    .mockResolvedValueOnce({ message: 'tapped' });
-
-  const result = await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 });
-
-  assert.deepEqual(result, { message: 'tapped' });
-  assert.equal(mockEnsureRunnerSession.mock.calls.length, 2);
-  assert.deepEqual(mockInvalidateRunnerSession.mock.calls[0], [
-    staleSession,
-    'runner_readiness_preflight_failed_before_command_send',
-  ]);
-  assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2);
-  assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession);
-});
-
-test('mutating commands restart stale sessions when readiness preflight times out before command send', async () => {
-  const staleSession = makeRunnerSession({ port: 8100, ready: true });
-  const freshSession = makeRunnerSession({ port: 8101, ready: false });
-
-  mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession);
-  mockExecuteRunnerCommandWithSession
-    .mockRejectedValueOnce(
-      new AppError('COMMAND_FAILED', 'Runner readiness timed out', {
-        runnerReadinessPreflightFailed: true,
-      }),
-    )
-    .mockResolvedValueOnce({ message: 'tapped' });
-
-  const result = await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 });
-
-  assert.deepEqual(result, { message: 'tapped' });
-  assert.equal(mockEnsureRunnerSession.mock.calls.length, 2);
-  assert.deepEqual(mockInvalidateRunnerSession.mock.calls[0], [
-    staleSession,
-    'runner_readiness_preflight_failed_before_command_send',
-  ]);
-  assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2);
-  assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession);
-});
-
 test('mutating commands emit readiness recovery diagnostics after failed preflight restart succeeds', async () => {
   const staleSession = makeRunnerSession({ port: 8100, ready: true });
   const freshSession = makeRunnerSession({ port: 8101, ready: false });

packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts

@@ -1,10 +1,12 @@
 import assert from 'node:assert/strict';
 import { test } from 'vitest';
-import { AppError } from '@agent-device/kernel/errors';
+import { AppError, createRequestCanceledError } from '@agent-device/kernel/errors';
 import {
   RUNNER_ERROR_RULES,
   isRetryableRunnerError,
   resolveRunnerFatalErrorReason,
+  shouldRebuildCachedRunnerArtifact,
+  shouldRestartRunnerAfterReadinessPreflight,
   shouldRestartRunnerBeforeCommandSend,
   shouldRetryRunnerConnectError,
 } from '../runner-contract.ts';
@@ -32,7 +34,12 @@ test('transport-shaped failures are retryable', () => {
 });
 
 test('boot-shaped failures are not retryable', () => {
-  assert.equal(isRetryableRunnerError(commandFailed('xcodebuild exited early (code 65)')), false);
+  assert.equal(
+    isRetryableRunnerError(
+      commandFailed('Runner did not accept connection (xcodebuild exited early)'),
+    ),
+    false,
+  );
   assert.equal(
     isRetryableRunnerError(commandFailed('Device is busy (Connecting to Simulator)')),
     false,
@@ -70,6 +77,99 @@ test('connect loop stops for terminal verdicts', () => {
   assert.equal(shouldRetryRunnerConnectError(new AppError('DEVICE_NOT_FOUND', 'gone')), true);
 });
 
+// --- readiness preflight ---
+
+test('the preflight marker alone decides the restart', () => {
+  // The marker is applied by the preflight's own catch, whatever it was waiting on when it gave
+  // up: a killed fallback, an exhausted probe, a refusal. Which of those arrived is not evidence
+  // about whether the command reached the runner, and the marker is.
+  const killedSpawn = commandFailed('xcrun timed out after 45000ms', {
+    cmd: 'xcrun',
+    timeoutMs: 45_000,
+    runnerReadinessPreflightFailed: true,
+  });
+  assert.equal(shouldRestartRunnerAfterReadinessPreflight(killedSpawn), true);
+  assert.equal(
+    shouldRestartRunnerAfterReadinessPreflight(
+      commandFailed('Runner readiness refused', { runnerReadinessPreflightFailed: true }),
+    ),
+    true,
+  );
+  // The restart the marker authorises is a new session, not more waiting inside this one.
+  assert.equal(shouldRetryRunnerConnectError(killedSpawn), true);
+  // Without the marker the same two shapes say nothing about the command having been written.
+  assert.equal(
+    shouldRestartRunnerAfterReadinessPreflight(
+      commandFailed('xcrun timed out after 45000ms', { cmd: 'xcrun', timeoutMs: 45_000 }),
+    ),
+    false,
+  );
+  assert.equal(
+    shouldRestartRunnerAfterReadinessPreflight(commandFailed('Runner readiness refused')),
+    false,
+  );
+  // The same catch marks a caller that stopped waiting. That mark is not a runner that stopped
+  // answering: the command was canceled, so no restart has a request left to serve.
+  assert.equal(
+    shouldRestartRunnerAfterReadinessPreflight(
+      createRequestCanceledError({ runnerReadinessPreflightFailed: true }),
+    ),
+    false,
+  );
+});
+
+test('a deadline on its own earns no recovery verdict', () => {
+  // The same recorded budget covers a wait inside the connect loop, where waiting is
+  // right, and a fetch that died after the command was written, where replaying is not.
+  const deadline = commandFailed('Runner command deadline exceeded', {
+    port: 8100,
+    timeoutMs: 45_000,
+  });
+  assert.equal(isRetryableRunnerError(deadline), false);
+  assert.equal(shouldRestartRunnerBeforeCommandSend(deadline), false);
+  assert.equal(shouldRestartRunnerAfterReadinessPreflight(deadline), false);
+  assert.equal(shouldRebuildCachedRunnerArtifact(deadline), false);
+  assert.equal(shouldRetryRunnerConnectError(deadline), true);
+});
+
+// --- restored-artifact axis (shouldRebuildCachedRunnerArtifact) ---
+
+test('only a runner that never accepted a connection indicts the cached artifact', () => {
+  assert.equal(
+    shouldRebuildCachedRunnerArtifact(commandFailed('Runner endpoint probe failed')),
+    true,
+  );
+  assert.equal(
+    shouldRebuildCachedRunnerArtifact(commandFailed('Runner did not accept connection')),
+    true,
+  );
+  assert.equal(
+    shouldRebuildCachedRunnerArtifact(
+      commandFailed('Runner did not accept connection (simctl spawn)', { port: 8100 }),
+    ),
+    true,
+  );
+  // Wiping derived data cannot fix a boot that refuses to compile, and its message
+  // otherwise reads as a refused connection.
+  assert.equal(
+    shouldRebuildCachedRunnerArtifact(
+      commandFailed('Runner did not accept connection (xcodebuild exited early)', {
+        port: 8100,
+        logPath: '/tmp/runner.log',
+      }),
+    ),
+    false,
+  );
+  assert.equal(shouldRebuildCachedRunnerArtifact(commandFailed('fetch failed')), false);
+});
+
+test('a device that is busy connecting is a terminal connect verdict', () => {
+  assert.equal(
+    shouldRetryRunnerConnectError(commandFailed('Device is busy (Connecting to Simulator)')),
+    false,
+  );
+});
+
 // --- session-fatal axis (resolveRunnerFatalErrorReason) ---
 
 test('session-fatal codes map to their invalidation reasons', () => {

packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts

@@ -0,0 +1,123 @@
+import assert from 'node:assert/strict';
+import { beforeEach, test, vi } from 'vitest';
+import { AppError } from '@agent-device/kernel/errors';
+import { appleRunnerTestHost } from '../test-host.ts';
+import { Deadline } from '../host.ts';
+import type { RunnerXctestrunArtifact } from '../runner-xctestrun.ts';
+import { IOS_SIMULATOR } from './device-fixtures.ts';
+import { createTestRequestCancellation, makeRunnerSession } from './runner-session-fixtures.ts';
+
+const {
+  mockEnsureRunnerSession,
+  mockExecuteRunnerCommandWithSession,
+  mockEmitDiagnostic,
+  mockGetRunnerSessionSnapshot,
+  mockInvalidateRunnerSession,
+  mockMarkRunnerXctestrunArtifactBadForRun,
+} = vi.hoisted(() => ({
+  mockEnsureRunnerSession: vi.fn(),
+  mockExecuteRunnerCommandWithSession: vi.fn(),
+  mockEmitDiagnostic: vi.fn(),
+  mockGetRunnerSessionSnapshot: vi.fn(),
+  mockInvalidateRunnerSession: vi.fn(),
+  mockMarkRunnerXctestrunArtifactBadForRun: vi.fn(),
+}));
+
+vi.mock('../runner-session.ts', async () => {
+  const actual =
+    await vi.importActual<typeof import('../runner-session.ts')>('../runner-session.ts');
+  return {
+    ...actual,
+    ensureRunnerSession: mockEnsureRunnerSession,
+    executeRunnerCommandWithSession: mockExecuteRunnerCommandWithSession,
+    getRunnerSessionSnapshot: mockGetRunnerSessionSnapshot,
+    invalidateRunnerSession: mockInvalidateRunnerSession,
+  };
+});
+
+vi.mock('../runner-xctestrun.ts', async () => {
+  const actual =
+    await vi.importActual<typeof import('../runner-xctestrun.ts')>('../runner-xctestrun.ts');
+  return {
+    ...actual,
+    markRunnerXctestrunArtifactBadForRun: mockMarkRunnerXctestrunArtifactBadForRun,
+  };
+});
+
+import { prepareIosRunner } from '../runner-client.ts';
+import { resetRunnerRecycleLedgerForTests } from '../runner-recycle-ledger.ts';
+
+function makeRunnerArtifact(
+  overrides: Partial<RunnerXctestrunArtifact> = {},
+): RunnerXctestrunArtifact {
+  return {
+    xctestrunPath: '/tmp/runner.xctestrun',
+    derived: '/tmp/derived',
+    cache: 'exact',
+    artifact: 'valid',
+    buildMs: 0,
+    xctestrunPathSource: 'manifest',
+    ...overrides,
+  };
+}
+
+const requestCancellation = createTestRequestCancellation();
+const { isRequestCanceled } = requestCancellation;
+
+beforeEach(() => {
+  vi.resetAllMocks();
+  resetRunnerRecycleLedgerForTests();
+  mockGetRunnerSessionSnapshot.mockReturnValue(null);
+  mockMarkRunnerXctestrunArtifactBadForRun.mockResolvedValue(undefined);
+  requestCancellation.reset();
+  appleRunnerTestHost.update({
+    emitDiagnostic: mockEmitDiagnostic,
+    isRequestCanceled,
+    getRequestSignal: () => undefined,
+  });
+});
+
+// What a spent prepare deadline does to a restored artifact. Prepare spends one `Deadline` across
+// boot and health check, so the failure this decision actually sees is the one
+// `readPreparePhaseTimeoutMs` raises when the boot ate the budget: "prepare ios-runner timed out"
+// with reason `prepare_deadline_expired`. That indicts the boot, not the derived data it was
+// launched from, so the artifact stays and prepare retries with a fresh session. The artifact is
+// only wiped by the rules that indict it, such as a runner that refused the connection.
+
+test('a prepare deadline spent during boot keeps the restored artifact and retries', async () => {
+  vi.useFakeTimers();
+  try {
+    vi.setSystemTime(1_000);
+    const restoredSession = makeRunnerSession({
+      port: 8100,
+      xctestrunPath: '/tmp/restored.xctestrun',
+      xctestrunArtifact: makeRunnerArtifact({ xctestrunPath: '/tmp/restored.xctestrun' }),
+    });
+    const prepareDeadline = Deadline.fromTimeoutMs(45_000);
+
+    mockEnsureRunnerSession.mockImplementation(async () => {
+      // The boot consumed the whole prepare budget, so no health phase time remains.
+      vi.setSystemTime(46_000);
+      return restoredSession;
+    });
+
+    await assert.rejects(
+      () => prepareIosRunner(IOS_SIMULATOR, { healthTimeoutMs: 90_000, prepareDeadline }),
+      (error: unknown) => {
+        assert.ok(error instanceof AppError);
+        assert.equal(error.message, 'prepare ios-runner timed out');
+        assert.equal(error.details?.reason, 'prepare_deadline_expired');
+        assert.equal(error.details?.phase, 'runner_session');
+        return true;
+      },
+    );
+
+    assert.equal(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls.length, 0);
+    assert.deepEqual(mockInvalidateRunnerSession.mock.calls.at(-1), [
+      restoredSession,
+      'prepare_runner_health_retry',
+    ]);
+  } finally {
+    vi.useRealTimers();
+  }
+});