From 56d63cbbbafdd89bc418d80618f1c9481b80872c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Kwas=CC=81niewski?= Date: Fri, 25 Sep 2026 17:41:33 +0200 Subject: [PATCH 1/2] fix(ios-runner): a caller deadline no longer kills a starting runner A wait poll bounds each attempt with a TimeoutError abort. That signal was merged with the request cancellation and handed to the xcodebuild launch, so a short wait timeout during runner start killed the runner and the retry paid the whole start again. The start now reacts only to request cancellation; a caller deadline leaves the session starting so the next request joins it. Closes #2894. --- .../runner-contract-request-deadline.test.ts | 85 +++++++++++++++++++ .../runner-request-cancellation.test.ts | 50 ++++++++++- .../src/runner/runner-client.ts | 18 +--- .../src/runner/runner-contract.ts | 58 +++++++++++++ .../src/runner/runner-lifecycle.ts | 6 +- .../src/runner/runner-session.ts | 13 +-- 6 files changed, 205 insertions(+), 25 deletions(-) create mode 100644 packages/platform-apple/src/runner/__tests__/runner-contract-request-deadline.test.ts diff --git a/packages/platform-apple/src/runner/__tests__/runner-contract-request-deadline.test.ts b/packages/platform-apple/src/runner/__tests__/runner-contract-request-deadline.test.ts new file mode 100644 index 0000000000..422547e56c --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-contract-request-deadline.test.ts @@ -0,0 +1,85 @@ +import assert from 'node:assert/strict'; +import { beforeEach, test } from 'vitest'; +import { createRequestCanceledError } from '@agent-device/kernel/errors'; +import { appleRunnerTestHost } from '../test-host.ts'; +import { + callerDeadlineExpired, + isCallerDeadlineAbortReason, + resolveRunnerStartupSignal, +} from '../runner-contract.ts'; + +const registered = new Map(); +const canceled = new Set(); + +function deadlineReason(): DOMException { + return new DOMException('Wait deadline exceeded', 'TimeoutError'); +} + +beforeEach(() => { + registered.clear(); + canceled.clear(); + appleRunnerTestHost.update({ + getRequestSignal: (requestId) => (requestId ? registered.get(requestId)?.signal : undefined), + isRequestCanceled: (requestId) => requestId !== undefined && canceled.has(requestId), + }); +}); + +test('a caller deadline is the typed TimeoutError reason, not its text', () => { + assert.equal(isCallerDeadlineAbortReason(deadlineReason()), true); + assert.equal(isCallerDeadlineAbortReason(new Error('Wait deadline exceeded')), false); + assert.equal(isCallerDeadlineAbortReason(createRequestCanceledError()), false); +}); + +test('the startup signal ignores a caller deadline and forwards every other abort', () => { + const deadline = new AbortController(); + const startup = resolveRunnerStartupSignal({ signal: deadline.signal }); + assert.ok(startup); + deadline.abort(deadlineReason()); + assert.equal(startup.aborted, false); + + const disconnect = new AbortController(); + const killed = resolveRunnerStartupSignal({ signal: disconnect.signal }); + assert.ok(killed); + const reason = new Error('client disconnected'); + disconnect.abort(reason); + assert.equal(killed.aborted, true); + assert.equal(killed.reason, reason); +}); + +test('a caller signal already aborted by its deadline does not abort the startup signal', () => { + const expired = new AbortController(); + expired.abort(deadlineReason()); + const startup = resolveRunnerStartupSignal({ signal: expired.signal }); + assert.equal(startup?.aborted, false); +}); + +test('the registered request signal kills a start even when it rides with a caller deadline', () => { + const request = new AbortController(); + registered.set('req-1', request); + const deadline = new AbortController(); + const startup = resolveRunnerStartupSignal({ requestId: 'req-1', signal: deadline.signal }); + assert.ok(startup); + deadline.abort(deadlineReason()); + assert.equal(startup.aborted, false); + request.abort(createRequestCanceledError()); + assert.equal(startup.aborted, true); +}); + +test('without a caller signal the registered request signal is the startup signal itself', () => { + const request = new AbortController(); + registered.set('req-2', request); + assert.equal(resolveRunnerStartupSignal({ requestId: 'req-2' }), request.signal); + assert.equal(resolveRunnerStartupSignal({}), undefined); +}); + +test('callerDeadlineExpired reads the deadline reason and yields to a cancelled request', () => { + const deadline = new AbortController(); + deadline.abort(deadlineReason()); + assert.equal(callerDeadlineExpired({ signal: deadline.signal }), true); + canceled.add('req-3'); + assert.equal(callerDeadlineExpired({ requestId: 'req-3', signal: deadline.signal }), false); + const plain = new AbortController(); + plain.abort(new Error('client disconnected')); + assert.equal(callerDeadlineExpired({ signal: plain.signal }), false); + assert.equal(callerDeadlineExpired({}), false); +}); diff --git a/packages/platform-apple/src/runner/__tests__/runner-request-cancellation.test.ts b/packages/platform-apple/src/runner/__tests__/runner-request-cancellation.test.ts index f6fbcc19d7..93c2363d4e 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-request-cancellation.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-request-cancellation.test.ts @@ -161,8 +161,9 @@ test('direct command cancellation reaches runner launch without a registered req const controller = new AbortController(); const device = { ...IOS_SIMULATOR, id: 'runner-direct-signal-sim' }; mockRunCmdBackground.mockImplementationOnce((_cmd, _args, options) => { - assert.equal(options?.signal, controller.signal); - controller.abort(new Error('wait deadline exceeded')); + assert.equal(options?.signal?.aborted, false); + controller.abort(new Error('client disconnected')); + assert.equal(options?.signal?.aborted, true); return makeBackgroundRunner(4141); }); @@ -178,6 +179,51 @@ test('direct command cancellation reaches runner launch without a registered req assert.equal(readRunnerSessionLiveness(device.id), null); }); +/** + * A `wait` poll bounds each attempt with a `TimeoutError` deadline. When that deadline lands while the + * runner is still starting, the start it interrupts is the one the retry needs: the launch must not be + * killed and the session must stay registered as starting, so the next request joins it (#2894). + */ +test('a caller deadline during runner start leaves the starting runner for the next request', async () => { + const controller = new AbortController(); + const device = { ...IOS_SIMULATOR, id: 'runner-caller-deadline-sim' }; + mockRunCmdBackground.mockImplementationOnce((_cmd, _args, options) => { + controller.abort(new DOMException('Wait deadline exceeded', 'TimeoutError')); + assert.equal(options?.signal?.aborted, false, 'the launch outlives the caller deadline'); + return makeBackgroundRunner(4545); + }); + mockWaitForRunner.mockImplementationOnce(async () => { + throw createRequestCanceledError(); + }); + + await assert.rejects( + executeRunnerCommand( + device, + { command: 'snapshot', appBundleId: 'com.example.demo' }, + { signal: controller.signal, logPath: '/tmp/runner.log' }, + ), + (error: unknown) => + isRequestCanceledError(error) && + (error as { details?: { readinessPhase?: string } }).details?.readinessPhase === + 'runner-start', + ); + assert.equal(readRunnerSessionLiveness(device.id)?.liveness, 'starting'); + assert.deepEqual(readRetainedLeaseDeviceIds(), [device.id]); + + await executeRunnerCommand( + device, + { command: 'snapshot', appBundleId: 'com.example.demo' }, + { logPath: '/tmp/runner.log' }, + ); + + assert.equal( + mockRunCmdBackground.mock.calls.length, + 1, + 'the retry did not launch a second runner', + ); + assert.equal(readRunnerSessionLiveness(device.id)?.liveness, 'ready'); +}); + test('prepare cancellation stops only its runner and preserves unrelated prep', async () => { const survivorRequestId = 'prepare-runner-survivor-B'; const canceledRequestId = 'prepare-runner-canceled-A'; diff --git a/packages/platform-apple/src/runner/runner-client.ts b/packages/platform-apple/src/runner/runner-client.ts index 76bcc29388..eb93d8f959 100644 --- a/packages/platform-apple/src/runner/runner-client.ts +++ b/packages/platform-apple/src/runner/runner-client.ts @@ -1,4 +1,4 @@ -import { retryWithPolicy, emitDiagnostic, getRequestSignal, isRequestCanceled } from './host.ts'; +import { retryWithPolicy, emitDiagnostic } from './host.ts'; import { isIosFamily, type DeviceInfo } from '@agent-device/kernel/device'; import { ensureRunnerSession, @@ -9,6 +9,7 @@ import { } from './runner-session.ts'; import { assertRunnerRequestActive, + callerDeadlineExpired, resolveRunnerRequestSignal, withRunnerCommandId, type RunnerCommand, @@ -57,21 +58,6 @@ function readOnlyResendBudget(error: unknown): number { return isRetryableRunnerError(error) ? TRANSPORT_RESEND_ATTEMPTS : 1; } -/** - * Whether the caller's own deadline ended this command, as opposed to the request being cancelled. - * A `wait` bounds each poll with an abort signal whose reason is a `TimeoutError` - * (`runWithinWaitDeadline`); a cancelled request aborts through the registered request signal or - * the cancellation registry. The typed reason decides, so a deadline that lands mid-fetch (surfacing - * as whatever the transport threw on abort) is read the same way as one that wakes a delay. - */ -function callerDeadlineExpired(options: AppleRunnerCommandOptions): boolean { - if (isRequestCanceled(options.requestId) || getRequestSignal(options.requestId)?.aborted) { - return false; - } - const reason: unknown = options.signal?.aborted ? options.signal.reason : undefined; - return reason instanceof DOMException && reason.name === 'TimeoutError'; -} - export async function runAppleRunnerCommand( device: DeviceInfo, command: RunnerCommand, diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index 86611eca91..5a180f963a 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -157,6 +157,64 @@ export function resolveRunnerRequestSignal(options: { return AbortSignal.any([registeredSignal, options.signal]); } +type RunnerRequestSignalOptions = { + requestId?: string; + signal?: AbortSignal; +}; + +/** + * Whether an abort reason is a caller's own deadline rather than a cancelled request. A `wait` + * bounds each poll with an abort signal whose reason is a `TimeoutError` (`runWithinWaitDeadline`); + * a cancelled request aborts through the registered request signal or the cancellation registry. + * The typed reason decides, never the error text the transport threw on abort. + */ +export function isCallerDeadlineAbortReason(reason: unknown): boolean { + return reason instanceof DOMException && reason.name === 'TimeoutError'; +} + +/** + * Whether the caller's own deadline ended this command, as opposed to the request being cancelled. + * A deadline that lands mid-fetch (surfacing as whatever the transport threw on abort) is read the + * same way as one that wakes a delay. + */ +export function callerDeadlineExpired(options: RunnerRequestSignalOptions): boolean { + if (isRequestCanceled(options.requestId) || getRequestSignal(options.requestId)?.aborted) { + return false; + } + return options.signal?.aborted === true && isCallerDeadlineAbortReason(options.signal.reason); +} + +/** + * The signal a runner start reacts to. A cancelled request (client disconnect) must kill the + * blocking xctestrun build and the runner launch instead of orphaning them, so the registered request + * signal passes through untouched. A caller's own deadline must not: the runner start it interrupts + * is the one the retry needs, and a start that pays itself again on every short-timeout poll never + * finishes on a slow host (#2894). The start keeps going on its own startup budget, and the caller's + * command is still cut off by its unfiltered signal once the runner answers. + */ +export function resolveRunnerStartupSignal( + options: RunnerRequestSignalOptions, +): AbortSignal | undefined { + const registeredSignal = getRequestSignal(options.requestId); + const callerSignal = options.signal; + if (!callerSignal || callerSignal === registeredSignal) return registeredSignal; + const controller = new AbortController(); + const forward = (signal: AbortSignal) => { + if (controller.signal.aborted) return; + if (isCallerDeadlineAbortReason(signal.reason)) return; + controller.abort(signal.reason); + }; + for (const signal of [registeredSignal, callerSignal]) { + if (!signal) continue; + if (signal.aborted) { + forward(signal); + continue; + } + signal.addEventListener('abort', () => forward(signal), { once: true }); + } + return controller.signal; +} + /** * The code the XCTest runner answers with when it declines to place a scroll gesture under the * on-screen keyboard (#2500). It is the runner's own vocabulary, so it is declared here beside the diff --git a/packages/platform-apple/src/runner/runner-lifecycle.ts b/packages/platform-apple/src/runner/runner-lifecycle.ts index 5d50e464e9..cd20e081d1 100644 --- a/packages/platform-apple/src/runner/runner-lifecycle.ts +++ b/packages/platform-apple/src/runner/runner-lifecycle.ts @@ -21,6 +21,7 @@ import { } from './runner-session.ts'; import { assertRunnerRequestActive, + callerDeadlineExpired, resolveRunnerRequestSignal, withRunnerCommandId, type RunnerCommand, @@ -311,7 +312,10 @@ export async function executeRunnerCommand( ? session.state === 'starting' : livenessAtEntry !== 'ready'; if (runnerNeverAnswered && isRequestCanceledError(appErr)) { - if (session) { + // A cancelled request leaves no half-started runner behind. A caller whose own deadline ran + // out mid-start leaves it running: the start is on its own budget, and the next request + // joins it instead of paying it again (#2894). + if (session && !callerDeadlineExpired(options)) { await invalidateRunnerSessionBestEffort(session, 'runner_startup_request_canceled'); } throw createRequestCanceledError( diff --git a/packages/platform-apple/src/runner/runner-session.ts b/packages/platform-apple/src/runner/runner-session.ts index 5319e7f0e3..26a255ffc4 100644 --- a/packages/platform-apple/src/runner/runner-session.ts +++ b/packages/platform-apple/src/runner/runner-session.ts @@ -35,7 +35,7 @@ import { decodeRunnerResponseBody, isRunnerResponseOk, readRunnerResponseData, - resolveRunnerRequestSignal, + resolveRunnerStartupSignal, withRunnerCommandId, type RunnerCommand, } from './runner-contract.ts'; @@ -138,14 +138,15 @@ export async function ensureRunnerSession( return await withRunnerSessionLock(device.id, async () => { // One budget for the whole startup phase, opened here from the request-level // `startupTimeoutMs`: the reuse check's toolchain probes, adoption and the startup - // itself all spend this one clock. The request's abort signal rides with it, so a + // itself all spend this one clock. The request's cancellation rides with it, so a // client disconnect kills the blocking xctestrun build and runner launch - // (killProcessTree via exec) instead of orphaning them. Request-scoped: only this - // request's device startup reacts, and a signal-less internal caller (shutdown) - // simply gets undefined. + // (killProcessTree via exec) instead of orphaning them; a caller's own deadline does + // not, so the start it interrupts is still there for the retry (#2894). Request-scoped: + // only this request's device startup reacts, and a signal-less internal caller + // (shutdown) simply gets undefined. const startupBudget = createRunnerPhaseBudget( options.startupTimeoutMs, - resolveRunnerRequestSignal(options), + resolveRunnerStartupSignal(options), ); const existing = runnerSessions.get(device.id); if (existing) { From eeac88328a7639767b944856ef7d43dac5b41d8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Kwas=CC=81niewski?= Date: Fri, 25 Sep 2026 18:13:03 +0200 Subject: [PATCH 2/2] refactor(ios-runner): compose the startup signal without retained listeners The caller signal is filtered through its own controller and the registered request signal is composed with AbortSignal.any, so a request that polls many times does not accumulate abort listeners on its cancellation signal. --- .../src/runner/runner-contract.ts | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index 5a180f963a..8cdc220bee 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -198,21 +198,17 @@ export function resolveRunnerStartupSignal( const registeredSignal = getRequestSignal(options.requestId); const callerSignal = options.signal; if (!callerSignal || callerSignal === registeredSignal) return registeredSignal; - const controller = new AbortController(); - const forward = (signal: AbortSignal) => { - if (controller.signal.aborted) return; - if (isCallerDeadlineAbortReason(signal.reason)) return; - controller.abort(signal.reason); + // The caller signal is filtered through its own controller, so a deadline never reaches the + // start; the registered signal is composed with `AbortSignal.any`, which detaches its own + // listener once the composed signal settles, so a request that polls many times does not + // accumulate listeners on its long-lived cancellation signal. + const filtered = new AbortController(); + const forward = () => { + if (!isCallerDeadlineAbortReason(callerSignal.reason)) filtered.abort(callerSignal.reason); }; - for (const signal of [registeredSignal, callerSignal]) { - if (!signal) continue; - if (signal.aborted) { - forward(signal); - continue; - } - signal.addEventListener('abort', () => forward(signal), { once: true }); - } - return controller.signal; + if (callerSignal.aborted) forward(); + else callerSignal.addEventListener('abort', forward, { once: true }); + return registeredSignal ? AbortSignal.any([registeredSignal, filtered.signal]) : filtered.signal; } /**