From 8c70a81eb0a98c93446acdadb8b5aa415f958088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 14 Sep 2026 17:20:51 +0200 Subject: [PATCH 1/7] fix(apple-runner): recovery decisions read typed rules, and a deadline gets a verdict Two private message-substring chains decided runner recovery outside RUNNER_ERROR_RULES, and both got the common case wrong. A command that ran out its connection deadline arrives in a shape whose message matches neither 'timeout' nor 'timed out', so executeRunnerCommand rethrew without restarting a session whose runner never received the command. And shouldRetryPrepareRunnerHealthFailure ORed in shouldRetryRunnerConnectError, whose ?? true default let the connect loop retry past a rule that had just denied it. fetchWithTimeout now reports its own expiry as a COMMAND_FAILED carrying timeoutMs, the shape isCommandTimeoutError already understands. Until then the bare AbortSignal.timeout rejection reached asAppError with no details at all, so neither a rule nor a timeout predicate could see it and the message chains were the only thing catching it. Only a rejection carrying that signal's own reason is relabelled: a refused connection or a canceled request keeps the error it actually failed with. RunnerErrorMatch.details is a predicate rather than a closed union, which is what makes a recorded deadline and the readiness-preflight marker expressible as evidence, and the table gains two axes: restartAfterReadinessPreflight, and artifactSuspect for the rules that say the restored xctestrun itself is at fault. The cache wipe reads that axis, so derived data is destroyed only when the runner refused or never answered on every route, and a boot that exited early says false even though its message also reads as a refused connection. Both chains are deleted; what depends on runtime state (a cached artifact being present, the request being canceled) stays in runner-lifecycle.ts. A bare deadline earns no replay verdict on purpose: 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 it is not. --- .../__tests__/runner-command-retry.test.ts | 47 +++++++- .../runner-error-classification.test.ts | 82 ++++++++++++- .../runner/__tests__/runner-transport.test.ts | 25 +++- .../src/runner/runner-contract.ts | 108 +++++++++++++++--- .../src/runner/runner-lifecycle.ts | 31 ++--- .../src/runner/runner-transport.ts | 17 ++- 6 files changed, 267 insertions(+), 43 deletions(-) diff --git a/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts b/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts index 0164168eaf..4b86a47cfc 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts @@ -367,14 +367,16 @@ test('mutating commands restart stale sessions when readiness preflight fails be assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession); }); -test('mutating commands restart stale sessions when readiness preflight times out before command send', async () => { +test('mutating commands restart stale sessions when readiness preflight outlives its deadline', 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', { + new AppError('COMMAND_FAILED', 'xcrun timed out after 45000ms', { + cmd: 'xcrun', + timeoutMs: 45_000, runnerReadinessPreflightFailed: true, }), ) @@ -392,6 +394,47 @@ test('mutating commands restart stale sessions when readiness preflight times ou assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession); }); +test('a readiness preflight refusal that is neither transport-shaped nor deadline-shaped surfaces', async () => { + const staleSession = makeRunnerSession({ port: 8100, ready: true }); + + mockEnsureRunnerSession.mockResolvedValueOnce(staleSession); + mockExecuteRunnerCommandWithSession.mockRejectedValueOnce( + new AppError('COMMAND_FAILED', 'Runner readiness refused', { + runnerReadinessPreflightFailed: true, + }), + ); + + await assert.rejects( + () => runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.message, 'Runner readiness refused'); + return true; + }, + ); + assert.equal(mockEnsureRunnerSession.mock.calls.length, 1); +}); + +test('a boot that exited early does not wipe a restored runner artifact', async () => { + const fixtures = makeBadCacheRecoveryFixtures(); + + mockEnsureRunnerSession.mockResolvedValueOnce(fixtures.restoredSession); + mockExecuteRunnerCommandWithSession.mockRejectedValueOnce( + new AppError('COMMAND_FAILED', 'Runner did not accept connection (xcodebuild exited early)'), + ); + + await assert.rejects( + () => prepareIosRunner(IOS_SIMULATOR, { healthTimeoutMs: 90_000 }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.message, 'Runner did not accept connection (xcodebuild exited early)'); + return true; + }, + ); + assert.equal(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls.length, 0); + assert.equal(mockEnsureRunnerSession.mock.calls.length, 1); +}); + 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 }); diff --git a/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts b/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts index 20772cd044..a59b8ca075 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts @@ -5,6 +5,8 @@ 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,79 @@ test('connect loop stops for terminal verdicts', () => { assert.equal(shouldRetryRunnerConnectError(new AppError('DEVICE_NOT_FOUND', 'gone')), true); }); +// --- connect deadline --- + +test('a readiness preflight that outlived its deadline restarts the session', () => { + // The marked shape this recovery actually sees: the simctl-spawn fallback killed at + // its budget, then marked by the readiness preflight that was waiting on it. + const killedSpawn = commandFailed('xcrun timed out after 45000ms', { + cmd: 'xcrun', + timeoutMs: 45_000, + runnerReadinessPreflightFailed: true, + }); + assert.equal(shouldRestartRunnerAfterReadinessPreflight(killedSpawn), true); + assert.equal(shouldRetryRunnerConnectError(killedSpawn), true); + // A preflight refusal with no deadline restarts nothing: the runner answered and said no. + assert.equal( + shouldRestartRunnerAfterReadinessPreflight( + commandFailed('Runner readiness refused', { 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', () => { diff --git a/packages/platform-apple/src/runner/__tests__/runner-transport.test.ts b/packages/platform-apple/src/runner/__tests__/runner-transport.test.ts index 71f0fc6712..edd7f7bc16 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-transport.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-transport.test.ts @@ -13,6 +13,7 @@ import { xctestIosDevice, } from './runner-transport.fixtures.ts'; import { appleRunnerTestHost } from '../test-host.ts'; +import { isCommandTimeoutError } from '../host.ts'; import type { IosPhysicalDeviceRunnerControl } from '../host.ts'; const { mockRunCmd, mockUsbmuxPostCommand } = vi.hoisted(() => ({ @@ -31,7 +32,7 @@ vi.mock('../runner-usbmux.ts', async (importOriginal) => { }); import { clearDeviceTunnelIpCache } from '../runner-command-route.ts'; -import { sendRunnerCommandOnce } from '../runner-transport.ts'; +import { fetchWithTimeout, sendRunnerCommandOnce } from '../runner-transport.ts'; // The real `resolveIosPhysicalDeviceControl` resolves the CoreDevice tunnel IP // through root-level tooling this package cannot reach; a fake control backed @@ -115,6 +116,28 @@ test('sendRunnerCommandOnce does not retry or simulator fallback after request f assert.equal(mockRunCmd.mock.calls.length, 0); }); +test('fetchWithTimeout reports its own deadline as a typed command timeout', async () => { + vi.stubGlobal( + 'fetch', + vi.fn((_url: string, init: RequestInit) => { + return new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => reject(init.signal?.reason)); + }); + }), + ); + + await assert.rejects( + () => fetchWithTimeout('http://127.0.0.1:8100/command', { method: 'POST' }, 5), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.timeoutMs, 5); + assert.equal(isCommandTimeoutError(error), true); + return true; + }, + ); +}); + test('sendRunnerCommandOnce routes xctest physical devices through usbmux', async () => { const fetchMock = vi.fn(); vi.stubGlobal('fetch', fetchMock); diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index 649ffce8f1..d3accedc8e 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -1,4 +1,10 @@ -import { AppError, createRequestCanceledError, toAppErrorCode } from '@agent-device/kernel/errors'; +import { + AppError, + createRequestCanceledError, + toAppErrorCode, + type AppErrorCode, + type AppErrorDetails, +} from '@agent-device/kernel/errors'; import crypto from 'node:crypto'; import { ALERT_NOT_FOUND_RUNNER_CODE } from '@agent-device/contracts/alert-contract'; import type { DeviceRotation } from '@agent-device/contracts/device'; @@ -143,15 +149,33 @@ export function resolveRunnerRequestSignal(options: { return AbortSignal.any([registeredSignal, options.signal]); } +/** + * Details evidence a rule requires beyond code and message. A predicate rather than a + * fixed vocabulary because the useful evidence is a shape: a recorded deadline, a + * preflight marker, a retriable flag. Every predicate below names one. + */ +type RunnerErrorDetailsMatch = (details: AppErrorDetails) => boolean; + type RunnerErrorMatch = { /** Required `AppError.code`; absent = any AppError. */ - code?: string; + code?: AppErrorCode; /** Every entry must appear in the lowercased message. */ messageIncludesAll?: readonly string[]; /** Required details evidence beyond code/message. */ - details?: 'retriable' | 'usbmux-device-unattached'; + details?: RunnerErrorDetailsMatch; }; +const hasRetriableFlag: RunnerErrorDetailsMatch = (details) => details.retriable === true; +const hasUsbmuxDeviceUnattached: RunnerErrorDetailsMatch = (details) => + details.usbmuxDeviceAttached === false; +/** The numeric deadline a command timeout records, including an `AbortSignal.timeout` wrap. */ +const hasCommandDeadline: RunnerErrorDetailsMatch = (details) => + typeof details.timeoutMs === 'number'; +const hasReadinessPreflightFailure: RunnerErrorDetailsMatch = (details) => + details.runnerReadinessPreflightFailed === true; +const hasReadinessPreflightDeadline: RunnerErrorDetailsMatch = (details) => + hasReadinessPreflightFailure(details) && hasCommandDeadline(details); + type RunnerErrorVerdicts = { /** isRetryableRunnerError: transport error worth a same-session resend. */ retryable?: boolean; @@ -161,6 +185,10 @@ type RunnerErrorVerdicts = { sessionFatalReason?: string; /** Connect-shaped failure before the command was sent: restart the session and replay. */ restartBeforeSend?: boolean; + /** Readiness preflight gave up on its deadline: restart the session and replay. */ + restartAfterReadinessPreflight?: boolean; + /** The runner never accepted a connection, so the restored artifact itself is suspect. */ + artifactSuspect?: boolean; }; type RunnerErrorRule = { @@ -183,43 +211,68 @@ type RunnerErrorRule = { export const RUNNER_ERROR_RULES: readonly RunnerErrorRule[] = [ { reason: 'usbmux_device_unattached', - match: { code: 'DEVICE_NOT_FOUND', details: 'usbmux-device-unattached' }, + match: { code: 'DEVICE_NOT_FOUND', details: hasUsbmuxDeviceUnattached }, verdicts: { connectRetry: false }, }, { reason: 'flagged_retriable', - match: { code: 'COMMAND_FAILED', details: 'retriable' }, - verdicts: { retryable: true }, + match: { code: 'COMMAND_FAILED', details: hasRetriableFlag }, + verdicts: { retryable: true, connectRetry: true }, }, { + // Says `artifactSuspect: false` on purpose: its message also reads as a refused + // connection, and a boot that cannot compile is not cured by wiping derived data. reason: 'xcodebuild_exited_early', match: { code: 'COMMAND_FAILED', messageIncludesAll: ['xcodebuild exited early'] }, - verdicts: { retryable: false, connectRetry: false }, + verdicts: { retryable: false, connectRetry: false, artifactSuspect: false }, }, { + // A device still mid-attachment is not a runner we can talk to yet, and waiting on + // it inside this request is what the caller's own retry is for. reason: 'device_busy_connecting', match: { code: 'COMMAND_FAILED', messageIncludesAll: ['device is busy', 'connecting'] }, - verdicts: { retryable: false }, + verdicts: { retryable: false, connectRetry: false }, + }, + { + // A deadline alone earns no 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 it is not. Only the preflight marker says the runner + // never saw the command, and it sits beside the deadline here for that reason. + reason: 'runner_readiness_preflight_deadline', + match: { code: 'COMMAND_FAILED', details: hasReadinessPreflightDeadline }, + verdicts: { restartAfterReadinessPreflight: true, connectRetry: true }, }, { reason: 'runner_connect_refused', match: { code: 'COMMAND_FAILED', messageIncludesAll: ['runner did not accept connection'] }, - verdicts: { retryable: true, restartBeforeSend: true }, + verdicts: { + retryable: true, + connectRetry: true, + restartBeforeSend: true, + artifactSuspect: true, + }, + }, + { + // Every endpoint answered and none of them had a runner: with a restored artifact + // in hand, that artifact is the common cause. + reason: 'runner_endpoint_probe_exhausted', + match: { code: 'COMMAND_FAILED', messageIncludesAll: ['runner endpoint probe failed'] }, + verdicts: { artifactSuspect: true }, }, { reason: 'fetch_failed', match: { code: 'COMMAND_FAILED', messageIncludesAll: ['fetch failed'] }, - verdicts: { retryable: true }, + verdicts: { retryable: true, connectRetry: true }, }, { reason: 'econnrefused', match: { code: 'COMMAND_FAILED', messageIncludesAll: ['econnrefused'] }, - verdicts: { retryable: true }, + verdicts: { retryable: true, connectRetry: true }, }, { reason: 'socket_hang_up', match: { code: 'COMMAND_FAILED', messageIncludesAll: ['socket hang up'] }, - verdicts: { retryable: true }, + verdicts: { retryable: true, connectRetry: true }, }, { reason: 'ax_snapshot_failure', @@ -249,8 +302,7 @@ function matchesRunnerErrorRule(error: AppError, match: RunnerErrorMatch): boole function matchesRunnerErrorDetails(error: AppError, details: RunnerErrorMatch['details']): boolean { if (details === undefined) return true; - if (details === 'retriable') return error.details?.retriable === true; - return isUsbmuxDeviceUnattachedError(error); + return details((error.details ?? {}) as AppErrorDetails); } function matchesRunnerErrorMessage(error: AppError, parts: readonly string[] | undefined): boolean { @@ -287,16 +339,36 @@ export function isRetryableRunnerError(err: unknown): boolean { */ export function isUsbmuxDeviceUnattachedError(error: unknown): boolean { if (!(error instanceof AppError) || error.code !== 'DEVICE_NOT_FOUND') return false; - return ( - (error.details as { usbmuxDeviceAttached?: unknown } | undefined)?.usbmuxDeviceAttached === - false - ); + return hasUsbmuxDeviceUnattached((error.details ?? {}) as AppErrorDetails); } +/** + * Default is true and not false: the common failure while the runner boots is an error + * no rule describes, and giving up on it would fail a command that the connect loop was + * about to succeed. A rule says false only when waiting cannot help. + */ export function shouldRetryRunnerConnectError(error: unknown): boolean { return runnerErrorVerdict(error, 'connectRetry') ?? true; } +/** + * The readiness preflight ran out of its own deadline, so the runner never saw the + * command: restarting the session and replaying is both safe and the only way out. + */ +export function shouldRestartRunnerAfterReadinessPreflight(error: unknown): boolean { + return runnerErrorVerdict(error, 'restartAfterReadinessPreflight') ?? false; +} + +/** + * The runner refused or never answered on every route, which is what a restored artifact + * that cannot boot looks like. Deliberately narrow: the recovery it authorises is a clean + * `xcodebuild` rebuild, so a slow boot, a busy device or a transport failure partway + * through a command must not pay that price. + */ +export function shouldRebuildCachedRunnerArtifact(error: unknown): boolean { + return runnerErrorVerdict(error, 'artifactSuspect') ?? false; +} + /** * Session-fatal classification for a runner response error: when defined, the * cached runner session must be invalidated with this reason instead of being diff --git a/packages/platform-apple/src/runner/runner-lifecycle.ts b/packages/platform-apple/src/runner/runner-lifecycle.ts index 1644092c78..f26c183f99 100644 --- a/packages/platform-apple/src/runner/runner-lifecycle.ts +++ b/packages/platform-apple/src/runner/runner-lifecycle.ts @@ -17,10 +17,12 @@ import { assertRunnerRequestActive, isRetryableRunnerError, resolveRunnerRequestSignal, + shouldRebuildCachedRunnerArtifact, + shouldRestartRunnerAfterReadinessPreflight, + shouldRestartRunnerBeforeCommandSend, shouldRetryRunnerConnectError, withRunnerCommandId, type RunnerCommand, - shouldRestartRunnerBeforeCommandSend, } from './runner-contract.ts'; import type { AppleRunnerCommandOptions, @@ -246,11 +248,7 @@ async function invalidateRunnerSessionBestEffort( function shouldRetryPrepareRunnerHealthFailure(error: AppError): boolean { if (isRequestCanceledError(error)) return false; - return ( - isRetryableRunnerError(error) || - shouldRetryRunnerConnectError(error) || - isPrepareHealthTimeout(error) - ); + return isRetryableRunnerError(error) || shouldRetryRunnerConnectError(error); } // fallow-ignore-next-line complexity @@ -498,6 +496,11 @@ function readPreparePhaseTimeoutMs( return remainingMs; } +/** + * A rebuild here is a clean `xcodebuild`, so the verdict comes from the rules that indict + * the artifact itself rather than from "anything the connect loop would tolerate". + * Whether the session even carries a cached artifact is this module's fact, not the table's. + */ function shouldRecoverBadCachedRunnerArtifact( error: AppError, session: RunnerSession, @@ -506,14 +509,7 @@ function shouldRecoverBadCachedRunnerArtifact( } { const artifact = session.xctestrunArtifact; if (!artifact || artifact.cache === 'miss') return false; - return shouldRetryPrepareRunnerHealthFailure(error); -} - -function isPrepareHealthTimeout(error: AppError): boolean { - const message = error.message.toLowerCase(); - return ( - message.includes('timeout') || message.includes('timed out') || message.includes('deadline') - ); + return shouldRebuildCachedRunnerArtifact(error); } function wrapPrepareHealthFailure( @@ -611,11 +607,6 @@ function isRunnerReadinessPreflightError(error: AppError): boolean { function shouldRestartAfterReadinessPreflightError(error: AppError): boolean { return ( isRunnerReadinessPreflightError(error) && - (isRetryableRunnerError(error) || isRunnerReadinessPreflightTimeout(error)) + (isRetryableRunnerError(error) || shouldRestartRunnerAfterReadinessPreflight(error)) ); } - -function isRunnerReadinessPreflightTimeout(error: AppError): boolean { - const message = error.message.toLowerCase(); - return message.includes('timeout') || message.includes('timed out'); -} diff --git a/packages/platform-apple/src/runner/runner-transport.ts b/packages/platform-apple/src/runner/runner-transport.ts index 6f8cdd4957..efb89a6c26 100644 --- a/packages/platform-apple/src/runner/runner-transport.ts +++ b/packages/platform-apple/src/runner/runner-transport.ts @@ -87,5 +87,20 @@ export async function fetchWithTimeout( ): Promise { const timeoutSignal = AbortSignal.timeout(timeoutMs); const signal = requestSignal ? AbortSignal.any([requestSignal, timeoutSignal]) : timeoutSignal; - return await fetch(url, { ...init, signal }); + try { + return await fetch(url, { ...init, signal }); + } catch (error) { + // `AbortSignal.timeout` rejects with a bare DOMException that no recovery rule can + // read. Only a rejection carrying that signal's own reason is our deadline: a + // refused connection or a canceled request keeps the error it actually failed with. + if (error === timeoutSignal.reason) { + throw new AppError( + 'COMMAND_FAILED', + 'Runner command deadline exceeded', + { timeoutMs }, + error, + ); + } + throw error; + } } From 4eb60b65c8cccc219fc8add221a90d2335ab8dcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 14 Sep 2026 18:45:50 +0200 Subject: [PATCH 2/7] test(apple-runner): readiness preflight decisions get their own file `runner-command-retry.test.ts` sits above the test-size tripwire and may not grow, and the four preflight verdicts did not belong in a retry aggregation anyway: they answer one question about what `executeRunnerCommand` does with a runner that refused before the command was written. `makeRunnerSession` comes from the fixtures module that already owns it. Co-authored-by: Apex by Callstack --- .../__tests__/runner-command-retry.test.ts | 93 --------- ...nner-lifecycle-readiness-preflight.test.ts | 179 ++++++++++++++++++ 2 files changed, 179 insertions(+), 93 deletions(-) create mode 100644 packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts diff --git a/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts b/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts index 4b86a47cfc..1713908ba2 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts @@ -342,99 +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 outlives its deadline', 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', 'xcrun timed out after 45000ms', { - cmd: 'xcrun', - timeoutMs: 45_000, - 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('a readiness preflight refusal that is neither transport-shaped nor deadline-shaped surfaces', async () => { - const staleSession = makeRunnerSession({ port: 8100, ready: true }); - - mockEnsureRunnerSession.mockResolvedValueOnce(staleSession); - mockExecuteRunnerCommandWithSession.mockRejectedValueOnce( - new AppError('COMMAND_FAILED', 'Runner readiness refused', { - runnerReadinessPreflightFailed: true, - }), - ); - - await assert.rejects( - () => runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.message, 'Runner readiness refused'); - return true; - }, - ); - assert.equal(mockEnsureRunnerSession.mock.calls.length, 1); -}); - -test('a boot that exited early does not wipe a restored runner artifact', async () => { - const fixtures = makeBadCacheRecoveryFixtures(); - - mockEnsureRunnerSession.mockResolvedValueOnce(fixtures.restoredSession); - mockExecuteRunnerCommandWithSession.mockRejectedValueOnce( - new AppError('COMMAND_FAILED', 'Runner did not accept connection (xcodebuild exited early)'), - ); - - await assert.rejects( - () => prepareIosRunner(IOS_SIMULATOR, { healthTimeoutMs: 90_000 }), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.message, 'Runner did not accept connection (xcodebuild exited early)'); - return true; - }, - ); - assert.equal(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls.length, 0); - assert.equal(mockEnsureRunnerSession.mock.calls.length, 1); -}); - 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 }); diff --git a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts new file mode 100644 index 0000000000..ee6bf064f3 --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts @@ -0,0 +1,179 @@ +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 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('../runner-session.ts'); + return { + ...actual, + ensureRunnerSession: mockEnsureRunnerSession, + executeRunnerCommandWithSession: mockExecuteRunnerCommandWithSession, + getRunnerSessionSnapshot: mockGetRunnerSessionSnapshot, + invalidateRunnerSession: mockInvalidateRunnerSession, + }; +}); + +vi.mock('../runner-xctestrun.ts', async () => { + const actual = + await vi.importActual('../runner-xctestrun.ts'); + return { + ...actual, + markRunnerXctestrunArtifactBadForRun: mockMarkRunnerXctestrunArtifactBadForRun, + }; +}); + +import { prepareIosRunner, runAppleRunnerCommand } from '../runner-client.ts'; +import { resetRunnerRecycleLedgerForTests } from '../runner-recycle-ledger.ts'; + +// What `executeRunnerCommand` decides when a readiness preflight refuses the runner +// before the command was ever written: the table's own verdict axes say whether the +// session restarts, whether the restored artifact is suspect, and whether the caller +// simply hears the refusal. + +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, + }); +}); + +function makeRunnerArtifact( + overrides: Partial = {}, +): RunnerXctestrunArtifact { + return { + xctestrunPath: '/tmp/runner.xctestrun', + derived: '/tmp/derived', + cache: 'exact', + artifact: 'valid', + buildMs: 0, + xctestrunPathSource: 'manifest', + ...overrides, + }; +} + +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 outlives its deadline', 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', 'xcrun timed out after 45000ms', { + cmd: 'xcrun', + timeoutMs: 45_000, + 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('a readiness preflight refusal that is neither transport-shaped nor deadline-shaped surfaces', async () => { + const staleSession = makeRunnerSession({ port: 8100, ready: true }); + + mockEnsureRunnerSession.mockResolvedValueOnce(staleSession); + mockExecuteRunnerCommandWithSession.mockRejectedValueOnce( + new AppError('COMMAND_FAILED', 'Runner readiness refused', { + runnerReadinessPreflightFailed: true, + }), + ); + + await assert.rejects( + () => runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.message, 'Runner readiness refused'); + return true; + }, + ); + assert.equal(mockEnsureRunnerSession.mock.calls.length, 1); +}); + +test('a boot that exited early does not wipe a restored runner artifact', async () => { + const restoredSession = makeRunnerSession({ + port: 8100, + xctestrunPath: '/tmp/restored.xctestrun', + xctestrunArtifact: makeRunnerArtifact({ xctestrunPath: '/tmp/restored.xctestrun' }), + }); + + mockEnsureRunnerSession.mockResolvedValueOnce(restoredSession); + mockExecuteRunnerCommandWithSession.mockRejectedValueOnce( + new AppError('COMMAND_FAILED', 'Runner did not accept connection (xcodebuild exited early)'), + ); + + await assert.rejects( + () => prepareIosRunner(IOS_SIMULATOR, { healthTimeoutMs: 90_000 }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.message, 'Runner did not accept connection (xcodebuild exited early)'); + return true; + }, + ); + assert.equal(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls.length, 0); + assert.equal(mockEnsureRunnerSession.mock.calls.length, 1); +}); From fe2876f55463480bfa41fa08467729083dd85548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 14 Sep 2026 19:57:37 +0200 Subject: [PATCH 3/7] test(apple-runner): a deadline route that the old wording check could not see MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The route-level case the review asked for: a readiness preflight failure that records the budget it ran out, carries the preflight marker, and says nothing about time in its message. The table restarts the session and replays; the deleted substring chain rethrew. Restoring that message check turns this one test red and leaves the rest of the file green, which is the difference the earlier cases could not show. The other half is the question the narrower wipe raises: a restored artifact whose runner never answers past its deadline on every attempt. It is not rebuilt. `runner-lifecycle-prepare-artifact.test.ts` pins what does happen — the artifact survives, the session is invalidated, the deadline is reported — and it goes red if the wipe goes back to firing on any prepare timeout. The same answer is now in the changelog, because "nothing rebuilds it" is a behavior someone will hit. `isRunnerReadinessPreflightError` moves into the contract module it was duplicating. Co-authored-by: Apex by Callstack --- CHANGELOG.md | 13 ++ .../runner-lifecycle-prepare-artifact.test.ts | 114 ++++++++++++++++++ ...nner-lifecycle-readiness-preflight.test.ts | 26 ++++ .../src/runner/runner-contract.ts | 10 ++ .../src/runner/runner-lifecycle.ts | 7 +- 5 files changed, 165 insertions(+), 5 deletions(-) create mode 100644 packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 961cc8406a..72e5669ac0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,19 @@ 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 command that ran out its connection deadline used to arrive as a bare + `AbortSignal.timeout` rejection whose message matched neither `timeout` nor `timed out`, so the + session was never restarted and the command failed for good. It now carries the budget it ran + out, and a readiness preflight that gave up on that budget restarts the runner and replays the + command whether or not the message mentions time. The other half of this is what no longer + happens: a prepare deadline, a slow boot, or a busy device no longer wipes a restored + `xcodebuild` artifact on the way to a rebuild, because only a runner that refused a connection or + never answered on any route says the artifact itself is at fault. A restored artifact whose runner + hangs past its deadline on every attempt does not rebuild itself: the runner session is + invalidated and the deadline is reported, and the rebuild needs either a failure that indicts the + artifact or the runner cache cleared by hand. + - 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::default`, one slot per checkout, and it stayed diff --git a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts new file mode 100644 index 0000000000..fd3932367b --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts @@ -0,0 +1,114 @@ +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 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('../runner-session.ts'); + return { + ...actual, + ensureRunnerSession: mockEnsureRunnerSession, + executeRunnerCommandWithSession: mockExecuteRunnerCommandWithSession, + getRunnerSessionSnapshot: mockGetRunnerSessionSnapshot, + invalidateRunnerSession: mockInvalidateRunnerSession, + }; +}); + +vi.mock('../runner-xctestrun.ts', async () => { + const actual = + await vi.importActual('../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 { + 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 prepare deadline does to a restored artifact. The wipe that rebuilds a suspect +// artifact comes from the rules that indict the artifact itself; a runner that never +// answers inside its budget indicts the boot, not the derived data it was launched from, +// so the artifact stays and the session goes. + +test('a restored artifact whose runner never answers past its deadline is kept while the session is dropped', async () => { + const restoredSession = makeRunnerSession({ + port: 8100, + xctestrunPath: '/tmp/restored.xctestrun', + xctestrunArtifact: makeRunnerArtifact({ xctestrunPath: '/tmp/restored.xctestrun' }), + }); + + mockEnsureRunnerSession.mockResolvedValue(restoredSession); + mockExecuteRunnerCommandWithSession.mockRejectedValue( + new AppError('COMMAND_FAILED', 'xcrun simctl spawn did not answer', { + cmd: 'xcrun', + timeoutMs: 45_000, + }), + ); + + await assert.rejects( + () => prepareIosRunner(IOS_SIMULATOR, { healthTimeoutMs: 90_000 }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.message, 'xcrun simctl spawn did not answer'); + return true; + }, + ); + + assert.equal(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls.length, 0); + assert.equal(mockEnsureRunnerSession.mock.calls.length, 2); + assert.deepEqual(mockInvalidateRunnerSession.mock.calls.at(-1), [ + restoredSession, + 'prepare_runner_health_failed', + ]); +}); diff --git a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts index ee6bf064f3..82be82a38e 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts @@ -133,6 +133,32 @@ test('mutating commands restart stale sessions when readiness preflight outlives assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession); }); +test('a readiness preflight deadline with no timeout wording still restarts the session', async () => { + const staleSession = makeRunnerSession({ port: 8100, ready: true }); + const freshSession = makeRunnerSession({ port: 8101, ready: false }); + + // The shape `fetchWithTimeout` reports its own expiry in: a recorded budget and the + // preflight marker, and a message that says nothing about time. A message check had + // been the only way to notice, and it would rethrow here without restarting a session + // whose runner never received the command. + mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession); + mockExecuteRunnerCommandWithSession + .mockRejectedValueOnce( + new AppError('COMMAND_FAILED', 'xcrun simctl spawn did not answer', { + cmd: 'xcrun', + timeoutMs: 45_000, + 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.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession); +}); + test('a readiness preflight refusal that is neither transport-shaped nor deadline-shaped surfaces', async () => { const staleSession = makeRunnerSession({ port: 8100, ready: true }); diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index d3accedc8e..b23541c7cc 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -351,6 +351,16 @@ export function shouldRetryRunnerConnectError(error: unknown): boolean { return runnerErrorVerdict(error, 'connectRetry') ?? true; } +/** + * The readiness preflight gave up before the command was written. The marker is the only + * evidence for that: an error carrying it arrives in whatever shape the preflight failed + * in, so no message check can be the test. + */ +export function isRunnerReadinessPreflightFailure(error: unknown): boolean { + if (!(error instanceof AppError)) return false; + return hasReadinessPreflightFailure((error.details ?? {}) as AppErrorDetails); +} + /** * The readiness preflight ran out of its own deadline, so the runner never saw the * command: restarting the session and replaying is both safe and the only way out. diff --git a/packages/platform-apple/src/runner/runner-lifecycle.ts b/packages/platform-apple/src/runner/runner-lifecycle.ts index f26c183f99..f2e5b570f8 100644 --- a/packages/platform-apple/src/runner/runner-lifecycle.ts +++ b/packages/platform-apple/src/runner/runner-lifecycle.ts @@ -16,6 +16,7 @@ import { import { assertRunnerRequestActive, isRetryableRunnerError, + isRunnerReadinessPreflightFailure, resolveRunnerRequestSignal, shouldRebuildCachedRunnerArtifact, shouldRestartRunnerAfterReadinessPreflight, @@ -600,13 +601,9 @@ function emitPrepareDiagnostic( }); } -function isRunnerReadinessPreflightError(error: AppError): boolean { - return error.details?.runnerReadinessPreflightFailed === true; -} - function shouldRestartAfterReadinessPreflightError(error: AppError): boolean { return ( - isRunnerReadinessPreflightError(error) && + isRunnerReadinessPreflightFailure(error) && (isRetryableRunnerError(error) || shouldRestartRunnerAfterReadinessPreflight(error)) ); } From b9f8139606191d115ab944439a3e27281e5df66c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 14 Sep 2026 21:05:04 +0200 Subject: [PATCH 4/7] test(apple-runner): the prepare fixture says what a real deadline says MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both fixtures invented wording, which is the mistake the review caught: the case they are supposed to pin is a specific producer's error, and a message nobody emits cannot show that a deleted message check used to fire on it. `ensureRunnerAttemptCanStart` reports an exhausted startup attempt as "Runner connection deadline exceeded" with the budget in details — the word the deleted `isPrepareHealthTimeout` matched. With that message the test fails when a wording check goes back into the wipe decision, and passes with the invented "xcrun simctl spawn did not answer" against the same mutant, which is the hole the fixture used to leave. `fetchWithTimeout` reports its own expiry as "Runner command deadline exceeded", and the preflight marker rides along on the way out of the preflight catch; neither deleted message chain looked for "deadline", so the route-level case is now built from that shape. The changelog said the `fetchWithTimeout` wrap reaches the lifecycle on the preflight route. It does not: `waitForRunner` turns each endpoint failure into "Runner did not accept connection". The reachable case is the direct post on the simulator and usbmux paths, which is what the entry says now. Co-authored-by: Apex by Callstack --- CHANGELOG.md | 22 +++++++++---------- .../runner-lifecycle-prepare-artifact.test.ts | 18 ++++++++------- ...nner-lifecycle-readiness-preflight.test.ts | 15 +++++++------ 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72e5669ac0..fe614c04c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,17 +21,17 @@ 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 command that ran out its connection deadline used to arrive as a bare - `AbortSignal.timeout` rejection whose message matched neither `timeout` nor `timed out`, so the - session was never restarted and the command failed for good. It now carries the budget it ran - out, and a readiness preflight that gave up on that budget restarts the runner and replays the - command whether or not the message mentions time. The other half of this is what no longer - happens: a prepare deadline, a slow boot, or a busy device no longer wipes a restored - `xcodebuild` artifact on the way to a rebuild, because only a runner that refused a connection or - never answered on any route says the artifact itself is at fault. A restored artifact whose runner - hangs past its deadline on every attempt does not rebuild itself: the runner session is - invalidated and the deadline is reported, and the rebuild needs either a failure that indicts the - artifact or the runner cache cleared by hand. + wording. A readiness preflight that runs out of time posting to the runner records the budget it + ran out and says "Runner command deadline exceeded" — neither of the two message checks on the + recovery paths looked for that phrasing, so the session was never restarted and the command was + never replayed. The rule reads the preflight marker and the recorded deadline now, so the restart + happens whatever the message happens to say. The other half of this is what no longer happens: a + prepare deadline, a slow boot, or a busy device no longer wipes a restored `xcodebuild` artifact on + the way to a rebuild, because only a runner that refused a connection or never answered on any + route says the artifact itself is at fault. A restored artifact whose runner hangs past its + deadline on every attempt does not rebuild itself: the runner session is invalidated and the + deadline is reported, and a rebuild needs either a failure that indicts the artifact or the runner + cache cleared by hand. - 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 diff --git a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts index fd3932367b..b0871b78fc 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts @@ -76,12 +76,14 @@ beforeEach(() => { }); }); -// What a prepare deadline does to a restored artifact. The wipe that rebuilds a suspect -// artifact comes from the rules that indict the artifact itself; a runner that never -// answers inside its budget indicts the boot, not the derived data it was launched from, -// so the artifact stays and the session goes. +// What a prepare deadline does to a restored artifact. The wipe that rebuilds a suspect artifact +// comes from the rules that indict the artifact itself; a runner that never answers inside its +// budget indicts the boot, not the derived data it was launched from, so the artifact stays and +// the session goes. The error is the one `ensureRunnerAttemptCanStart` reports when the startup +// attempt is already out of time: "Runner connection deadline exceeded" is what a real prepare +// deadline looks like, and the word in it is exactly what the deleted message check matched. -test('a restored artifact whose runner never answers past its deadline is kept while the session is dropped', async () => { +test('a restored artifact whose runner outlives the prepare deadline is kept while the session goes', async () => { const restoredSession = makeRunnerSession({ port: 8100, xctestrunPath: '/tmp/restored.xctestrun', @@ -90,8 +92,8 @@ test('a restored artifact whose runner never answers past its deadline is kept w mockEnsureRunnerSession.mockResolvedValue(restoredSession); mockExecuteRunnerCommandWithSession.mockRejectedValue( - new AppError('COMMAND_FAILED', 'xcrun simctl spawn did not answer', { - cmd: 'xcrun', + new AppError('COMMAND_FAILED', 'Runner connection deadline exceeded', { + port: 8100, timeoutMs: 45_000, }), ); @@ -100,7 +102,7 @@ test('a restored artifact whose runner never answers past its deadline is kept w () => prepareIosRunner(IOS_SIMULATOR, { healthTimeoutMs: 90_000 }), (error: unknown) => { assert.ok(error instanceof AppError); - assert.equal(error.message, 'xcrun simctl spawn did not answer'); + assert.equal(error.message, 'Runner connection deadline exceeded'); return true; }, ); diff --git a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts index 82be82a38e..b24666b6c7 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts @@ -133,19 +133,20 @@ test('mutating commands restart stale sessions when readiness preflight outlives assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession); }); -test('a readiness preflight deadline with no timeout wording still restarts the session', async () => { +test('a readiness preflight that runs out a post deadline restarts the session and replays', async () => { const staleSession = makeRunnerSession({ port: 8100, ready: true }); const freshSession = makeRunnerSession({ port: 8101, ready: false }); - // The shape `fetchWithTimeout` reports its own expiry in: a recorded budget and the - // preflight marker, and a message that says nothing about time. A message check had - // been the only way to notice, and it would rethrow here without restarting a session - // whose runner never received the command. + // The real shape of a preflight that ran out of time on a direct post: the simulator and + // usbmux paths post to the runner themselves, and `fetchWithTimeout` reports its expiry as + // "Runner command deadline exceeded" with the budget it ran out. The preflight marker is + // added on the way past the preflight catch. Neither of the deleted message checks matched + // that wording, so the session was never restarted and the command was never replayed. mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession); mockExecuteRunnerCommandWithSession .mockRejectedValueOnce( - new AppError('COMMAND_FAILED', 'xcrun simctl spawn did not answer', { - cmd: 'xcrun', + new AppError('COMMAND_FAILED', 'Runner command deadline exceeded', { + port: 8100, timeoutMs: 45_000, runnerReadinessPreflightFailed: true, }), From 131980cf1ddf60d7a69f85101ddc98b73f8110b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 14 Sep 2026 21:13:28 +0200 Subject: [PATCH 5/7] test(apple-runner): let a real prepare deadline fail the artifact check --- CHANGELOG.md | 13 ++-- .../runner-lifecycle-prepare-artifact.test.ts | 73 ++++++++++--------- 2 files changed, 46 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe614c04c8..33cc648703 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,13 +25,12 @@ ran out and says "Runner command deadline exceeded" — neither of the two message checks on the recovery paths looked for that phrasing, so the session was never restarted and the command was never replayed. The rule reads the preflight marker and the recorded deadline now, so the restart - happens whatever the message happens to say. The other half of this is what no longer happens: a - prepare deadline, a slow boot, or a busy device no longer wipes a restored `xcodebuild` artifact on - the way to a rebuild, because only a runner that refused a connection or never answered on any - route says the artifact itself is at fault. A restored artifact whose runner hangs past its - deadline on every attempt does not rebuild itself: the runner session is invalidated and the - deadline is reported, and a rebuild needs either a failure that indicts the artifact or the runner - cache cleared by hand. + happens whatever the message happens to say. 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 diff --git a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts index b0871b78fc..3309284b0f 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-prepare-artifact.test.ts @@ -2,6 +2,7 @@ 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'; @@ -76,41 +77,47 @@ beforeEach(() => { }); }); -// What a prepare deadline does to a restored artifact. The wipe that rebuilds a suspect artifact -// comes from the rules that indict the artifact itself; a runner that never answers inside its -// budget indicts the boot, not the derived data it was launched from, so the artifact stays and -// the session goes. The error is the one `ensureRunnerAttemptCanStart` reports when the startup -// attempt is already out of time: "Runner connection deadline exceeded" is what a real prepare -// deadline looks like, and the word in it is exactly what the deleted message check matched. +// 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 restored artifact whose runner outlives the prepare deadline is kept while the session goes', async () => { - const restoredSession = makeRunnerSession({ - port: 8100, - xctestrunPath: '/tmp/restored.xctestrun', - xctestrunArtifact: makeRunnerArtifact({ xctestrunPath: '/tmp/restored.xctestrun' }), - }); - - mockEnsureRunnerSession.mockResolvedValue(restoredSession); - mockExecuteRunnerCommandWithSession.mockRejectedValue( - new AppError('COMMAND_FAILED', 'Runner connection deadline exceeded', { +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, - timeoutMs: 45_000, - }), - ); + 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 }), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.message, 'Runner connection deadline exceeded'); - return true; - }, - ); + 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.equal(mockEnsureRunnerSession.mock.calls.length, 2); - assert.deepEqual(mockInvalidateRunnerSession.mock.calls.at(-1), [ - restoredSession, - 'prepare_runner_health_failed', - ]); + assert.equal(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls.length, 0); + assert.deepEqual(mockInvalidateRunnerSession.mock.calls.at(-1), [ + restoredSession, + 'prepare_runner_health_retry', + ]); + } finally { + vi.useRealTimers(); + } }); From 9b4a130eb9e48a31b1844b8f6c6b8b5051199491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 15 Sep 2026 08:38:31 +0200 Subject: [PATCH 6/7] fix(apple-runner): the preflight marker alone decides the restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule asked for two facts: the marker the readiness preflight puts on its way out, and a recorded budget in the same details. A device run says those two rarely arrive together. Wedging a warm runner (`kill -STOP`, then a `tap`) produced "Runner did not accept connection" and invalidated with `runner_connect_failed_before_command_send`; the budget-carrying shape appeared only post-send, where replaying is not safe and no restart is wanted. A preflight reaches the caller in whatever shape its connect loop ended with — a refusal, an exhausted probe, a killed `simctl` fallback, a post that ran out of its budget — and only the last of those carries `timeoutMs` at the top level. The marker is the fact the decision needs, so it is now the only one asked for. `hasCommandDeadline` and `hasReadinessPreflightDeadline` go with it, `shouldRestartAfterReadinessPreflightError` reduces to the table lookup and is gone, and `isRunnerReadinessPreflightFailure` loses its only consumer. `connectRetry` left the verdicts too: the marker is applied after the connect loop has returned, so no loop can ever consult it. The mocked decision tests stop asserting a message, which is what makes them enough: the shape is no longer part of the rule, and the shapes a real preflight produces are what the device run recorded rather than what a fixture invents. Restoring the budget conjunction turns the refusal and the `fetch failed` cases red again, so the tests still discriminate the claim. --- CHANGELOG.md | 11 +++-- .../runner-error-classification.test.ts | 24 +++++++--- ...nner-lifecycle-readiness-preflight.test.ts | 47 +++++++++---------- .../src/runner/runner-contract.ts | 35 ++++---------- .../src/runner/runner-lifecycle.ts | 10 +--- 5 files changed, 57 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33cc648703..64f4bd866f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,11 +21,12 @@ 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 that runs out of time posting to the runner records the budget it - ran out and says "Runner command deadline exceeded" — neither of the two message checks on the - recovery paths looked for that phrasing, so the session was never restarted and the command was - never replayed. The rule reads the preflight marker and the recorded deadline now, so the restart - happens whatever the message happens to say. The other half is what no longer happens: when a slow + 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. 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 diff --git a/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts b/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts index a59b8ca075..2460c88264 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts @@ -77,23 +77,35 @@ test('connect loop stops for terminal verdicts', () => { assert.equal(shouldRetryRunnerConnectError(new AppError('DEVICE_NOT_FOUND', 'gone')), true); }); -// --- connect deadline --- +// --- readiness preflight --- -test('a readiness preflight that outlived its deadline restarts the session', () => { - // The marked shape this recovery actually sees: the simctl-spawn fallback killed at - // its budget, then marked by the readiness preflight that was waiting on it. +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(shouldRetryRunnerConnectError(killedSpawn), true); - // A preflight refusal with no deadline restarts nothing: the runner answered and said no. 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, ); }); diff --git a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts index b24666b6c7..eb030bbebf 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts @@ -106,15 +106,19 @@ test('mutating commands restart stale sessions when readiness preflight fails be assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession); }); -test('mutating commands restart stale sessions when readiness preflight outlives its deadline', async () => { +test('a readiness preflight that runs out a post deadline restarts the session and replays', async () => { const staleSession = makeRunnerSession({ port: 8100, ready: true }); const freshSession = makeRunnerSession({ port: 8101, ready: false }); + // The simulator and usbmux routes post to the runner themselves, and `fetchWithTimeout` reports + // an expiry as "Runner command deadline exceeded" with the budget it ran out. Neither of the two + // message checks this rule replaced matched that wording, so the command was never replayed. + // What routes it is the marker the preflight catch puts on its way out, not the wording. mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession); mockExecuteRunnerCommandWithSession .mockRejectedValueOnce( - new AppError('COMMAND_FAILED', 'xcrun timed out after 45000ms', { - cmd: 'xcrun', + new AppError('COMMAND_FAILED', 'Runner command deadline exceeded', { + port: 8100, timeoutMs: 45_000, runnerReadinessPreflightFailed: true, }), @@ -125,29 +129,20 @@ test('mutating commands restart stale sessions when readiness preflight outlives 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('a readiness preflight that runs out a post deadline restarts the session and replays', async () => { +test('a readiness preflight refusal restarts the session like any other preflight failure', async () => { const staleSession = makeRunnerSession({ port: 8100, ready: true }); const freshSession = makeRunnerSession({ port: 8101, ready: false }); - // The real shape of a preflight that ran out of time on a direct post: the simulator and - // usbmux paths post to the runner themselves, and `fetchWithTimeout` reports its expiry as - // "Runner command deadline exceeded" with the budget it ran out. The preflight marker is - // added on the way past the preflight catch. Neither of the deleted message checks matched - // that wording, so the session was never restarted and the command was never replayed. + // This is the shape no message check could have been written for: the runner answered the probe + // and the answer was no, which says nothing about whether the command was written. The marker + // says that, and it says it for every shape at once. mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession); mockExecuteRunnerCommandWithSession .mockRejectedValueOnce( - new AppError('COMMAND_FAILED', 'Runner command deadline exceeded', { - port: 8100, - timeoutMs: 45_000, + new AppError('COMMAND_FAILED', 'Runner readiness refused', { runnerReadinessPreflightFailed: true, }), ) @@ -156,18 +151,20 @@ test('a readiness preflight that runs out a post deadline restarts the session a 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.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], freshSession); + assert.deepEqual(mockInvalidateRunnerSession.mock.calls[0], [ + staleSession, + 'runner_readiness_preflight_failed_before_command_send', + ]); }); -test('a readiness preflight refusal that is neither transport-shaped nor deadline-shaped surfaces', async () => { - const staleSession = makeRunnerSession({ port: 8100, ready: true }); +test('a failed readiness probe without the marker does not restart the session', async () => { + const session = makeRunnerSession({ port: 8100, ready: true }); - mockEnsureRunnerSession.mockResolvedValueOnce(staleSession); + // Without the marker the failure is just a transport shape, and the one that says the command + // was never written is the reason this restart is safe at all. + mockEnsureRunnerSession.mockResolvedValueOnce(session); mockExecuteRunnerCommandWithSession.mockRejectedValueOnce( - new AppError('COMMAND_FAILED', 'Runner readiness refused', { - runnerReadinessPreflightFailed: true, - }), + new AppError('COMMAND_FAILED', 'Runner readiness refused'), ); await assert.rejects( diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index b23541c7cc..0fa90ccf3a 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -168,13 +168,8 @@ type RunnerErrorMatch = { const hasRetriableFlag: RunnerErrorDetailsMatch = (details) => details.retriable === true; const hasUsbmuxDeviceUnattached: RunnerErrorDetailsMatch = (details) => details.usbmuxDeviceAttached === false; -/** The numeric deadline a command timeout records, including an `AbortSignal.timeout` wrap. */ -const hasCommandDeadline: RunnerErrorDetailsMatch = (details) => - typeof details.timeoutMs === 'number'; const hasReadinessPreflightFailure: RunnerErrorDetailsMatch = (details) => details.runnerReadinessPreflightFailed === true; -const hasReadinessPreflightDeadline: RunnerErrorDetailsMatch = (details) => - hasReadinessPreflightFailure(details) && hasCommandDeadline(details); type RunnerErrorVerdicts = { /** isRetryableRunnerError: transport error worth a same-session resend. */ @@ -185,7 +180,7 @@ type RunnerErrorVerdicts = { sessionFatalReason?: string; /** Connect-shaped failure before the command was sent: restart the session and replay. */ restartBeforeSend?: boolean; - /** Readiness preflight gave up on its deadline: restart the session and replay. */ + /** Readiness preflight gave up before the command was written: restart the session and replay. */ restartAfterReadinessPreflight?: boolean; /** The runner never accepted a connection, so the restored artifact itself is suspect. */ artifactSuspect?: boolean; @@ -234,13 +229,13 @@ export const RUNNER_ERROR_RULES: readonly RunnerErrorRule[] = [ verdicts: { retryable: false, connectRetry: false }, }, { - // A deadline alone earns no 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 it is not. Only the preflight marker says the runner - // never saw the command, and it sits beside the deadline here for that reason. - reason: 'runner_readiness_preflight_deadline', - match: { code: 'COMMAND_FAILED', details: hasReadinessPreflightDeadline }, - verdicts: { restartAfterReadinessPreflight: true, connectRetry: true }, + // The marker is the whole fact: the preflight gave up before the command was written, so + // replaying it cannot duplicate anything. It is asked for alone because the preflight fails in + // whatever shape the connect loop ended with — a refusal, an exhausted probe, a killed + // fallback — and a rule that also required a recorded budget would fire on only some of them. + reason: 'runner_readiness_preflight_failed', + match: { code: 'COMMAND_FAILED', details: hasReadinessPreflightFailure }, + verdicts: { restartAfterReadinessPreflight: true }, }, { reason: 'runner_connect_refused', @@ -352,18 +347,8 @@ export function shouldRetryRunnerConnectError(error: unknown): boolean { } /** - * The readiness preflight gave up before the command was written. The marker is the only - * evidence for that: an error carrying it arrives in whatever shape the preflight failed - * in, so no message check can be the test. - */ -export function isRunnerReadinessPreflightFailure(error: unknown): boolean { - if (!(error instanceof AppError)) return false; - return hasReadinessPreflightFailure((error.details ?? {}) as AppErrorDetails); -} - -/** - * The readiness preflight ran out of its own deadline, so the runner never saw the - * command: restarting the session and replaying is both safe and the only way out. + * The readiness preflight gave up, so the runner never saw the command: restarting the session and + * replaying is both safe and the only way out. The marker carries the rule; the message does not. */ export function shouldRestartRunnerAfterReadinessPreflight(error: unknown): boolean { return runnerErrorVerdict(error, 'restartAfterReadinessPreflight') ?? false; diff --git a/packages/platform-apple/src/runner/runner-lifecycle.ts b/packages/platform-apple/src/runner/runner-lifecycle.ts index f2e5b570f8..3c0c309f75 100644 --- a/packages/platform-apple/src/runner/runner-lifecycle.ts +++ b/packages/platform-apple/src/runner/runner-lifecycle.ts @@ -16,7 +16,6 @@ import { import { assertRunnerRequestActive, isRetryableRunnerError, - isRunnerReadinessPreflightFailure, resolveRunnerRequestSignal, shouldRebuildCachedRunnerArtifact, shouldRestartRunnerAfterReadinessPreflight, @@ -309,7 +308,7 @@ export async function executeRunnerCommand( restartReason: 'runner_connect_failed_before_command_send', }); } - if (session && shouldRestartAfterReadinessPreflightError(appErr)) { + if (session && shouldRestartRunnerAfterReadinessPreflight(appErr)) { assertRunnerRequestActive(options.requestId); return await restartSessionAndRunCommand({ device, @@ -600,10 +599,3 @@ function emitPrepareDiagnostic( }, }); } - -function shouldRestartAfterReadinessPreflightError(error: AppError): boolean { - return ( - isRunnerReadinessPreflightFailure(error) && - (isRetryableRunnerError(error) || shouldRestartRunnerAfterReadinessPreflight(error)) - ); -} From f83bdfe5bde7e158e5d7d5b5180f2a4826b2f8ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 15 Sep 2026 10:51:29 +0200 Subject: [PATCH 7/7] fix(apple-runner): a canceled request is not a runner that stopped answering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The marker says the preflight gave up, and one of the things a preflight gives up on is a caller that stopped waiting. `createRequestCanceledError` answers `COMMAND_FAILED` too, so the rule written last round matched a cancellation exactly as well as a wedged runner: on a session that was ready and still working, the cancel invalidated the session and replayed the canceled command against a fresh boot nobody was going to wait for. The main branch rethrew cancellations before any restart branch could see them; the marker rule made the marker the only question, and the marker does not distinguish the two shapes it is applied to. The exception belongs on the rule, where the verdict is, rather than on the call site that happens to notice: a marked error restarts the session unless the typed reason on it says the request was canceled. `isRequestCanceledDetails` is exported from the owning type so the rule reads that reason instead of restating the literal, and `isRequestCanceledError` now asks it. Deleting the conjunct turns both the classification test and the lifecycle test red — the lifecycle one pins the exact shape from the report: ready session, marked cancellation, error rethrown, no invalidate called, no second boot. --- CHANGELOG.md | 4 ++- packages/kernel/src/errors.ts | 11 ++++++- .../runner-error-classification.test.ts | 10 +++++- ...nner-lifecycle-readiness-preflight.test.ts | 31 ++++++++++++++++++- .../src/runner/runner-contract.ts | 9 +++++- 5 files changed, 60 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64f4bd866f..1430c33046 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,9 @@ `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. Two message checks decided it before, + 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 diff --git a/packages/kernel/src/errors.ts b/packages/kernel/src/errors.ts index 95a1c70ece..b482a210dd 100644 --- a/packages/kernel/src/errors.ts +++ b/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; diff --git a/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts b/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts index 2460c88264..eb8590480d 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts @@ -1,6 +1,6 @@ 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, @@ -108,6 +108,14 @@ test('the preflight marker alone decides the restart', () => { 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', () => { diff --git a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts index eb030bbebf..c105197a6a 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts @@ -1,6 +1,10 @@ import assert from 'node:assert/strict'; import { beforeEach, test, vi } from 'vitest'; -import { AppError } from '@agent-device/kernel/errors'; +import { + AppError, + createRequestCanceledError, + isRequestCanceledError, +} from '@agent-device/kernel/errors'; import { appleRunnerTestHost } from '../test-host.ts'; import type { RunnerXctestrunArtifact } from '../runner-xctestrun.ts'; import { IOS_SIMULATOR } from './device-fixtures.ts'; @@ -178,6 +182,31 @@ test('a failed readiness probe without the marker does not restart the session', assert.equal(mockEnsureRunnerSession.mock.calls.length, 1); }); +test('a cancellation during the readiness preflight does not restart the session it canceled', async () => { + const session = makeRunnerSession({ port: 8100, ready: true }); + + // The preflight's catch marks whatever it was waiting on when it gave up, and one of the things + // it waits on is a caller that stopped waiting. That mark describes a walkaway, not a wedged + // runner, and a session that is ready is the one the caller just left: restarting it would boot a + // runner for a command nobody is going to send again, and take down a session that still works. + mockEnsureRunnerSession.mockResolvedValueOnce(session); + mockExecuteRunnerCommandWithSession.mockRejectedValueOnce( + createRequestCanceledError({ runnerReadinessPreflightFailed: true, command: 'tap' }), + ); + + await assert.rejects( + () => runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.ok(isRequestCanceledError(error)); + return true; + }, + ); + assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0); + assert.equal(mockEnsureRunnerSession.mock.calls.length, 1); + assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 1); +}); + test('a boot that exited early does not wipe a restored runner artifact', async () => { const restoredSession = makeRunnerSession({ port: 8100, diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index 0fa90ccf3a..7b7501a140 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -1,6 +1,7 @@ import { AppError, createRequestCanceledError, + isRequestCanceledDetails, toAppErrorCode, type AppErrorCode, type AppErrorDetails, @@ -168,8 +169,14 @@ type RunnerErrorMatch = { const hasRetriableFlag: RunnerErrorDetailsMatch = (details) => details.retriable === true; const hasUsbmuxDeviceUnattached: RunnerErrorDetailsMatch = (details) => details.usbmuxDeviceAttached === false; +/** + * The preflight marks whatever it was waiting on when it stopped, and one of the things it waits on + * is a caller that stopped waiting. A canceled request is not a wedged runner: the restart this + * marker authorises would boot a runner for a command nobody is going to send again. Every abort in + * the connect loop normalizes to the typed canceled reason before it reaches here. + */ const hasReadinessPreflightFailure: RunnerErrorDetailsMatch = (details) => - details.runnerReadinessPreflightFailed === true; + details.runnerReadinessPreflightFailed === true && !isRequestCanceledDetails(details); type RunnerErrorVerdicts = { /** isRetryableRunnerError: transport error worth a same-session resend. */