diff --git a/packages/platform-apple/src/runner/__tests__/runner-adoption.test.ts b/packages/platform-apple/src/runner/__tests__/runner-adoption.test.ts index a96525252..b9bb459d9 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-adoption.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-adoption.test.ts @@ -13,7 +13,10 @@ import { } from '../runner-lease.ts'; import { isIosRunnerDetachEnabled, tryAdoptRunnerSessionFromLease } from '../runner-adoption.ts'; import { sendRunnerCommandOnce } from '../runner-transport.ts'; -import { resolveExpectedRunnerCacheMetadata } from '../runner-xctestrun.ts'; +import { + createRunnerPhaseBudget, + resolveExpectedRunnerCacheMetadata, +} from '../runner-xctestrun.ts'; import { appleRunnerTestHost } from '../test-host.ts'; import { appleToolchainProbeResult } from './apple-toolchain-fixtures.ts'; import { mkdtempForTestSync } from './tmp-dir.ts'; @@ -161,7 +164,9 @@ test('a request canceled during the fingerprint probe fails adoption instead of }); await expect( - tryAdoptRunnerSessionFromLease(simulator, { signal: request.signal }), + tryAdoptRunnerSessionFromLease(simulator, { + budget: createRunnerPhaseBudget(undefined, request.signal), + }), ).rejects.toSatisfy(isRequestCanceledError); expect(mockSendRunnerCommandOnce).not.toHaveBeenCalled(); @@ -186,7 +191,9 @@ test('a request canceled during the fingerprint probe fails adoption instead of ); await expect( - tryAdoptRunnerSessionFromLease(simulator, { signal: warmRequest.signal }), + tryAdoptRunnerSessionFromLease(simulator, { + budget: createRunnerPhaseBudget(undefined, warmRequest.signal), + }), ).rejects.toSatisfy(isRequestCanceledError); expect(mockSendRunnerCommandOnce).not.toHaveBeenCalled(); // Ownership was never transferred: the stale lease is untouched. diff --git a/packages/platform-apple/src/runner/__tests__/runner-artifact-phase-budget.test.ts b/packages/platform-apple/src/runner/__tests__/runner-artifact-phase-budget.test.ts index f641aa481..463ac1c45 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-artifact-phase-budget.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-artifact-phase-budget.test.ts @@ -6,7 +6,7 @@ import { AppError } from '@agent-device/kernel/errors'; import { resetAllProcessMemosForTests } from '@agent-device/kernel/ttl-memo'; import { appleRunnerTestHost } from '../test-host.ts'; import type { ExecOptions, ExecResult, ExecStreamOptions } from '../host.ts'; -import { ensureXctestrunArtifact } from '../runner-xctestrun.ts'; +import { createRunnerPhaseBudget, ensureXctestrunArtifact } from '../runner-xctestrun.ts'; import { appleToolchainProbeResult } from './apple-toolchain-fixtures.ts'; import { MACOS_DEVICE } from './device-fixtures.ts'; import { mkdtempForTestSync } from './tmp-dir.ts'; @@ -67,7 +67,7 @@ afterEach(() => { test('a warm toolchain leaves the build the whole phase budget', async () => { await assert.rejects( - ensureXctestrunArtifact(MACOS_DEVICE, { buildTimeoutMs: 120_000 }), + ensureXctestrunArtifact(MACOS_DEVICE, { budget: createRunnerPhaseBudget(120_000, undefined) }), missingXctestrun, ); @@ -88,7 +88,7 @@ test('a cold-start probe stall comes out of the build budget instead of being ad }); await assert.rejects( - ensureXctestrunArtifact(MACOS_DEVICE, { buildTimeoutMs: 120_000 }), + ensureXctestrunArtifact(MACOS_DEVICE, { budget: createRunnerPhaseBudget(120_000, undefined) }), missingXctestrun, ); @@ -107,7 +107,7 @@ test('a probe that spends the whole phase fails before xcodebuild is spawned', a }); await assert.rejects( - ensureXctestrunArtifact(MACOS_DEVICE, { buildTimeoutMs: 30_000 }), + ensureXctestrunArtifact(MACOS_DEVICE, { budget: createRunnerPhaseBudget(30_000, undefined) }), (error: unknown) => error instanceof AppError && error.details?.reason === 'runner_phase_budget_exhausted' && diff --git a/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts b/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts index 91415f15a..782572327 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts @@ -6,7 +6,7 @@ import { IOS_DEVICE, IOS_SIMULATOR, MACOS_DEVICE } from './device-fixtures.ts'; import { appleRunnerTestHost } from '../test-host.ts'; import type { ExecOptions } from '../host.ts'; import { - createRunnerPhaseDeadline, + createRunnerPhaseBudget, diffComparableRunnerCacheMetadata, resolveRunnerBundleBuildSettings, resolveRunnerMaxConcurrentDestinationsFlag, @@ -278,37 +278,16 @@ describe('toolchain probe budget', () => { assert.deepEqual(xcodebuildTimeouts, [COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, 15_000]); }); - test('a toolchain host that never returns stops at the shared budget instead of once per probe', () => { - const clock = installFakeToolchainClock(); - runCmdSync.mockImplementation((command: string, args: string[], options: ExecOptions) => { - throw blockForWholeTimeout(clock, command, args, options); - }); - runCmdSync.mockClear(); - - assert.throws( - () => resolveExpectedRunnerCacheMetadata(MACOS_DEVICE), - // A budget spent before the remaining probes could start is not an - // unreadable toolchain: nothing probed it, so the error says the budget - // ran out rather than pointing at `xcode-select`. - (error: unknown) => expectRunnerPhaseBudgetExhausted(error), - ); - // 30 s + a 15 s retry spends the whole budget on the first probe; the two - // xcrun probes then fail on the budget instead of blocking for 30 s each. - assert.equal(runCmdSync.mock.calls.length, 2); - assert.equal(clock.nowMs, 45_000); - }); - test('an owning phase with 4 s left gets one 4 s attempt and no retry', () => { const clock = installFakeToolchainClock(); - const phaseDeadline = createRunnerPhaseDeadline(4_000); + const phaseBudget = createRunnerPhaseBudget(4_000, undefined); runCmdSync.mockImplementation((command: string, args: string[], options: ExecOptions) => { throw blockForWholeTimeout(clock, command, args, options); }); runCmdSync.mockClear(); assert.throws( - () => - resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR, undefined, { deadline: phaseDeadline }), + () => resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR, undefined, phaseBudget), (error: unknown) => expectRunnerPhaseBudgetExhausted(error), ); assert.equal(runCmdSync.mock.calls.length, 1); @@ -317,75 +296,6 @@ describe('toolchain probe budget', () => { assert.equal(clock.nowMs, 4_000); }); - test('a request canceled while a probe blocked surfaces the cancellation instead of retrying', () => { - const clock = installFakeToolchainClock(); - const request = new AbortController(); - runCmdSync.mockImplementation((command: string, args: string[], options: ExecOptions) => { - const timeout = blockForWholeTimeout(clock, command, args, options); - request.abort(); - throw timeout; - }); - runCmdSync.mockClear(); - - assert.throws( - () => - resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR, undefined, { signal: request.signal }), - (error: unknown) => isRequestCanceledError(error), - ); - assert.equal(runCmdSync.mock.calls.length, 1); - }); - - test('a non-timeout error that also cancels the request on the last probe surfaces cancellation, not an unavailable toolchain', () => { - const request = new AbortController(); - runCmdSync.mockImplementation((command: string, args: string[]) => { - // The final probe: abort the request and fail with a plain command - // error, not the exec layer's structured timeout -- there is no next - // attempt left to catch the cancellation, so the catch here must. - if (command === 'xcrun' && args.includes('--show-sdk-build-version')) { - request.abort(); - throw new AppError('COMMAND_FAILED', 'xcrun: unexpected error', {}); - } - return appleToolchainProbeResult(command, args); - }); - runCmdSync.mockClear(); - - assert.throws( - () => - resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR, undefined, { signal: request.signal }), - (error: unknown) => isRequestCanceledError(error), - ); - assert.equal(runCmdSync.mock.calls.length, 3); - }); - - test('an already-canceled request runs no toolchain probe at all, cold or with the fingerprint cache warm', () => { - installFakeToolchainClock(); - runCmdSync.mockClear(); - - assert.throws( - () => - resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR, undefined, { - signal: AbortSignal.abort(), - }), - (error: unknown) => isRequestCanceledError(error), - ); - assert.equal(runCmdSync.mock.calls.length, 0); - - // Warm the real fingerprint memo with an ordinary request, then repeat - // with an already-aborted signal: the cache-hit path must check - // cancellation before it returns the memoized value, not skip it (#2422). - resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); - runCmdSync.mockClear(); - - assert.throws( - () => - resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR, undefined, { - signal: AbortSignal.abort(), - }), - (error: unknown) => isRequestCanceledError(error), - ); - assert.equal(runCmdSync.mock.calls.length, 0); - }); - test('a probe that failed on its own and merely says "timed out" in its message is not retried', () => { installFakeToolchainClock(); runCmdSync.mockImplementation((command: string, args: string[]) => { @@ -409,6 +319,174 @@ describe('toolchain probe budget', () => { ); expect(runCmdSync.mock.calls.filter(([command]) => command === 'xcodebuild')).toHaveLength(1); }); + + /** + * One row per way a fingerprint read can be interrupted: when the owning request aborts, + * what the probe that abort lands on does, and what the phase must then surface. + * `spawnSync` cannot be interrupted once it has started, so a cancellation is only ever + * observed between attempts -- the exec count is what pins which attempt each row stopped. + */ + type ToolchainProbeCancellationCase = { + label: string; + /** Which execs refuse to answer, and how they end; every other exec answers. */ + failing?: { at: 'first' | 'final' | 'every'; as: 'exec-timeout' | 'command-failure' }; + /** + * When the owning request aborts: never, before the phase runs a probe at all, while + * the failing exec is still blocked, or as that exec's timeout unwinds. + */ + aborts: 'never' | 'before-any-probe' | 'while-it-blocks' | 'as-it-unwinds'; + /** Whether the process-wide fingerprint memo already holds an answer. */ + fingerprintCache?: 'warm'; + expected: 'request-canceled' | 'budget-exhausted' | 'fingerprint'; + /** Execs the phase is allowed to have run by the time it settles. */ + execs: number; + /** Wall clock the phase spent: only an exec that blocks for its whole timeout moves it. */ + clockMs: number; + }; + + const CANCELLATION_CASES: ToolchainProbeCancellationCase[] = [ + { + label: 'aborted before the first probe, cold cache', + aborts: 'before-any-probe', + expected: 'request-canceled', + execs: 0, + clockMs: 0, + }, + { + // A cache hit must not answer a request that is already gone (#2422 round 4). + label: 'aborted before the first probe, fingerprint cache warm', + aborts: 'before-any-probe', + fingerprintCache: 'warm', + expected: 'request-canceled', + execs: 0, + clockMs: 0, + }, + { + label: 'aborted while the first probe blocks, and it then times out', + failing: { at: 'first', as: 'exec-timeout' }, + aborts: 'while-it-blocks', + expected: 'request-canceled', + execs: 1, + clockMs: COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, + }, + { + label: 'aborted while the first probe fails with a non-timeout error', + failing: { at: 'first', as: 'command-failure' }, + aborts: 'while-it-blocks', + expected: 'request-canceled', + execs: 1, + clockMs: 0, + }, + { + // The budget still had 15 s: only the cancellation stops the retry. + label: "aborted as the first attempt's timeout unwinds, before its retry", + failing: { at: 'first', as: 'exec-timeout' }, + aborts: 'as-it-unwinds', + expected: 'request-canceled', + execs: 1, + clockMs: COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, + }, + { + // Nothing is left to catch the cancellation on a later attempt, so the failing + // probe's own catch must: an unavailable toolchain would be the wrong verdict. + label: 'aborted while the final probe fails with a non-timeout error', + failing: { at: 'final', as: 'command-failure' }, + aborts: 'while-it-blocks', + expected: 'request-canceled', + execs: 3, + clockMs: 0, + }, + { + label: 'aborted while the final probe blocks, and it then times out', + failing: { at: 'final', as: 'exec-timeout' }, + aborts: 'while-it-blocks', + expected: 'request-canceled', + execs: 3, + clockMs: COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, + }, + { + // 30 s plus a 15 s retry spends the whole 45 s ceiling on the first probe. The two + // xcrun probes never ran, so the verdict is the budget, not an unreadable toolchain: + // an `xcode-select` hint here would point at the wrong thing. + label: 'never aborted, the first probe and its retry spend the whole budget', + failing: { at: 'every', as: 'exec-timeout' }, + aborts: 'never', + expected: 'budget-exhausted', + execs: 2, + clockMs: 45_000, + }, + { + label: 'never aborted, the first probe times out and its retry recovers', + failing: { at: 'first', as: 'exec-timeout' }, + aborts: 'never', + expected: 'fingerprint', + execs: 4, + clockMs: COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, + }, + ]; + + test.each(CANCELLATION_CASES)('cancellation matrix: $label', (testCase) => { + const clock = installFakeToolchainClock(); + const request = new AbortController(); + if (testCase.fingerprintCache === 'warm') { + runCmdSync.mockImplementation(appleToolchainProbeResult); + resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); + } + if (testCase.aborts === 'before-any-probe') request.abort(); + + let execs = 0; + runCmdSync.mockImplementation((command: string, args: string[], options: ExecOptions) => { + execs += 1; + if (!failsOnExec(testCase, execs)) return appleToolchainProbeResult(command, args); + if (testCase.aborts === 'while-it-blocks') request.abort(); + const failure = + testCase.failing?.as === 'exec-timeout' + ? blockForWholeTimeout(clock, command, args, options) + : // No `timeoutMs` detail: the tool failed on its own, so nothing retries it. + new AppError('COMMAND_FAILED', `${command}: unexpected error`, { cmd: command, args }); + if (testCase.aborts === 'as-it-unwinds') request.abort(); + throw failure; + }); + + const readFingerprint = () => + resolveExpectedRunnerCacheMetadata( + IOS_SIMULATOR, + undefined, + createRunnerPhaseBudget(undefined, request.signal), + ); + + if (testCase.expected === 'fingerprint') { + const metadata = readFingerprint(); + assert.equal(metadata.xcodeVersion, '26.2', testCase.label); + assert.equal(metadata.xcodeBuildVersion, '17C52', testCase.label); + } else { + assert.throws(readFingerprint, (error: unknown) => { + assert.ok( + testCase.expected === 'request-canceled' + ? isRequestCanceledError(error) + : expectRunnerPhaseBudgetExhausted(error), + `${testCase.label}: expected ${testCase.expected}, got ${String(error)}`, + ); + return true; + }); + } + assert.equal(execs, testCase.execs, `${testCase.label}: exec count`); + assert.equal(clock.nowMs, testCase.clockMs, `${testCase.label}: wall clock spent`); + }); + + /** Which exec a row's failing probe is: the first, the last of the three, or all of them. */ + function failsOnExec(testCase: ToolchainProbeCancellationCase, exec: number): boolean { + switch (testCase.failing?.at) { + case 'first': + return exec === 1; + case 'final': + return exec === 3; + case 'every': + return true; + default: + return false; + } + } }); /** The error a runner phase raises when a step is reached with nothing left to spend. */ diff --git a/packages/platform-apple/src/runner/__tests__/runner-client.test.ts b/packages/platform-apple/src/runner/__tests__/runner-client.test.ts index ba9be5f86..0afa695a0 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-client.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-client.test.ts @@ -52,6 +52,7 @@ import { } from '../runner-cache.ts'; import { ensureXctestrunArtifact, xctestrunReferencesProjectRoot } from '../runner-artifact.ts'; import { + createRunnerPhaseBudget, markRunnerXctestrunArtifactBadForRun, resolveExpectedRunnerCacheMetadata, resolveRunnerDerivedPath, @@ -1099,10 +1100,10 @@ test('ensureXctestrunArtifact aborts only the disconnected request build and pre }); const canceledPromise = ensureXctestrunArtifact(canceledDevice, { - signal: canceledController.signal, + budget: createRunnerPhaseBudget(undefined, canceledController.signal), }); const survivorPromise = ensureXctestrunArtifact(survivorDevice, { - signal: survivorController.signal, + budget: createRunnerPhaseBudget(undefined, survivorController.signal), }); await Promise.all([canceledBuildStarted.promise, survivorBuildStarted.promise]); @@ -1408,7 +1409,7 @@ test('ensureXctestrunArtifact stress-recovers after a bad restored artifact', as }); const rebuilt = await ensureXctestrunArtifact(macOsDevice, { - buildTimeoutMs: 300_000, + budget: createRunnerPhaseBudget(300_000, undefined), }); assert.equal(rebuilt.xctestrunPath, rebuiltXctestrunPath); diff --git a/packages/platform-apple/src/runner/runner-adoption.ts b/packages/platform-apple/src/runner/runner-adoption.ts index ad97f5e7d..3c2819def 100644 --- a/packages/platform-apple/src/runner/runner-adoption.ts +++ b/packages/platform-apple/src/runner/runner-adoption.ts @@ -1,7 +1,6 @@ import path from 'node:path'; import { resolveIosSimulatorDeviceSetPath, - type Deadline, emitDiagnostic, isProcessAlive, parseBooleanLiteral, @@ -22,7 +21,7 @@ import { requireRunnerPhaseRemainingMs, resolveExpectedRunnerCacheMetadata, resolveRunnerDerivedPath, - type RunnerCacheProbeBudget, + type RunnerPhaseBudget, type RunnerXctestrunArtifact, } from './runner-xctestrun.ts'; import { @@ -54,11 +53,11 @@ export function isIosRunnerDetachEnabled(env: NodeJS.ProcessEnv = process.env): export async function tryAdoptRunnerSessionFromLease( device: DeviceInfo, options: { - startupTimeoutMs?: number; - /** The startup phase's clock: the fingerprint check below spends from it (#2422). */ - phaseDeadline?: Deadline; - /** The owning request's cancellation signal, forwarded to those probes. */ - signal?: AbortSignal; + /** + * The startup phase's one budget: the fingerprint check below spends from it, its + * cancellation reaches those probes, and the adopted session inherits the rest (#2422). + */ + budget?: RunnerPhaseBudget; expectedRunnerSessionId?: string; }, ): Promise { @@ -97,10 +96,7 @@ export async function tryAdoptRunnerSessionFromLease( if (!verifyLeaseRunnerPidIdentity(lease, runnerPid)) { return skip('runner_pid_recycled'); } - const expectedDerived = resolveExpectedDerivedPath(device, { - deadline: options.phaseDeadline, - signal: options.signal, - }); + const expectedDerived = resolveExpectedDerivedPath(device, options.budget); if (!expectedDerived) return skip('expected_derived_unresolved'); if (!lease.xctestrunPath.startsWith(`${expectedDerived}${path.sep}`)) { return skip('artifact_fingerprint_mismatch'); @@ -150,7 +146,7 @@ async function probeRunnerAnswersUptime(device: DeviceInfo, port: number): Promi function resolveExpectedDerivedPath( device: DeviceInfo, - budget: RunnerCacheProbeBudget, + budget: RunnerPhaseBudget | undefined, ): string | null { try { return resolveRunnerDerivedPath( @@ -169,7 +165,7 @@ function buildAdoptedRunnerSession( lease: RunnerLease, runnerPid: number, expectedDerived: string, - options: { startupTimeoutMs?: number; phaseDeadline?: Deadline }, + options: { budget?: RunnerPhaseBudget }, ): RunnerSession & { lease: RunnerLease } { const sessionId = lease.sessionId; const artifact: RunnerXctestrunArtifact = { @@ -195,11 +191,7 @@ function buildAdoptedRunnerSession( // The probe already proved the runner answers commands. ready: true, startupTimeoutMs: normalizeRunnerStartupTimeoutMs( - requireRunnerPhaseRemainingMs( - options.phaseDeadline, - options.startupTimeoutMs, - 'runner_session_adoption', - ), + requireRunnerPhaseRemainingMs(options.budget, 'runner_session_adoption'), ), lease: buildRunnerLease({ deviceId: device.id, diff --git a/packages/platform-apple/src/runner/runner-artifact.ts b/packages/platform-apple/src/runner/runner-artifact.ts index af7156d46..5ed1d8168 100644 --- a/packages/platform-apple/src/runner/runner-artifact.ts +++ b/packages/platform-apple/src/runner/runner-artifact.ts @@ -5,7 +5,6 @@ import os from 'node:os'; import path from 'node:path'; import { runCmdStreaming, - type Deadline, type ExecBackgroundResult, withKeyedLock, emitRequestProgress, @@ -20,7 +19,6 @@ import { assertSafeDerivedCleanup, cleanRunnerDerivedArtifacts, cleanRunnerDerivedBeforeEvaluation, - createRunnerPhaseDeadline, emitRunnerXctestrunDecision, emitRunnerXctestrunRebuildDecision, evaluateExistingXctestrun, @@ -34,6 +32,7 @@ import { resolveRunnerSigningBuildSettings, writeRunnerCacheMetadataForArtifacts, type ExistingXctestrunState, + type RunnerPhaseBudget, type RunnerXctestrunCacheKind, type RunnerXctestrunCacheMetadata, } from './runner-cache.ts'; @@ -71,13 +70,17 @@ export type ExternalXctestRunnerOptions = { iosXctestEnvDir?: string; }; -/** What the build phase reads: its budget, where it logs, and how it is canceled. */ +/** What the build phase reads: its budget, and where it logs. */ type RunnerXctestrunBuildOptions = { verbose?: boolean; logPath?: string; traceLogPath?: string; - buildTimeoutMs?: number; - signal?: AbortSignal; + /** + * The build phase's one budget, opened by whoever owns the build: the cache decision's + * blocking toolchain probes and `xcodebuild` spend the same clock, and the owning + * request cancels both (#2422). + */ + budget?: RunnerPhaseBudget; }; export async function ensureXctestrunArtifact( @@ -90,12 +93,11 @@ export async function ensureXctestrunArtifact( if (external) return external; const projectRoot = findProjectRoot(); - // One clock for the whole build phase: the toolchain probes and the xcodebuild share it. - const phaseDeadline = createRunnerPhaseDeadline(options.buildTimeoutMs); - const expectedCacheMetadata = resolveExpectedRunnerCacheMetadata(device, projectRoot, { - deadline: phaseDeadline, - signal: options.signal, - }); + const expectedCacheMetadata = resolveExpectedRunnerCacheMetadata( + device, + projectRoot, + options.budget, + ); const derived = resolveRunnerDerivedPath(device, expectedCacheMetadata); return await withKeyedLock(runnerXctestrunBuildLocks, derived, async () => { const releaseCacheLock = await acquireRunnerXctestrunCacheLock(derived); @@ -103,7 +105,6 @@ export async function ensureXctestrunArtifact( return await ensureXctestrunUnderCacheLock({ device, options, - phaseDeadline, projectRoot, expectedCacheMetadata, derived, @@ -161,13 +162,12 @@ function resolveExternalXctestDerivedDataPath(xctestrunPath: string): string { async function ensureXctestrunUnderCacheLock(params: { device: DeviceInfo; options: RunnerXctestrunBuildOptions; - phaseDeadline: Deadline | undefined; projectRoot: string; expectedCacheMetadata: RunnerXctestrunCacheMetadata; derived: string; forceRebuild: boolean; }): Promise { - const { device, options, phaseDeadline, projectRoot, expectedCacheMetadata, derived } = params; + const { device, options, projectRoot, expectedCacheMetadata, derived } = params; cleanRunnerDerivedBeforeEvaluation(derived, params.forceRebuild); const existing = await evaluateExistingXctestrunForDevice({ device, @@ -195,7 +195,6 @@ async function ensureXctestrunUnderCacheLock(params: { return await buildXctestrunArtifact({ device, options, - phaseDeadline, projectRoot, expectedCacheMetadata, derived, @@ -233,41 +232,27 @@ async function resolveReusableXctestrunArtifact(params: { async function buildXctestrunArtifact(params: { device: DeviceInfo; options: RunnerXctestrunBuildOptions; - phaseDeadline: Deadline | undefined; projectRoot: string; expectedCacheMetadata: RunnerXctestrunCacheMetadata; derived: string; cache: RunnerXctestrunArtifact['cache']; reason: ExistingXctestrunState['reason']; }): Promise { - const { - device, - options, - phaseDeadline, - projectRoot, - expectedCacheMetadata, - derived, - cache, - reason, - } = params; + const { device, options, projectRoot, expectedCacheMetadata, derived, cache, reason } = params; const projectPath = resolveAppleRunnerProjectPath(projectRoot); if (!fs.existsSync(projectPath)) { throw new AppError('COMMAND_FAILED', 'iOS runner project not found', { projectPath }); } - const buildTimeoutMs = requireRunnerPhaseRemainingMs( - phaseDeadline, - options.buildTimeoutMs, - 'runner_xctestrun_build', - ); + const buildTimeoutMs = requireRunnerPhaseRemainingMs(options.budget, 'runner_xctestrun_build'); const buildStartedAt = Date.now(); emitRequestProgress({ type: 'command', status: 'progress', message: 'Building Apple runner...', }); - await buildRunnerXctestrun(device, projectPath, derived, { ...options, buildTimeoutMs }); + await buildRunnerXctestrun(device, projectPath, derived, options, buildTimeoutMs); const buildMs = Math.max(0, Date.now() - buildStartedAt); const built = findXctestrun(derived, device); @@ -474,6 +459,8 @@ async function buildRunnerXctestrun( projectPath: string, derived: string, options: RunnerXctestrunBuildOptions, + /** What {@link requireRunnerPhaseRemainingMs} left of the build phase, for the exec layer. */ + buildTimeoutMs: number | undefined, ): Promise { const runnerBundleBuildSettings = resolveRunnerBundleBuildSettings(process.env); const signingBuildSettings = resolveRunnerSigningBuildSettings( @@ -510,8 +497,8 @@ async function buildRunnerXctestrun( ], { detached: true, - timeoutMs: options.buildTimeoutMs, - signal: options.signal, + timeoutMs: buildTimeoutMs, + signal: options.budget?.signal, onSpawn: (child) => { runnerPrepProcesses.add(child); child.on('close', () => { diff --git a/packages/platform-apple/src/runner/runner-cache-metadata.ts b/packages/platform-apple/src/runner/runner-cache-metadata.ts index 15a402d84..702aec6bd 100644 --- a/packages/platform-apple/src/runner/runner-cache-metadata.ts +++ b/packages/platform-apple/src/runner/runner-cache-metadata.ts @@ -67,24 +67,43 @@ type ToolchainProbeFailure = { }; /** - * The one clock a runner phase spends, created once per phase and read with - * {@link requireRunnerPhaseRemainingMs}; `undefined` for a caller carrying no budget. + * Everything one runner phase may spend: the single clock every step of the phase reads, + * and the owning request's cancellation. Created once, where the phase begins, and handed + * on as this object — no step below receives a timeout number it could open a second phase + * with, which is how a cold probe stall and the build each spent the same budget (#2422). */ -export function createRunnerPhaseDeadline(timeoutMs: number | undefined): Deadline | undefined { - if (timeoutMs === undefined || !Number.isFinite(timeoutMs)) return undefined; - return Deadline.fromTimeoutMs(Math.max(0, timeoutMs)); +export type RunnerPhaseBudget = Readonly<{ + /** The phase's clock; absent when its owner carries no budget at all. */ + deadline?: Deadline; + /** The owning request's cancellation signal, if it carries one. */ + signal?: AbortSignal; +}>; + +/** + * Opens a phase from the numeric timeout its public option carries: the one place a number + * becomes a budget, so every boundary below it takes the {@link RunnerPhaseBudget} instead. + */ +export function createRunnerPhaseBudget( + timeoutMs: number | undefined, + signal: AbortSignal | undefined, +): RunnerPhaseBudget { + const bounded = timeoutMs !== undefined && Number.isFinite(timeoutMs); + return { + deadline: bounded ? Deadline.fromTimeoutMs(Math.max(0, timeoutMs)) : undefined, + signal, + }; } /** - * What the phase has left for its next step, or `fallbackTimeoutMs` when it carries no - * deadline. Throws rather than returning zero, so a spent phase fails before it spawns. + * What the phase has left for its next step, or `undefined` when it carries no deadline. + * Throws rather than returning zero, so a spent phase fails before it spawns. */ export function requireRunnerPhaseRemainingMs( - deadline: Deadline | undefined, - fallbackTimeoutMs: number | undefined, + budget: RunnerPhaseBudget | undefined, phase: string, ): number | undefined { - if (!deadline) return fallbackTimeoutMs; + const deadline = budget?.deadline; + if (!deadline) return undefined; const remainingMs = Math.floor(deadline.remainingMs()); if (remainingMs <= 0) throw runnerPhaseBudgetExhaustedError(phase); return remainingMs; @@ -99,20 +118,10 @@ function runnerPhaseBudgetExhaustedError(phase: string): AppError { }); } -/** - * What the phase that wants a runner cache decision has left to spend on it. A caller - * with neither field still gets {@link TOOLCHAIN_FINGERPRINT_BUDGET_MS} as the ceiling. - */ -export type RunnerCacheProbeBudget = { - /** The owning phase's clock, shared with whatever the phase does next. */ - deadline?: Deadline; - /** The owning request's cancellation signal, if it carries one. */ - signal?: AbortSignal; -}; - /** * The remaining-time and cancellation view the probes consult: one per fingerprint read, - * so the three probes and their retries share a single budget. + * so the three probes and their retries share a single budget. A phase with no deadline + * still gets {@link TOOLCHAIN_FINGERPRINT_BUDGET_MS} as the ceiling. * * `spawnSync` cannot be interrupted once it has started, so cancellation is observed * between attempts; the per-attempt cap is what bounds how long that takes. @@ -124,9 +133,7 @@ type ToolchainProbeClock = { throwIfCanceled(): void; }; -function createToolchainProbeClock( - budget: RunnerCacheProbeBudget | undefined, -): ToolchainProbeClock { +function createToolchainProbeClock(budget: RunnerPhaseBudget | undefined): ToolchainProbeClock { const phaseDeadline = budget?.deadline; const deadline = Deadline.fromTimeoutMs( Math.min( @@ -217,7 +224,7 @@ export const IOS_RUNNER_CONTAINER_BUNDLE_IDS: string[] = resolveRunnerContainerB export function resolveExpectedRunnerCacheMetadata( device: DeviceInfo, projectRoot: string = findProjectRoot(), - budget?: RunnerCacheProbeBudget, + budget?: RunnerPhaseBudget, ): RunnerXctestrunCacheMetadata { const platformName = resolveRunnerPlatformName(device); return { @@ -256,7 +263,7 @@ function toolchainFingerprintCache(): TtlMemo { - await ensureXctestrunArtifact(device, runnerOptions); + // A cache prewarm owns the build phase it starts: one budget for the cache + // decision's toolchain probes and the `xcodebuild` that may follow them. + await ensureXctestrunArtifact(device, { + ...runnerOptions, + budget: createRunnerPhaseBudget(runnerOptions.buildTimeoutMs, runnerOptions.signal), + }); }, }); } diff --git a/packages/platform-apple/src/runner/runner-session.ts b/packages/platform-apple/src/runner/runner-session.ts index 40198d4b6..5e070f3c4 100644 --- a/packages/platform-apple/src/runner/runner-session.ts +++ b/packages/platform-apple/src/runner/runner-session.ts @@ -19,14 +19,14 @@ import { waitForRunner, RUNNER_STARTUP_TIMEOUT_MS } from './runner-startup-trans import { sendRunnerCommandOnce } from './runner-transport.ts'; import { acquireXcodebuildSimulatorSetRedirect, - createRunnerPhaseDeadline, + createRunnerPhaseBudget, ensureXctestrunArtifact, IOS_RUNNER_CONTAINER_BUNDLE_IDS, prepareXctestrunWithEnv, requireRunnerPhaseRemainingMs, resolveExpectedRunnerCacheMetadata, resolveRunnerDerivedPath, - type RunnerCacheProbeBudget, + type RunnerPhaseBudget, } from './runner-xctestrun.ts'; import { resolveRunnerRequestSignal, @@ -112,21 +112,27 @@ export async function ensureRunnerSession( // from a retained-after-close runner no longer applies. cancelIosRunnerIdleStop(device.id); return await withRunnerSessionLock(device.id, async () => { - // One clock for the whole startup phase: the toolchain probes and the startup share it. - const phaseDeadline = createRunnerPhaseDeadline(options.startupTimeoutMs); + // 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 + // 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. + const startupBudget = createRunnerPhaseBudget( + options.startupTimeoutMs, + resolveRunnerRequestSignal(options), + ); const existing = runnerSessions.get(device.id); if (existing) { assertExpectedRunnerSession(existing, options.expectedRunnerSessionId); - const reusable = await resolveReusableRunnerSession(device, existing, { - deadline: phaseDeadline, - signal: resolveRunnerRequestSignal(options), - }); + const reusable = await resolveReusableRunnerSession(device, existing, startupBudget); if (reusable) return reusable; } return await withRunnerLeaseLock( device.id, - async () => await startRunnerSessionWithLease(device, options, phaseDeadline), + async () => await startRunnerSessionWithLease(device, options, startupBudget), ); }); } @@ -134,14 +140,10 @@ export async function ensureRunnerSession( async function startRunnerSessionWithLease( device: DeviceInfo, options: RunnerSessionOptions, - phaseDeadline: Deadline | undefined, + startupBudget: RunnerPhaseBudget, ): Promise { const startupTimings: Record = {}; - // The owning request's abort signal 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. - const signal = resolveRunnerRequestSignal(options); + const signal = startupBudget.signal; const logicalLeaseContext = normalizeRunnerLogicalLeaseContext( options.runnerLeaseContext, device.id, @@ -159,9 +161,7 @@ async function startRunnerSessionWithLease( 'adopt_detached_runner', async () => await tryAdoptRunnerSessionFromLease(device, { - startupTimeoutMs: options.startupTimeoutMs, - phaseDeadline, - signal, + budget: startupBudget, expectedRunnerSessionId: options.expectedRunnerSessionId, }), ); @@ -192,16 +192,16 @@ async function startRunnerSessionWithLease( phase: 'ios_runner_startup_cleanup_stale_bundles_skipped', }); } - // Read before the build, which answers to its own `buildTimeoutMs` deadline (#2422). - const startupTimeoutMs = requireRunnerPhaseRemainingMs( - phaseDeadline, - options.startupTimeoutMs, - 'runner_session_startup', - ); + // Read before the build, which is a phase of its own with its own budget (#2422). + const startupTimeoutMs = requireRunnerPhaseRemainingMs(startupBudget, 'runner_session_startup'); const xctestrunArtifact = await measureRunnerStartupStep( startupTimings, 'ensure_xctestrun', - async () => await ensureXctestrunArtifact(device, { ...options, signal }), + async () => + await ensureXctestrunArtifact(device, { + ...options, + budget: createRunnerPhaseBudget(options.buildTimeoutMs, signal), + }), ); startupTimings.build_xctestrun = xctestrunArtifact.buildMs; const port = await measureRunnerStartupStep( @@ -324,7 +324,7 @@ function runnerSessionOwnershipChanged(): AppError { async function resolveReusableRunnerSession( device: DeviceInfo, existing: RunnerSession, - cacheProbeBudget: RunnerCacheProbeBudget, + startupBudget: RunnerPhaseBudget, ): Promise { if (!isRunnerProcessAlive(existing.child.pid)) { await measureRunnerStartupStep({}, 'stop_stale_session', async () => { @@ -354,7 +354,7 @@ async function resolveReusableRunnerSession( const expectedDerived = resolveRunnerDerivedPath( device, - resolveExpectedRunnerCacheMetadata(device, undefined, cacheProbeBudget), + resolveExpectedRunnerCacheMetadata(device, undefined, startupBudget), ); if (existingArtifact?.derived !== expectedDerived) { emitDiagnostic({ diff --git a/packages/platform-apple/src/runner/runner-xctestrun.ts b/packages/platform-apple/src/runner/runner-xctestrun.ts index ca024295f..41e4694db 100644 --- a/packages/platform-apple/src/runner/runner-xctestrun.ts +++ b/packages/platform-apple/src/runner/runner-xctestrun.ts @@ -12,12 +12,12 @@ export { type RunnerXctestrunCacheKind, } from './runner-cache.ts'; export { - createRunnerPhaseDeadline, + createRunnerPhaseBudget, IOS_RUNNER_CONTAINER_BUNDLE_IDS, requireRunnerPhaseRemainingMs, resolveExpectedRunnerCacheMetadata, resolveRunnerAppBundleId, resolveRunnerDerivedPath, - type RunnerCacheProbeBudget, + type RunnerPhaseBudget, } from './runner-cache-metadata.ts'; export { acquireXcodebuildSimulatorSetRedirect } from './runner-device-set.ts'; diff --git a/packages/platform-apple/src/snapshot-source/cache-identity.test.ts b/packages/platform-apple/src/snapshot-source/cache-identity.test.ts index 0a8196502..fc71799fd 100644 --- a/packages/platform-apple/src/snapshot-source/cache-identity.test.ts +++ b/packages/platform-apple/src/snapshot-source/cache-identity.test.ts @@ -46,24 +46,6 @@ test('a cold-start toolchain probe recovers on retry, and the retry gets only wh assert.equal(calls, 6); }); -test('a probe that spends the whole deadline is not retried', async () => { - const clock = { nowMs: 0 }; - const timeouts: number[] = []; - const host = fakeToolchainHost((command, _args, options) => { - timeouts.push(options.timeoutMs ?? 0); - throw blockForWholeTimeout(clock, command, options); - }); - - await assert.rejects( - readSnapshotSourceToolchain(host, 'iOS 26.2', fakeClockDeadline(30_000, clock)), - (error: unknown) => - error instanceof AppError && error.message === 'xcodebuild timed out after 30000ms', - ); - // Nothing left to retry on, so the original timeout propagates unchanged. - assert.deepEqual(timeouts, [30_000]); - assert.equal(clock.nowMs, 30_000); -}); - test('a toolchain host that never returns still fails at the deadline with the same timeout error', async () => { const clock = { nowMs: 0 }; const timeouts: number[] = []; @@ -83,38 +65,6 @@ test('a toolchain host that never returns still fails at the deadline with the s assert.equal(clock.nowMs, 60_000); }); -test('a request canceled while a probe blocked surfaces the cancellation, not the timeout', async () => { - const clock = { nowMs: 0 }; - const request = new AbortController(); - let calls = 0; - const host = fakeToolchainHost((command, _args, options) => { - calls += 1; - const timeout = blockForWholeTimeout(clock, command, options); - // The abort lands while the attempt is still blocked, which is the case the - // deadline alone cannot tell from a plain timeout: it still has room. - request.abort(); - throw timeout; - }); - - await assert.rejects( - readSnapshotSourceToolchain( - host, - 'iOS 26.2', - createSnapshotSourceDeadline(120_000, request.signal, () => clock.nowMs), - ), - (error: unknown) => { - assert.ok(error instanceof SnapshotSourceError); - assert.equal(error.failureKind, 'cancelled'); - assert.equal(error.failureCode, 'abort-signal'); - assert.equal(error.details?.reason, 'request_canceled'); - return true; - }, - ); - // The deadline still had 90 s, so only the cancellation stops the retry. - assert.equal(calls, 1); - assert.equal(clock.nowMs, 30_000); -}); - test('a probe that failed on its own and merely says "timed out" in its message is not retried', async () => { const clock = { nowMs: 0 }; let calls = 0; @@ -134,6 +84,136 @@ test('a probe that failed on its own and merely says "timed out" in its message assert.equal(calls, 1); }); +/** + * One row per way a toolchain read can be interrupted: how the probe the row exercises + * ends, when the owning request aborts relative to it, and what the caller must then see. + * The retry is the only place a cancellation can be observed -- an exec already running + * cannot be taken back -- so the exec count is what pins where each row stopped. + */ +type ToolchainProbeCancellationCase = { + label: string; + /** How the first probe ends; later probes answer. Absent when no probe runs at all. */ + firstProbe?: 'exec-timeout' | 'command-failure'; + /** + * When the owning request aborts: never, before the phase even opens its deadline, while + * the first probe is still blocked, or as that probe's timeout unwinds. + */ + aborts: 'never' | 'before-the-deadline' | 'while-it-blocks' | 'as-it-unwinds'; + /** The phase deadline. 30 s is spent in full by one stalled probe, leaving no retry. */ + deadlineMs: number; + expected: 'cancelled' | 'exec-timeout' | 'command-failure'; + execs: number; + clockMs: number; +}; + +const CANCELLATION_CASES: ToolchainProbeCancellationCase[] = [ + { + label: 'aborted before the phase opened its deadline', + aborts: 'before-the-deadline', + deadlineMs: 120_000, + expected: 'cancelled', + execs: 0, + clockMs: 0, + }, + { + // The deadline still had 90 s, so only the cancellation stops the retry. + label: 'aborted while the first probe blocks, and it then times out', + firstProbe: 'exec-timeout', + aborts: 'while-it-blocks', + deadlineMs: 120_000, + expected: 'cancelled', + execs: 1, + clockMs: 30_000, + }, + { + // A probe that failed on its own is never retried, so there is no retry to cancel: + // the tool's own failure is what the caller sees, and the request fails either way. + label: 'aborted while the first probe fails with a non-timeout error', + firstProbe: 'command-failure', + aborts: 'while-it-blocks', + deadlineMs: 120_000, + expected: 'command-failure', + execs: 1, + clockMs: 0, + }, + { + label: "aborted as the first probe's timeout unwinds, before its retry", + firstProbe: 'exec-timeout', + aborts: 'as-it-unwinds', + deadlineMs: 120_000, + expected: 'cancelled', + execs: 1, + clockMs: 30_000, + }, + { + // Nothing left to retry on, so the original timeout propagates unchanged. + label: 'never aborted, the first probe spends the whole deadline', + firstProbe: 'exec-timeout', + aborts: 'never', + deadlineMs: 30_000, + expected: 'exec-timeout', + execs: 1, + clockMs: 30_000, + }, +]; + +test.each(CANCELLATION_CASES)('cancellation matrix: $label', async (testCase) => { + const clock = { nowMs: 0 }; + const request = new AbortController(); + if (testCase.aborts === 'before-the-deadline') request.abort(); + let execs = 0; + const host = fakeToolchainHost((command, args, options) => { + execs += 1; + if (execs > 1 || !testCase.firstProbe) return toolchainAnswer(command, args); + if (testCase.aborts === 'while-it-blocks') request.abort(); + const failure = + testCase.firstProbe === 'exec-timeout' + ? blockForWholeTimeout(clock, command, options) + : // No `timeoutMs` detail: the tool failed on its own, so nothing retries it. + new AppError('COMMAND_FAILED', `${command}: unexpected error`, { cmd: command }); + if (testCase.aborts === 'as-it-unwinds') request.abort(); + throw failure; + }); + + await assert.rejects( + // The deadline is opened inside the rejected call: an already-aborted request must + // fail as it is opened, before any probe runs. + async () => + await readSnapshotSourceToolchain( + host, + 'iOS 26.2', + createSnapshotSourceDeadline(testCase.deadlineMs, request.signal, () => clock.nowMs), + ), + (error: unknown) => { + assertExpectedToolchainFailure(error, testCase); + return true; + }, + ); + assert.equal(execs, testCase.execs, `${testCase.label}: exec count`); + assert.equal(clock.nowMs, testCase.clockMs, `${testCase.label}: wall clock spent`); +}); + +function assertExpectedToolchainFailure( + error: unknown, + testCase: ToolchainProbeCancellationCase, +): void { + if (testCase.expected === 'cancelled') { + assert.ok(error instanceof SnapshotSourceError, `${testCase.label}: expected a cancellation`); + assert.equal(error.failureKind, 'cancelled', testCase.label); + assert.equal(error.failureCode, 'abort-signal', testCase.label); + assert.equal(error.details?.reason, 'request_canceled', testCase.label); + return; + } + assert.ok(error instanceof AppError, `${testCase.label}: expected the probe's own failure`); + assert.equal( + error.message, + testCase.expected === 'exec-timeout' + ? 'xcodebuild timed out after 30000ms' + : 'xcodebuild: unexpected error', + testCase.label, + ); +} + /** A deadline read against a clock only {@link blockForWholeTimeout} advances. */ function fakeClockDeadline(timeoutMs: number, clock: { nowMs: number }): SnapshotSourceDeadline { return createSnapshotSourceDeadline(timeoutMs, undefined, () => clock.nowMs);