From f2d87e337cf8fb6301e9aaa27cef64e78c981475 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 16:31:23 +0200 Subject: [PATCH 1/8] fix(apple): key the toolchain fingerprint memo on the resolved xcode-select dir `toolchainFingerprintCacheKey` keyed the memo on `DEVELOPER_DIR` alone, so every default `xcode-select` selection shared one entry. A daemon that answered a cache decision before someone ran `sudo xcode-select -s` kept answering with the Xcode the host had left, and because every hit re-set the entry, a busy daemon served it indefinitely rather than for one TTL (#3300). - Resolve the default selection per read and key on it, keeping a client's `DEVELOPER_DIR` choice as before. - Run that resolution under the probe clock and cancellation, capped below the probes it precedes. A read that cannot resolve its selection skips the memo: a key shared by every unresolved selection would let one host selection answer for the next. - Measure the TTL from the read that stored an entry; a hit no longer extends it. - Keep the report path probe-free by looking entries up under the dir the read that stored one resolved. Related: #3449, whose Xcode-too-old reports should name the toolchain a decision actually read. --- .../__tests__/apple-toolchain-fixtures.ts | 72 +++- .../__tests__/runner-cache-metadata.test.ts | 115 +++--- .../__tests__/runner-toolchain-probe.test.ts | 358 ++++++++++++++++++ .../src/runner/runner-toolchain-probe.ts | 107 ++++-- 4 files changed, 565 insertions(+), 87 deletions(-) create mode 100644 packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts diff --git a/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts b/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts index 08ba597f34..0cee4324b8 100644 --- a/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts +++ b/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts @@ -1,6 +1,7 @@ import { beforeEach, vi } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; import { appleRunnerTestHost } from '../test-host.ts'; -import type { ExecResult } from '@agent-device/host-kit/command'; +import type { ExecOptions, ExecResult } from '@agent-device/host-kit/command'; export const STUBBED_APPLE_TOOLCHAIN = { xcodeVersion: '26.2', @@ -9,7 +10,16 @@ export const STUBBED_APPLE_TOOLCHAIN = { sdkBuildVersion: '23C53', } as const; +const STUBBED_SELECTED_DEVELOPER_DIR = '/Applications/Xcode-Stub.app/Contents/Developer'; + +export function selectedDeveloperDirResult(developerDir: string): ExecResult { + return { exitCode: 0, stdout: `${developerDir}\n`, stderr: '' }; +} + export function appleToolchainProbeResult(command: string, args: readonly string[]): ExecResult { + if (command === 'xcode-select' && args[0] === '-p') { + return selectedDeveloperDirResult(STUBBED_SELECTED_DEVELOPER_DIR); + } if (command === 'xcodebuild' && args[0] === '-version') { return { exitCode: 0, @@ -27,15 +37,67 @@ export function appleToolchainProbeResult(command: string, args: readonly string } /** - * Answers the runner cache's toolchain probes from a fixed toolchain, so cases - * that key the cache neither read the host's Xcode nor depend on one existing. - * Returns the mock so a case can make a probe fail. + * Answers the runner cache's toolchain probes from a fixed toolchain, so cases that key the cache + * neither read the host's Xcode nor depend on one existing. No client chooses a `DEVELOPER_DIR`, so + * the selection a key probe resolves is the one the case scripts. Returns the mock so a case can + * make a probe fail. */ export function stubAppleToolchainProbes(): ReturnType { const runCmdSync = vi.fn(appleToolchainProbeResult); beforeEach(() => { runCmdSync.mockImplementation(appleToolchainProbeResult); - appleRunnerTestHost.update({ runCmdSync }); + appleRunnerTestHost.update({ + runCmdSync, + commandDeveloperDir: () => undefined, + }); }); return runCmdSync; } + +/** + * The fingerprint budget's clock, advanced only by an exec that blocks for the whole timeout it was + * handed. Without it a mock that throws immediately proves nothing about a deadline: no time passes, + * so every budget looks untouched however many attempts run. + */ +export function installToolchainProbeClock(): { nowMs: number } { + const clock = { nowMs: 0 }; + appleRunnerTestHost.update({ + deadlineFromTimeoutMs: (timeoutMs: number) => { + const startedAtMs = clock.nowMs; + const expiresAtMs = startedAtMs + Math.max(0, timeoutMs); + return { + remainingMs: () => Math.max(0, expiresAtMs - clock.nowMs), + elapsedMs: () => Math.max(0, clock.nowMs - startedAtMs), + isExpired: () => expiresAtMs - clock.nowMs <= 0, + }; + }, + }); + return clock; +} + +/** An exec that blocked for its whole timeout and was then killed, as the exec layer reports it. */ +export function blockForWholeTimeout( + clock: { nowMs: number }, + command: string, + args: readonly string[], + options: ExecOptions, +): AppError { + const timeoutMs = options.timeoutMs ?? 0; + clock.nowMs += timeoutMs; + return new AppError('COMMAND_FAILED', `${command} timed out after ${timeoutMs}ms`, { + cmd: command, + args, + timeoutMs, + }); +} + +/** + * The execs that read the toolchain itself, as opposed to the `xcode-select -p` a read runs to + * decide which fingerprint entry it belongs to. A case that says a cache hit must not re-read the + * toolchain says nothing about that one exec, so it asserts through this. + */ +export function valueProbeCommands(calls: readonly (readonly unknown[])[]): string[] { + return calls + .filter(([command]) => command !== 'xcode-select') + .map(([command]) => String(command)); +} diff --git a/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts b/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts index 4025b7a879..63e8bc3829 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts @@ -23,8 +23,12 @@ import { memoizedRunnerXcodeVersion } from '../runner-toolchain-probe.ts'; import { COLD_TOOLCHAIN_PROBE_TIMEOUT_MS } from '../apple-runner-platform.ts'; import { appleToolchainProbeResult, + blockForWholeTimeout, + installToolchainProbeClock, + selectedDeveloperDirResult, STUBBED_APPLE_TOOLCHAIN, stubAppleToolchainProbes, + valueProbeCommands, } from './apple-toolchain-fixtures.ts'; import { mkdtempForTestSync } from './tmp-dir.ts'; import { xcodebuildLogWithBuildArguments } from './runner-build-log.fixtures.ts'; @@ -272,7 +276,7 @@ describe('toolchain probe budget', () => { beforeEach(resetAllProcessMemosForTests); test('a cold-start probe recovers on retry, and the stall it survived is charged to the budget', () => { - const clock = installFakeToolchainClock(); + const clock = installToolchainProbeClock(); const xcodebuildTimeouts: number[] = []; runCmdSync.mockImplementation((command: string, args: string[], options: ExecOptions) => { if (command !== 'xcodebuild') return appleToolchainProbeResult(command, args); @@ -292,9 +296,10 @@ describe('toolchain probe budget', () => { }); test('an owning phase with 4 s left gets one 4 s attempt and no retry', () => { - const clock = installFakeToolchainClock(); + const clock = installToolchainProbeClock(); const phaseBudget = createRunnerPhaseBudget(4_000, undefined); runCmdSync.mockImplementation((command: string, args: string[], options: ExecOptions) => { + if (command === 'xcode-select') return appleToolchainProbeResult(command, args); throw blockForWholeTimeout(clock, command, args, options); }); runCmdSync.mockClear(); @@ -303,14 +308,14 @@ describe('toolchain probe budget', () => { () => resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR, undefined, phaseBudget), (error: unknown) => expectRunnerPhaseBudgetExhausted(error), ); - assert.equal(runCmdSync.mock.calls.length, 1); + assert.equal(valueProbeCommands(runCmdSync.mock.calls).length, 1); // The one attempt was capped by the phase, not by the 30 s per-call ceiling. - assert.equal(runCmdSync.mock.calls[0]?.[2]?.timeoutMs, 4_000); + assert.equal(runCmdSync.mock.calls.at(-1)?.[2]?.timeoutMs, 4_000); assert.equal(clock.nowMs, 4_000); }); test('a probe that failed on its own and merely says "timed out" in its message is not retried', () => { - installFakeToolchainClock(); + installToolchainProbeClock(); runCmdSync.mockImplementation((command: string, args: string[]) => { if (command !== 'xcodebuild') return appleToolchainProbeResult(command, args); // No `timeoutMs` detail: this is the tool reporting its own failure, not @@ -439,7 +444,7 @@ describe('toolchain probe budget', () => { ]; test.each(CANCELLATION_CASES)('cancellation matrix: $label', (testCase) => { - const clock = installFakeToolchainClock(); + const clock = installToolchainProbeClock(); const request = new AbortController(); if (testCase.fingerprintCache === 'warm') { runCmdSync.mockImplementation(appleToolchainProbeResult); @@ -449,6 +454,9 @@ describe('toolchain probe budget', () => { let execs = 0; runCmdSync.mockImplementation((command: string, args: string[], options: ExecOptions) => { + // The cache-key resolution picks which entry the read belongs to; it is never one of the + // three probes this matrix interrupts. + if (command === 'xcode-select') return appleToolchainProbeResult(command, args); execs += 1; if (!failsOnExec(testCase, execs)) return appleToolchainProbeResult(command, args); if (testCase.aborts === 'while-it-blocks') request.abort(); @@ -513,44 +521,6 @@ function expectRunnerPhaseBudgetExhausted(error: unknown): boolean { return true; } -/** - * A clock the probes' own budget reads, advanced only by - * {@link blockForWholeTimeout}. Without it a mock that throws immediately - * proves nothing about a deadline: no time passes, so every budget looks - * untouched however many attempts run. - */ -function installFakeToolchainClock(): { nowMs: number } { - const clock = { nowMs: 0 }; - appleRunnerTestHost.update({ - deadlineFromTimeoutMs: (timeoutMs: number) => { - const startedAtMs = clock.nowMs; - const expiresAtMs = startedAtMs + Math.max(0, timeoutMs); - return { - remainingMs: () => Math.max(0, expiresAtMs - clock.nowMs), - elapsedMs: () => Math.max(0, clock.nowMs - startedAtMs), - isExpired: () => expiresAtMs - clock.nowMs <= 0, - }; - }, - }); - return clock; -} - -/** A probe that blocked for its whole timeout and was then killed, as the exec layer reports it. */ -function blockForWholeTimeout( - clock: { nowMs: number }, - command: string, - args: string[], - options: ExecOptions, -): AppError { - const timeoutMs = options.timeoutMs ?? 0; - clock.nowMs += timeoutMs; - return new AppError('COMMAND_FAILED', `${command} timed out after ${timeoutMs}ms`, { - cmd: command, - args, - timeoutMs, - }); -} - test('a failing probe reports its exit status rather than a fabricated SDK version', () => { runCmdSync.mockImplementation((command: string, args: readonly string[]) => command === 'xcrun' @@ -652,16 +622,14 @@ test('a malformed xcodebuild answer is not memoized: the next request re-probes assert.deepEqual(unavailableProbes(), [ { probe: 'xcodebuild -version', reason: 'unparsable_output' }, ]); - const probeCallsWhileMalformed = runCmdSync.mock.calls.length; runCmdSync.mockImplementation(appleToolchainProbeResult); + runCmdSync.mockClear(); const metadata = resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); assert.equal(metadata.xcodeVersion, '26.2'); assert.equal(metadata.xcodeBuildVersion, '17C52'); - expect(runCmdSync.mock.calls.slice(probeCallsWhileMalformed).map(([command]) => command)).toEqual( - ['xcodebuild', 'xcrun', 'xcrun'], - ); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual(['xcodebuild', 'xcrun', 'xcrun']); }); test('only a complete, parsed toolchain fingerprint is memoized', () => { @@ -681,15 +649,11 @@ test('only a complete, parsed toolchain fingerprint is memoized', () => { runCmdSync.mockClear(); const first = resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); // The healthy xcodebuild answer from the failed round was not kept either: all three re-run. - expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual([ - 'xcodebuild', - 'xcrun', - 'xcrun', - ]); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual(['xcodebuild', 'xcrun', 'xcrun']); runCmdSync.mockClear(); const second = resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); - expect(runCmdSync).not.toHaveBeenCalled(); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual([]); assert.deepEqual( [second.xcodeVersion, second.xcodeBuildVersion, second.sdkVersion, second.sdkBuildVersion], [first.xcodeVersion, first.xcodeBuildVersion, first.sdkVersion, first.sdkBuildVersion], @@ -793,10 +757,42 @@ test('a toolchain read under one DEVELOPER_DIR does not answer for another, so i resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR).xcodeBuildVersion, STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion, ); - expect(runCmdSync).not.toHaveBeenCalled(); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual([]); }); -test('a toolchain fingerprint expires once no request uses it; each use renews it', () => { +test('a default selection the host moved does not answer from the fingerprint it left behind', () => { + // Same consequence as the DEVELOPER_DIR case above, reached through `xcode-select` instead: + // `sudo xcode-select -s` changes what a read without a client dir is answering for (#3300). + withoutRunnerDerivedPathEnv(); + let selectedDir = '/Applications/Xcode-A.app/Contents/Developer'; + runCmdSync.mockImplementation((command: string, args: readonly string[]) => + command === 'xcode-select' + ? selectedDeveloperDirResult(selectedDir) + : command === 'xcodebuild' && selectedDir === '/Applications/Xcode-B.app/Contents/Developer' + ? { exitCode: 0, stdout: 'Xcode 27.0\nBuild version 18A100\n', stderr: '' } + : appleToolchainProbeResult(command, args), + ); + + const onA = resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); + selectedDir = '/Applications/Xcode-B.app/Contents/Developer'; + runCmdSync.mockClear(); + const onB = resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); + expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual([ + 'xcode-select', + 'xcodebuild', + 'xcrun', + 'xcrun', + ]); + assert.equal(onA.xcodeBuildVersion, STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + assert.equal(onB.xcodeBuildVersion, '18A100'); + // The derived path is what a retained runner is reused by, so a differing one makes it stale. + assert.notEqual( + resolveRunnerDerivedPath(IOS_SIMULATOR, onA), + resolveRunnerDerivedPath(IOS_SIMULATOR, onB), + ); +}); + +test('a toolchain fingerprint expires at the end of the TTL its read started, hits included', () => { vi.useFakeTimers(); try { appleRunnerTestHost.update({ @@ -804,15 +800,14 @@ test('a toolchain fingerprint expires once no request uses it; each use renews i }); resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); runCmdSync.mockClear(); - // Used every 9 minutes, it outlives a 10-minute window without another probe. - vi.advanceTimersByTime(9 * 60_000); - resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); + // A hit answers the read and leaves the expiry where the read put it. Hits used to push it + // ahead of themselves, so a busy daemon never re-resolved what it was answering for (#3300). vi.advanceTimersByTime(9 * 60_000); resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); expect(runCmdSync).not.toHaveBeenCalled(); expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); - vi.advanceTimersByTime(10 * 60_000); + vi.advanceTimersByTime(60_001); expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBeUndefined(); resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual([ diff --git a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts new file mode 100644 index 0000000000..c2b4136b48 --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts @@ -0,0 +1,358 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import assert from 'node:assert/strict'; +import { isRequestCanceledError } from '@agent-device/kernel/errors'; +import { resetAllProcessMemosForTests } from '@agent-device/kernel/ttl-memo'; +import type { ExecOptions, ExecResult } from '@agent-device/host-kit/command'; +import { IOS_SIMULATOR } from './device-fixtures.ts'; +import { appleRunnerTestHost } from '../test-host.ts'; +import { + appleToolchainProbeResult, + blockForWholeTimeout, + installToolchainProbeClock, + selectedDeveloperDirResult, + STUBBED_APPLE_TOOLCHAIN, + valueProbeCommands, +} from './apple-toolchain-fixtures.ts'; +import { + memoizedRunnerXcodeVersion, + requireRunnerToolchainFingerprint, +} from '../runner-toolchain-probe.ts'; +import { createRunnerPhaseBudget, type RunnerPhaseBudget } from '../runner-phase-budget.ts'; +import { COLD_TOOLCHAIN_PROBE_TIMEOUT_MS } from '../apple-runner-platform.ts'; + +// The fingerprint memo is process-wide and the selection it keys on is host state, so these cases +// script both: `xcode-select -p` answers `selectedDeveloperDir`, and `xcodebuild` names the Xcode +// belonging to that selection. +const XCODE_A = '/Applications/Xcode-A.app/Contents/Developer'; +const XCODE_B = '/Applications/Xcode-B.app/Contents/Developer'; +const IPHONESIMULATOR_SDK = 'iphonesimulator'; +const XCODE_B_BUILD_VERSION = '18A100'; +const FINGERPRINT_TTL_MS = 10 * 60_000; +/** The ceiling one `xcode-select -p` attempt gets, below the 30 s a value probe gets. */ +const XCODE_SELECT_TIMEOUT_MS = 2_000; + +const runCmdSync = vi.fn(); +let selectedDeveloperDir = XCODE_A; +let nowMs = 1_700_000_000_000; + +beforeEach(() => { + resetAllProcessMemosForTests(); + selectedDeveloperDir = XCODE_A; + runCmdSync.mockReset().mockImplementation(appleToolchainProbeResult); + appleRunnerTestHost.update({ + runCmdSync, + commandDeveloperDir: () => undefined, + }); + vi.spyOn(Date, 'now').mockImplementation(() => nowMs); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** Two Xcodes and the `sudo xcode-select -s` that moves the host's default selection between them. */ +function scriptSelectableXcodes(): void { + runCmdSync.mockImplementation((command: string, args: readonly string[]) => + command === 'xcode-select' + ? selectedDeveloperDirResult(selectedDeveloperDir) + : command === 'xcodebuild' && selectedDeveloperDir === XCODE_B + ? { + exitCode: 0, + stdout: `Xcode 27.0\nBuild version ${XCODE_B_BUILD_VERSION}\n`, + stderr: '', + } + : appleToolchainProbeResult(command, args), + ); +} + +function readFingerprint( + budget?: RunnerPhaseBudget, +): ReturnType { + return requireRunnerToolchainFingerprint(IPHONESIMULATOR_SDK, budget); +} + +function attemptedTimeoutMs(command: string): number | undefined { + const call = runCmdSync.mock.calls.find(([executed]) => executed === command); + return (call?.[2] as ExecOptions | undefined)?.timeoutMs; +} + +function blockResolutionForWholeTimeout(clock: { nowMs: number }): void { + runCmdSync.mockImplementation((command: string, args: string[], options: ExecOptions) => { + if (command !== 'xcode-select') return appleToolchainProbeResult(command, args); + throw blockForWholeTimeout(clock, command, args, options); + }); +} + +describe('the developer dir a toolchain fingerprint answers for', () => { + test('a default selection that moved re-reads instead of answering the Xcode it replaced', () => { + scriptSelectableXcodes(); + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + + selectedDeveloperDir = XCODE_B; + runCmdSync.mockClear(); + expect(readFingerprint().xcodeBuildVersion).toBe(XCODE_B_BUILD_VERSION); + + // The switch costs one resolution and one full re-read. The entry keyed on the old dir cannot + // answer for the new one, which is what let a daemon serve the Xcode the host had left (#3300). + expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual([ + 'xcode-select', + 'xcodebuild', + 'xcrun', + 'xcrun', + ]); + }); + + test('a selection that comes back to a dir answers from that dir, not from the one it left', () => { + scriptSelectableXcodes(); + readFingerprint(); + selectedDeveloperDir = XCODE_B; + readFingerprint(); + + selectedDeveloperDir = XCODE_A; + runCmdSync.mockClear(); + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual([]); + }); + + test('each read resolves the selection it keys on, and one resolution is enough', () => { + readFingerprint(); + runCmdSync.mockClear(); + + readFingerprint(); + + expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual(['xcode-select']); + }); + + test('a failure report names the Xcode of the selection this process resolved last', () => { + scriptSelectableXcodes(); + readFingerprint(); + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); + + selectedDeveloperDir = XCODE_B; + readFingerprint(); + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe('27.0'); + + // Naming a selection is not worth waiting on one: only a fingerprint read resolves it. + runCmdSync.mockClear(); + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe('27.0'); + expect(runCmdSync.mock.calls).toEqual([]); + }); + + test('a client that chooses DEVELOPER_DIR keys its read without resolving the default', () => { + scriptSelectableXcodes(); + appleRunnerTestHost.update({ commandDeveloperDir: () => XCODE_A }); + + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + runCmdSync.mockClear(); + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + + expect(runCmdSync.mock.calls).toEqual([]); + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); + }); + + test('a read that fails on its probes leaves a report naming the selection that stored one', () => { + scriptSelectableXcodes(); + readFingerprint(); + + selectedDeveloperDir = XCODE_B; + runCmdSync.mockImplementation((command: string) => + command === 'xcode-select' + ? selectedDeveloperDirResult(XCODE_B) + : { exitCode: 1, stdout: '', stderr: 'xcodebuild: no such tool\n' }, + ); + expect(() => readFingerprint()).toThrow(/Could not read the Xcode toolchain versions/); + + // XCODE_B stored nothing, so the only report still owed an answer is the one made under XCODE_A. + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); + }); + + test('a selection that will not resolve reads a fingerprint and caches nothing for it', () => { + const unresolvable = (): ExecResult => ({ + exitCode: 72, + stdout: '', + stderr: 'xcode-select: error: no developer directory\n', + }); + runCmdSync.mockImplementation((command: string, args: readonly string[]) => + command === 'xcode-select' ? unresolvable() : appleToolchainProbeResult(command, args), + ); + + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + runCmdSync.mockClear(); + // Nothing is cached under an unknown dir: the next read cannot tell which Xcode the entry would + // answer for, so it pays the probes again rather than risking another selection's answer. + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual([ + 'xcode-select', + 'xcodebuild', + 'xcrun', + 'xcrun', + ]); + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBeUndefined(); + }); + + test('a selection that answers nothing, or dies on exec, is no dir rather than a bad one', () => { + for (const unanswered of [ + { exitCode: 0, stdout: ' \n', stderr: '' } satisfies ExecResult, + null, + ]) { + runCmdSync.mockImplementation((command: string, args: readonly string[]) => { + if (command !== 'xcode-select') return appleToolchainProbeResult(command, args); + if (!unanswered) throw new Error('ENOENT: xcode-select not found'); + return unanswered; + }); + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + } + + expect(runCmdSync.mock.calls.filter(([command]) => command === 'xcode-select')).toHaveLength(2); + }); + + test('a default-selection read keys on what the resolution answers, not on a dir it saw before', () => { + scriptSelectableXcodes(); + appleRunnerTestHost.update({ commandDeveloperDir: () => XCODE_A }); + readFingerprint(); + + selectedDeveloperDir = XCODE_B; + appleRunnerTestHost.update({ commandDeveloperDir: () => undefined }); + runCmdSync.mockClear(); + expect(readFingerprint().xcodeBuildVersion).toBe(XCODE_B_BUILD_VERSION); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual(['xcodebuild', 'xcrun', 'xcrun']); + + // The resolution is what puts this read back on the client-dir entry: without believing its + // answer, a selection that came home would re-probe a toolchain this process already knows. + selectedDeveloperDir = XCODE_A; + runCmdSync.mockClear(); + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual([]); + }); +}); + +describe('the budget and cancellation a default-selection resolution answers to', () => { + test('a resolution is capped below the value probe whose entry it decides', () => { + readFingerprint(); + + expect(attemptedTimeoutMs('xcode-select')).toBe(XCODE_SELECT_TIMEOUT_MS); + expect(attemptedTimeoutMs('xcodebuild')).toBe(COLD_TOOLCHAIN_PROBE_TIMEOUT_MS); + }); + + test('an owning phase leaves the resolution half of what it has and no more', () => { + installToolchainProbeClock(); + + readFingerprint(createRunnerPhaseBudget(1_200, undefined)); + + expect(attemptedTimeoutMs('xcode-select')).toBe(600); + }); + + test('a stalled resolution spends its own ceiling and never the fingerprint behind it', () => { + const clock = installToolchainProbeClock(); + blockResolutionForWholeTimeout(clock); + + const fingerprint = readFingerprint(); + + // A stalled `xcode-select` is a host not saying which Xcode it means, so the read continues + // with no dir and pays for the whole fingerprint: the optional probe cannot cost the answer. + expect(fingerprint.xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(clock.nowMs).toBe(XCODE_SELECT_TIMEOUT_MS); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual(['xcodebuild', 'xcrun', 'xcrun']); + }); + + test('a stalled resolution costs a cold-start retry its headroom, and only that', () => { + const clock = installToolchainProbeClock(); + const xcodebuildTimeouts: number[] = []; + runCmdSync.mockImplementation((command: string, args: string[], options: ExecOptions) => { + if (command === 'xcode-select') throw blockForWholeTimeout(clock, command, args, options); + if (command !== 'xcodebuild') return appleToolchainProbeResult(command, args); + xcodebuildTimeouts.push(options.timeoutMs ?? 0); + if (xcodebuildTimeouts.length > 1) return appleToolchainProbeResult(command, args); + throw blockForWholeTimeout(clock, command, args, options); + }); + + const fingerprint = readFingerprint(); + + // The same cold-start stall the budget is sized around, on a read that also lost 2 s deciding + // its key: the retry runs on what is left, so one ceiling still bounds the whole fingerprint. + expect(fingerprint.xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(xcodebuildTimeouts).toEqual([ + COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, + 45_000 - COLD_TOOLCHAIN_PROBE_TIMEOUT_MS - XCODE_SELECT_TIMEOUT_MS, + ]); + expect(clock.nowMs).toBe(COLD_TOOLCHAIN_PROBE_TIMEOUT_MS + XCODE_SELECT_TIMEOUT_MS); + }); + + for (const cache of ['cold', 'warm'] as const) { + test(`a request aborted while the resolution blocks is canceled with a ${cache} cache`, () => { + const request = new AbortController(); + if (cache === 'warm') { + readFingerprint(); + runCmdSync.mockClear(); + } + runCmdSync.mockImplementation((command: string) => { + if (command === 'xcode-select') { + request.abort(); + throw new Error('spawnSync was killed'); + } + throw new Error(`no value probe may run once the resolution was aborted: ${command}`); + }); + + try { + readFingerprint(createRunnerPhaseBudget(undefined, request.signal)); + assert.fail('expected the aborted request to be canceled at the resolution'); + } catch (error) { + assert.ok(isRequestCanceledError(error), `got ${String(error)}`); + } + assert.deepEqual( + runCmdSync.mock.calls.filter(([command]) => command !== 'xcode-select').flat(), + [], + ); + }); + } +}); + +describe('the lifetime of a memoized toolchain fingerprint', () => { + function advance(ms: number): void { + nowMs += ms; + } + + test('a hit answers to the end of the TTL its read started, and no longer', () => { + readFingerprint(); + + // Hits used to push expiry ahead of themselves, so a daemon reading more often than the TTL + // never re-resolved its selection at all (#3300). + advance(FINGERPRINT_TTL_MS / 2); + runCmdSync.mockClear(); + readFingerprint(); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual([]); + + advance(FINGERPRINT_TTL_MS / 2); + runCmdSync.mockClear(); + readFingerprint(); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual(['xcodebuild', 'xcrun', 'xcrun']); + }); + + test('a hit well inside the TTL answers the read without touching the toolchain', () => { + readFingerprint(); + runCmdSync.mockClear(); + + advance(FINGERPRINT_TTL_MS / 4); + readFingerprint(); + + expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual(['xcode-select']); + }); + + test('a fingerprint expired under one selection is read again under the current one', () => { + scriptSelectableXcodes(); + readFingerprint(); + selectedDeveloperDir = XCODE_B; + + advance(FINGERPRINT_TTL_MS); + runCmdSync.mockClear(); + readFingerprint(); + + expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual([ + 'xcode-select', + 'xcodebuild', + 'xcrun', + 'xcrun', + ]); + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe('27.0'); + }); +}); diff --git a/packages/platform-apple/src/runner/runner-toolchain-probe.ts b/packages/platform-apple/src/runner/runner-toolchain-probe.ts index 547939152a..26b196ff79 100644 --- a/packages/platform-apple/src/runner/runner-toolchain-probe.ts +++ b/packages/platform-apple/src/runner/runner-toolchain-probe.ts @@ -24,11 +24,21 @@ import { } from './runner-phase-budget.ts'; /** - * Ceiling on the wall clock the whole toolchain fingerprint may spend, across all three - * probes and their retries, when the owning phase carries no shorter budget: one stalled - * probe, its warm retry, and the two probes still to run (#2422). + * Ceiling on the wall clock the whole toolchain fingerprint may spend, across the cache-key probe, + * the three value probes, and their retries, when the owning phase carries no shorter budget: one + * stalled probe, its warm retry, and the two probes still to run (#2422). The cache-key probe takes + * at most {@link XCODE_SELECT_PROBE_TIMEOUT_MS} of it and half of what the phase has left, so a + * stalled `xcode-select` costs a value probe that headroom and nothing more. */ const TOOLCHAIN_FINGERPRINT_BUDGET_MS = 45_000; + +/** + * Ceiling on one `xcode-select -p` attempt. `xcode-select -p` answers in tens of milliseconds, so + * this is headroom rather than an expected cost, and it stays small enough that a stalled probe + * cannot take the cold-start ceiling away from the value probes whose entry it is deciding. A + * selection this process cannot resolve leaves the read with no dir to key on. + */ +const XCODE_SELECT_PROBE_TIMEOUT_MS = 2_000; const TOOLCHAIN_PROBE_MAX_BUFFER = 128 * 1024; const TOOLCHAIN_PROBE_DETAIL_MAX_LENGTH = 200; const TOOLCHAIN_PROBE_HINT = @@ -55,8 +65,8 @@ type ProbeResult = /** * The remaining-time and cancellation view the probes consult: one per fingerprint read, - * so the three probes and their retries share a single budget. A phase with no deadline - * still gets {@link TOOLCHAIN_FINGERPRINT_BUDGET_MS} as the ceiling. + * so the cache-key probe, the three value probes, and their retries share a single budget. + * A phase with no deadline still gets {@link TOOLCHAIN_FINGERPRINT_BUDGET_MS} as the ceiling. * * `spawnSync` cannot be interrupted once it has started, so cancellation is observed * between attempts; the per-attempt cap is what bounds how long that takes. @@ -64,6 +74,13 @@ type ProbeResult = type ToolchainProbeClock = { /** Milliseconds the next attempt may block for; 0 once the budget is spent. */ attemptTimeoutMs(): number; + /** + * Milliseconds the cache-key probe may block for. Deciding which entry to read is optional + * work that precedes the read which answers for it, so it may spend neither more than + * {@link XCODE_SELECT_PROBE_TIMEOUT_MS} nor more than half of what that read still needs. + * Zero once the budget is spent, which skips the optional probe entirely. + */ + keyProbeTimeoutMs(): number; /** Throws the owning request's cancellation error once it has aborted. */ throwIfCanceled(): void; }; @@ -79,6 +96,8 @@ function createToolchainProbeClock(budget: RunnerPhaseBudget | undefined): Toolc return { attemptTimeoutMs: () => Math.min(COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, Math.floor(deadline.remainingMs())), + keyProbeTimeoutMs: () => + Math.min(XCODE_SELECT_PROBE_TIMEOUT_MS, Math.floor(deadline.remainingMs() / 2)), throwIfCanceled: () => { if (budget?.signal?.aborted) { throw createRequestCanceledError({ phase: 'apple_toolchain_probe' }); @@ -90,11 +109,12 @@ function createToolchainProbeClock(budget: RunnerPhaseBudget | undefined): Toolc // Lazy: createTtlMemo is a host capability, and module evaluation happens // before the composition root binds the host. Only a complete, parsed // fingerprint is ever memoized, so nothing unavailable can outlive the probe -// that could not answer. The key carries a client-chosen DEVELOPER_DIR, so an -// entry expires once no request has used it for TOOLCHAIN_FINGERPRINT_TTL_MS: -// a dir no client uses any more must not stay for the daemon's lifetime. A hit -// renews the entry, so a failure report can name the Xcode a decision read for -// at least TOOLCHAIN_FINGERPRINT_TTL_MS after that read (the default start budget). +// that could not answer. The key carries the developer dir the read answers for, +// and the TTL runs from the read rather than from the last hit, so a host that +// switched its default Xcode selection is answered for at most +// TOOLCHAIN_FINGERPRINT_TTL_MS after that switch (#3300). The same ceiling bounds +// how long a failure report can name a selection no request is using any more, +// which is why a dir no request reads must not stay for the daemon's lifetime. const TOOLCHAIN_FINGERPRINT_TTL_MS = 10 * 60_000; let lazyToolchainFingerprintCache: TtlMemo | undefined; function toolchainFingerprintCache(): TtlMemo { @@ -105,6 +125,13 @@ function toolchainFingerprintCache(): TtlMemo Date: Fri, 16 Oct 2026 17:06:03 +0200 Subject: [PATCH 2/8] fix(apple): point the runner failure report at the selection a hit resolved A default-selection read that hit the memo returned before updating the report pointer, so after `xcode-select` moved A to B and back to A, a failure report named B for an A-backed runner. Route the read and the hit through one memo lookup so both record the dir the read resolved, and say so in that pointer's comment. Also drop "or skip" from the value-probe guard's comment: that guard only ever throws. Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- .../__tests__/runner-toolchain-probe.test.ts | 13 +++++++++++ .../src/runner/runner-toolchain-probe.ts | 22 ++++++++++++++----- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts index c2b4136b48..8c3d05f69d 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts @@ -138,6 +138,19 @@ describe('the developer dir a toolchain fingerprint answers for', () => { expect(runCmdSync.mock.calls).toEqual([]); }); + test('a selection that comes back to a dir answers its report from that dir as well', () => { + scriptSelectableXcodes(); + readFingerprint(); + selectedDeveloperDir = XCODE_B; + readFingerprint(); + + selectedDeveloperDir = XCODE_A; + // The read itself is a hit on A's entry, so it stores nothing; the report pointer still has to + // follow the selection this read resolved, or it would blame B for an A-backed runner (#3300). + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); + }); + test('a client that chooses DEVELOPER_DIR keys its read without resolving the default', () => { scriptSelectableXcodes(); appleRunnerTestHost.update({ commandDeveloperDir: () => XCODE_A }); diff --git a/packages/platform-apple/src/runner/runner-toolchain-probe.ts b/packages/platform-apple/src/runner/runner-toolchain-probe.ts index 26b196ff79..8fdadc4b23 100644 --- a/packages/platform-apple/src/runner/runner-toolchain-probe.ts +++ b/packages/platform-apple/src/runner/runner-toolchain-probe.ts @@ -126,9 +126,9 @@ function toolchainFingerprintCache(): TtlMemo Date: Fri, 16 Oct 2026 17:21:21 +0200 Subject: [PATCH 3/8] fix(apple): say what a runner failure report guarantees about its entry The recorded selection outlives the entry it names: the memo's TTL is measured from the read that stored the entry, so a report can look it up after that entry is gone. Say that, and pin the case with a test that a report then names no Xcode rather than another selection's. Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- .../__tests__/runner-toolchain-probe.test.ts | 14 ++++++++++++++ .../src/runner/runner-toolchain-probe.ts | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts index 8c3d05f69d..54cd701321 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts @@ -351,6 +351,20 @@ describe('the lifetime of a memoized toolchain fingerprint', () => { expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual(['xcode-select']); }); + test('a report names no Xcode once the entry its selection recorded has expired', () => { + scriptSelectableXcodes(); + readFingerprint(); + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); + + // The selection stays recorded past expiry; what goes away is the entry behind it, and a report + // answers with nothing rather than probing or naming another selection's Xcode. + advance(FINGERPRINT_TTL_MS); + runCmdSync.mockClear(); + + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBeUndefined(); + expect(runCmdSync.mock.calls).toEqual([]); + }); + test('a fingerprint expired under one selection is read again under the current one', () => { scriptSelectableXcodes(); readFingerprint(); diff --git a/packages/platform-apple/src/runner/runner-toolchain-probe.ts b/packages/platform-apple/src/runner/runner-toolchain-probe.ts index 8fdadc4b23..e75d3823bd 100644 --- a/packages/platform-apple/src/runner/runner-toolchain-probe.ts +++ b/packages/platform-apple/src/runner/runner-toolchain-probe.ts @@ -128,7 +128,7 @@ function toolchainFingerprintCache(): TtlMemo Date: Fri, 16 Oct 2026 19:14:56 +0200 Subject: [PATCH 4/8] fix(apple): key the runner toolchain memo on the selection's real dir The reviewer's xcodes case: a tool swaps the Xcode bundle behind a stable path, so `xcode-select -p` answers the same string and the key never moves. Resolve the host's selection link to the dir it really names, and follow the same path for a client's DEVELOPER_DIR so one toolchain lands on one entry. The link is read first, so a warm read costs no spawn; `xcode-select -p` is the fallback. Restore TTL renewal on a hit, which #3300 asked to keep: the key now moves with the selection, so a hit is evidence the entry still answers for this host. Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- .../__tests__/apple-toolchain-fixtures.ts | 28 ++++ .../__tests__/runner-cache-metadata.test.ts | 9 +- .../__tests__/runner-toolchain-probe.test.ts | 121 +++++++++++++++++- .../src/runner/runner-toolchain-probe.ts | 78 +++++++++-- 4 files changed, 217 insertions(+), 19 deletions(-) diff --git a/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts b/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts index 0cee4324b8..b730c4f30d 100644 --- a/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts +++ b/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts @@ -1,6 +1,8 @@ import { beforeEach, vi } from 'vitest'; +import fs from 'node:fs'; import { AppError } from '@agent-device/kernel/errors'; import { appleRunnerTestHost } from '../test-host.ts'; +import { XCODE_SELECT_LINK_PATH } from '../runner-toolchain-probe.ts'; import type { ExecOptions, ExecResult } from '@agent-device/host-kit/command'; export const STUBBED_APPLE_TOOLCHAIN = { @@ -43,6 +45,7 @@ export function appleToolchainProbeResult(command: string, args: readonly string * make a probe fail. */ export function stubAppleToolchainProbes(): ReturnType { + stubAppleSelectionLink(); const runCmdSync = vi.fn(appleToolchainProbeResult); beforeEach(() => { runCmdSync.mockImplementation(appleToolchainProbeResult); @@ -54,6 +57,31 @@ export function stubAppleToolchainProbes(): ReturnType { return runCmdSync; } +/** + * The host's default selection link as the Apple toolchain probes see it. Undefined keeps the link + * unreadable, which sends every read down the `xcode-select -p` fallback a case already scripts; a + * case that owns a link points it at a path it created. + */ +export const appleSelectionLink: { target: string | undefined } = { target: undefined }; + +/** + * Hides the selection link of the machine running the suite, so a case decides the host's selection + * rather than inheriting it. The real host has one, and where it points is not this suite's business. + */ +export function stubAppleSelectionLink(): void { + const realRealpathSync = fs.realpathSync; + beforeEach(() => { + appleSelectionLink.target = undefined; + vi.spyOn(fs, 'realpathSync').mockImplementation((target: fs.PathLike) => { + if (String(target) !== XCODE_SELECT_LINK_PATH) return realRealpathSync(target); + if (appleSelectionLink.target === undefined) { + throw Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' }); + } + return realRealpathSync(appleSelectionLink.target); + }); + }); +} + /** * The fingerprint budget's clock, advanced only by an exec that blocks for the whole timeout it was * handed. Without it a mock that throws immediately proves nothing about a deadline: no time passes, diff --git a/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts b/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts index 63e8bc3829..b579b038c9 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts @@ -792,7 +792,7 @@ test('a default selection the host moved does not answer from the fingerprint it ); }); -test('a toolchain fingerprint expires at the end of the TTL its read started, hits included', () => { +test('a toolchain fingerprint is renewed by each hit and expires once nothing uses it', () => { vi.useFakeTimers(); try { appleRunnerTestHost.update({ @@ -800,14 +800,15 @@ test('a toolchain fingerprint expires at the end of the TTL its read started, hi }); resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); runCmdSync.mockClear(); - // A hit answers the read and leaves the expiry where the read put it. Hits used to push it - // ahead of themselves, so a busy daemon never re-resolved what it was answering for (#3300). + // The key names the dir the read resolved, so a hit is evidence the entry still answers for + // this host and renews itself (#3300 asked to keep this). What an idle entry cannot outlast is + // the TTL, measured from the hit that last renewed it. vi.advanceTimersByTime(9 * 60_000); resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); expect(runCmdSync).not.toHaveBeenCalled(); expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); - vi.advanceTimersByTime(60_001); + vi.advanceTimersByTime(10 * 60_001); expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBeUndefined(); resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); expect(runCmdSync.mock.calls.map(([command]) => command)).toEqual([ diff --git a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts index 54cd701321..9144b19917 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts @@ -1,5 +1,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; import { isRequestCanceledError } from '@agent-device/kernel/errors'; import { resetAllProcessMemosForTests } from '@agent-device/kernel/ttl-memo'; import type { ExecOptions, ExecResult } from '@agent-device/host-kit/command'; @@ -7,10 +9,12 @@ import { IOS_SIMULATOR } from './device-fixtures.ts'; import { appleRunnerTestHost } from '../test-host.ts'; import { appleToolchainProbeResult, + appleSelectionLink, blockForWholeTimeout, installToolchainProbeClock, selectedDeveloperDirResult, STUBBED_APPLE_TOOLCHAIN, + stubAppleSelectionLink, valueProbeCommands, } from './apple-toolchain-fixtures.ts'; import { @@ -18,6 +22,7 @@ import { requireRunnerToolchainFingerprint, } from '../runner-toolchain-probe.ts'; import { createRunnerPhaseBudget, type RunnerPhaseBudget } from '../runner-phase-budget.ts'; +import { mkdtempForTestSync } from './tmp-dir.ts'; import { COLD_TOOLCHAIN_PROBE_TIMEOUT_MS } from '../apple-runner-platform.ts'; // The fingerprint memo is process-wide and the selection it keys on is host state, so these cases @@ -35,9 +40,15 @@ const runCmdSync = vi.fn(); let selectedDeveloperDir = XCODE_A; let nowMs = 1_700_000_000_000; +stubAppleSelectionLink(); + +/** A scratch root the case's fake developer dirs and selection links live under. */ +let tempRoot: string; + beforeEach(() => { resetAllProcessMemosForTests(); selectedDeveloperDir = XCODE_A; + tempRoot = mkdtempForTestSync('runner-selection-'); runCmdSync.mockReset().mockImplementation(appleToolchainProbeResult); appleRunnerTestHost.update({ runCmdSync, @@ -50,6 +61,16 @@ afterEach(() => { vi.restoreAllMocks(); }); +/** + * A developer dir the host can resolve, standing in for an installed Xcode at `dir`. `realpath` only + * answers for a dir that exists, so a key built from a missing one would be no key at all. + */ +function realSelectionDir(dir: string): string { + const created = path.join(tempRoot, dir); + fs.mkdirSync(created, { recursive: true }); + return created; +} + /** Two Xcodes and the `sudo xcode-select -s` that moves the host's default selection between them. */ function scriptSelectableXcodes(): void { runCmdSync.mockImplementation((command: string, args: readonly string[]) => @@ -203,6 +224,87 @@ describe('the developer dir a toolchain fingerprint answers for', () => { expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBeUndefined(); }); + test('a warm read resolves its key from the selection link without spawning anything', () => { + appleSelectionLink.target = realSelectionDir(XCODE_A); + readFingerprint(); + runCmdSync.mockClear(); + + readFingerprint(); + + // The link is what a warm read reads, so the memo costs no process at all. `xcode-select -p` is + // the fallback, not the per-read cost, which is what made the key change expensive before. + expect(runCmdSync.mock.calls).toEqual([]); + }); + + test('a selection link naming a toolchain answers from that toolchain, not from xcode-select', () => { + appleSelectionLink.target = realSelectionDir(XCODE_A); + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + + // The two sources disagree, which is what a host whose selection moved without its link keeping + // up looks like. The link is what `xcode-select -s` writes, so it is the one that names it. + scriptSelectableXcodes(); + selectedDeveloperDir = XCODE_B; + runCmdSync.mockClear(); + + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(runCmdSync.mock.calls).toEqual([]); + }); + + test('a selection link moved to another developer dir moves the key with it', () => { + appleSelectionLink.target = realSelectionDir(XCODE_A); + readFingerprint(); + + appleSelectionLink.target = realSelectionDir(XCODE_B); + runCmdSync.mockClear(); + + // The entry under the dir the host left cannot answer for the dir it moved to. + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual(['xcodebuild', 'xcrun', 'xcrun']); + expect(runCmdSync.mock.calls.some(([command]) => command === 'xcode-select')).toBe(false); + }); + + test('a bundle swapped behind a stable selection path moves the key', () => { + // What a tool like xcodes does: it replaces the Xcode.app bundle, so the selection keeps naming + // the same path while the toolchain behind it changes. Keying on the link's raw target would + // keep answering for the Xcode that path used to reach, so the key follows the real dir. + const stable = path.join(tempRoot, 'Xcode.app'); + const first = path.join(tempRoot, 'Xcode-1.app'); + const second = path.join(tempRoot, 'Xcode-2.app'); + for (const bundle of [first, second]) { + fs.mkdirSync(path.join(bundle, 'Contents', 'Developer'), { recursive: true }); + } + fs.symlinkSync(path.basename(first), stable); + appleSelectionLink.target = stable; + + readFingerprint(); + fs.unlinkSync(stable); + fs.symlinkSync(path.basename(second), stable); + + runCmdSync.mockClear(); + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual(['xcodebuild', 'xcrun', 'xcrun']); + }); + + test('a selection reported with its trailing newline keys on the dir alone', () => { + // `xcode-select -p` answers with a newline. A key that kept it would never match the dir a + // client's DEVELOPER_DIR names, so one Xcode would look like two toolchains. + const reported = realSelectionDir(XCODE_A); + runCmdSync.mockImplementation((command: string, args: readonly string[]) => + command === 'xcode-select' + ? { exitCode: 0, stdout: `${reported}\n`, stderr: '' } + : appleToolchainProbeResult(command, args), + ); + + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + runCmdSync.mockClear(); + + appleRunnerTestHost.update({ commandDeveloperDir: () => reported }); + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + + // The client-named dir hit the entry the newline-built key stored, so nothing re-read. + expect(runCmdSync.mock.calls).toEqual([]); + }); + test('a selection that answers nothing, or dies on exec, is no dir rather than a bad one', () => { for (const unanswered of [ { exitCode: 0, stdout: ' \n', stderr: '' } satisfies ExecResult, @@ -325,11 +427,12 @@ describe('the lifetime of a memoized toolchain fingerprint', () => { nowMs += ms; } - test('a hit answers to the end of the TTL its read started, and no longer', () => { + test('a hit renews the entry it answered from, because its key is still the selection', () => { readFingerprint(); - // Hits used to push expiry ahead of themselves, so a daemon reading more often than the TTL - // never re-resolved its selection at all (#3300). + // The key names the dir this host selects, so a hit is itself evidence the entry is still the + // answer for that host. #3300 asked to keep this renewal; the stale answer it could once serve + // now costs a key change instead of a TTL. advance(FINGERPRINT_TTL_MS / 2); runCmdSync.mockClear(); readFingerprint(); @@ -338,6 +441,18 @@ describe('the lifetime of a memoized toolchain fingerprint', () => { advance(FINGERPRINT_TTL_MS / 2); runCmdSync.mockClear(); readFingerprint(); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual([]); + }); + + test('an entry no read takes expires at the TTL its storing read started', () => { + readFingerprint(); + + advance(FINGERPRINT_TTL_MS); + runCmdSync.mockClear(); + readFingerprint(); + + // Renewal is for entries in use. One that nothing has asked for goes away, which is also the + // ceiling on how long a failure report can name a selection no request is using. expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual(['xcodebuild', 'xcrun', 'xcrun']); }); diff --git a/packages/platform-apple/src/runner/runner-toolchain-probe.ts b/packages/platform-apple/src/runner/runner-toolchain-probe.ts index e75d3823bd..e26b05c08c 100644 --- a/packages/platform-apple/src/runner/runner-toolchain-probe.ts +++ b/packages/platform-apple/src/runner/runner-toolchain-probe.ts @@ -12,6 +12,7 @@ import { isCommandTimeoutError, runCmdSync, } from './host.ts'; +import fs from 'node:fs'; import { COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, resolveRunnerPlatformName, @@ -33,12 +34,20 @@ import { const TOOLCHAIN_FINGERPRINT_BUDGET_MS = 45_000; /** - * Ceiling on one `xcode-select -p` attempt. `xcode-select -p` answers in tens of milliseconds, so - * this is headroom rather than an expected cost, and it stays small enough that a stalled probe - * cannot take the cold-start ceiling away from the value probes whose entry it is deciding. A - * selection this process cannot resolve leaves the read with no dir to key on. + * Ceiling on one `xcode-select -p` attempt, which the cache key needs only when the host's selection + * link cannot be read. `xcode-select -p` answers in tens of milliseconds, so this is headroom rather + * than an expected cost, and it stays small enough that a stalled probe cannot take the cold-start + * ceiling away from the value probes whose entry it is deciding. A selection this process cannot + * resolve leaves the read with no dir to key on. */ const XCODE_SELECT_PROBE_TIMEOUT_MS = 2_000; + +/** + * The symlink `xcode-select -s` writes and `xcode-select -p` reads back. Reading it directly answers + * which developer dir the host selects, without a subprocess, and the cache needs that answer on + * every hit and not only on a miss. Named so a suite can script the host's selection. + */ +export const XCODE_SELECT_LINK_PATH = '/var/db/xcode_select_link'; const TOOLCHAIN_PROBE_MAX_BUFFER = 128 * 1024; const TOOLCHAIN_PROBE_DETAIL_MAX_LENGTH = 200; const TOOLCHAIN_PROBE_HINT = @@ -110,11 +119,11 @@ function createToolchainProbeClock(budget: RunnerPhaseBudget | undefined): Toolc // before the composition root binds the host. Only a complete, parsed // fingerprint is ever memoized, so nothing unavailable can outlive the probe // that could not answer. The key carries the developer dir the read answers for, -// and the TTL runs from the read rather than from the last hit, so a host that -// switched its default Xcode selection is answered for at most -// TOOLCHAIN_FINGERPRINT_TTL_MS after that switch (#3300). The same ceiling bounds -// how long a failure report can name a selection no request is using any more, -// which is why a dir no request reads must not stay for the daemon's lifetime. +// so an entry is stale only until its own key stops being the host's selection, +// which is what let a daemon serve the Xcode a host had left (#3300). A hit +// therefore renews the entry, as it did before: an idle entry still goes away +// after TOOLCHAIN_FINGERPRINT_TTL_MS, which also bounds how long a failure report +// can name a selection no request is using any more. const TOOLCHAIN_FINGERPRINT_TTL_MS = 10 * 60_000; let lazyToolchainFingerprintCache: TtlMemo | undefined; function toolchainFingerprintCache(): TtlMemo { @@ -144,7 +153,12 @@ export function requireRunnerToolchainFingerprint( const clock = createToolchainProbeClock(budget); clock.throwIfCanceled(); const chosenDir = commandDeveloperDir(); - const developerDir = chosenDir ?? probeSelectedDeveloperDir(clock); + // Both sources are resolved to the dir they really name, so a client naming a symlinked Xcode and + // a daemon reading the selection link land on one entry for one toolchain. + const developerDir = + chosenDir !== undefined + ? (resolveRealDeveloperDir(chosenDir) ?? chosenDir) + : resolveDefaultDeveloperDir(clock); // A read that cannot tell which Xcode it answers for has no entry to read or write: a shared // unknown key would let one host selection answer for the next, which is the fault here. if (developerDir === undefined) return requireFreshRunnerToolchainFingerprint(sdkName, clock); @@ -162,7 +176,12 @@ function requireMemoizedRunnerToolchainFingerprint( clock: ToolchainProbeClock, ): RunnerToolchainFingerprint { const cached = toolchainFingerprintCache().get(cacheKey); - if (cached) return cached; + if (cached) { + // A hit can only be an entry keyed on the dir this read resolved, so it is still the host's + // selection and renews itself. Only an entry no request reads expires (#3300). + toolchainFingerprintCache().set(cacheKey, cached); + return cached; + } const fingerprint = requireFreshRunnerToolchainFingerprint(sdkName, clock); toolchainFingerprintCache().set(cacheKey, fingerprint); return fingerprint; @@ -181,6 +200,37 @@ function requireFreshRunnerToolchainFingerprint( return fingerprint.value; } +/** + * The dir the host's default selection points at, symlinks followed. The selection link is read + * first because the memo needs its key on every hit and not only on a miss, and spawning + * `xcode-select` costs a few milliseconds on a warm read. A link this process cannot read falls back + * to `xcode-select -p`, and a selection neither can name leaves the read with no key at all. + */ +function resolveDefaultDeveloperDir(clock: ToolchainProbeClock): string | undefined { + const selectedLink = resolveRealDeveloperDir(XCODE_SELECT_LINK_PATH); + if (selectedLink) return selectedLink; + const reported = probeSelectedDeveloperDir(clock); + if (reported === undefined) return undefined; + // A selection the host names but this process cannot follow is still that host's selection, so it + // keys on the name. Keying on nothing instead would let one host answer for the next. + return resolveRealDeveloperDir(reported) ?? reported; +} + +/** + * The dir `candidate` really names, or undefined when nothing is there to name. `realpath` rather + * than the link's raw target because a tool like xcodes swaps the Xcode.app bundle behind a stable + * path: the raw target survives that swap, so a key built from it would keep answering for the Xcode + * the host replaced. + */ +function resolveRealDeveloperDir(candidate: string | undefined): string | undefined { + if (candidate === undefined) return undefined; + try { + return fs.realpathSync(candidate.trim()) || undefined; + } catch { + return undefined; + } +} + /** * The dir `xcode-select` currently selects, or undefined when it will not say. Best effort: a read * that cannot resolve its own entry spends what is left of its budget on the probes that carry the @@ -213,7 +263,11 @@ function probeSelectedDeveloperDir(clock: ToolchainProbeClock): string | undefin * describes, and the dir its entry is keyed on is the one that read resolved. */ export function memoizedRunnerXcodeVersion(device: DeviceInfo): string | undefined { - const developerDir = commandDeveloperDir() ?? recordedDefaultSelection; + const chosenDir = commandDeveloperDir(); + const developerDir = + chosenDir !== undefined + ? (resolveRealDeveloperDir(chosenDir) ?? chosenDir) + : recordedDefaultSelection; if (developerDir === undefined) return undefined; return toolchainFingerprintCache().get( toolchainFingerprintCacheKey( From 03834c02315f0e3426f675912f6ca81a3886cdb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 19:53:00 +0200 Subject: [PATCH 5/8] fix(apple): key the runner toolchain memo on the bundle standing in the dir A hit renews its entry, so a key that sees only the dir pins a stale fingerprint for as long as a daemon stays busy when an update replaces the bundle in place: an installer or App Store update at /Applications/Xcode.app keeps the path. Add the inode and mtime of the version file xcodebuild reads to the key, so the replaced bundle answers under a new key and the old entry expires once nothing renews it. The failure report now reads only the memo. It reused the key halves a real read built instead of resolving and stating the selection itself, which is what lets a hit renew at all: a report has no budget to spend and no deadline to answer to. Cancellation is observed after the key's two filesystem reads, so a warm hit cannot hide an abort. Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- .../__tests__/runner-cache-metadata.test.ts | 8 +- .../__tests__/runner-toolchain-probe.test.ts | 85 ++++++++++++++- .../src/runner/runner-toolchain-probe.ts | 103 +++++++++++++----- 3 files changed, 165 insertions(+), 31 deletions(-) diff --git a/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts b/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts index b579b038c9..fc9924ac20 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-cache-metadata.test.ts @@ -800,9 +800,11 @@ test('a toolchain fingerprint is renewed by each hit and expires once nothing us }); resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); runCmdSync.mockClear(); - // The key names the dir the read resolved, so a hit is evidence the entry still answers for - // this host and renews itself (#3300 asked to keep this). What an idle entry cannot outlast is - // the TTL, measured from the hit that last renewed it. + // A hit is an entry keyed on the resolved dir AND the identity of the bundle standing in it, so + // the toolchain behind that key is still the one in place and the entry renews itself (#3300 + // asked to keep this). A bundle replaced in place moves the identity, not just the dir; see + // `a bundle replaced in place at the same path moves the key`. What an idle entry cannot + // outlast is the TTL, measured from the hit that last renewed it. vi.advanceTimersByTime(9 * 60_000); resolveExpectedRunnerCacheMetadata(IOS_SIMULATOR); expect(runCmdSync).not.toHaveBeenCalled(); diff --git a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts index 9144b19917..c3d3ee09c7 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts @@ -62,12 +62,14 @@ afterEach(() => { }); /** - * A developer dir the host can resolve, standing in for an installed Xcode at `dir`. `realpath` only - * answers for a dir that exists, so a key built from a missing one would be no key at all. + * A developer dir the host can resolve, standing in for an installed Xcode at `dir`. Cases below + * compare keys through the memo, and a dir that does not exist cannot be resolved to its real path, + * so its key would fall back to the name rather than the resolved dir the case means. */ function realSelectionDir(dir: string): string { const created = path.join(tempRoot, dir); fs.mkdirSync(created, { recursive: true }); + fs.writeFileSync(path.join(created, '..', 'version.plist'), 'build 1\n'); return created; } @@ -305,6 +307,61 @@ describe('the developer dir a toolchain fingerprint answers for', () => { expect(runCmdSync.mock.calls).toEqual([]); }); + test('a bundle replaced in place at the same path moves the key', () => { + // An App Store or installer update replaces the bundle without moving it, and the selection keeps + // naming the same dir. Renewal on a hit means a key blind to that would keep serving the + // fingerprint the update replaced for as long as the daemon stays busy, which is #3300 again. + const selected = realSelectionDir(XCODE_A); + appleSelectionLink.target = selected; + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + + // The installer writes a new bundle: same path, new inode for the version file. + const versionFile = path.join(selected, '..', 'version.plist'); + fs.unlinkSync(versionFile); + fs.writeFileSync(versionFile, 'build 2\n'); + + runCmdSync.mockClear(); + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + expect(valueProbeCommands(runCmdSync.mock.calls)).toEqual(['xcodebuild', 'xcrun', 'xcrun']); + }); + + test('a report reads only the memo, never the filesystem', () => { + // A report runs after a failure, with no budget of its own to spend and no deadline to answer + // to. Everything it needs is the key a real read already built, so a slow or vanished bundle + // cannot make reporting wait, on either of its two paths. + const selected = realSelectionDir(XCODE_A); + appleSelectionLink.target = selected; + readFingerprint(); + appleRunnerTestHost.update({ commandDeveloperDir: () => selected }); + readFingerprint(); + + // The selection-link stub already spies realpathSync, so its history from the reads above is + // cleared before the report is asked to answer. + const statSync = vi.spyOn(fs, 'statSync').mockClear(); + const realpathSync = vi.spyOn(fs, 'realpathSync').mockClear(); + + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); + appleRunnerTestHost.update({ commandDeveloperDir: () => undefined }); + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); + + expect(statSync.mock.calls).toEqual([]); + expect(realpathSync.mock.calls).toEqual([]); + }); + + test('a report for a client dir this process never read answers with nothing', () => { + appleSelectionLink.target = realSelectionDir(XCODE_A); + readFingerprint(); + + const otherDir = realSelectionDir(XCODE_B); + appleRunnerTestHost.update({ commandDeveloperDir: () => otherDir }); + runCmdSync.mockClear(); + + // That dir has an entry only after a read resolves it, and looking one up would mean going and + // stat-ing a bundle a report must not wait on. + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBeUndefined(); + expect(runCmdSync.mock.calls).toEqual([]); + }); + test('a selection that answers nothing, or dies on exec, is no dir rather than a bad one', () => { for (const unanswered of [ { exitCode: 0, stdout: ' \n', stderr: '' } satisfies ExecResult, @@ -393,6 +450,30 @@ describe('the budget and cancellation a default-selection resolution answers to' expect(clock.nowMs).toBe(COLD_TOOLCHAIN_PROBE_TIMEOUT_MS + XCODE_SELECT_TIMEOUT_MS); }); + test('a request aborted during the key reads is canceled even on a warm cache', () => { + const request = new AbortController(); + appleSelectionLink.target = realSelectionDir(XCODE_A); + readFingerprint(createRunnerPhaseBudget(undefined, request.signal)); + + // The key's filesystem reads cannot be interrupted, so cancellation is observed after them. On a + // warm cache the answer is already in hand, which is exactly when a hit could otherwise hide the + // abort from the owner that asked for this request to stop. + const statTheBundle = fs.statSync; + vi.spyOn(fs, 'statSync').mockImplementation(((...args: Parameters) => { + request.abort(); + return statTheBundle(...args); + }) as typeof fs.statSync); + runCmdSync.mockClear(); + + try { + readFingerprint(createRunnerPhaseBudget(undefined, request.signal)); + assert.fail('expected the aborted request to be canceled at the key reads'); + } catch (error) { + assert.ok(isRequestCanceledError(error), `got ${String(error)}`); + } + assert.deepEqual(valueProbeCommands(runCmdSync.mock.calls), []); + }); + for (const cache of ['cold', 'warm'] as const) { test(`a request aborted while the resolution blocks is canceled with a ${cache} cache`, () => { const request = new AbortController(); diff --git a/packages/platform-apple/src/runner/runner-toolchain-probe.ts b/packages/platform-apple/src/runner/runner-toolchain-probe.ts index e26b05c08c..49cc595448 100644 --- a/packages/platform-apple/src/runner/runner-toolchain-probe.ts +++ b/packages/platform-apple/src/runner/runner-toolchain-probe.ts @@ -13,6 +13,7 @@ import { runCmdSync, } from './host.ts'; import fs from 'node:fs'; +import path from 'node:path'; import { COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, resolveRunnerPlatformName, @@ -118,11 +119,12 @@ function createToolchainProbeClock(budget: RunnerPhaseBudget | undefined): Toolc // Lazy: createTtlMemo is a host capability, and module evaluation happens // before the composition root binds the host. Only a complete, parsed // fingerprint is ever memoized, so nothing unavailable can outlive the probe -// that could not answer. The key carries the developer dir the read answers for, -// so an entry is stale only until its own key stops being the host's selection, -// which is what let a daemon serve the Xcode a host had left (#3300). A hit -// therefore renews the entry, as it did before: an idle entry still goes away -// after TOOLCHAIN_FINGERPRINT_TTL_MS, which also bounds how long a failure report +// that could not answer. The key carries the developer dir the read answers for +// and the identity of the bundle standing in it, so an entry is stale only until +// its own key stops describing the host's selection, which is what let a daemon +// serve the Xcode a host had left (#3300). A hit therefore renews the entry, as +// it did before: an idle entry still goes away after +// TOOLCHAIN_FINGERPRINT_TTL_MS, which also bounds how long a failure report // can name a selection no request is using any more. const TOOLCHAIN_FINGERPRINT_TTL_MS = 10 * 60_000; let lazyToolchainFingerprintCache: TtlMemo | undefined; @@ -135,11 +137,27 @@ function toolchainFingerprintCache(): TtlMemo Date: Fri, 16 Oct 2026 20:04:16 +0200 Subject: [PATCH 6/8] refactor(apple): own the selection link path beside the probe ceiling The reviewer's nit: it was exported only so the suite could match it. It belongs with the other Apple toolchain probe constant both probers read, where it can be named without that. Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- .../src/runner/__tests__/apple-toolchain-fixtures.ts | 2 +- .../platform-apple/src/runner/apple-runner-platform.ts | 8 ++++++++ .../platform-apple/src/runner/runner-toolchain-probe.ts | 7 +------ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts b/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts index b730c4f30d..d5e9ff6953 100644 --- a/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts +++ b/packages/platform-apple/src/runner/__tests__/apple-toolchain-fixtures.ts @@ -2,7 +2,7 @@ import { beforeEach, vi } from 'vitest'; import fs from 'node:fs'; import { AppError } from '@agent-device/kernel/errors'; import { appleRunnerTestHost } from '../test-host.ts'; -import { XCODE_SELECT_LINK_PATH } from '../runner-toolchain-probe.ts'; +import { XCODE_SELECT_LINK_PATH } from '../apple-runner-platform.ts'; import type { ExecOptions, ExecResult } from '@agent-device/host-kit/command'; export const STUBBED_APPLE_TOOLCHAIN = { diff --git a/packages/platform-apple/src/runner/apple-runner-platform.ts b/packages/platform-apple/src/runner/apple-runner-platform.ts index c06f9ec30b..16c14ea86b 100644 --- a/packages/platform-apple/src/runner/apple-runner-platform.ts +++ b/packages/platform-apple/src/runner/apple-runner-platform.ts @@ -21,6 +21,14 @@ import { readHostCpuArchSync } from './host.ts'; */ export const COLD_TOOLCHAIN_PROBE_TIMEOUT_MS = 30_000; +/** + * The symlink `xcode-select -s` writes and `xcode-select -p` reads back. Resolving it answers which + * developer dir the host selects, without a subprocess, so the toolchain memo can key on that answer + * on a hit as well as on a miss. Beside the probe ceiling above so both Apple toolchain probers read + * one value without either owning it. + */ +export const XCODE_SELECT_LINK_PATH = '/var/db/xcode_select_link'; + export type RunnerApplePlatformName = 'iOS' | 'tvOS' | 'macOS' | 'visionOS'; type RunnerPlatformDeviceKind = 'simulator' | 'device'; diff --git a/packages/platform-apple/src/runner/runner-toolchain-probe.ts b/packages/platform-apple/src/runner/runner-toolchain-probe.ts index 49cc595448..7976475765 100644 --- a/packages/platform-apple/src/runner/runner-toolchain-probe.ts +++ b/packages/platform-apple/src/runner/runner-toolchain-probe.ts @@ -16,6 +16,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, + XCODE_SELECT_LINK_PATH, resolveRunnerPlatformName, resolveRunnerSdkName, } from './apple-runner-platform.ts'; @@ -43,12 +44,6 @@ const TOOLCHAIN_FINGERPRINT_BUDGET_MS = 45_000; */ const XCODE_SELECT_PROBE_TIMEOUT_MS = 2_000; -/** - * The symlink `xcode-select -s` writes and `xcode-select -p` reads back. Reading it directly answers - * which developer dir the host selects, without a subprocess, and the cache needs that answer on - * every hit and not only on a miss. Named so a suite can script the host's selection. - */ -export const XCODE_SELECT_LINK_PATH = '/var/db/xcode_select_link'; const TOOLCHAIN_PROBE_MAX_BUFFER = 128 * 1024; const TOOLCHAIN_PROBE_DETAIL_MAX_LENGTH = 200; const TOOLCHAIN_PROBE_HINT = From d33b8049e34efc48f836c58a3e40cb71e6cd772e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 20:13:12 +0200 Subject: [PATCH 7/8] fix(apple): keep each client's toolchain answer for its own failure report One global record meant the last client-named DEVELOPER_DIR read erased what an earlier client needed to report on, even while that client's fingerprint was still cached. A daemon serves request-scoped selections concurrently, so retain the answer per name instead. It is a TTL memo, so a name whose reads stopped drops out on the same clock as the entry it names. Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- .../__tests__/runner-toolchain-probe.test.ts | 24 +++++++++++++ .../src/runner/runner-toolchain-probe.ts | 35 +++++++++++++------ 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts index c3d3ee09c7..a88a920741 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts @@ -348,6 +348,30 @@ describe('the developer dir a toolchain fingerprint answers for', () => { expect(realpathSync.mock.calls).toEqual([]); }); + test('each client-named selection keeps its own report across another client read', () => { + // A daemon serves clients with different request-scoped DEVELOPER_DIRs at once. The second + // client's read must not erase what the first one needs to report on, even though both of their + // fingerprints are still cached. + const dirA = realSelectionDir(XCODE_A); + const dirB = realSelectionDir(XCODE_B); + let chosen: string | undefined = dirA; + appleRunnerTestHost.update({ commandDeveloperDir: () => chosen }); + runCmdSync.mockImplementation((command: string, args: readonly string[]) => + command === 'xcodebuild' && chosen === dirA + ? { exitCode: 0, stdout: 'Xcode 15.1\nBuild version 1A111\n', stderr: '' } + : appleToolchainProbeResult(command, args), + ); + + expect(readFingerprint().xcodeBuildVersion).toBe('1A111'); + chosen = dirB; + expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); + + chosen = dirA; + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe('15.1'); + chosen = dirB; + expect(memoizedRunnerXcodeVersion(IOS_SIMULATOR)).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeVersion); + }); + test('a report for a client dir this process never read answers with nothing', () => { appleSelectionLink.target = realSelectionDir(XCODE_A); readFingerprint(); diff --git a/packages/platform-apple/src/runner/runner-toolchain-probe.ts b/packages/platform-apple/src/runner/runner-toolchain-probe.ts index 7976475765..1f10433211 100644 --- a/packages/platform-apple/src/runner/runner-toolchain-probe.ts +++ b/packages/platform-apple/src/runner/runner-toolchain-probe.ts @@ -140,6 +140,17 @@ type RecordedSelection = { bundleIdentity: string | undefined; }; +// One clock with the fingerprints it points at, and expiry scheduled for the same reason: nothing +// renews a client dir whose reads stopped, so its record cannot outlive the entry it names. +let lazyRecordedClients: TtlMemo | undefined; +function lazyRecordedClientSelections(): TtlMemo | undefined { + lazyRecordedClients ??= createTtlMemo({ + ttlMs: TOOLCHAIN_FINGERPRINT_TTL_MS, + scheduleExpiry: true, + }); + return lazyRecordedClients; +} + /** * What the last default-selection read answered for, which is what a report names when the client * chose no `DEVELOPER_DIR`. A read that threw or could not resolve its selection leaves it alone, @@ -148,11 +159,19 @@ type RecordedSelection = { let recordedDefaultSelection: RecordedSelection | undefined; /** - * What the last client-named read answered for, under the exact name that client used. A report for - * a client naming some other dir gets nothing: the key for that dir needs the identity of a bundle - * this path must not go and look up. + * What each client-named selection answered for, under the exact name that client used. Kept per + * name rather than in one slot because a daemon serves clients with different request-scoped + * `DEVELOPER_DIR`s at once: one global record would let the last read erase what an earlier client + * needs to report on, even though its fingerprint is still cached. It is a memo and not a map, so a + * name drops out on the same clock as the entry it points at. */ -let recordedChosenSelection: ({ readonly named: string } & RecordedSelection) | undefined; +function recordedClientSelection(named: string): RecordedSelection | undefined { + return lazyRecordedClientSelections()?.get(named); +} + +function rememberRecordedClientSelection(named: string, answered: RecordedSelection): void { + lazyRecordedClientSelections()?.set(named, answered); +} /** * The toolchain half of the cache key. It also names the derived-data directory, so an @@ -184,7 +203,7 @@ export function requireRunnerToolchainFingerprint( const answered = { developerDir: resolvedDir, bundleIdentity }; // A report finds its entry by the key this read built, whether the read stored it or hit it, so a // report never has to resolve or stat anything itself. - if (chosenDir !== undefined) recordedChosenSelection = { named: chosenDir, ...answered }; + if (chosenDir !== undefined) rememberRecordedClientSelection(chosenDir, answered); else recordedDefaultSelection = answered; return fingerprint; } @@ -308,11 +327,7 @@ function probeSelectedDeveloperDir(clock: ToolchainProbeClock): string | undefin export function memoizedRunnerXcodeVersion(device: DeviceInfo): string | undefined { const chosenDir = commandDeveloperDir(); const answered = - chosenDir === undefined - ? recordedDefaultSelection - : recordedChosenSelection?.named === chosenDir - ? recordedChosenSelection - : undefined; + chosenDir === undefined ? recordedDefaultSelection : recordedClientSelection(chosenDir); if (answered === undefined) return undefined; return toolchainFingerprintCache().get( toolchainFingerprintCacheKey( From 4161d65cb1f16840003dc6605fff95a68d51dd65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 17 Oct 2026 07:05:58 +0200 Subject: [PATCH 8/8] test(apple): pin the replacement bundle's mtime so the key must move The in-place replacement case relied on the filesystem to make both halves of the memo key differ. On ext4 or tmpfs a freed inode is reused and two writes can land in one truncated millisecond, leaving the key untouched and the case red. Set the replacement's mtime explicitly. Also drop the claim that two probers read the selection link path; only the toolchain probe does. --- .../src/runner/__tests__/runner-toolchain-probe.test.ts | 7 +++++++ .../platform-apple/src/runner/apple-runner-platform.ts | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts index a88a920741..fb3fd5d3c6 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-toolchain-probe.test.ts @@ -317,8 +317,15 @@ describe('the developer dir a toolchain fingerprint answers for', () => { // The installer writes a new bundle: same path, new inode for the version file. const versionFile = path.join(selected, '..', 'version.plist'); + const replacedAt = fs.statSync(versionFile).mtimeMs + 1_000; fs.unlinkSync(versionFile); fs.writeFileSync(versionFile, 'build 2\n'); + // Not left to the filesystem to make the two halves of the identity differ. On ext4 or tmpfs a + // freed inode comes straight back out, and the two writes can land in one truncated millisecond, + // which would leave the key untouched and this case red for a reason the fix has nothing to do + // with. A replacement bundle is written after the one it replaces, so one second later says that + // wherever the temp dir lives. + fs.utimesSync(versionFile, new Date(replacedAt), new Date(replacedAt)); runCmdSync.mockClear(); expect(readFingerprint().xcodeBuildVersion).toBe(STUBBED_APPLE_TOOLCHAIN.xcodeBuildVersion); diff --git a/packages/platform-apple/src/runner/apple-runner-platform.ts b/packages/platform-apple/src/runner/apple-runner-platform.ts index 16c14ea86b..28180c4ddd 100644 --- a/packages/platform-apple/src/runner/apple-runner-platform.ts +++ b/packages/platform-apple/src/runner/apple-runner-platform.ts @@ -24,8 +24,8 @@ export const COLD_TOOLCHAIN_PROBE_TIMEOUT_MS = 30_000; /** * The symlink `xcode-select -s` writes and `xcode-select -p` reads back. Resolving it answers which * developer dir the host selects, without a subprocess, so the toolchain memo can key on that answer - * on a hit as well as on a miss. Beside the probe ceiling above so both Apple toolchain probers read - * one value without either owning it. + * on a hit as well as on a miss. Beside the probe ceiling above so the constant sits with the + * toolchain constants this package shares rather than inside the one module that reads it. */ export const XCODE_SELECT_LINK_PATH = '/var/db/xcode_select_link';