From 7ca81475888925dc43a28579c82ce9a97dcf4800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 6 Oct 2026 11:38:59 +0200 Subject: [PATCH 1/2] fix(apple-runner): fence prep spawns behind a start-owned admission (#3220) A runner start builds, launches, and health-checks without holding the device session lock, so a teardown racing that window could kill the build and still let the start retry into a second concurrent `xcodebuild build-for-testing` on the same DerivedData directory. Every start now carries an admission token. Teardown of an in-flight start closes the token before killing prep processes, so later calls see an explicit retired verdict instead of retrying into the race; an open that only queued behind a settled fence is readmitted and proceeds normally. The prepare loop owns one token across its retry so the replacement build stays authorized, and a caller leaving on its own deadline marks the start retry-pending so teardown does not stop a build another owner still waits on. abortAll/stopAll fence all device starts for their duration instead of awaiting per-device session locks. The machinery lives in runner-artifact.ts beside the prep ledger it gates; no new static module edges, no host allowlist. --- .../runner-artifact-start-admission.test.ts | 206 ++++++++++ .../__tests__/runner-command-retry.test.ts | 12 +- .../runner-session-close-prep-fence.test.ts | 385 ++++++++++++++++++ .../__tests__/runner-start-admission.test.ts | 280 +++++++++++++ .../__tests__/runner-start-budget.test.ts | 230 +++++++++-- .../src/runner/runner-adoption.ts | 106 +++++ .../src/runner/runner-artifact.ts | 370 +++++++++++++++-- .../src/runner/runner-disposal.ts | 19 +- .../src/runner/runner-lifecycle.ts | 36 +- .../src/runner/runner-provider.ts | 6 + .../src/runner/runner-session.ts | 277 +++++++------ .../src/runner/runner-start-budget.ts | 87 +++- .../src/runner/runner-xctestrun.ts | 17 +- 13 files changed, 1790 insertions(+), 241 deletions(-) create mode 100644 packages/platform-apple/src/runner/__tests__/runner-artifact-start-admission.test.ts create mode 100644 packages/platform-apple/src/runner/__tests__/runner-session-close-prep-fence.test.ts create mode 100644 packages/platform-apple/src/runner/__tests__/runner-start-admission.test.ts diff --git a/packages/platform-apple/src/runner/__tests__/runner-artifact-start-admission.test.ts b/packages/platform-apple/src/runner/__tests__/runner-artifact-start-admission.test.ts new file mode 100644 index 0000000000..98048ef08f --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-artifact-start-admission.test.ts @@ -0,0 +1,206 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { afterEach, beforeEach, test, vi } from 'vitest'; +import { isRequestCanceledError } from '@agent-device/kernel/errors'; +import type { ExecResult } from '@agent-device/host-kit/command'; +import { appleRunnerTestHost } from '../test-host.ts'; +import { + addRunnerStartWaiter, + cancelRunnerStartWaiter, + ensureXctestrunArtifact, + fenceRunnerStartAdmissionsForTeardown, + openRunnerStartAdmission, + runnerStartAdmitsPreparation, +} from '../runner-xctestrun.ts'; +import { appleToolchainProbeResult } from './apple-toolchain-fixtures.ts'; +import { IOS_SIMULATOR } from './device-fixtures.ts'; +import { seedRunnerProductBundle } from './runner-xctestrun.fixtures.ts'; +import { mkdtempForTestSync } from './tmp-dir.ts'; + +// The preparation-spawn seam of #3220: admission is read immediately before `xcodebuild +// build-for-testing` is created, so a start whose device went down never answers the kill with +// a replacement build. The tests drive `ensureXctestrunArtifact` with a stand-in `xcodebuild` +// so the gate is proven at the spawn itself, not at a mock above it. + +const runCmdStreaming = vi.fn(); +let projectRoot: string; +let derived: string; + +beforeEach(() => { + projectRoot = mkdtempForTestSync('agent-device-start-admission-root-'); + fs.mkdirSync( + path.join(projectRoot, 'apple', 'runner', 'AgentDeviceRunner', 'AgentDeviceRunner.xcodeproj'), + { recursive: true }, + ); + derived = mkdtempForTestSync('agent-device-start-admission-derived-'); + process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH = derived; + runCmdStreaming.mockReset(); + appleRunnerTestHost.update({ + runCmdSync: vi.fn().mockImplementation(appleToolchainProbeResult), + runCmdStreaming, + findProjectRoot: () => projectRoot, + readVersion: () => '0.0.0-test', + }); +}); + +afterEach(() => { + delete process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH; +}); + +/** + * The cold-build retry of the incident: a teardown kills the first `build-for-testing` before it + * ever takes the session lock, and the retired start re-enters to build again. The second spawn + * is what made close wait out its timeout, and it is exactly what the pre-spawn gate refuses — + * the child that would be the replacement never exists. + */ +test('a start whose device was torn down spawns no replacement build after its first was killed', async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-admission-retry-sim' }; + const admission = openRunnerStartAdmission(device.id); + let killFirstBuild: () => void = () => {}; + const firstBuildKilled = new Promise((resolve) => { + killFirstBuild = resolve; + }); + runCmdStreaming.mockImplementationOnce(() => { + killFirstBuild(); + // What a killed build surfaces as at this seam: the exec layer rejects once the tree was + // signaled, and the start's retry re-enters from here. + return Promise.reject(new Error('Command was aborted')); + }); + + const firstStart = ensureXctestrunArtifact(device, { startAdmission: admission }).catch( + (error: unknown) => error, + ); + await firstBuildKilled; + // The teardown fences the device and lifts once it settles. The start's own verdict survives + // that: the retry which re-enters with a closed admission is the replacement build #3220 is + // about, so it stays refused. + fenceRunnerStartAdmissionsForTeardown(device.id)(); + + const failure = await firstStart; + assert.equal(runCmdStreaming.mock.calls.length, 1, 'the killed build was the only spawn'); + assert.ok(failure instanceof Error, 'the killed build failed its start'); + + // The health retry the incident measured: same start, same options, after the kill. + const retry = await ensureXctestrunArtifact(device, { + startAdmission: admission, + }).catch((error: unknown) => error); + + assert.ok(isRequestCanceledError(retry), 'the fenced start fails as a canceled start'); + assert.equal(runCmdStreaming.mock.calls.length, 1, 'no replacement build was spawned'); +}); + +/** + * The nearest negative: the same retry on a device no teardown touched must still build. A + * guard that refused preparation on any hint of a prior failure would pass the case above and + * break every cold start. + */ +test('a start nobody retired still builds after a killed build', async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-admission-survivor-sim' }; + const admission = openRunnerStartAdmission(device.id); + runCmdStreaming + .mockResolvedValueOnce({ exitCode: 143, stdout: '', stderr: '' } satisfies ExecResult) + .mockImplementationOnce(async () => { + await seedBuiltRunner(); + return { exitCode: 0, stdout: '', stderr: '' } satisfies ExecResult; + }); + + await assert.rejects(() => ensureXctestrunArtifact(device, { startAdmission: admission })); + const rebuilt = await ensureXctestrunArtifact(device, { startAdmission: admission }); + + assert.equal(runCmdStreaming.mock.calls.length, 2, 'the retry built the artifact'); + assert.equal(rebuilt.artifact, 'rebuilt'); +}); + +/** + * A device under a teardown admits preparation no start carried: the prewarm's own build. The + * fence answers by device, so a build phase carrying no token is refused for the length of the + * close that raised it (#3220). + */ +test('a fenced device admits a preparation that carries no start', async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-admission-prewarm-sim' }; + const settleFence = fenceRunnerStartAdmissionsForTeardown(device.id); + runCmdStreaming.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' } satisfies ExecResult); + + try { + const failure = await ensureXctestrunArtifact(device, {}).catch((error: unknown) => error); + assert.ok(isRequestCanceledError(failure)); + assert.equal(runCmdStreaming.mock.calls.length, 0, 'the prewarm build was never spawned'); + } finally { + settleFence(); + } + assert.equal( + runnerStartAdmitsPreparation(device.id), + true, + 'the fence was the close: once it settles, preparation is admitted again', + ); +}); + +/** + * The window #3193 left measured and open: a waiter cancels before the first prep child exists, + * so a ledger sweep has nothing to stop and the build would simply start. The cancellation closes + * admission, and the spawn that follows it is refused. + */ +test('a cancellation that arrives before the first prep spawn refuses the build that would follow', async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-admission-early-cancel-sim' }; + const admission = openRunnerStartAdmission(device.id); + const waiter = new AbortController(); + addRunnerStartWaiter(admission, waiter.signal); + runCmdStreaming.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' } satisfies ExecResult); + + assert.equal(cancelRunnerStartWaiter(admission, waiter.signal), true); + const failure = await ensureXctestrunArtifact(device, { startAdmission: admission }).catch( + (error: unknown) => error, + ); + + assert.ok(isRequestCanceledError(failure)); + assert.equal(runCmdStreaming.mock.calls.length, 0, 'the never-canceled build was never spawned'); +}); + +/** + * The nearest negative of the waiter rule, taken at the spawn seam: while another waiter is + * still interested, a cancellation preserves the work — a start that asks next still builds. + */ +test('a spawn still admitted by a remaining waiter builds', async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-admission-peer-waiter-sim' }; + const admission = openRunnerStartAdmission(device.id); + addRunnerStartWaiter(admission, new AbortController().signal); + const leaving = new AbortController(); + addRunnerStartWaiter(admission, leaving.signal); + runCmdStreaming.mockImplementation(async () => { + await seedBuiltRunner(); + return { exitCode: 0, stdout: '', stderr: '' } satisfies ExecResult; + }); + + assert.equal(cancelRunnerStartWaiter(admission, leaving.signal), false); + const built = await ensureXctestrunArtifact(device, { startAdmission: admission }); + + assert.equal(runCmdStreaming.mock.calls.length, 1, 'the surviving waiter kept the build alive'); + assert.equal(built.artifact, 'rebuilt'); +}); + +/** Stands in for a successful `xcodebuild build-for-testing`: the products land under SYMROOT. */ +async function seedBuiltRunner(): Promise { + const symroot = path.join(derived, 'Build', 'Products'); + await seedRunnerProductBundle( + path.join(symroot, 'Debug-iphonesimulator', 'AgentDeviceRunner.app'), + ); + fs.writeFileSync( + path.join( + symroot, + 'AgentDeviceRunner_AgentDeviceRunnerUITests_iphonesimulator27.0-arm64.xctestrun', + ), + ` + + + + ProjectRootHint + ${projectRoot} + ProductPaths + + __TESTROOT__/Debug-iphonesimulator/AgentDeviceRunner.app + + +`, + ); +} 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 d399e1f36a..6711176f51 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 @@ -1061,12 +1061,22 @@ function assertBadCacheRecoverySideEffects( fixtures.restoredArtifact, 'Runner did not accept connection', ]); - assert.deepEqual(mockEnsureRunnerSession.mock.calls[1]?.[1], { + const firstCallOptions = mockEnsureRunnerSession.mock.calls[0]?.[1] as + | { startAdmission?: unknown } + | undefined; + const retryOptions = mockEnsureRunnerSession.mock.calls[1]?.[1] as + | Record + | undefined; + assert.deepEqual(retryOptions, { healthTimeoutMs: 90_000, buildTimeoutMs: 300_000, cleanStaleBundles: true, forceRunnerXctestrunRebuild: true, + startAdmission: firstCallOptions?.startAdmission, }); + // Both attempts of the prepare loop share one admission token: the retry + // is the replacement build for the first attempt, not a newcomer. + assert.ok(firstCallOptions?.startAdmission); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[0]?.[2].command, 'uptime'); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[0]?.[4], 90_000); diff --git a/packages/platform-apple/src/runner/__tests__/runner-session-close-prep-fence.test.ts b/packages/platform-apple/src/runner/__tests__/runner-session-close-prep-fence.test.ts new file mode 100644 index 0000000000..9540029d88 --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-session-close-prep-fence.test.ts @@ -0,0 +1,385 @@ +import assert from 'node:assert/strict'; +import { EventEmitter } from 'node:events'; +import fs from 'node:fs'; +import path from 'node:path'; +import { afterEach, beforeEach, test, vi } from 'vitest'; +import { createRequestCanceledError, isRequestCanceledError } from '@agent-device/kernel/errors'; +import type { ExecBackgroundResult, ExecResult } from '@agent-device/host-kit/command'; +import type { DeviceInfo } from '@agent-device/kernel/device'; +import { appleRunnerTestHost } from '../test-host.ts'; +import { + makeClassifyOwnerLivenessViaMocks, + makeBackgroundRunner, + runnerResponse, +} from './runner-session-fixtures.ts'; +import { appleToolchainProbeResult } from './apple-toolchain-fixtures.ts'; +import { seedRunnerProductBundle } from './runner-xctestrun.fixtures.ts'; +import { mkdtempForTestSync } from './tmp-dir.ts'; + +/** + * The live incident of #3220, as a control: a start holds the runner session lock through a cold + * `build-for-testing`, a non-retained `close` begins and kills that build, and the same caller + * retries the start while close still waits for the lock. No second prep child is spawned, and + * close settles instead of waiting on a replacement build. + * + * Everything below the exec seam is production code: the session lock, the admission fence, the + * prep ledger, the artifact cache decision, and the spawn decision itself. Only `xcodebuild`, + * the Apple tools, and the process table are stood in for. + */ + +const XCTESTRUN_NAME = + 'AgentDeviceRunner_AgentDeviceRunnerUITests_iphonesimulator27.0-arm64.xctestrun'; + +const { + mockBuildForTesting, + mockCleanupTempFile, + mockGetFreePort, + mockIsProcessAlive, + mockIsProcessGroupAlive, + mockPrepareXctestrunWithEnv, + mockReadProcessCommand, + mockReadProcessStartTime, + mockRunAppleToolCommand, + mockRunCmdBackground, + mockRunXcrun, + mockSignalPidsBestEffort, + mockSignalProcessGroupBestEffort, + mockWaitForRunner, +} = vi.hoisted(() => ({ + mockBuildForTesting: vi.fn(), + mockCleanupTempFile: vi.fn(), + mockGetFreePort: vi.fn(), + mockIsProcessAlive: vi.fn(), + mockIsProcessGroupAlive: vi.fn(), + mockPrepareXctestrunWithEnv: vi.fn(), + mockReadProcessCommand: vi.fn((_pid: number) => null as string | null), + mockReadProcessStartTime: vi.fn((_pid: number) => 'fixed-test-owner-start-time' as string | null), + mockRunAppleToolCommand: vi.fn(), + mockRunCmdBackground: vi.fn(), + mockRunXcrun: vi.fn(), + mockSignalPidsBestEffort: vi.fn(), + mockSignalProcessGroupBestEffort: vi.fn(), + mockWaitForRunner: vi.fn(), +})); + +vi.mock('../runner-io.ts', async () => { + const actual = await vi.importActual('../runner-io.ts'); + return { + ...actual, + cleanupTempFile: mockCleanupTempFile, + getFreePort: mockGetFreePort, + }; +}); + +vi.mock('../runner-startup-transport.ts', async () => { + const actual = await vi.importActual( + '../runner-startup-transport.ts', + ); + return { ...actual, waitForRunner: mockWaitForRunner }; +}); + +// The session-xctestrun env step shells out to `plutil`; it is not the subject here, and the +// fixture build's plist is not something the host's tools have to agree to read. +vi.mock('../runner-artifact-env.ts', async () => { + const actual = await vi.importActual( + '../runner-artifact-env.ts', + ); + return { + ...actual, + prepareXctestrunWithEnv: mockPrepareXctestrunWithEnv, + }; +}); + +import { + abortAllIosRunnerSessions, + ensureRunnerSession, + releaseIosRunnerOnClose, +} from '../runner-session.ts'; +import { runnerPrepProcessChildren, runnerStartTeardownPending } from '../runner-xctestrun.ts'; + +let projectRoot: string; +let derived: string; +/** The `build-for-testing` children this test's seam created, in spawn order. */ +let builds: { pid: number; kill: () => void }[]; + +const IOS_ADMISSION_SIMULATOR: DeviceInfo = { + platform: 'apple', + appleOs: 'ios', + id: 'runner-close-fence-sim', + name: 'iPhone 17 Pro', + kind: 'simulator', + booted: true, +}; + +// The overrides below are this file's scratch dirs; a later test in this worker +// must not inherit them. Nothing between module load and the first beforeEach +// changes these, so capturing them here is the same state beforeEach restores. +const previousDerived = process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH; +const previousLeaseDir = process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR; +afterEach(() => { + if (previousDerived === undefined) delete process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH; + else process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH = previousDerived; + if (previousLeaseDir === undefined) delete process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR; + else process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR = previousLeaseDir; +}); + +beforeEach(async () => { + projectRoot = mkdtempForTestSync('agent-device-close-fence-root-'); + fs.mkdirSync( + path.join(projectRoot, 'apple', 'runner', 'AgentDeviceRunner', 'AgentDeviceRunner.xcodeproj'), + { recursive: true }, + ); + derived = mkdtempForTestSync('agent-device-close-fence-derived-'); + process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH = derived; + process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR = mkdtempForTestSync('agent-device-close-fence-'); + builds = []; + appleRunnerTestHost.update({ + runCmdStreaming: mockBuildForTesting, + runCmdSync: vi.fn().mockImplementation(appleToolchainProbeResult), + runCmdBackground: mockRunCmdBackground, + findProjectRoot: () => projectRoot, + readVersion: () => '0.0.0-test', + isProcessAlive: mockIsProcessAlive, + isProcessGroupAlive: mockIsProcessGroupAlive, + readProcessCommand: mockReadProcessCommand, + readProcessStartTime: mockReadProcessStartTime, + signalPidsBestEffort: mockSignalPidsBestEffort, + signalProcessGroupBestEffort: mockSignalProcessGroupBestEffort, + runAppleToolCommand: mockRunAppleToolCommand, + runXcrun: mockRunXcrun, + leaseOwnerStateDir: () => undefined, + classifyOwnerLiveness: makeClassifyOwnerLivenessViaMocks({ + isProcessAlive: (pid) => Boolean(mockIsProcessAlive(pid)), + readProcessStartTime: (pid) => (mockReadProcessStartTime(pid) as string | null) ?? null, + }), + }); + await abortAllIosRunnerSessions(); + vi.resetAllMocks(); + appleRunnerTestHost.update({ + runCmdStreaming: mockBuildForTesting, + runCmdSync: vi.fn().mockImplementation(appleToolchainProbeResult), + findProjectRoot: () => projectRoot, + readVersion: () => '0.0.0-test', + }); + mockRunXcrun.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }); + mockRunAppleToolCommand.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }); + mockRunCmdBackground.mockReturnValue(makeBackgroundRunner(4242)); + mockGetFreePort.mockResolvedValue(8123); + mockPrepareXctestrunWithEnv.mockResolvedValue({ + xctestrunPath: '/tmp/session-runner.xctestrun', + jsonPath: '/tmp/session-runner.json', + }); + mockIsProcessAlive.mockReturnValue(true); + mockIsProcessGroupAlive.mockReturnValue(false); + mockReadProcessCommand.mockReturnValue(null); + mockReadProcessStartTime.mockImplementation((pid: number) => + pid === process.pid ? 'fixed-test-owner-start-time' : null, + ); + mockWaitForRunner.mockResolvedValue(runnerResponse({ uptimeMs: 1 })); + mockBuildForTesting.mockImplementation((_command: string, _args: string[], options: any) => + hangBuildForTesting(options), + ); +}); + +test('close during a cold build leaves the start no way to spawn a replacement build', async () => { + const device = IOS_ADMISSION_SIMULATOR; + // Park close inside its prep stop: the kill is attempted, but the stop has not returned, so + // close provably has not reached the session lock yet (the start holds it) and no later fence + // could have run. That is the moment the device must already be fenced. + let releasePrepStop: () => void = () => {}; + const prepStopParked = new Promise((resolve) => { + releasePrepStop = resolve; + }); + let signalAttempted = () => {}; + const killAttempted = new Promise((resolve) => { + signalAttempted = resolve; + }); + mockRunAppleToolCommand.mockImplementation(async (tool: string, args: string[]) => { + // `killRunnerProcessTree` is the only pkill that signals by parent pid (`-P`); the lease + // cleanup's stale-launch sweep matches argv (`-f`), so only the prep tree-kill parks. + if (tool === 'pkill' && args.includes('-P')) { + signalAttempted(); + await prepStopParked; + } + return { exitCode: 0, stdout: '', stderr: '' }; + }); + + // A cold start: holds the session lock for the whole build, as a real one does. + const firstStart = ensureRunnerSession(device, {}).catch((error: unknown) => error); + await vi.waitFor(() => assert.equal(builds.length, 1)); + assert.equal(runnerPrepProcessChildren(device.id).length, 1, 'the build is on the prep ledger'); + + const closing = releaseIosRunnerOnClose(device.id, { retain: false }); + await killAttempted; + assert.equal( + runnerStartTeardownPending(device.id), + true, + 'the device was fenced before the prep stop finished', + ); + + // The prewarm health retry of the incident: the same caller starts over while close waits. + const retriedStart = ensureRunnerSession(device, {}).catch((error: unknown) => error); + releasePrepStop(); + builds[0]!.kill(); + + const [first, retried] = await Promise.all([firstStart, retriedStart, closing]); + + assert.ok(first instanceof Error, 'the start whose build was killed failed'); + assert.ok(isRequestCanceledError(retried), 'the retry is refused as a canceled start'); + assert.equal( + (retried as { details?: { runnerStartRetirementReason?: unknown } }).details + ?.runnerStartRetirementReason, + 'device_teardown', + 'the refusal carries the typed retirement reason, not just a cancellation', + ); + assert.equal(mockBuildForTesting.mock.calls.length, 1, 'no replacement build was ever spawned'); + assert.equal(runnerPrepProcessChildren(device.id).length, 0, 'the killed build left the ledger'); +}); + +/** + * The reviewer's counter-case to the fence: a caller that merely QUEUED behind the close — it + * never owned the killed build and is not that teardown's retry — must not inherit the fence + * once the close has settled. It wakes, re-routes to a fresh admission, and starts on its own. + */ +test('a start queued behind a close starts fresh once that close has settled', async () => { + const device = { ...IOS_ADMISSION_SIMULATOR, id: 'runner-close-queued-sim' }; + const firstStart = ensureRunnerSession(device, {}).catch((error: unknown) => error); + await vi.waitFor(() => assert.equal(builds.length, 1)); + + const closing = releaseIosRunnerOnClose(device.id, { retain: false }); + // Close has killed the prep child (proven by the empty ledger) and now waits on the session + // lock; the fence is in effect. The killed build frees that lock, and this caller is already + // queued behind close's own stop task, capturing the fenced admission while it still governs. + await vi.waitFor(() => assert.equal(runnerPrepProcessChildren(device.id).length, 0)); + builds[0]!.kill(); + const queuedStart = ensureRunnerSession(device, {}).catch((error: unknown) => error); + mockBuildForTesting.mockImplementationOnce(async () => { + await seedBuiltRunner(); + return { exitCode: 0, stdout: '', stderr: '' } satisfies ExecResult; + }); + + await closing; + await firstStart; + const started = await queuedStart; + assert.ok(!(started instanceof Error), 'the queued caller started fresh: ' + String(started)); + assert.equal( + mockBuildForTesting.mock.calls.length, + 2, + 'close spawned no replacement; the queued caller built its own runner', + ); +}); + +/** + * The nearest negative: with no close in sight, the next start after a killed build is a real + * start and does build. A fence that simply refused the device after any kill would pass the + * case above and make every retry impossible. + */ +test('a start after a settled teardown opens fresh and builds', async () => { + const device = { ...IOS_ADMISSION_SIMULATOR, id: 'runner-close-fence-next-sim' }; + const firstStart = ensureRunnerSession(device, {}).catch((error: unknown) => error); + await vi.waitFor(() => assert.equal(builds.length, 1)); + const closing = releaseIosRunnerOnClose(device.id, { retain: false }); + await vi.waitFor(() => assert.equal(runnerPrepProcessChildren(device.id).length, 0)); + builds[0]!.kill(); + await closing; + await firstStart; + assert.equal(mockBuildForTesting.mock.calls.length, 1); + + mockBuildForTesting.mockImplementationOnce(async () => { + await seedBuiltRunner(); + return { exitCode: 0, stdout: '', stderr: '' } satisfies ExecResult; + }); + const next = await ensureRunnerSession(device, {}); + + assert.equal( + mockBuildForTesting.mock.calls.length, + 2, + 'the independent open built its own runner', + ); + assert.equal(next.deviceId, device.id); +}); + +/** + * The reviewer's requestId-only control (#3220 review): R1 holds a cold build and counts as an + * owner through its registered request signal, with no caller `signal`; R2, also requestId-only, + * is queued on the session lock behind it. R1's request disconnecting must stop R1's own build + * and must not refuse R2, which main would have let build its own runner. + */ +test('a queued requestId-only start survives the starting caller disconnecting', async () => { + const device = { ...IOS_ADMISSION_SIMULATOR, id: 'runner-queued-requestid-sim' }; + const owner = new AbortController(); + appleRunnerTestHost.update({ + getRequestSignal: (id?: string) => (id === 'owner-request' ? owner.signal : undefined), + }); + + const firstStart = ensureRunnerSession(device, { requestId: 'owner-request' }).catch( + (error: unknown) => error, + ); + await vi.waitFor(() => assert.equal(builds.length, 1)); + const queued = ensureRunnerSession(device, { requestId: 'queued-request' }).catch( + (error: unknown) => error, + ); + mockBuildForTesting.mockImplementationOnce(async () => { + await seedBuiltRunner(); + return { exitCode: 0, stdout: '', stderr: '' } satisfies ExecResult; + }); + + // The owner's request disconnects: a cancellation, so its own build is stopped... + owner.abort(createRequestCanceledError()); + builds[0]!.kill(); + + const [first, second] = await Promise.all([firstStart, queued]); + assert.ok(first instanceof Error, 'the disconnected owner start failed'); + assert.ok(!(second instanceof Error), 'the queued start proceeds: ' + String(second)); + assert.equal( + mockBuildForTesting.mock.calls.length, + 2, + 'one caller disconnecting did not refuse the work of the other', + ); +}); + +/** + * A `build-for-testing` that hangs until the test kills it: the exec seam's promise rejects on + * the kill, the way the tree-kill makes a real one, and its child is registered the way the + * production spawn seam registers it. + */ +function hangBuildForTesting(options: { + onSpawn?: (child: ExecBackgroundResult['child']) => void; +}): Promise { + const pid = 6100 + builds.length; + const emitter = new EventEmitter(); + const child = Object.assign(emitter, { pid, exitCode: null }) as ExecBackgroundResult['child']; + let kill: () => void = () => {}; + const wait = new Promise((_, reject) => { + kill = () => { + emitter.emit('close', 143, null); + reject(new Error('Command was aborted')); + }; + }); + builds.push({ pid, kill }); + options.onSpawn?.(child); + return wait; +} + +/** Stands in for a successful `build-for-testing`: the products land under SYMROOT. */ +async function seedBuiltRunner(): Promise { + const symroot = path.join(derived, 'Build', 'Products'); + await seedRunnerProductBundle( + path.join(symroot, 'Debug-iphonesimulator', 'AgentDeviceRunner.app'), + ); + fs.writeFileSync( + path.join(symroot, XCTESTRUN_NAME), + ` + + + + ProjectRootHint + ${projectRoot} + ProductPaths + + __TESTROOT__/Debug-iphonesimulator/AgentDeviceRunner.app + + +`, + ); +} diff --git a/packages/platform-apple/src/runner/__tests__/runner-start-admission.test.ts b/packages/platform-apple/src/runner/__tests__/runner-start-admission.test.ts new file mode 100644 index 0000000000..0ee10d5de1 --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-start-admission.test.ts @@ -0,0 +1,280 @@ +import assert from 'node:assert/strict'; +import { beforeEach, test, vi } from 'vitest'; +import { isRequestCanceledError } from '@agent-device/kernel/errors'; +import { appleRunnerTestHost } from '../test-host.ts'; +import { + addRunnerStartWaiter, + cancelRunnerStartWaiter, + fenceRunnerStartAdmissionsForTeardown, + finishRunnerStartAdmission, + openRunnerStartAdmission, + openRunnerStartLoopAdmission, + readmitRunnerStartAdmission, + runnerStartAdmitsPreparation, + runnerStartRetiredError, + runnerStartTeardownPending, +} from '../runner-xctestrun.ts'; + +// The start-admission decisions #3220 puts in one place: whether one start may still prepare, and +// which cancellation is the last one. These are the primitives the spawn seam, the publish point, +// and the caller race read; the spawn and publish paths themselves are exercised at their own +// seams (runner-artifact-start-admission, runner-session-close-prep-fence). + +beforeEach(() => { + appleRunnerTestHost.update({ emitDiagnostic: vi.fn() }); +}); + +function retiredReason(error: unknown): unknown { + assert.ok(error instanceof Error); + return (error as { details?: { runnerStartRetirementReason?: unknown } }).details + ?.runnerStartRetirementReason; +} + +/** + * A start cannot prepare once a teardown has closed its admission, and the refusal says which + * reason closed it: a caller switching on the reason must never read a message to learn whether + * the device went down or its waiters left. + */ +test('a closed start admits no preparation and says why', () => { + const admission = openRunnerStartAdmission('admission-retire-sim'); + const settle = fenceRunnerStartAdmissionsForTeardown('admission-retire-sim'); + + assert.equal(admission.admitted, false); + assert.equal(runnerStartAdmitsPreparation('admission-retire-sim', admission), false); + assert.equal(runnerStartAdmitsPreparation('admission-retire-sim'), false); + const error = runnerStartRetiredError(admission.retired ?? 'last_waiter_canceled'); + assert.ok(isRequestCanceledError(error), 'a retirement is a cancellation of the work'); + assert.equal(retiredReason(error), 'device_teardown'); + settle(); + finishRunnerStartAdmission(admission); +}); + +/** + * A device under a teardown admits no preparation at all — the fence answers by device, so it + * reaches preparation carrying no start's token, a cache prewarm's build — and the answer lifts + * the moment the teardown settles (#3220). A start that opened mid-close was never that close's + * victim: its own verdict is untouched, so it runs once the fence lifts instead of failing for a + * close it took no part in. + */ +test('a device under a teardown admits no preparation and frees once it settles', () => { + const settle = fenceRunnerStartAdmissionsForTeardown('admission-fence-sim'); + assert.equal(runnerStartTeardownPending('admission-fence-sim'), true); + + const midClose = openRunnerStartAdmission('admission-fence-sim'); + assert.equal( + runnerStartAdmitsPreparation('admission-fence-sim', midClose), + false, + 'a start that opens mid-close is refused', + ); + + settle(); + assert.equal(runnerStartTeardownPending('admission-fence-sim'), false); + assert.equal( + runnerStartAdmitsPreparation('admission-fence-sim', midClose), + true, + 'the queued start is not the close: it runs once the fence lifts', + ); + assert.equal(midClose.admitted, true); + finishRunnerStartAdmission(midClose); +}); + +/** + * A start that was in flight when the close began keeps its closed verdict after the fence lifts: + * its retry is the replacement build the fence exists for, so it can never resume into the fresh + * start a later open makes (#3220). + */ +test('a start the close found in flight keeps its verdict after the fence lifts', () => { + const device = 'admission-verdict-sim'; + const inFlight = openRunnerStartAdmission(device); + const settle = fenceRunnerStartAdmissionsForTeardown(device); + settle(); + + assert.equal(inFlight.admitted, false, 'the close closed it while it was in flight'); + assert.equal(inFlight.retired, 'device_teardown'); + assert.equal(runnerStartAdmitsPreparation(device, inFlight), false); + assert.equal( + runnerStartAdmitsPreparation(device), + true, + 'and the device itself is free for the next open', + ); + finishRunnerStartAdmission(inFlight); +}); + +/** + * A settle is idempotent: a teardown that settles inside the lock it took and again in a + * `finally` must not lift another teardown's fence with the second call. + */ +test('one teardown settling twice lifts only its own fence', () => { + const device = 'admission-double-settle-sim'; + const first = fenceRunnerStartAdmissionsForTeardown(device); + const second = fenceRunnerStartAdmissionsForTeardown(device); + + first(); + first(); + assert.equal(runnerStartTeardownPending(device), true, 'the second teardown still fences'); + second(); + assert.equal(runnerStartTeardownPending(device), false); +}); + +/** + * Waiters are counted: canceling one preserves the work another still expects, and only the + * final cancellation is allowed to stop it. Without the count, one client disconnecting would + * SIGTERM a build a live request is waiting on. + */ +test('only the final interested waiter closes admission and owns the stop', () => { + const admission = openRunnerStartAdmission('admission-waiters-sim'); + const first = new AbortController(); + const second = new AbortController(); + addRunnerStartWaiter(admission, first.signal); + addRunnerStartWaiter(admission, second.signal); + + assert.equal(cancelRunnerStartWaiter(admission, first.signal), false); + assert.equal(admission.admitted, true, 'a waiter still interested keeps the work admitted'); + + assert.equal(cancelRunnerStartWaiter(admission, second.signal), true); + assert.equal(admission.admitted, false); + assert.equal(admission.retired, 'last_waiter_canceled'); + finishRunnerStartAdmission(admission); +}); + +/** + * A start closed by a cancellation is never reopened: a caller whose start was canceled for lack + * of interest has no claim on re-admission, and a device that has since gone quiet again must not + * hand its work back to it. + */ +test('a cancellation-closed start is never readmitted', () => { + const device = 'admission-cancel-closed-sim'; + const admission = openRunnerStartAdmission(device); + const waiter = new AbortController(); + addRunnerStartWaiter(admission, waiter.signal); + assert.equal(cancelRunnerStartWaiter(admission, waiter.signal), true); + + assert.equal(readmitRunnerStartAdmission(admission), false); + assert.equal(admission.admitted, false); + finishRunnerStartAdmission(admission); +}); + +/** + * A start that only QUEUED behind a teardown is not that teardown's retry: once the fence has + * settled, the woken start is readmitted and runs, instead of failing forever for a close it never + * took part in (#3220 review). While the fence still stands, nothing re-admits — that is the + * mid-close retry the fence exists for. + */ +test('a start queued behind a settled teardown is readmitted; one under the fence is not', () => { + const device = 'admission-readmit-sim'; + const admission = openRunnerStartAdmission(device); + const settle = fenceRunnerStartAdmissionsForTeardown(device); + + // Mid-close: the fence still stands, so the queued start stays refused. + assert.equal(readmitRunnerStartAdmission(admission), false); + assert.equal(admission.admitted, false); + + settle(); + assert.equal(readmitRunnerStartAdmission(admission), true); + assert.equal(admission.admitted, true); + assert.equal(runnerStartAdmitsPreparation(device, admission), true); + finishRunnerStartAdmission(admission); +}); + +/** + * A readmit never steps into a newer teardown: a second close that fences the device again leaves + * the woken start refused rather than handing it a verdict it already lost. + */ +test('a queued start is not readmitted under a newer fence', () => { + const device = 'admission-readmit-fenced-sim'; + const admission = openRunnerStartAdmission(device); + const first = fenceRunnerStartAdmissionsForTeardown(device); + first(); + const second = fenceRunnerStartAdmissionsForTeardown(device); + + assert.equal(readmitRunnerStartAdmission(admission), false); + second(); + finishRunnerStartAdmission(admission); +}); + +/** + * Two starts on one device are independent: a teardown closes both while it runs, and the one + * that merely queued is readmitted once it settles while the other keeps its verdict until it + * finishes its own work. + */ +test('independent starts on one device carry independent verdicts', () => { + const device = 'admission-two-starts-sim'; + const queued = openRunnerStartAdmission(device); + const settle = fenceRunnerStartAdmissionsForTeardown(device); + const later = openRunnerStartAdmission(device); + + assert.equal(queued.admitted, false, 'the start in flight when close began is closed'); + assert.equal( + runnerStartAdmitsPreparation(device, later), + false, + 'the later open is refused while the fence stands', + ); + + settle(); + assert.equal(readmitRunnerStartAdmission(queued), true); + assert.equal( + readmitRunnerStartAdmission(later), + false, + 'a token still open needs no readmit — its verdict was never closed', + ); + assert.equal(later.admitted, true); + finishRunnerStartAdmission(queued); + finishRunnerStartAdmission(later); +}); + +/** + * A start that settled leaves the device's in-flight set, so a later teardown does not close a + * verdict nobody is waiting on any more; the state leaves the registry with its last member. + */ +test('a settled start leaves the device state, and an empty device leaves the registry', () => { + const device = 'admission-finish-sim'; + const admission = openRunnerStartAdmission(device); + finishRunnerStartAdmission(admission); + + const settle = fenceRunnerStartAdmissionsForTeardown(device); + assert.equal(admission.admitted, true, 'the finished start answers to no fence'); + settle(); + assert.equal(runnerStartTeardownPending(device), false); +}); + +/** + * The prepare loop owns its token across retries, and its health retry is exactly the + * replacement build the fence exists for (#3220 review): a fence that closes the loop + * retires the whole loop, and the settle of that fence must not hand the loop's next + * attempt back a verdict it lost. A loop token is only ever released by the loop. + */ +test('a loop-supplied token is never reopened by a settled fence', () => { + const device = 'admission-loop-sim'; + const loop = openRunnerStartLoopAdmission(device); + const settle = fenceRunnerStartAdmissionsForTeardown(device); + settle(); + + assert.equal(loop.admitted, false, 'the close closed the loop in flight'); + assert.equal( + readmitRunnerStartAdmission(loop), + false, + 'the settled fence never reopens a loop token', + ); + assert.equal(runnerStartAdmitsPreparation(device, loop), false); + finishRunnerStartAdmission(loop); +}); + +/** + * Both halves of the loop rule at once: the retiring loop keeps its refusal after the + * fence lifts, while an independent open that merely queued behind that same fence is + * readmitted and builds on its own. + */ +test('a settled fence refuses the retiring loop and readmits the queued open', () => { + const device = 'admission-loop-and-open-sim'; + const loop = openRunnerStartLoopAdmission(device); + const queued = openRunnerStartAdmission(device); + const settle = fenceRunnerStartAdmissionsForTeardown(device); + settle(); + + assert.equal(readmitRunnerStartAdmission(loop), false); + assert.equal(loop.admitted, false, 'the loop stays retired: its retry was the replacement'); + assert.equal(readmitRunnerStartAdmission(queued), true); + assert.equal(queued.admitted, true, 'the queued open is a fresh start'); + finishRunnerStartAdmission(loop); + finishRunnerStartAdmission(queued); +}); diff --git a/packages/platform-apple/src/runner/__tests__/runner-start-budget.test.ts b/packages/platform-apple/src/runner/__tests__/runner-start-budget.test.ts index dbe8e555f2..117f4d9d95 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-start-budget.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-start-budget.test.ts @@ -9,24 +9,29 @@ import { } from '@agent-device/kernel/errors'; import { IOS_SIMULATOR } from './device-fixtures.ts'; import { appleRunnerTestHost } from '../test-host.ts'; -import { raceRunnerStartAgainstCaller } from '../runner-start-budget.ts'; +import { + raceRunnerStartAgainstCaller, + reserveRunnerStartOwnerInterest, +} from '../runner-start-budget.ts'; import { registerRunnerPrepProcess } from '../runner-artifact.ts'; -import { runnerPrepProcessChildren } from '../runner-xctestrun.ts'; +import { + addRunnerStartWaiter, + fenceRunnerStartAdmissionsForTeardown, + openRunnerStartAdmission, + runnerPrepProcessChildren, +} from '../runner-xctestrun.ts'; const mockSignalPidsBestEffort = vi.fn(); const mockSignalProcessGroupBestEffort = vi.fn(); const mockRunAppleToolCommand = vi.fn(); -const mockGetRequestSignal = vi.fn(); beforeEach(() => { vi.resetAllMocks(); mockRunAppleToolCommand.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }); - mockGetRequestSignal.mockReturnValue(undefined); appleRunnerTestHost.update({ signalPidsBestEffort: mockSignalPidsBestEffort, signalProcessGroupBestEffort: mockSignalProcessGroupBestEffort, runAppleToolCommand: mockRunAppleToolCommand, - getRequestSignal: mockGetRequestSignal, }); }); @@ -61,58 +66,51 @@ async function settleAsyncWork(): Promise { } /** - * The waiter's cancel owns the build it waited on (#3177). A request canceled while queued behind - * a detached cold build must stop that build: its spawn carried only the STARTING request's - * cancellation signal, so without the waiter's device-scoped prep kill the build would keep - * compiling under the daemon on the shared runner derived-data root, and a retried `open` would - * race it. The kill is the same tree-kill escalation a session stop uses. Here the build's owner - * has no live request signal (its start is detached), so the waiter is allowed to stop it. + * The last interested waiter owns the build it waited on (#3177, #3220). A request canceled + * while queued behind a detached cold build must stop that build: its spawn carried only the + * STARTING request's cancellation signal, so without the waiter's device-scoped prep kill the + * build would keep compiling under the daemon on the shared runner derived-data root, where a + * retried `open` would race it. The kill is the same tree-kill escalation a session stop uses. */ -test('a request canceled while waiting on the detached start stops that device build', async () => { +test('the last waiter canceled while waiting on the start stops that device build', async () => { const device = { ...IOS_SIMULATOR, id: 'runner-waiter-cancel-sim' }; + const admission = openRunnerStartAdmission(device.id); const build = makePrepChild(4848); - registerRunnerPrepProcess(device.id, build, 'owner-request-gone'); - mockGetRequestSignal.mockImplementation((requestId?: string) => - requestId === 'owner-request-gone' ? AbortSignal.abort() : undefined, - ); + registerRunnerPrepProcess(device.id, build, admission); const controller = new AbortController(); controller.abort(createRequestCanceledError()); await assert.rejects( - raceRunnerStartAgainstCaller(hangingStart(), controller.signal, device.id), + raceRunnerStartAgainstCaller(hangingStart(), admission, controller.signal), canceled, ); await settleAsyncWork(); assert.ok( signaledPids().includes(4848), - 'the waiter cancel reached the build through the prep tree-kill path', - ); - assert.equal( - runnerPrepProcessChildren(device.id).length, - 0, - 'the killed build left the prep ledger', + 'the last waiter cancel reached the build through the prep tree-kill path', ); + assert.equal(runnerPrepProcessChildren(device.id).length, 0, 'the killed build left the ledger'); + assert.equal(admission.admitted, false, 'and admission closed so nothing rebuilds (#3220)'); }); /** - * A canceled waiter must not reach a build still owned by an in-flight request (#3177 review). - * The owner cancels its own build through its live request signal at the exec layer; a waiter - * tearing it down would SIGTERM another active request's work out from under it. Here the build's - * owner still has a registered, un-aborted signal, so the canceled waiter leaves it running. + * A canceled waiter must not reach a build another waiter still expects (#3177 review, #3220). + * The build belongs to work somebody is waiting on, and one request tearing it down would + * SIGTERM that waiter's work out from under it. Here a second caller still holds interest, so + * the canceled waiter leaves the build running and admitted. */ -test('a canceled waiter leaves a build owned by a still-active request', async () => { - const device = { ...IOS_SIMULATOR, id: 'runner-waiter-owner-active-sim' }; +test('a canceled waiter leaves the build while another waiter is still interested', async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-waiter-peer-sim' }; + const admission = openRunnerStartAdmission(device.id); const build = makePrepChild(4850); - registerRunnerPrepProcess(device.id, build, 'owner-request-live'); - mockGetRequestSignal.mockImplementation((requestId?: string) => - requestId === 'owner-request-live' ? new AbortController().signal : undefined, - ); + registerRunnerPrepProcess(device.id, build, admission); + addRunnerStartWaiter(admission, new AbortController().signal); const controller = new AbortController(); controller.abort(createRequestCanceledError()); await assert.rejects( - raceRunnerStartAgainstCaller(hangingStart(), controller.signal, device.id), + raceRunnerStartAgainstCaller(hangingStart(), admission, controller.signal), canceled, ); await settleAsyncWork(); @@ -120,13 +118,14 @@ test('a canceled waiter leaves a build owned by a still-active request', async ( assert.equal( mockSignalProcessGroupBestEffort.mock.calls.length, 0, - 'a canceled waiter never signals a build another active request owns', + 'a canceled waiter never signals a build another waiter still needs', ); assert.deepEqual( runnerPrepProcessChildren(device.id).map((child) => child.pid), [4850], - 'the owned build stayed registered', + 'the needed build stayed registered', ); + assert.equal(admission.admitted, true, 'a live waiter keeps preparation admitted'); }); /** @@ -136,11 +135,12 @@ test('a canceled waiter leaves a build owned by a still-active request', async ( */ test('a caller deadline on the same waiter leaves the build running', async () => { const device = { ...IOS_SIMULATOR, id: 'runner-waiter-deadline-sim' }; + const admission = openRunnerStartAdmission(device.id); const build = makePrepChild(4949); - registerRunnerPrepProcess(device.id, build); + registerRunnerPrepProcess(device.id, build, admission); const controller = new AbortController(); - const waiting = raceRunnerStartAgainstCaller(hangingStart(), controller.signal, device.id); + const waiting = raceRunnerStartAgainstCaller(hangingStart(), admission, controller.signal); controller.abort(callerDeadline()); await assert.rejects(waiting, canceled); await settleAsyncWork(); @@ -155,24 +155,106 @@ test('a caller deadline on the same waiter leaves the build running', async () = [4949], 'the deadline left the build registered', ); + assert.equal(admission.admitted, true, 'a deadline never closes admission'); +}); + +/** + * A build its caller left on a deadline is still owned work: the start it belongs to keeps + * running for that caller's retry (#2894), so a DIFFERENT caller's cancellation must not reach + * it. This is the case #3193's owner sniff protected by reading the owning request's liveness; + * the start's own token carries the fact now (#3220). + */ +test("a canceled caller leaves the build another caller's deadline detached", async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-waiter-deadline-owner-sim' }; + const detachedStart = openRunnerStartAdmission(device.id); + const build = makePrepChild(4960); + registerRunnerPrepProcess(device.id, build, detachedStart); + + // Caller one leaves its start running on its own deadline. + const deadlineController = new AbortController(); + const detachedWait = raceRunnerStartAgainstCaller( + hangingStart(), + detachedStart, + deadlineController.signal, + ).catch((error: unknown) => error); + deadlineController.abort(callerDeadline()); + await detachedWait; + + // A second caller, on its own start, then cancels. + const otherAdmission = openRunnerStartAdmission(device.id); + const cancelController = new AbortController(); + cancelController.abort(createRequestCanceledError()); + await assert.rejects( + raceRunnerStartAgainstCaller(hangingStart(), otherAdmission, cancelController.signal), + canceled, + ); + await settleAsyncWork(); + + assert.equal( + mockSignalProcessGroupBestEffort.mock.calls.length, + 0, + 'a cancel reaches only builds no live caller owns', + ); + assert.deepEqual( + runnerPrepProcessChildren(device.id).map((child) => child.pid), + [4960], + 'the deadline-detached build the retry will join is still running', + ); +}); + +/** + * A cancellation that arrives after a teardown already fenced the device stops nothing new: the + * teardown killed the device's builds under the same fence, and a late cancel must not claim a + * build a later open on that device has already started paying for. The later start's child is + * registered here so the assertion says the new build is still running, not merely that no + * signal was attempted. + */ +test('a canceled waiter after a teardown stops nothing a later start needs', async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-waiter-after-teardown-sim' }; + const fenced = openRunnerStartAdmission(device.id); + // The teardown's own sweep is over; the fence lifts, and the next open pays for its own build. + fenceRunnerStartAdmissionsForTeardown(device.id)(); + const freshAdmission = openRunnerStartAdmission(device.id); + const neededBuild = makePrepChild(4951); + registerRunnerPrepProcess(device.id, neededBuild, freshAdmission); + + const controller = new AbortController(); + controller.abort(createRequestCanceledError()); + await assert.rejects( + raceRunnerStartAgainstCaller(hangingStart(), fenced, controller.signal), + canceled, + ); + await settleAsyncWork(); + + assert.equal( + mockSignalProcessGroupBestEffort.mock.calls.length, + 0, + 'the teardown already did the stopping', + ); + assert.deepEqual( + runnerPrepProcessChildren(device.id).map((child) => child.pid), + [4951], + "the later start's build is still running and still registered", + ); }); /** * The kill is scoped to the waiting device. A canceled waiter for device A must not signal the - * build device B is still paying for — the same request-scoping rule this PR applies to the + * build device B is still paying for — the same request-scoping rule #3177 applies to the * daemon-side timeout recovery. */ test('a canceled waiter signals only its own device build', async () => { const device = { ...IOS_SIMULATOR, id: 'runner-waiter-scope-sim' }; + const admission = openRunnerStartAdmission(device.id); const ownBuild = makePrepChild(5050); const siblingBuild = makePrepChild(5151); - registerRunnerPrepProcess(device.id, ownBuild); + registerRunnerPrepProcess(device.id, ownBuild, admission); registerRunnerPrepProcess('other-device', siblingBuild); const controller = new AbortController(); controller.abort(createRequestCanceledError()); await assert.rejects( - raceRunnerStartAgainstCaller(hangingStart(), controller.signal, device.id), + raceRunnerStartAgainstCaller(hangingStart(), admission, controller.signal), canceled, ); await settleAsyncWork(); @@ -196,3 +278,69 @@ test('a closed build leaves the prep ledger on its own', () => { build.emit('close'); assert.equal(runnerPrepProcessChildren(device.id).length, 0); }); + +/** + * The starting request counts as interested even where the caller passes no `signal` (#3220 + * review): a joiner's cancellation must not outvote the live owner and SIGTERM its build out + * from under it — the protection the removed #3193 owner sniff carried, now by mechanism. + */ +test('owner interest keeps a joiner cancel from stopping the starting request build', async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-owner-interest-sim' }; + const requestId = 'owner-request-live-' + device.id; + const owner = new AbortController(); + appleRunnerTestHost.update({ + getRequestSignal: (id?: string) => (id === requestId ? owner.signal : undefined), + }); + const admission = openRunnerStartAdmission(device.id); + const build = makePrepChild(5353); + registerRunnerPrepProcess(device.id, build, admission); + const releaseOwner = reserveRunnerStartOwnerInterest(admission, { requestId }); + + const joiner = new AbortController(); + joiner.abort(createRequestCanceledError()); + await assert.rejects( + raceRunnerStartAgainstCaller(hangingStart(), admission, joiner.signal), + canceled, + ); + await settleAsyncWork(); + + assert.equal(mockSignalProcessGroupBestEffort.mock.calls.length, 0); + assert.deepEqual( + runnerPrepProcessChildren(device.id).map((child) => child.pid), + [5353], + 'the owner still counts: the joiner cancel stopped nothing', + ); + assert.equal(admission.admitted, true); + + // The owner's own disconnect is the last interest leaving: it closes and stops. + owner.abort(createRequestCanceledError()); + await settleAsyncWork(); + assert.ok(signaledPids().includes(5353), 'the owner cancel reaches its own build'); + assert.equal(admission.admitted, false, 'and admission closed so nothing rebuilds'); + releaseOwner(); +}); + +/** A start that settles releases its owner interest; the next cancel owns the stop. */ +test('a settled start releases owner interest', async () => { + const device = { ...IOS_SIMULATOR, id: 'runner-owner-released-sim' }; + const requestId = 'owner-request-settled-' + device.id; + const owner = new AbortController(); + appleRunnerTestHost.update({ + getRequestSignal: (id?: string) => (id === requestId ? owner.signal : undefined), + }); + const admission = openRunnerStartAdmission(device.id); + const build = makePrepChild(5454); + registerRunnerPrepProcess(device.id, build, admission); + reserveRunnerStartOwnerInterest(admission, { requestId })(); + + const joiner = new AbortController(); + joiner.abort(createRequestCanceledError()); + await assert.rejects( + raceRunnerStartAgainstCaller(hangingStart(), admission, joiner.signal), + canceled, + ); + await settleAsyncWork(); + + assert.ok(signaledPids().includes(5454), 'with the owner counted out, the joiner is last'); + assert.equal(admission.admitted, false); +}); diff --git a/packages/platform-apple/src/runner/runner-adoption.ts b/packages/platform-apple/src/runner/runner-adoption.ts index a8eb4d2472..4ea3d0316f 100644 --- a/packages/platform-apple/src/runner/runner-adoption.ts +++ b/packages/platform-apple/src/runner/runner-adoption.ts @@ -16,6 +16,7 @@ import { withRunnerCommandId, } from './runner-contract.ts'; import { + buildDetachedRunnerLease, buildRunnerLease, isLeaseRunnerProcessIntact, readRunnerLeaseForAdoption, @@ -24,6 +25,7 @@ import { type RunnerLease, type RunnerLeaseAdoptionRefusal, } from './runner-lease.ts'; +import { isRunnerProcessAlive } from './runner-disposal.ts'; import { resolveExpectedRunnerCacheMetadata, resolveRunnerDerivedPath, @@ -32,7 +34,10 @@ import { } from './runner-xctestrun.ts'; import { resolveRunnerCacheKey } from './runner-cache-metadata.ts'; import { + advanceRunnerSessionState, + resolveRunnerDetachDecision, RunnerCommandAccounting, + type RunnerDetachRefusal, type RunnerProcessHandle, type RunnerSession, } from './runner-session-types.ts'; @@ -58,6 +63,107 @@ export function isIosRunnerDetachEnabled(env: NodeJS.ProcessEnv = process.env): return parseBooleanLiteral(env.AGENT_DEVICE_IOS_RUNNER_DETACH ?? '') !== false; } +// Graceful daemon shutdown hands a request-proven runner off to the next daemon instead of paying +// the xcodebuild ramp again: the lease token is rewritten to a detached form (so this daemon's own +// teardown paths no longer classify it as owned), this process gives up its sides of the runner's +// log, and `onDetached` takes the session out of the caller's registry. Once this process exits the +// lease is stale and the adoption path picks it up. Explicit cleanup still works: clean:daemon kills +// by the lease's runnerPid, and the runner's XCTWaiter self-expires after 24h. +// +// Every gate that keeps a session on the kill path is named and reported, because a handoff that +// silently declines is indistinguishable from a rebuild: the handoff lanes +// (`resolveRunnerHandoffTarget`), a session that never served a command, still owes a response, or +// last reported main-thread work still draining (`resolveRunnerDetachDecision`), a missing or +// unwritable lease, and a runner this process cannot prove alive. What stays in the registry is +// torn down by `stopAllIosRunnerSessions`, which the daemon's shutdown runs right after this — so a +// shutdown during a startup tears that runner down rather than handing off one that never reached +// its listener (#2681). +export async function detachRunnerSessionsForShutdown( + sessions: ReadonlyMap, + onDetached: (deviceId: string, session: RunnerSession) => void, +): Promise { + if (!isIosRunnerDetachEnabled()) return 0; + let detached = 0; + for (const [deviceId, session] of sessions) { + const outcome = detachRunnerSessionForShutdown(session); + if (!outcome.detached) { + emitDiagnostic({ + level: 'debug', + phase: 'ios_runner_session_detach_skipped', + data: { + deviceId, + sessionId: session.sessionId, + lane: outcome.lane, + reason: outcome.reason, + // A refused handoff is read from the daemon log, and the two refusals that name a charge look + // identical without this: an exchange still awaited is recovered by its own answer, while an + // abandoned residue waits for terminal evidence for its `commandId` (#2965). + outstandingCharges: session.commandCharges.outstandingChargeCount, + hasAbandonedCharges: session.commandCharges.hasAbandonedCharges, + }, + }); + continue; + } + detached += 1; + onDetached(deviceId, session); + emitDiagnostic({ + level: 'info', + phase: 'ios_runner_session_detached', + data: { + deviceId, + lane: outcome.lane, + sessionId: session.sessionId, + runnerPid: session.child.pid, + port: session.port, + runnerLogPath: session.runnerLogPath, + }, + }); + } + return detached; +} + +type RunnerDetachSkippedReason = + | RunnerHandoffRefusal + | RunnerDetachRefusal + | 'lease_absent' + | 'runner_process_dead' + | 'lease_write_failed'; + +type RunnerDetachOutcome = + | { detached: true; lane: RunnerHandoffLane } + | { detached: false; lane: RunnerHandoffLane | undefined; reason: RunnerDetachSkippedReason }; + +function detachRunnerSessionForShutdown(session: RunnerSession): RunnerDetachOutcome { + const target = resolveRunnerHandoffTarget(session.device); + if (!target.handoff) { + return { detached: false, lane: undefined, reason: target.reason }; + } + const lane = target.lane; + const decision = resolveRunnerDetachDecision(session); + if (!decision.detach) { + return { detached: false, lane, reason: decision.reason }; + } + const lease = session.lease; + if (!lease) { + return { detached: false, lane, reason: 'lease_absent' }; + } + if (!isRunnerProcessAlive(session.child.pid)) { + return { detached: false, lane, reason: 'runner_process_dead' }; + } + try { + writeRunnerLease(buildDetachedRunnerLease(lease)); + } catch { + return { detached: false, lane, reason: 'lease_write_failed' }; + } + // Only once the lease says the runner is handed over does this process give up its own sides of + // the runner's log: until that write lands the session is still owned, and an owned session that + // stopped following its runner's output is worse off than one that never handed anything off. + // The runner holds its own descriptor, so this cannot disturb it either way (#2681). + session.endOutputObservation?.(); + advanceRunnerSessionState(session, 'stopped'); + return { detached: true, lane }; +} + type RunnerAdoptionRefusal = | RunnerHandoffRefusal | 'lease_absent' diff --git a/packages/platform-apple/src/runner/runner-artifact.ts b/packages/platform-apple/src/runner/runner-artifact.ts index defe03ce6c..f8bedcb7f6 100644 --- a/packages/platform-apple/src/runner/runner-artifact.ts +++ b/packages/platform-apple/src/runner/runner-artifact.ts @@ -1,4 +1,8 @@ -import { AppError, isRequestCanceledError } from '@agent-device/kernel/errors'; +import { + AppError, + createRequestCanceledError, + isRequestCanceledError, +} from '@agent-device/kernel/errors'; import fs from 'node:fs'; import crypto from 'node:crypto'; import os from 'node:os'; @@ -7,9 +11,9 @@ import { runCmdStreaming, withKeyedLock, withProcessLock, + emitDiagnostic, emitRequestProgress, findProjectRoot, - getRequestSignal, isCommandTimeoutError, } from './host.ts'; import type { ExecBackgroundResult } from '@agent-device/host-kit/command'; @@ -62,9 +66,297 @@ export { prepareXctestrunWithEnv } from './runner-artifact-env.ts'; const runnerXctestrunBuildLocks = new Map>(); +/** + * Why a runner start can no longer prepare its device. `device_teardown` is an explicit teardown + * (a non-retained `close`, a daemon stop); `last_waiter_canceled` is the final interested waiter + * cancelling the start. + */ +export type RunnerStartRetirementReason = 'device_teardown' | 'last_waiter_canceled'; + +/** + * Whether one runner start may still prepare its device: spawn a preparation subprocess + * (`xcodebuild build-for-testing`) or publish the runner it built. + * + * The token belongs to the START, not to the device, and it closes on the first of: + * + * - **An explicit teardown** ({@link fenceRunnerStartAdmissionsForTeardown}), which fences every + * start in flight on the device BEFORE the teardown stops the current prep children or waits for + * the session lock. A start parked mid-build answers the kill by failing its own spawn check + * instead of rebuilding, which is what keeps close from waiting on a replacement build (#3220). + * - **The last interested waiter cancelling** ({@link cancelRunnerStartWaiter}). Waiters are + * counted, so one waiter leaving while another still expects the session preserves the work; + * only the final cancellation closes admission. + * + * Independent opens are separate starts: they queue on the session lock as they always have and + * carry their own token, which no other start's teardown touches. A start that merely QUEUED + * behind a teardown is readmitted once that teardown settles + * ({@link readmitRunnerStartAdmission}), so the fence refuses the retiring start's own retries and + * mid-close work, not every later open. The preparation-spawn seam and the session-publish point + * read the verdict immediately before they act. + */ +export type RunnerStartAdmission = Readonly<{ + deviceId: string; + /** Whether preparation and publication are still admitted for a start holding this token. */ + admitted: boolean; + /** Why admission closed, or `undefined` while it is open. */ + retired: RunnerStartRetirementReason | undefined; +}>; + +type StartAdmission = RunnerStartAdmission & { + readonly interestedWaiters: Set; + /** True for a token an `ensureRunnerSession` call minted for itself, never a supplied one. */ + readonly mintedByStart: boolean; + /** A caller left this start on its own deadline while it runs: its retry is still owed (#2894). */ + retryPending: boolean; + close(reason: RunnerStartRetirementReason): boolean; + readmit(): void; +}; + +type DeviceStartState = { + /** Explicit teardowns in flight on this device; while positive, no preparation is admitted. */ + pendingTeardowns: number; + /** The tokens of the starts in flight on this device — the fence's sweep set, nothing more. */ + inFlight: Set; +}; + +const deviceStartStates = new Map(); + +/** + * Opens the admission one start answers to, taken when a caller asks for a runner. Registration is + * synchronous and precedes any await, so a teardown beginning in the same turn as the start finds + * the token and closes it even while the start queues behind the work another start holds the + * session lock for (#3220). + */ +export function openRunnerStartAdmission(deviceId: string): RunnerStartAdmission { + return registerStartAdmission(createStartAdmission(deviceId, true)); +} + +/** + * Opens the one admission a prepare attempt loop owns and supplies to every start that loop makes + * (#3220). A fence landing mid-loop is a verdict on the whole loop — the health retry that + * re-entered the start is the replacement build the incident measured — so a loop token is never + * reopened once the fence lifts, and the loop's own settle is what releases it. + */ +export function openRunnerStartLoopAdmission(deviceId: string): RunnerStartAdmission { + return registerStartAdmission(createStartAdmission(deviceId, false)); +} + +/** The start settled: its token leaves the device's in-flight set. Verdicts live on the token. */ +export function finishRunnerStartAdmission(admission: RunnerStartAdmission): void { + (admission as StartAdmission).retryPending = false; + const state = deviceStartStates.get(admission.deviceId); + if (!state) return; + state.inFlight.delete(admission as StartAdmission); + forgetDeviceStartState(admission.deviceId, state); +} + +/** + * An explicit teardown fences the device: every start in flight closes BEFORE the teardown stops + * the current prep children or takes the session lock, and while the fence stands no start + * prepares the device — including preparation carrying no start's token, a cache prewarm's build. + * The returned settle lifts this teardown's fence and runs as the teardown's last step; + * overlapping teardowns count each other, so a fence never lifts while another still runs (#3220). + */ +export function fenceRunnerStartAdmissionsForTeardown(deviceId: string): () => void { + const state = stateForDevice(deviceId); + state.pendingTeardowns += 1; + for (const admission of state.inFlight) { + admission.close('device_teardown'); + } + let settled = false; + return () => { + // Idempotent: a teardown may call its settle inside the lock it took and again in a + // `finally`, and only the first call may lift the fence it raised. + if (settled) return; + settled = true; + state.pendingTeardowns -= 1; + forgetDeviceStartState(deviceId, state); + }; +} + +/** + * Daemon-wide teardown: no start may prepare any device until the settle runs. Devices carrying a + * runner session are fenced with the rest, so a newcomer open on a session's device cannot slip a + * build past the sweep that is about to stop it. + */ +export function retireAllRunnerStartAdmissions(additionalDeviceIds?: Iterable): () => void { + const deviceIds = new Set([...deviceStartStates.keys(), ...(additionalDeviceIds ?? [])]); + const settles = [...deviceIds].map((deviceId) => fenceRunnerStartAdmissionsForTeardown(deviceId)); + return () => { + for (const settle of settles.splice(0)) settle(); + }; +} + +/** + * Whether an explicit teardown is in flight on this device. Read where a start decides what a + * closed token means: a queued independent open whose close has since settled is a fresh start, + * not that teardown's retry. + */ +export function runnerStartTeardownPending(deviceId: string): boolean { + return (deviceStartStates.get(deviceId)?.pendingTeardowns ?? 0) > 0; +} + +/** + * Re-admits a start that only ever queued behind a settled teardown: its token closed while the + * teardown ran, no teardown is pending now, and every gate refused it work in the meantime, so it + * runs on rather than failing for a close it never took part in (#3220 review). + */ +export function readmitRunnerStartAdmission(admission: RunnerStartAdmission): boolean { + const start = admission as StartAdmission; + // A loop-supplied token is a verdict on the whole loop, and the loop's own retry is exactly the + // replacement build the fence exists for: only a start that minted its own token reruns. + if (!start.mintedByStart || start.admitted || start.retired !== 'device_teardown') return false; + if (runnerStartTeardownPending(start.deviceId)) return false; + start.readmit(); + return true; +} +/** + * The gate the preparation-spawn seam, the publish point, and the lock-entry check share. Two + * questions, one answer: the start that owns the work must still be admitted, and the device must + * not sit under an explicit teardown in progress. The second is what reaches preparation carrying + * no start's token — a cache prewarm's build enqueued while close runs — because the device's + * fence alone answers it. + */ +export function runnerStartAdmitsPreparation( + deviceId: string, + startAdmission?: RunnerStartAdmission, +): boolean { + if (startAdmission && !startAdmission.admitted) return false; + return !runnerStartTeardownPending(deviceId); +} + +/** + * The refusal a start publishes when its admission closed under it: a canceled-request error, + * because a teardown and a last-waiter cancellation both cancel the work, carrying the typed + * retirement reason so no caller reads a message to learn which. + */ +export function runnerStartRetiredError(reason: RunnerStartRetirementReason): AppError { + return createRequestCanceledError({ + runnerStartRetired: true, + runnerStartRetirementReason: reason, + }); +} + +/** Throws {@link runnerStartRetiredError} once a start may no longer prepare. */ +export function assertRunnerStartAdmitsPreparation( + deviceId: string, + startAdmission?: RunnerStartAdmission, +): void { + if (runnerStartAdmitsPreparation(deviceId, startAdmission)) return; + throw runnerStartRetiredError(startAdmission?.retired ?? 'device_teardown'); +} + +/** + * Records a caller's interest in the outcome of this start's work, keyed by the signal through + * which that caller can cancel. Keyed on the signal because an interested waiter is exactly a + * caller that can still say it is not interested: a fire-and-forget prewarm carries no signal, + * registers nothing, and must not be what keeps a live waiter's cancellation from fencing the work + * it just abandoned (#3220). + */ +export function addRunnerStartWaiter( + admission: RunnerStartAdmission, + signal: AbortSignal, +): 'interested' | 'already-spent' { + if (signal.aborted) return 'already-spent'; + (admission as StartAdmission).interestedWaiters.add(signal); + return 'interested'; +} + +/** + * A cancellation landed for this caller: reports whether it now owns stopping this start's prep + * children — true only when admission still stands and no OTHER waiter still expects this start's + * outcome. Owning the stop means closing admission in the same motion, or the cancelled caller's + * build would respawn the moment the start retries (#3220). + * + * A cancellation that finds another waiter interested closes and stops nothing: that build belongs + * to work somebody is still waiting for, and reaching it would be one request killing another's + * work out from under it. A cancellation arriving after a teardown fenced the start stops nothing + * new — the teardown owns that stop. A caller canceled before its interest ever registered is + * covered too: if nobody else cares about this start, that caller is the only one its work can + * still be orphaned on. + */ +export function cancelRunnerStartWaiter( + admission: RunnerStartAdmission, + signal: AbortSignal, +): boolean { + const start = admission as StartAdmission; + start.interestedWaiters.delete(signal); + if (!start.admitted || start.interestedWaiters.size > 0) return false; + return start.close('last_waiter_canceled'); +} + +/** + * Drops one waiter's interest that was not a cancellation — its start settled, or the caller's own + * deadline ended the wait while leaving the start running for the retry (#2894). Spent interest + * stops counting toward the last-waiter rule; nothing closes and nothing stops, which is what keeps + * a bounded poll from tearing down the build its own retry needs. + */ +export function releaseRunnerStartWaiter( + admission: RunnerStartAdmission, + signal: AbortSignal, +): void { + (admission as StartAdmission).interestedWaiters.delete(signal); +} + +function stateForDevice(deviceId: string): DeviceStartState { + const existing = deviceStartStates.get(deviceId); + if (existing) return existing; + const state: DeviceStartState = { pendingTeardowns: 0, inFlight: new Set() }; + deviceStartStates.set(deviceId, state); + return state; +} + +/** Nothing fences the device and no start is in flight on it once the last of both is gone. */ +function forgetDeviceStartState(deviceId: string, state: DeviceStartState): void { + if (state.pendingTeardowns <= 0 && state.inFlight.size === 0) { + deviceStartStates.delete(deviceId); + } +} + +function registerStartAdmission(admission: StartAdmission): StartAdmission { + stateForDevice(admission.deviceId).inFlight.add(admission); + return admission; +} + +function createStartAdmission(deviceId: string, mintedByStart: boolean): StartAdmission { + const interestedWaiters = new Set(); + let retired: RunnerStartRetirementReason | undefined; + return { + deviceId, + interestedWaiters, + mintedByStart, + retryPending: false, + get admitted() { + return retired === undefined; + }, + get retired() { + return retired; + }, + close(reason) { + if (retired) return false; + retired = reason; + emitDiagnostic({ + level: 'debug', + phase: 'ios_runner_start_admission_retired', + data: { deviceId, reason }, + }); + return true; + }, + readmit() { + retired = undefined; + emitDiagnostic({ + level: 'debug', + phase: 'ios_runner_start_admission_readmitted', + data: { deviceId }, + }); + }, + }; +} + type RunnerPrepProcess = Readonly<{ deviceId: string; - requestId: string | undefined; + /** The start whose admission admitted this child; a closed token owns stopping exactly these. */ + startAdmission: RunnerStartAdmission | undefined; child: ExecBackgroundResult['child']; }>; @@ -72,18 +364,19 @@ const runnerPrepProcessLedger = new Set(); /** * Records a prep subprocess (`xcodebuild build-for-testing`) against the device it builds for and - * the request whose start spawned it, so a request canceled while waiting on that build can stop - * it (#3177). The build child keeps its owning start's signal as its first cancel path; this - * ledger is the device-scoped second one, for the waiters whose cancellation the spawn never saw. - * The owner is what keeps the second path from reaching a build another, still-active request - * launched: a canceled waiter stops only builds whose owner can no longer cancel them. + * the start admitted to build it, so a cancellation can stop the right children (#3177). The build + * child keeps its owning start's signal as its first cancel path; this ledger is the second one, + * for the waiters whose cancellation the spawn never saw. Which cancellations may stop what is + * registered here is the start admission's verdict (#3220), not a property of the child: a + * last-waiter cancellation stops the children its own start spawned, and an explicit teardown + * stops the device's — never a build another live start owns. */ export function registerRunnerPrepProcess( deviceId: string, child: ExecBackgroundResult['child'], - requestId?: string, + startAdmission?: RunnerStartAdmission, ): void { - const entry: RunnerPrepProcess = { deviceId, requestId, child }; + const entry: RunnerPrepProcess = { deviceId, startAdmission, child }; runnerPrepProcessLedger.add(entry); child.on('close', () => { runnerPrepProcessLedger.delete(entry); @@ -98,34 +391,44 @@ export function runnerPrepProcessChildren( } /** - * The prep subprocesses whose owning start is detached: the request that spawned it has finished, - * was canceled, or never existed (an in-process build with no request), so no live cancellation - * can reach the build anymore. A canceled waiter stops exactly these, never a build still owned by - * an in-flight request — that one belongs to its owner and dies through the owner's own signal. + * The prep subprocesses a start's cancellation owns stopping: the device's builds that no live + * caller can cancel any more (#3177). Ownership is the start admission recorded on the ledger + * entry, and an owner is live while an interested waiter remains on that token — so a canceled + * waiter reaches its own build (its cancel emptied the token) and the build of a start nobody is + * waiting for any more, and never reaches a build another live caller is still waiting on. This + * is #3193's protection carried by a structural fact rather than a lookup of whether some + * request id still resolves to a live signal (#3220). A build no start owns — a cache prewarm's + * direct artifact build — is nobody's to cancel, so a canceler stops it too. */ -export function runnerPrepProcessChildrenWithoutActiveOwner( +export function runnerPrepProcessChildrenWithoutLiveOwner( deviceId?: string, ): readonly ExecBackgroundResult['child'][] { return prepProcessEntries(deviceId) - .filter((entry) => !isRunnerPrepOwnerActive(entry.requestId)) + .filter( + (entry) => entry.startAdmission === undefined || !hasRunnerStartOwner(entry.startAdmission), + ) .map((entry) => entry.child); } -function prepProcessEntries(deviceId?: string): readonly RunnerPrepProcess[] { - return [...runnerPrepProcessLedger].filter( - (entry) => deviceId === undefined || entry.deviceId === deviceId, - ); +function hasRunnerStartOwner(admission: RunnerStartAdmission): boolean { + const start = admission as StartAdmission; + return start.interestedWaiters.size > 0 || start.retryPending; } /** - * Whether the request owning a prep build can still cancel it: a registered, un-aborted request - * signal means the owner is in flight and its cancellation reaches the build through the exec - * layer directly. + * A caller left this start on its own deadline rather than cancelling it, so the start keeps + * running for the retry that will join it (#2894). Marking the token keeps its build owned: a + * different caller's cancellation must not reach work a retry is still owed, exactly the build + * #3193's owner sniff refused to touch. Cleared when the start settles. */ -function isRunnerPrepOwnerActive(requestId: string | undefined): boolean { - if (!requestId) return false; - const ownerSignal = getRequestSignal(requestId); - return ownerSignal !== undefined && !ownerSignal.aborted; +export function markRunnerStartRetryPending(admission: RunnerStartAdmission): void { + (admission as StartAdmission).retryPending = true; +} + +function prepProcessEntries(deviceId?: string): readonly RunnerPrepProcess[] { + return [...runnerPrepProcessLedger].filter( + (entry) => deviceId === undefined || entry.deviceId === deviceId, + ); } export function forgetRunnerPrepProcess(child: ExecBackgroundResult['child']): void { @@ -159,8 +462,11 @@ type RunnerXctestrunBuildOptions = { verbose?: boolean; logPath?: string; traceLogPath?: string; - /** The request whose start owns this build; recorded so cancel rules respect the owner. */ - requestId?: string; + /** + * The start whose admission admits this build. A build phase a start does not own — a cache + * prewarm — passes none and is admitted by the device's current admission alone (#3220). + */ + startAdmission?: RunnerStartAdmission; /** * 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 @@ -519,6 +825,10 @@ async function buildRunnerXctestrun( /** What {@link requireRunnerPhaseRemainingMs} left of the build phase, for the exec layer. */ buildTimeoutMs: number | undefined, ): Promise { + // Read immediately before the spawn: a build retried after its first child was killed by a + // teardown or a last-waiter cancellation reaches this same line again, and admission is what + // refuses it. Checking at registration would be a check after the child exists (#3220). + assertRunnerStartAdmitsPreparation(device.id, options.startAdmission); const runnerBundleBuildSettings = resolveRunnerBundleBuildSettings(process.env); const signingBuildSettings = resolveRunnerSigningBuildSettings( process.env, @@ -558,7 +868,7 @@ async function buildRunnerXctestrun( timeoutMs: buildTimeoutMs, signal: options.budget?.signal, onSpawn: (child) => { - registerRunnerPrepProcess(device.id, child, options.requestId); + registerRunnerPrepProcess(device.id, child, options.startAdmission); }, onStdoutChunk: (chunk) => { logChunk(chunk, options.logPath, options.traceLogPath, options.verbose); diff --git a/packages/platform-apple/src/runner/runner-disposal.ts b/packages/platform-apple/src/runner/runner-disposal.ts index 0aa61ec94d..a046d1bfdb 100644 --- a/packages/platform-apple/src/runner/runner-disposal.ts +++ b/packages/platform-apple/src/runner/runner-disposal.ts @@ -29,7 +29,7 @@ import { forgetRunnerPrepProcess, IOS_RUNNER_CONTAINER_BUNDLE_IDS, runnerPrepProcessChildren, - runnerPrepProcessChildrenWithoutActiveOwner, + runnerPrepProcessChildrenWithoutLiveOwner, } from './runner-xctestrun.ts'; import { advanceRunnerSessionState, type RunnerSession } from './runner-session-types.ts'; @@ -116,20 +116,23 @@ export async function abortRunnerSessionsAndPrepProcesses( /** * Stops the prep subprocesses (the `xcodebuild build-for-testing` behind a cold runner start) * with the tree-kill escalation the sessions get. A device stops only its own builds: the caller - * that stops device A's session must not sweep device B's in-flight build (#3177). + * that stops device A's session must not sweep device B's in-flight build (#3177). This device-wide + * form belongs to an explicit teardown, which has fenced the device and owns every build on it. */ export async function stopRunnerPrepProcesses(deviceId?: string): Promise { await stopPrepProcessList(runnerPrepProcessChildren(deviceId)); } /** - * Stops the device builds no active request owns anymore (#3177). A canceled waiter may stop the - * build it waited on only once that build's owning start is detached; a build still owned by an - * in-flight request belongs to its owner, which cancels it through its own signal, and a waiter - * must not SIGTERM another request's work out from under it. + * Stops the device builds no live caller owns any more (#3177), the way #3193 did it and for the + * same reason: a canceled waiter may stop the build it waited on, but a build still owned by a + * caller who can cancel it belongs to that caller and dies through that caller's own signal. The + * difference is the mechanism — the waiter set on the start's own admission answers who owns a + * build, instead of a lookup of whether the owning request id still resolves to a live signal + * (#3220). A last-waiter cancellation calls this; an explicit teardown calls the device-wide form. */ -export async function stopRunnerPrepProcessesWithoutActiveOwner(deviceId?: string): Promise { - await stopPrepProcessList(runnerPrepProcessChildrenWithoutActiveOwner(deviceId)); +export async function stopRunnerPrepProcessesWithoutLiveOwner(deviceId?: string): Promise { + await stopPrepProcessList(runnerPrepProcessChildrenWithoutLiveOwner(deviceId)); } async function stopPrepProcessList( diff --git a/packages/platform-apple/src/runner/runner-lifecycle.ts b/packages/platform-apple/src/runner/runner-lifecycle.ts index 4f5e51b5a0..76c5218e4d 100644 --- a/packages/platform-apple/src/runner/runner-lifecycle.ts +++ b/packages/platform-apple/src/runner/runner-lifecycle.ts @@ -43,7 +43,11 @@ import type { AppleRunnerPrepareOptions, AppleRunnerPrepareResult, } from './runner-provider.ts'; -import { markRunnerXctestrunArtifactBadForRun } from './runner-xctestrun.ts'; +import { + finishRunnerStartAdmission, + markRunnerXctestrunArtifactBadForRun, + openRunnerStartLoopAdmission, +} from './runner-xctestrun.ts'; import { handleRunnerTransportErrorAfterCommandSend } from './runner-command-recovery.ts'; import { buildRunnerRecycleBudgetExhaustedError, @@ -70,18 +74,26 @@ export async function prepareLocalIosRunner( assertRunnerRequestActive(options.requestId); const signal = resolveRunnerRequestSignal(options); const command = withRunnerCommandId({ command: 'uptime' }); + // One admission for the whole attempt loop, supplied to every start it makes: a teardown that + // lands mid-loop closes it, and the health retry that would re-enter the start and rebuild the + // runner the loop's killed child was building meets a closed gate instead (#3220). + const startAdmission = openRunnerStartLoopAdmission(device.id); let recoveryReason: string | undefined; - for (let attempt = 1; attempt <= PREPARE_RUNNER_HEALTH_MAX_SESSION_ATTEMPTS; attempt += 1) { - const result = await runPrepareAttempt({ - device, - command, - options, - signal, - attempt, - recoveryReason, - }); - if (result.kind === 'prepared') return result.result; - recoveryReason = result.recoveryReason; + try { + for (let attempt = 1; attempt <= PREPARE_RUNNER_HEALTH_MAX_SESSION_ATTEMPTS; attempt += 1) { + const result = await runPrepareAttempt({ + device, + command, + options: { ...options, startAdmission }, + signal, + attempt, + recoveryReason, + }); + if (result.kind === 'prepared') return result.result; + recoveryReason = result.recoveryReason; + } + } finally { + finishRunnerStartAdmission(startAdmission); } // Unreachable while PREPARE_RUNNER_HEALTH_MAX_SESSION_ATTEMPTS is positive. diff --git a/packages/platform-apple/src/runner/runner-provider.ts b/packages/platform-apple/src/runner/runner-provider.ts index 77def22778..6a4d9263c9 100644 --- a/packages/platform-apple/src/runner/runner-provider.ts +++ b/packages/platform-apple/src/runner/runner-provider.ts @@ -21,6 +21,12 @@ export type AppleRunnerLifecycleOptions = AppleRunnerCommandOptions & { forceRunnerXctestrunRebuild?: boolean; /** The session is started ahead of any command that needs it (a prewarm). */ speculative?: boolean; + /** + * The admission held by the caller that owns this start across its retries (#3220). The prepare + * attempt loop carries one into every start it makes, so a teardown landing mid-loop refuses the + * retry that would otherwise rebuild the runner its first child was killed building. + */ + startAdmission?: import('./runner-artifact.ts').RunnerStartAdmission; }; export type AppleRunnerPrewarmOptions = AppleRunnerLifecycleOptions & { diff --git a/packages/platform-apple/src/runner/runner-session.ts b/packages/platform-apple/src/runner/runner-session.ts index ce4fac520e..27c9944cc5 100644 --- a/packages/platform-apple/src/runner/runner-session.ts +++ b/packages/platform-apple/src/runner/runner-session.ts @@ -9,38 +9,44 @@ import { } from './host.ts'; import type { ExecResult } from '@agent-device/host-kit/command'; import { isApplePlatform, type DeviceInfo } from '@agent-device/kernel/device'; -import { - resolveRunnerHandoffTarget, - type RunnerHandoffLane, - type RunnerHandoffRefusal, -} from './apple-runner-platform.ts'; import type { RunnerLogicalLeaseContext } from '@agent-device/contracts/runner-lease-context'; import type { AppleRunnerLifecycleOptions } from './runner-provider.ts'; import { flushRunnerLogAppends, getFreePort, resolveRunnerLaunchLogPath } from './runner-io.ts'; import { RUNNER_STARTUP_TIMEOUT_MS } from './runner-startup-transport.ts'; import { + assertRunnerStartAdmitsPreparation, createRunnerPhaseBudget, ensureXctestrunArtifact, + fenceRunnerStartAdmissionsForTeardown, + finishRunnerStartAdmission, IOS_RUNNER_CONTAINER_BUNDLE_IDS, + openRunnerStartAdmission, prepareXctestrunWithEnv, + readmitRunnerStartAdmission, requireRunnerPhaseRemainingMs, resolveExpectedRunnerCacheMetadata, resolveRunnerDerivedPath, + retireAllRunnerStartAdmissions, + runnerStartAdmitsPreparation, + runnerStartRetiredError, type RunnerPhaseBudget, + type RunnerStartAdmission, } from './runner-xctestrun.ts'; import { resolveRunnerCacheKey } from './runner-cache-metadata.ts'; import type { RunnerCommand } from './runner-contract.ts'; import { enrichRunnerStartupFailureWithDeviceStates } from './runner-error-classification.ts'; import { isRunnerReadinessProbeCommand } from './runner-command-traits.ts'; import { - buildDetachedRunnerLease, buildRunnerLease, prepareRunnerLeaseForStartup, runnerOwnerToken, withRunnerLeaseLock, writeRunnerLease, } from './runner-lease.ts'; -import { isIosRunnerDetachEnabled, tryAdoptRunnerSessionFromLease } from './runner-adoption.ts'; +import { + detachRunnerSessionsForShutdown, + tryAdoptRunnerSessionFromLease, +} from './runner-adoption.ts'; import { buildRunnerSessionXctestrunSuffix } from './runner-artifact-env.ts'; import { abortRunnerSessionsAndPrepProcesses, @@ -53,21 +59,21 @@ import { type RunnerDisposalOptions, } from './runner-disposal.ts'; import { - advanceRunnerSessionState, buildRunnerSessionId, canWorkWithRunnerSession, isRunnerMainThreadOccupied, normalizeRunnerStartupTimeoutMs, - resolveRunnerDetachDecision, resolveRunnerSessionLiveness, RunnerCommandAccounting, - type RunnerDetachRefusal, type RunnerSession, type RunnerSessionLiveness, type RunnerSessionRegistration, } from './runner-session-types.ts'; import { launchRunnerProcess, type LaunchedRunnerProcess } from './runner-process-launch.ts'; import { isSameRunnerSimulator } from './runner-device-set.ts'; +// The start-budget members are read through function-scoped imports: this module sits in the +// façade closures the eager-closure budget holds at its merge-base size, and the budget module is +// only ever needed at a start, never on an import path. import type { RunnerStartBudget } from './runner-start-budget.ts'; export type { RunnerSession } from './runner-session-types.ts'; @@ -91,8 +97,20 @@ export async function ensureRunnerSession( // Any runner use means the device is active again: a pending idle stop // from a retained-after-close runner no longer applies. cancelIosRunnerIdleStop(device.id); + // This start's admission: the loop that owns the start across retries supplies one, and every + // other start takes a token for itself. Both are taken synchronously, before the first await and + // registered with the device, so a teardown beginning in this same turn closes it even while + // this start queues behind the work another start holds the session lock for (#3220). + const ownedAdmission = options.startAdmission; + const startAdmission = ownedAdmission ?? openRunnerStartAdmission(device.id); const start = withRunnerSessionLock(device.id, async () => { - const { openRunnerStartBudget } = await import('./runner-start-budget.ts'); + const { openRunnerStartBudget, reserveRunnerStartOwnerInterest } = + await import('./runner-start-budget.ts'); + // A start woken after the close that closed its admission has settled did no work under the + // fence and runs on as the fresh start it always was; one woken WHILE that close still runs + // is refused here, before it adopts, boots, or builds (#3220). + readmitRunnerStartAdmission(startAdmission); + assertRunnerStartAdmitsPreparation(device.id, startAdmission); // One budget for the whole start, opened once the lock is held so a start queued behind // another does not spend its clock waiting: the reuse check's toolchain probes, adoption and // the startup itself all read it. The request's cancellation rides with it, so a client @@ -101,6 +119,9 @@ export async function ensureRunnerSession( // still there for the retry (#2894). A start that outlives its caller is still bounded: once // the budget is spent the same signal ends it, the lock is released and the device is usable. const budget = openRunnerStartBudget(options); + // The starting request counts as interested even on the surface that passes no `signal`, so + // a joiner's cancellation can never outvote the live owner and stop its build (#3220). + const releaseOwnerInterest = reserveRunnerStartOwnerInterest(startAdmission, options); try { const existing = runnerSessions.get(device.id); if (existing) { @@ -111,16 +132,23 @@ export async function ensureRunnerSession( return await withRunnerLeaseLock( device.id, - async () => await startRunnerSessionWithLease(device, options, budget), + async () => await startRunnerSessionWithLease(device, options, budget, startAdmission), ); } catch (error) { throw budget.exhausted.aborted ? budget.exhausted.reason : error; } finally { budget.close(); + releaseOwnerInterest(); } }); const { raceRunnerStartAgainstCaller } = await import('./runner-start-budget.ts'); - return await raceRunnerStartAgainstCaller(start, options.signal, device.id); + return await raceRunnerStartAgainstCaller( + // A start that minted its own token releases it; a loop-supplied token belongs to the loop, + // which finishes it when the loop itself is done retrying (#3220). + ownedAdmission ? start : start.finally(() => finishRunnerStartAdmission(startAdmission)), + startAdmission, + options.signal, + ); } /** How long the device-readiness probe may take, bounded by the startup budget it runs inside. */ @@ -130,6 +158,7 @@ async function startRunnerSessionWithLease( device: DeviceInfo, options: RunnerSessionOptions, budget: RunnerStartBudget, + startAdmission: RunnerStartAdmission, ): Promise { const startupTimings: Record = {}; const startupBudget = budget.phase; @@ -158,6 +187,10 @@ async function startRunnerSessionWithLease( if (adopted) { adopted.startupTimings = startupTimings; adopted.logicalLeaseContext = logicalLeaseContext; + // An adoption publishes a runner exactly the way a build does, so it answers to the same + // gate: a start fenced by a teardown registers its runner into no device (#3220). The + // runner itself keeps its lease for the next open to adopt. + assertRunnerStartAdmitsPreparation(device.id, startAdmission); runnerSessions.set(device.id, adopted); return adopted; } @@ -221,6 +254,7 @@ async function startRunnerSessionWithLease( async () => await ensureXctestrunArtifact(device, { ...options, + startAdmission, budget: createRunnerPhaseBudget(resolveRunnerBuildTimeoutMs(options, budget), signal), }), ); @@ -317,6 +351,10 @@ async function startRunnerSessionWithLease( }); throw createRequestCanceledError(); } + // Publication is the second gate: a start whose preparation was admitted before a teardown + // must not register a runner into the device that teardown is settling, or into the fresh + // start a later open has already begun (#3220). + await refuseRetiredRunnerPublication(device, session, startAdmission); try { writeRunnerLease(lease); } catch (error) { @@ -331,6 +369,26 @@ async function startRunnerSessionWithLease( return session; } +/** + * Refuses a launched runner whose start lost admission while it was building: the publication + * a fenced start would make lands on a device teardown is settling or a fresh start already + * owns, so the launch this start already paid for is its own to stop, and it stops it under + * the lease lock the start still holds (#3220). + */ +async function refuseRetiredRunnerPublication( + device: DeviceInfo, + session: RunnerSession, + startAdmission: RunnerStartAdmission, +): Promise { + if (runnerStartAdmitsPreparation(device.id, startAdmission)) return; + await disposeRunnerSession(session, { + graceful: false, + waitTimeoutMs: RUNNER_INVALIDATE_WAIT_TIMEOUT_MS, + leaseLockHeld: true, + }); + throw runnerStartRetiredError(startAdmission.retired ?? 'device_teardown'); +} + export function assertExpectedRunnerSession( session: Pick, expectedRunnerSessionId: string | undefined, @@ -659,13 +717,30 @@ export async function releaseSpeculativeIosRunnerSession(deviceId: string): Prom } export async function stopIosRunnerSession(deviceId: string): Promise { + await stopIosRunnerSessionDevice(deviceId); +} + +/** + * Stops the device's runner under the session lock. `fenceSettle`, when given, is called as the + * last step INSIDE that lock, so a start queued behind this stop wakes to a device whose teardown + * has fully settled and runs on rather than reading the still-pending fence as its own refusal. + */ +async function stopIosRunnerSessionDevice( + deviceId: string, + fenceSettle?: () => void, +): Promise { cancelIosRunnerIdleStop(deviceId); - await withRunnerSessionLock(deviceId, async () => { - await withRunnerLeaseLock(deviceId, async () => { - await stopRunnerSessionInternal(deviceId, undefined, { leaseLockHeld: true }); - await cleanupOwnedIosRunnerLease(deviceId); + try { + await withRunnerSessionLock(deviceId, async () => { + await withRunnerLeaseLock(deviceId, async () => { + await stopRunnerSessionInternal(deviceId, undefined, { leaseLockHeld: true }); + await cleanupOwnedIosRunnerLease(deviceId); + }); + fenceSettle?.(); }); - }); + } finally { + fenceSettle?.(); + } } /** @@ -675,8 +750,10 @@ export async function stopIosRunnerSession(deviceId: string): Promise { * or wedges, so pooling it back hands the same stalled process to the next `open` (#2552). An idle * retained runner keeps warm reuse via the idle-stop timer. The decision is owned here because the * occupancy fact lives on the session, and awaited so `close` returns only once the lease is gone. - * Non-retained close stops the device's current prep processes before taking the session lock, - * which an in-flight cold start holds through its build. Later prep spawns are not fenced here. + * Non-retained close first fences the device's start admission, then stops the device's current + * prep processes, before taking the session lock an in-flight cold start holds through its build: + * a start that answers the kill by retrying its build finds the spawn refused, which is what keeps + * close from waiting on the replacement build (#3220). */ export async function releaseIosRunnerOnClose( deviceId: string, @@ -694,130 +771,68 @@ export async function releaseIosRunnerOnClose( data: { deviceId }, }); } - await stopRunnerPrepProcesses(deviceId); - await stopIosRunnerSession(deviceId); + // First the fence, then the kill: an in-flight start that answers the kill by retrying its + // build finds admission closed and stops, and the fence outlives the kill and the session stop + // until this close settles — while it stands, nothing prepares this device (#3220). + const settleFence = fenceRunnerStartAdmissionsForTeardown(deviceId); + try { + await stopRunnerPrepProcesses(deviceId); + await stopIosRunnerSessionDevice(deviceId, settleFence); + } finally { + // The error path settles too: a close that throws mid-teardown must not fence the device + // past its own failure. + settleFence(); + } } export async function abortAllIosRunnerSessions(): Promise { + // The same fence a close raises, for every device at once: the prep sweep the abort performs is + // one-shot, so an in-flight start that would answer it with another build is refused before the + // sweep runs, and stays refused until the sweep settles (#3220). const activeSessions = Array.from(runnerSessions.values()); - await abortRunnerSessionsAndPrepProcesses(activeSessions); - for (const session of activeSessions) { - if (runnerSessions.get(session.deviceId) === session) { - runnerSessions.delete(session.deviceId); + const settleFences = retireAllRunnerStartAdmissions( + activeSessions.map((session) => session.deviceId), + ); + try { + await abortRunnerSessionsAndPrepProcesses(activeSessions); + for (const session of activeSessions) { + if (runnerSessions.get(session.deviceId) === session) { + runnerSessions.delete(session.deviceId); + } } + } finally { + // The abort settles even on its error path, so no device stays fenced past this teardown. + settleFences(); } } -type RunnerDetachSkippedReason = - | RunnerHandoffRefusal - | RunnerDetachRefusal - | 'lease_absent' - | 'runner_process_dead' - | 'lease_write_failed'; - -// Graceful daemon shutdown hands a request-proven runner off to the next daemon instead of paying -// the xcodebuild ramp again: the lease token is rewritten to a detached form (so this daemon's own -// teardown paths no longer classify it as owned), this process gives up its sides of the runner's -// log, and the session simply leaves the in-memory map. Once this process exits the lease is stale -// and the adoption path picks it up. Explicit cleanup still works: clean:daemon kills by the lease's -// runnerPid, and the runner's XCTWaiter self-expires after 24h. -// -// Every gate that keeps a session on the kill path is named and reported, because a handoff that -// silently declines is indistinguishable from a rebuild: the handoff lanes -// (`resolveRunnerHandoffTarget`), a session that never served a command, still owes a response, or -// last reported main-thread work still draining (`resolveRunnerDetachDecision`), a missing or -// unwritable lease, and a runner this process cannot prove alive. What stays in the map is torn down by `stopAllIosRunnerSessions`, which the daemon's -// shutdown runs right after this — so a shutdown during a startup tears that runner down rather than -// handing off one that never reached its listener (#2681). +// The detach decision itself lives in runner-adoption.ts beside the adoption it hands off to +// (#2681); this is the map side: a detached session leaves the registry and drops its idle +// timer (the detached module gives up the log observation and marks the session stopped). export async function detachIosRunnerSessionsForShutdown(): Promise { - if (!isIosRunnerDetachEnabled()) return 0; - let detached = 0; - for (const [deviceId, session] of runnerSessions) { - const outcome = detachRunnerSessionForShutdown(deviceId, session); - if (!outcome.detached) { - emitDiagnostic({ - level: 'debug', - phase: 'ios_runner_session_detach_skipped', - data: { - deviceId, - sessionId: session.sessionId, - lane: outcome.lane, - reason: outcome.reason, - // A refused handoff is read from the daemon log, and the two refusals that name a charge look - // identical without this: an exchange still awaited is recovered by its own answer, while an - // abandoned residue waits for terminal evidence for its `commandId` (#2965). - outstandingCharges: session.commandCharges.outstandingChargeCount, - hasAbandonedCharges: session.commandCharges.hasAbandonedCharges, - }, - }); - continue; - } - detached += 1; - emitDiagnostic({ - level: 'info', - phase: 'ios_runner_session_detached', - data: { - deviceId, - lane: outcome.lane, - sessionId: session.sessionId, - runnerPid: session.child.pid, - port: session.port, - runnerLogPath: session.runnerLogPath, - }, - }); - } - return detached; + return await detachRunnerSessionsForShutdown(runnerSessions, (deviceId) => { + runnerSessions.delete(deviceId); + cancelIosRunnerIdleStop(deviceId); + }); } -type RunnerDetachOutcome = - | { detached: true; lane: RunnerHandoffLane } - | { detached: false; lane: RunnerHandoffLane | undefined; reason: RunnerDetachSkippedReason }; - -function detachRunnerSessionForShutdown( - deviceId: string, - session: RunnerSession, -): RunnerDetachOutcome { - const target = resolveRunnerHandoffTarget(session.device); - if (!target.handoff) { - return { detached: false, lane: undefined, reason: target.reason }; - } - const lane = target.lane; - const decision = resolveRunnerDetachDecision(session); - if (!decision.detach) { - return { detached: false, lane, reason: decision.reason }; - } - const lease = session.lease; - if (!lease) { - return { detached: false, lane, reason: 'lease_absent' }; - } - if (!isRunnerProcessAlive(session.child.pid)) { - return { detached: false, lane, reason: 'runner_process_dead' }; - } +export async function stopAllIosRunnerSessions(): Promise { + // This is the daemon's teardown: the fence spans the whole sweep, not just the abort, so a + // start that began while the per-session stops ran meets a closed admission at its next gate + // instead of answering the final prep sweep with another build (#3220). + const settleFences = retireAllRunnerStartAdmissions(runnerSessions.keys()); try { - writeRunnerLease(buildDetachedRunnerLease(lease)); - } catch { - return { detached: false, lane, reason: 'lease_write_failed' }; + await abortAllIosRunnerSessions(); + const pending = Array.from(runnerSessions.keys()); + await Promise.allSettled( + pending.map(async (deviceId) => { + await stopIosRunnerSession(deviceId); + }), + ); + await stopRunnerPrepProcesses(); + } finally { + settleFences(); } - // Only once the lease says the runner is handed over does this process give up its own sides of - // the runner's log: until that write lands the session is still owned, and an owned session that - // stopped following its runner's output is worse off than one that never handed anything off. - // The runner holds its own descriptor, so this cannot disturb it either way (#2681). - session.endOutputObservation?.(); - runnerSessions.delete(deviceId); - cancelIosRunnerIdleStop(deviceId); - advanceRunnerSessionState(session, 'stopped'); - return { detached: true, lane }; -} - -export async function stopAllIosRunnerSessions(): Promise { - await abortAllIosRunnerSessions(); - const pending = Array.from(runnerSessions.keys()); - await Promise.allSettled( - pending.map(async (deviceId) => { - await stopIosRunnerSession(deviceId); - }), - ); - await stopRunnerPrepProcesses(); } function ensureBootedIfNeeded(device: DeviceInfo): Promise { diff --git a/packages/platform-apple/src/runner/runner-start-budget.ts b/packages/platform-apple/src/runner/runner-start-budget.ts index 50d087d917..ea9dc30dc7 100644 --- a/packages/platform-apple/src/runner/runner-start-budget.ts +++ b/packages/platform-apple/src/runner/runner-start-budget.ts @@ -5,8 +5,16 @@ import { } from '@agent-device/kernel/errors'; import { emitDiagnostic } from './host.ts'; import { isCallerDeadlineAbortReason, resolveRunnerStartupSignal } from './runner-contract.ts'; -import { stopRunnerPrepProcessesWithoutActiveOwner } from './runner-disposal.ts'; -import { createRunnerPhaseBudget, type RunnerPhaseBudget } from './runner-xctestrun.ts'; +import { + addRunnerStartWaiter, + cancelRunnerStartWaiter, + createRunnerPhaseBudget, + markRunnerStartRetryPending, + releaseRunnerStartWaiter, + type RunnerPhaseBudget, + type RunnerStartAdmission, +} from './runner-xctestrun.ts'; +import { stopRunnerPrepProcessesWithoutLiveOwner } from './runner-disposal.ts'; import { normalizeRunnerStartupTimeoutMs, type RunnerSession } from './runner-session-types.ts'; import type { AppleRunnerLifecycleOptions } from './runner-provider.ts'; @@ -91,40 +99,85 @@ function runnerStartBudgetExhaustedError(timeoutMs: number, explicit: boolean): * * The two abort reasons get opposite treatment of the detached start, and the difference is the * whole point (#2894 vs #3177). A caller's own deadline (a bounded poll) must leave the start - * running: it is the start the retry joins. A cancelled request means the client is gone, and the - * start it was waiting for belongs to nobody — its spawn carried the *waiting request's* - * cancellation signal only when that request opened the start, so a request that merely joined a - * start another request spawned has no path to it otherwise. On cancel the waiter stops the - * device's prep subprocesses through the same tree-kill path a session stop uses, so a timed-out - * `open` cannot orphan a `build-for-testing` on the shared runner derived-data root where a - * retried `open` would race it. Only builds whose owning start is detached are stopped: a build - * still owned by an in-flight request belongs to its owner and dies through the owner's own - * signal, never under a canceled waiter. The start itself keeps running under the lock (bounded - * by its own budget); its build is left running only while its owner is still there to cancel it. + * running: it is the start the retry joins. A cancelled request means the caller wants nothing + * more from this start, and the waiters are what hold a detached start's work alive: its spawn + * carried the *starting* request's cancellation signal only when that request opened the start, so + * a request that merely joined a start another request spawned has no path to it otherwise. + * + * The verdict belongs to the admission, not to this race: a waiter's cancellation closes the + * start's admission and stops the device's prep children no live caller owns any more — and only + * when it is the LAST interested one, because a build somebody is still waiting on belongs to that + * waiter's work and one request must not SIGTERM another's out from under it. The closed admission + * is what keeps the build it killed from being replaced by the start's next retry (#3220). */ export async function raceRunnerStartAgainstCaller( start: Promise, + admission: RunnerStartAdmission, signal: AbortSignal | undefined, - deviceId: string, ): Promise { if (!signal) return await start; return await new Promise((resolve, reject) => { + const interested = signal.aborted + ? ('already-spent' as const) + : addRunnerStartWaiter(admission, signal); const abort = () => { reject(createRequestCanceledError(undefined, signal.reason)); - if (!isCallerDeadlineAbortReason(signal.reason)) { - void stopRunnerPrepProcessesWithoutActiveOwner(deviceId); - } start.catch(emitDetachedRunnerStartFailed); + if (isCallerDeadlineAbortReason(signal.reason)) { + if (interested === 'interested') releaseRunnerStartWaiter(admission, signal); + // The start keeps running for this caller's retry (#2894), so its build stays owned: + // another caller's cancellation must not reach work a retry is still owed. + markRunnerStartRetryPending(admission); + return; + } + // Even a caller canceled before its interest registered closes the start's admission: the + // issue's pre-spawn window is exactly a cancel that arrives while the ledger is still + // empty, and the closed admission is what refuses the spawn that would otherwise follow. + if (cancelRunnerStartWaiter(admission, signal)) { + void stopRunnerPrepProcessesWithoutLiveOwner(admission.deviceId); + } }; if (signal.aborted) { abort(); } else { signal.addEventListener('abort', abort, { once: true }); } - start.then(resolve, reject).finally(() => signal.removeEventListener('abort', abort)); + start.then(resolve, reject).finally(() => { + signal.removeEventListener('abort', abort); + if (interested === 'interested') releaseRunnerStartWaiter(admission, signal); + }); }); } +/** + * The starting request's own interest in the start it opened, for the surface where the caller + * hands over no `signal` at all: `raceRunnerStartAgainstCaller` registers nothing when there is + * no signal to listen on, and without an owner entry here the start is invisible to the + * last-waiter rule — a joiner canceling could then SIGTERM the live owner's build, which is the + * exact protection the removed #3193 owner sniff used to provide. Keyed on the startup signal + * (a cancelled request, never a deadline), so the owner's disconnect closes admission and stops + * the build the same way a last waiter's does. Returns the release, called once the start + * settles. A caller that did pass a signal already counts through the race; double-keying the + * same caller would keep its cancellation from ever being the last one. + */ +export function reserveRunnerStartOwnerInterest( + admission: RunnerStartAdmission, + options: RunnerSessionOptions, +): () => void { + const signal = options.signal ? undefined : resolveRunnerStartupSignal(options); + if (!signal || addRunnerStartWaiter(admission, signal) !== 'interested') return () => {}; + const onAbort = () => { + if (cancelRunnerStartWaiter(admission, signal)) { + void stopRunnerPrepProcessesWithoutLiveOwner(admission.deviceId); + } + }; + signal.addEventListener('abort', onAbort, { once: true }); + return () => { + signal.removeEventListener('abort', onAbort); + releaseRunnerStartWaiter(admission, signal); + }; +} + /** A cancelled request killed its start on purpose; any other failure of a start nobody awaits is news. */ function emitDetachedRunnerStartFailed(error: unknown): void { if (isRequestCanceledError(error)) return; diff --git a/packages/platform-apple/src/runner/runner-xctestrun.ts b/packages/platform-apple/src/runner/runner-xctestrun.ts index 2be0a78a3a..b3c9de1f48 100644 --- a/packages/platform-apple/src/runner/runner-xctestrun.ts +++ b/packages/platform-apple/src/runner/runner-xctestrun.ts @@ -1,10 +1,25 @@ export { + addRunnerStartWaiter, + assertRunnerStartAdmitsPreparation, + cancelRunnerStartWaiter, ensureXctestrunArtifact, + fenceRunnerStartAdmissionsForTeardown, + finishRunnerStartAdmission, forgetRunnerPrepProcess, hasCachedAppleRunnerArtifact, + markRunnerStartRetryPending, + openRunnerStartAdmission, + openRunnerStartLoopAdmission, prepareXctestrunWithEnv, + readmitRunnerStartAdmission, + releaseRunnerStartWaiter, + retireAllRunnerStartAdmissions, runnerPrepProcessChildren, - runnerPrepProcessChildrenWithoutActiveOwner, + runnerPrepProcessChildrenWithoutLiveOwner, + runnerStartAdmitsPreparation, + runnerStartRetiredError, + runnerStartTeardownPending, + type RunnerStartAdmission, type RunnerXctestrunArtifact, type RunnerXctestrunArtifactState, } from './runner-artifact.ts'; From 38527ee5ca4869f73a4647554da9992c666197b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 6 Oct 2026 12:20:29 +0200 Subject: [PATCH 2/2] test(apple-runner): split bad-cache recovery out of the retry ratchet file (#3220) The fence work grew runner-command-retry.test.ts past its merge-base length, which the test-file size ratchet refuses. The two bad-cache recovery tests mirror the prepare artifact decision, so they move to runner-lifecycle-prepare-artifact.test.ts, which already carries that harness. --- .../__tests__/runner-command-retry.test.ts | 169 ----------------- .../runner-lifecycle-prepare-artifact.test.ts | 175 ++++++++++++++++++ 2 files changed, 175 insertions(+), 169 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 6711176f51..055abcf199 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 @@ -68,78 +68,6 @@ beforeEach(() => { }); }); -test('prepareIosRunner marks a bad restored artifact and rebuilds once after health failure', async () => { - const fixtures = makeBadCacheRecoveryFixtures(); - - mockEnsureRunnerSession - .mockResolvedValueOnce(fixtures.restoredSession) - .mockResolvedValueOnce(fixtures.rebuiltSession); - mockExecuteRunnerCommandWithSession - .mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused')) - .mockResolvedValueOnce({ uptimeMs: 42 }); - - const result = await prepareIosRunner(IOS_SIMULATOR, { - healthTimeoutMs: 90_000, - buildTimeoutMs: 300_000, - }); - - assertRecoveredPrepareResult(result); - assertBadCacheRecoverySideEffects(fixtures); - assertRecoveredPrepareDiagnostics(); -}); - -test('prepareIosRunner invalidates rebuilt sessions when bad-cache recovery health fails', async () => { - const restoredArtifact = makeRunnerArtifact({ - xctestrunPath: '/tmp/restored.xctestrun', - cache: 'exact', - artifact: 'valid', - }); - const rebuiltArtifact = makeRunnerArtifact({ - xctestrunPath: '/tmp/rebuilt.xctestrun', - cache: 'miss', - artifact: 'rebuilt', - }); - const restoredSession = makeRunnerSession({ - port: 8100, - xctestrunPath: restoredArtifact.xctestrunPath, - xctestrunArtifact: restoredArtifact, - }); - const rebuiltSession = makeRunnerSession({ - port: 8101, - xctestrunPath: rebuiltArtifact.xctestrunPath, - xctestrunArtifact: rebuiltArtifact, - }); - - mockEnsureRunnerSession - .mockResolvedValueOnce(restoredSession) - .mockResolvedValueOnce(rebuiltSession); - mockExecuteRunnerCommandWithSession - .mockRejectedValueOnce(runnerConnectFailure('runner_endpoint_probe_exhausted')) - .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'Runner health timed out')); - - await assert.rejects( - () => prepareIosRunner(IOS_SIMULATOR, { healthTimeoutMs: 90_000 }), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.message, 'artifact restored but runner did not connect'); - assert.equal(error.details?.restoredFailureReason, 'Runner endpoint probe failed'); - assert.equal(error.details?.xctestrunPath, '/tmp/rebuilt.xctestrun'); - assert.equal(error.details?.artifact, 'rebuilt'); - assert.equal(error.details?.cache, 'miss'); - return true; - }, - ); - - assert.deepEqual(mockInvalidateRunnerSession.mock.calls, [ - [restoredSession, 'prepare_cached_runner_health_failed'], - [rebuiltSession, 'prepare_rebuilt_runner_health_failed'], - ]); - assert.deepEqual(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls[0], [ - restoredArtifact, - 'Runner endpoint probe failed', - ]); -}); - test('prepareIosRunner retries a fresh launch session when the health check cannot connect', async () => { const stuckSession = makeRunnerSession({ port: 8100 }); const relaunchedSession = makeRunnerSession({ port: 8101 }); @@ -1008,103 +936,6 @@ test('sequence invalidates the session when the status probe fails', async () => }); }); -function makeBadCacheRecoveryFixtures() { - const restoredArtifact = makeRunnerArtifact({ - xctestrunPath: '/tmp/restored.xctestrun', - cache: 'exact', - artifact: 'valid', - }); - const rebuiltArtifact = makeRunnerArtifact({ - xctestrunPath: '/tmp/rebuilt.xctestrun', - cache: 'miss', - artifact: 'rebuilt', - buildMs: 123, - }); - const restoredSession = makeRunnerSession({ - port: 8100, - xctestrunPath: restoredArtifact.xctestrunPath, - xctestrunArtifact: restoredArtifact, - }); - const rebuiltSession = makeRunnerSession({ - port: 8101, - xctestrunPath: rebuiltArtifact.xctestrunPath, - xctestrunArtifact: rebuiltArtifact, - }); - - return { restoredArtifact, restoredSession, rebuiltSession }; -} - -function assertRecoveredPrepareResult(result: Awaited>): void { - assert.deepEqual(result, { - runner: { uptimeMs: 42 }, - cache: 'miss', - artifact: 'rebuilt', - buildMs: 123, - connectMs: result.connectMs, - healthCheckMs: result.healthCheckMs, - xctestrunPath: '/tmp/rebuilt.xctestrun', - recoveryReason: 'Runner did not accept connection', - }); - assert.equal(result.failureReason, undefined); - assert.equal(result.connectMs >= 0, true); - assert.equal(result.healthCheckMs >= 0, true); -} - -function assertBadCacheRecoverySideEffects( - fixtures: ReturnType, -): void { - assert.deepEqual(mockInvalidateRunnerSession.mock.calls[0], [ - fixtures.restoredSession, - 'prepare_cached_runner_health_failed', - ]); - assert.deepEqual(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls[0], [ - fixtures.restoredArtifact, - 'Runner did not accept connection', - ]); - const firstCallOptions = mockEnsureRunnerSession.mock.calls[0]?.[1] as - | { startAdmission?: unknown } - | undefined; - const retryOptions = mockEnsureRunnerSession.mock.calls[1]?.[1] as - | Record - | undefined; - assert.deepEqual(retryOptions, { - healthTimeoutMs: 90_000, - buildTimeoutMs: 300_000, - cleanStaleBundles: true, - forceRunnerXctestrunRebuild: true, - startAdmission: firstCallOptions?.startAdmission, - }); - // Both attempts of the prepare loop share one admission token: the retry - // is the replacement build for the first attempt, not a newcomer. - assert.ok(firstCallOptions?.startAdmission); - assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); - assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[0]?.[2].command, 'uptime'); - assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[0]?.[4], 90_000); - assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], fixtures.rebuiltSession); -} - -function assertRecoveredPrepareDiagnostics(): void { - assert.ok( - mockEmitDiagnostic.mock.calls.some( - ([event]) => event.phase === 'ios_runner_prepare_bad_cache_recovered', - ), - ); - const prepareDiagnostic = mockEmitDiagnostic.mock.calls.find( - ([event]) => event.phase === 'apple_runner_prepare', - )?.[0]; - assert.ok(prepareDiagnostic); - assert.equal(prepareDiagnostic.level, 'info'); - assert.equal(prepareDiagnostic.data?.cache, 'miss'); - assert.equal(prepareDiagnostic.data?.artifact, 'rebuilt'); - assert.equal(prepareDiagnostic.data?.xctestrunPath, '/tmp/rebuilt.xctestrun'); - assert.equal(prepareDiagnostic.data?.recoveryReason, 'Runner did not accept connection'); - assert.equal(prepareDiagnostic.data?.failureReason, undefined); - assert.deepEqual(prepareDiagnostic.data?.timingContainment, { - connectMs: ['buildMs'], - healthCheckMs: [], - }); -} - function assertDiagnosticDecision(expected: { decision: 'skipped' | 'retained'; reason: string; 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 28e2868586..8ccc263af4 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 @@ -8,6 +8,7 @@ import { makeRunnerArtifact, createTestRequestCancellation, makeRunnerSession, + runnerConnectFailure, } from './runner-session-fixtures.ts'; const { @@ -110,3 +111,177 @@ test('a prepare deadline spent during boot keeps the restored artifact and retri vi.useRealTimers(); } }); + +// The bad-cache recovery decision: a restored artifact whose runner refuses the connection is +// indicted, wiped for the run, and replaced by one forced rebuild — under one admission token +// owned by the prepare loop, so the rebuild is the retry a fence governs rather than a fresh +// start that slips past it (#3220). + +test('prepareIosRunner marks a bad restored artifact and rebuilds once after health failure', async () => { + const fixtures = makeBadCacheRecoveryFixtures(); + + mockEnsureRunnerSession + .mockResolvedValueOnce(fixtures.restoredSession) + .mockResolvedValueOnce(fixtures.rebuiltSession); + mockExecuteRunnerCommandWithSession + .mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused')) + .mockResolvedValueOnce({ uptimeMs: 42 }); + + const result = await prepareIosRunner(IOS_SIMULATOR, { + healthTimeoutMs: 90_000, + buildTimeoutMs: 300_000, + }); + + assertRecoveredPrepareResult(result); + assertBadCacheRecoverySideEffects(fixtures); + assertRecoveredPrepareDiagnostics(); +}); + +test('prepareIosRunner invalidates rebuilt sessions when bad-cache recovery health fails', async () => { + const restoredArtifact = makeRunnerArtifact({ + xctestrunPath: '/tmp/restored.xctestrun', + cache: 'exact', + artifact: 'valid', + }); + const rebuiltArtifact = makeRunnerArtifact({ + xctestrunPath: '/tmp/rebuilt.xctestrun', + cache: 'miss', + artifact: 'rebuilt', + }); + const restoredSession = makeRunnerSession({ + port: 8100, + xctestrunPath: restoredArtifact.xctestrunPath, + xctestrunArtifact: restoredArtifact, + }); + const rebuiltSession = makeRunnerSession({ + port: 8101, + xctestrunPath: rebuiltArtifact.xctestrunPath, + xctestrunArtifact: rebuiltArtifact, + }); + + mockEnsureRunnerSession + .mockResolvedValueOnce(restoredSession) + .mockResolvedValueOnce(rebuiltSession); + mockExecuteRunnerCommandWithSession + .mockRejectedValueOnce(runnerConnectFailure('runner_endpoint_probe_exhausted')) + .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'Runner health timed out')); + + await assert.rejects( + () => prepareIosRunner(IOS_SIMULATOR, { healthTimeoutMs: 90_000 }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.message, 'artifact restored but runner did not connect'); + assert.equal(error.details?.restoredFailureReason, 'Runner endpoint probe failed'); + assert.equal(error.details?.xctestrunPath, '/tmp/rebuilt.xctestrun'); + assert.equal(error.details?.artifact, 'rebuilt'); + assert.equal(error.details?.cache, 'miss'); + return true; + }, + ); + + assert.deepEqual(mockInvalidateRunnerSession.mock.calls, [ + [restoredSession, 'prepare_cached_runner_health_failed'], + [rebuiltSession, 'prepare_rebuilt_runner_health_failed'], + ]); + assert.deepEqual(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls[0], [ + restoredArtifact, + 'Runner endpoint probe failed', + ]); +}); + +function makeBadCacheRecoveryFixtures() { + const restoredArtifact = makeRunnerArtifact({ + xctestrunPath: '/tmp/restored.xctestrun', + cache: 'exact', + artifact: 'valid', + }); + const rebuiltArtifact = makeRunnerArtifact({ + xctestrunPath: '/tmp/rebuilt.xctestrun', + cache: 'miss', + artifact: 'rebuilt', + buildMs: 123, + }); + const restoredSession = makeRunnerSession({ + port: 8100, + xctestrunPath: restoredArtifact.xctestrunPath, + xctestrunArtifact: restoredArtifact, + }); + const rebuiltSession = makeRunnerSession({ + port: 8101, + xctestrunPath: rebuiltArtifact.xctestrunPath, + xctestrunArtifact: rebuiltArtifact, + }); + + return { restoredArtifact, restoredSession, rebuiltSession }; +} + +function assertRecoveredPrepareResult(result: Awaited>): void { + assert.deepEqual(result, { + runner: { uptimeMs: 42 }, + cache: 'miss', + artifact: 'rebuilt', + buildMs: 123, + connectMs: result.connectMs, + healthCheckMs: result.healthCheckMs, + xctestrunPath: '/tmp/rebuilt.xctestrun', + recoveryReason: 'Runner did not accept connection', + }); + assert.equal(result.failureReason, undefined); + assert.equal(result.connectMs >= 0, true); + assert.equal(result.healthCheckMs >= 0, true); +} + +function assertBadCacheRecoverySideEffects( + fixtures: ReturnType, +): void { + assert.deepEqual(mockInvalidateRunnerSession.mock.calls[0], [ + fixtures.restoredSession, + 'prepare_cached_runner_health_failed', + ]); + assert.deepEqual(mockMarkRunnerXctestrunArtifactBadForRun.mock.calls[0], [ + fixtures.restoredArtifact, + 'Runner did not accept connection', + ]); + const firstCallOptions = mockEnsureRunnerSession.mock.calls[0]?.[1] as + | { startAdmission?: unknown } + | undefined; + const retryOptions = mockEnsureRunnerSession.mock.calls[1]?.[1] as + | Record + | undefined; + assert.deepEqual(retryOptions, { + healthTimeoutMs: 90_000, + buildTimeoutMs: 300_000, + cleanStaleBundles: true, + forceRunnerXctestrunRebuild: true, + startAdmission: firstCallOptions?.startAdmission, + }); + // Both attempts of the prepare loop share one admission token: the retry + // is the replacement build for the first attempt, not a newcomer. + assert.ok(firstCallOptions?.startAdmission); + assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); + assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[0]?.[2].command, 'uptime'); + assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[0]?.[4], 90_000); + assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[1], fixtures.rebuiltSession); +} + +function assertRecoveredPrepareDiagnostics(): void { + assert.ok( + mockEmitDiagnostic.mock.calls.some( + ([event]) => event.phase === 'ios_runner_prepare_bad_cache_recovered', + ), + ); + const prepareDiagnostic = mockEmitDiagnostic.mock.calls.find( + ([event]) => event.phase === 'apple_runner_prepare', + )?.[0]; + assert.ok(prepareDiagnostic); + assert.equal(prepareDiagnostic.level, 'info'); + assert.equal(prepareDiagnostic.data?.cache, 'miss'); + assert.equal(prepareDiagnostic.data?.artifact, 'rebuilt'); + assert.equal(prepareDiagnostic.data?.xctestrunPath, '/tmp/rebuilt.xctestrun'); + assert.equal(prepareDiagnostic.data?.recoveryReason, 'Runner did not accept connection'); + assert.equal(prepareDiagnostic.data?.failureReason, undefined); + assert.deepEqual(prepareDiagnostic.data?.timingContainment, { + connectMs: ['buildMs'], + healthCheckMs: [], + }); +}