Skip to content

Commit 22cea00

Browse files
thymikeeclaude
thymikee
and
claude
committed
fix(apple): scope start admissions, prep builds and bad-artifact marks to the runtime
Closing one daemon runtime no longer retires another runtime's start admissions or signals its prep builds. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
1 parent c047776 commit 22cea00

3 files changed

Lines changed: 91 additions & 17 deletions

File tree

‎packages/platform-apple/src/runner/__tests__/runner-session-runtime-isolation.test.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import assert from 'node:assert/strict';
2+
import { EventEmitter } from 'node:events';
3+
import { beforeEach, test, vi } from 'vitest';
4+
import type { ExecBackgroundResult } from '@agent-device/host-kit/command';
5+
import { createAppleRunnerRuntimeState } from '../../core/runner-owner-state.ts';
6+
import { appleRunnerTestHost } from '../test-host.ts';
7+
import { registerRunnerPrepProcess } from '../runner-artifact.ts';
8+
import { stopAllIosRunnerSessions } from '../runner-session.ts';
9+
import { openRunnerStartAdmission, runnerPrepProcessChildren } from '../runner-xctestrun.ts';
10+
11+
const mockSignalProcessGroupBestEffort = vi.fn();
12+
const mockRunAppleToolCommand = vi.fn();
13+
14+
beforeEach(() => {
15+
vi.resetAllMocks();
16+
mockRunAppleToolCommand.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' });
17+
appleRunnerTestHost.update({
18+
runAppleToolCommand: mockRunAppleToolCommand,
19+
signalProcessGroupBestEffort: mockSignalProcessGroupBestEffort,
20+
signalPidsBestEffort: vi.fn(),
21+
});
22+
});
23+
24+
function prepChild(pid: number): ExecBackgroundResult['child'] {
25+
return Object.assign(new EventEmitter(), {
26+
pid,
27+
exitCode: null,
28+
}) as ExecBackgroundResult['child'];
29+
}
30+
31+
test("stopping every runner of one runtime leaves another runtime's prep build and admission alone", async () => {
32+
const runtimeA = createAppleRunnerRuntimeState();
33+
const runtimeB = createAppleRunnerRuntimeState();
34+
const deviceId = 'runner-runtime-isolation-sim';
35+
const buildA = prepChild(6101);
36+
const buildB = prepChild(6102);
37+
38+
runtimeA.run(() => {
39+
registerRunnerPrepProcess(deviceId, buildA, openRunnerStartAdmission(deviceId));
40+
});
41+
const admissionB = runtimeB.run(() => {
42+
const admission = openRunnerStartAdmission(deviceId);
43+
registerRunnerPrepProcess(deviceId, buildB, admission);
44+
return admission;
45+
});
46+
47+
await runtimeA.run(async () => await stopAllIosRunnerSessions());
48+
49+
const signaled = mockSignalProcessGroupBestEffort.mock.calls.map(([pid]) => pid as number);
50+
assert.ok(signaled.includes(6101), 'runtime A stopped its own build');
51+
assert.ok(!signaled.includes(6102), "runtime A never signaled runtime B's build");
52+
assert.equal(
53+
runtimeA.run(() => runnerPrepProcessChildren(deviceId).length),
54+
0,
55+
);
56+
assert.equal(
57+
runtimeB.run(() => runnerPrepProcessChildren(deviceId).length),
58+
1,
59+
);
60+
assert.equal(admissionB.admitted, true, "runtime B's start admission stays open");
61+
});

‎packages/platform-apple/src/runner/runner-artifact.ts‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
emitRequestProgress,
1616
findProjectRoot,
1717
isCommandTimeoutError,
18+
runnerRuntimeSlot,
1819
} from './host.ts';
1920
import type { ExecBackgroundResult } from '@agent-device/host-kit/command';
2021
import type { DeviceInfo } from '@agent-device/kernel/device';
@@ -119,7 +120,10 @@ type DeviceStartState = {
119120
inFlight: Set<StartAdmission>;
120121
};
121122

122-
const deviceStartStates = new Map<string, DeviceStartState>();
123+
const DEVICE_START_STATES_SLOT = Symbol('apple-runner-device-start-states');
124+
// Per daemon runtime: a runtime's teardown retires only the start admissions it opened.
125+
const deviceStartStates = () =>
126+
runnerRuntimeSlot(DEVICE_START_STATES_SLOT, () => new Map<string, DeviceStartState>());
123127

124128
/**
125129
* Opens the admission one start answers to, taken when a caller asks for a runner. Registration is
@@ -144,7 +148,7 @@ export function openRunnerStartLoopAdmission(deviceId: string): RunnerStartAdmis
144148
/** The start settled: its token leaves the device's in-flight set. Verdicts live on the token. */
145149
export function finishRunnerStartAdmission(admission: RunnerStartAdmission): void {
146150
(admission as StartAdmission).retryPending = false;
147-
const state = deviceStartStates.get(admission.deviceId);
151+
const state = deviceStartStates().get(admission.deviceId);
148152
if (!state) return;
149153
state.inFlight.delete(admission as StartAdmission);
150154
forgetDeviceStartState(admission.deviceId, state);
@@ -180,7 +184,10 @@ export function fenceRunnerStartAdmissionsForTeardown(deviceId: string): () => v
180184
* build past the sweep that is about to stop it.
181185
*/
182186
export function retireAllRunnerStartAdmissions(additionalDeviceIds?: Iterable<string>): () => void {
183-
const deviceIds = new Set<string>([...deviceStartStates.keys(), ...(additionalDeviceIds ?? [])]);
187+
const deviceIds = new Set<string>([
188+
...deviceStartStates().keys(),
189+
...(additionalDeviceIds ?? []),
190+
]);
184191
const settles = [...deviceIds].map((deviceId) => fenceRunnerStartAdmissionsForTeardown(deviceId));
185192
return () => {
186193
for (const settle of settles.splice(0)) settle();
@@ -193,7 +200,7 @@ export function retireAllRunnerStartAdmissions(additionalDeviceIds?: Iterable<st
193200
* not that teardown's retry.
194201
*/
195202
export function runnerStartTeardownPending(deviceId: string): boolean {
196-
return (deviceStartStates.get(deviceId)?.pendingTeardowns ?? 0) > 0;
203+
return (deviceStartStates().get(deviceId)?.pendingTeardowns ?? 0) > 0;
197204
}
198205

199206
/**
@@ -299,17 +306,17 @@ export function releaseRunnerStartWaiter(
299306
}
300307

301308
function stateForDevice(deviceId: string): DeviceStartState {
302-
const existing = deviceStartStates.get(deviceId);
309+
const existing = deviceStartStates().get(deviceId);
303310
if (existing) return existing;
304311
const state: DeviceStartState = { pendingTeardowns: 0, inFlight: new Set() };
305-
deviceStartStates.set(deviceId, state);
312+
deviceStartStates().set(deviceId, state);
306313
return state;
307314
}
308315

309316
/** Nothing fences the device and no start is in flight on it once the last of both is gone. */
310317
function forgetDeviceStartState(deviceId: string, state: DeviceStartState): void {
311318
if (state.pendingTeardowns <= 0 && state.inFlight.size === 0) {
312-
deviceStartStates.delete(deviceId);
319+
deviceStartStates().delete(deviceId);
313320
}
314321
}
315322

@@ -360,7 +367,10 @@ type RunnerPrepProcess = Readonly<{
360367
child: ExecBackgroundResult['child'];
361368
}>;
362369

363-
const runnerPrepProcessLedger = new Set<RunnerPrepProcess>();
370+
const RUNNER_PREP_LEDGER_SLOT = Symbol('apple-runner-prep-process-ledger');
371+
// Per daemon runtime: a runtime's stop signals only the prep children it started.
372+
const runnerPrepProcessLedger = () =>
373+
runnerRuntimeSlot(RUNNER_PREP_LEDGER_SLOT, () => new Set<RunnerPrepProcess>());
364374

365375
/**
366376
* Records a prep subprocess (`xcodebuild build-for-testing`) against the device it builds for and
@@ -377,9 +387,9 @@ export function registerRunnerPrepProcess(
377387
startAdmission?: RunnerStartAdmission,
378388
): void {
379389
const entry: RunnerPrepProcess = { deviceId, startAdmission, child };
380-
runnerPrepProcessLedger.add(entry);
390+
runnerPrepProcessLedger().add(entry);
381391
child.on('close', () => {
382-
runnerPrepProcessLedger.delete(entry);
392+
runnerPrepProcessLedger().delete(entry);
383393
});
384394
}
385395

@@ -426,14 +436,14 @@ export function markRunnerStartRetryPending(admission: RunnerStartAdmission): vo
426436
}
427437

428438
function prepProcessEntries(deviceId?: string): readonly RunnerPrepProcess[] {
429-
return [...runnerPrepProcessLedger].filter(
439+
return [...runnerPrepProcessLedger()].filter(
430440
(entry) => deviceId === undefined || entry.deviceId === deviceId,
431441
);
432442
}
433443

434444
export function forgetRunnerPrepProcess(child: ExecBackgroundResult['child']): void {
435-
for (const entry of runnerPrepProcessLedger) {
436-
if (entry.child === child) runnerPrepProcessLedger.delete(entry);
445+
for (const entry of runnerPrepProcessLedger()) {
446+
if (entry.child === child) runnerPrepProcessLedger().delete(entry);
437447
}
438448
}
439449

‎packages/platform-apple/src/runner/runner-cache.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
withProcessLock,
99
isEnvTruthy,
1010
findProjectRoot,
11+
runnerRuntimeSlot,
1112
} from './host.ts';
1213
import {
1314
RUNNER_CACHE_METADATA_FILE,
@@ -38,7 +39,9 @@ export { requireRunnerPhaseRemainingMs, type RunnerPhaseBudget } from './runner-
3839
const RUNNER_XCTESTRUN_CACHE_LOCK_TIMEOUT_MS = 10 * 60_000;
3940
const RUNNER_XCTESTRUN_CACHE_LOCK_POLL_MS = 100;
4041

41-
const badRunnerArtifactsForRun = new Set<string>();
42+
const BAD_ARTIFACTS_SLOT = Symbol('apple-runner-bad-artifacts');
43+
const badRunnerArtifactsForRun = () =>
44+
runnerRuntimeSlot(BAD_ARTIFACTS_SLOT, () => new Set<string>());
4245

4346
export type RunnerXctestrunCacheKind = 'exact' | 'miss' | 'external';
4447

@@ -102,7 +105,7 @@ export async function markRunnerXctestrunArtifactBadForRun(
102105
return;
103106
}
104107

105-
badRunnerArtifactsForRun.add(artifact.derived);
108+
badRunnerArtifactsForRun().add(artifact.derived);
106109
await withProcessLock({
107110
acquire: () => acquireRunnerXctestrunCacheLock(artifact.derived),
108111
task: async () => {
@@ -139,15 +142,15 @@ function resolveRunnerXctestrunCacheLockPath(derived: string): string {
139142
}
140143

141144
export function cleanRunnerDerivedBeforeEvaluation(derived: string, forceRebuild: boolean): void {
142-
if (!shouldCleanDerived() && !forceRebuild && !badRunnerArtifactsForRun.has(derived)) {
145+
if (!shouldCleanDerived() && !forceRebuild && !badRunnerArtifactsForRun().has(derived)) {
143146
return;
144147
}
145148
emitRunnerXctestrunDecision('clean', forceRebuild ? 'forced_rebuild' : 'forced_clean', {
146149
derived,
147150
});
148151
assertSafeDerivedCleanup(derived);
149152
cleanRunnerDerivedArtifacts(derived);
150-
badRunnerArtifactsForRun.delete(derived);
153+
badRunnerArtifactsForRun().delete(derived);
151154
}
152155

153156
/**

0 commit comments

Comments
 (0)