Skip to content

Commit 2eb4198

Browse files
committed
feat(apple): reclaim retained runners under device-claim authority
Implement the #1320 retained-runner rule: a daemon holding the host-global device claim may stop and replace a warm XCTest runner whose owner no longer holds that claim. Previously worktree B's open failed with an unstructured COMMAND_FAILED (IOS_RUNNER_OWNED_BY_OTHER_DAEMON) for up to five minutes after worktree A closed, until A's daemon idled out. - Runner leases now record deviceClaimProtocol: 1; takeover is gated on the lease declaring claim arbitration, so owners from pre-claims builds are never preempted. - The claim-authority probe is daemon-bound through the existing runner-owner seam and answers from the claim store by process identity; unbound embedders answer false and keep today's refusal. - Disposal now skips device-wide runner container-app termination when the on-disk lease is owned by someone else, so the losing daemon's idle stop or shutdown cannot kill the successor's runner on the shared simulator. - Help topics updated: the live-owner runner rejection now names claim arbitration instead of being unconditional. Live-validated with two daemons sharing one claim store against a throwaway simulator: live-owner open still rejects with structured DEVICE_IN_USE; after close, the contender opens in ~8s with the lease re-owned while the loser daemon is still alive; stopping the loser afterwards leaves the winner's runner healthy.
1 parent caa3dc2 commit 2eb4198

15 files changed

Lines changed: 327 additions & 16 deletions

‎packages/platform-apple/src/core/runner-host.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@ import {
3636
import { bootFailureHint, classifyBootFailure } from '@agent-device/provision-kit/boot-diagnostics';
3737
import { resolveIosPhysicalDeviceControl } from './physical-device-control.ts';
3838
import { visitXmlPlistEntries } from './plist-xml.ts';
39-
import { getRunnerLeaseOwnerStateDir } from './runner-owner-state.ts';
39+
import {
40+
getRunnerDeviceClaimAuthorityProbe,
41+
getRunnerLeaseOwnerStateDir,
42+
} from './runner-owner-state.ts';
4043
import { buildSimctlArgsForDevice } from './simctl.ts';
4144
import { readApplePlistJson, runAppleToolCommand, runXcrun } from './tool-provider.ts';
4245

@@ -85,4 +88,5 @@ export const appleRunnerHost: AppleRunnerHost = {
8588
resolveIosPhysicalDeviceControl,
8689
visitXmlPlistEntries,
8790
leaseOwnerStateDir: getRunnerLeaseOwnerStateDir,
91+
hasDeviceClaimAuthority: (deviceId) => getRunnerDeviceClaimAuthorityProbe()?.(deviceId) ?? false,
8892
};

‎packages/platform-apple/src/core/runner-owner-state.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,23 @@ export function setRunnerLeaseOwnerStateDir(stateDir: string | undefined): void
1313
export function getRunnerLeaseOwnerStateDir(): string | undefined {
1414
return runnerLeaseOwnerStateDir;
1515
}
16+
17+
/**
18+
* Daemon-owned device-claim arbitration probe: does the embedding process hold
19+
* the host-global local device claim for this device id right now? Unbound (in
20+
* embedders that run no claim store, such as package tests) it answers false,
21+
* which keeps runner-lease takeover disabled there.
22+
*/
23+
export type RunnerDeviceClaimAuthorityProbe = (deviceId: string) => boolean;
24+
25+
let runnerDeviceClaimAuthorityProbe: RunnerDeviceClaimAuthorityProbe | undefined;
26+
27+
export function setRunnerDeviceClaimAuthorityProbe(
28+
probe: RunnerDeviceClaimAuthorityProbe | undefined,
29+
): void {
30+
runnerDeviceClaimAuthorityProbe = probe;
31+
}
32+
33+
export function getRunnerDeviceClaimAuthorityProbe(): RunnerDeviceClaimAuthorityProbe | undefined {
34+
return runnerDeviceClaimAuthorityProbe;
35+
}
Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,5 @@
1-
export { setRunnerLeaseOwnerStateDir } from './core/runner-owner-state.ts';
1+
export {
2+
setRunnerDeviceClaimAuthorityProbe,
3+
setRunnerLeaseOwnerStateDir,
4+
type RunnerDeviceClaimAuthorityProbe,
5+
} from './core/runner-owner-state.ts';

‎packages/platform-apple/src/runner/__tests__/runner-disposal.test.ts‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { IOS_SIMULATOR, MACOS_DEVICE, TVOS_SIMULATOR } from './device-fixtures.t
33
import type { ExecResult } from '../host.ts';
44
import type { RunnerSession } from '../runner-session-types.ts';
55
import { appleRunnerTestHost } from '../test-host.ts';
6+
import { makeRunnerLease } from './runner-session-fixtures.ts';
7+
import { mkdtempForTestSync } from './tmp-dir.ts';
68

79
const { mockCleanupTempFile } = vi.hoisted(() => ({
810
mockCleanupTempFile: vi.fn(),
@@ -13,7 +15,8 @@ vi.mock('../runner-io.ts', async (importOriginal) => {
1315
return { ...actual, cleanupTempFile: mockCleanupTempFile };
1416
});
1517

16-
import { abortRunnerSessionsAndPrepProcesses } from '../runner-disposal.ts';
18+
import { abortRunnerSessionsAndPrepProcesses, disposeRunnerSession } from '../runner-disposal.ts';
19+
import { writeRunnerLease } from '../runner-lease.ts';
1720

1821
const mockIsProcessAlive = vi.fn();
1922
const mockIsProcessGroupAlive = vi.fn();
@@ -23,6 +26,9 @@ const mockSignalPidsBestEffort = vi.fn();
2326
const mockSignalProcessGroupBestEffort = vi.fn();
2427

2528
beforeEach(() => {
29+
process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR = mkdtempForTestSync(
30+
'agent-device-runner-disposal-test-',
31+
);
2632
appleRunnerTestHost.update({
2733
isProcessAlive: mockIsProcessAlive,
2834
isProcessGroupAlive: mockIsProcessGroupAlive,
@@ -106,9 +112,44 @@ test.each([IOS_SIMULATOR, TVOS_SIMULATOR])(
106112
},
107113
);
108114

115+
test('simulator disposal terminates runner container apps while it still owns the on-disk lease', async () => {
116+
vi.useRealTimers();
117+
mockIsProcessAlive.mockReturnValue(false);
118+
const lease = makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-disposal-own' });
119+
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), { lease });
120+
writeRunnerLease(lease);
121+
122+
await disposeRunnerSession(session, { graceful: false, waitTimeoutMs: 1 });
123+
124+
expect(simulatorTerminateCalls()).not.toEqual([]);
125+
});
126+
127+
test('simulator disposal skips container-app termination after a foreign takeover replaced the lease', async () => {
128+
vi.useRealTimers();
129+
mockIsProcessAlive.mockReturnValue(false);
130+
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), {
131+
lease: makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-disposal-loser' }),
132+
});
133+
// The successor's runner lives in the same container bundles on the shared
134+
// simulator; terminating them here would stop the new owner's runner.
135+
writeRunnerLease(
136+
makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-disposal-successor' }),
137+
);
138+
139+
await disposeRunnerSession(session, { graceful: false, waitTimeoutMs: 1 });
140+
141+
expect(simulatorTerminateCalls()).toEqual([]);
142+
expect(mockCleanupTempFile).toHaveBeenCalledWith(session.xctestrunPath);
143+
});
144+
145+
function simulatorTerminateCalls(): unknown[] {
146+
return mockRunXcrun.mock.calls.filter(([args]) => (args as string[]).includes('terminate'));
147+
}
148+
109149
function makeRunnerSession(
110150
device: RunnerSession['device'],
111151
testPromise: Promise<ExecResult>,
152+
overrides: Partial<RunnerSession> = {},
112153
): RunnerSession {
113154
return {
114155
sessionId: `${device.id}:8123:test`,
@@ -120,6 +161,7 @@ function makeRunnerSession(
120161
testPromise,
121162
child: { pid: 42, exitCode: null },
122163
ready: true,
164+
...overrides,
123165
};
124166
}
125167

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import assert from 'node:assert/strict';
2+
import fs from 'node:fs';
3+
import { afterEach, beforeEach, test } from 'vitest';
4+
import { appleRunnerTestHost } from '../test-host.ts';
5+
import {
6+
prepareRunnerLeaseForStartup,
7+
runnerOwnerStartTime,
8+
writeRunnerLease,
9+
type RunnerLease,
10+
type RunnerLeaseCleanupAdapter,
11+
} from '../runner-lease.ts';
12+
import { makeRunnerLease } from './runner-session-fixtures.ts';
13+
import { mkdtempForTestSync } from './tmp-dir.ts';
14+
15+
// #1320 retained-runner rule, tested at the lease seam directly: device claims
16+
// are exclusive per device, so claim authority proves a busy lease's owner
17+
// released the device and merely kept its runner warm.
18+
19+
let ownerStateDir: string;
20+
21+
beforeEach(() => {
22+
process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR = mkdtempForTestSync(
23+
'agent-device-claim-takeover-lease-',
24+
);
25+
// The owner state dir must exist on disk: an owner whose state dir is gone
26+
// classifies as stale (reclaimable) instead of busy.
27+
ownerStateDir = mkdtempForTestSync('agent-device-claim-takeover-owner-');
28+
});
29+
30+
afterEach(() => {
31+
fs.rmSync(ownerStateDir, { recursive: true, force: true });
32+
});
33+
34+
function liveForeignLease(deviceId: string, overrides: Partial<RunnerLease> = {}): RunnerLease {
35+
return makeRunnerLease({
36+
deviceId,
37+
ownerToken: 'owner-foreign-live',
38+
ownerPid: process.pid,
39+
ownerStartTime: runnerOwnerStartTime(),
40+
ownerStateDir,
41+
...overrides,
42+
});
43+
}
44+
45+
function recordingCleanupAdapter(): RunnerLeaseCleanupAdapter & { calls: string[] } {
46+
const calls: string[] = [];
47+
return {
48+
calls,
49+
cleanupRunnerProcessTree: async (_pid, signal) => {
50+
calls.push(`process-tree:${signal}`);
51+
},
52+
cleanupRunnerXcodebuildProcesses: async (_deviceId, ownerToken) => {
53+
calls.push(`xcodebuild:${ownerToken ?? 'any'}`);
54+
},
55+
cleanupTempFile: (filePath) => {
56+
calls.push(`temp:${filePath}`);
57+
},
58+
};
59+
}
60+
61+
test('device-claim authority reclaims a live foreign claim-aware lease', async () => {
62+
const deviceId = 'claim-takeover-sim';
63+
writeRunnerLease(liveForeignLease(deviceId, { deviceClaimProtocol: 1 }));
64+
appleRunnerTestHost.update({ hasDeviceClaimAuthority: (id) => id === deviceId });
65+
const cleanup = recordingCleanupAdapter();
66+
67+
await prepareRunnerLeaseForStartup(deviceId, cleanup);
68+
69+
assert.ok(cleanup.calls.includes('xcodebuild:owner-foreign-live'));
70+
// The lease was released as part of the takeover, so the next classification
71+
// sees an empty store instead of the foreign owner.
72+
const emptyCleanup = recordingCleanupAdapter();
73+
await prepareRunnerLeaseForStartup(deviceId, emptyCleanup);
74+
assert.ok(emptyCleanup.calls.includes('xcodebuild:any'));
75+
});
76+
77+
test('a claim-aware lease still refuses without device-claim authority', async () => {
78+
const deviceId = 'claim-no-authority-sim';
79+
writeRunnerLease(liveForeignLease(deviceId, { deviceClaimProtocol: 1 }));
80+
const cleanup = recordingCleanupAdapter();
81+
82+
await assert.rejects(
83+
async () => await prepareRunnerLeaseForStartup(deviceId, cleanup),
84+
/already owned by another agent-device daemon/,
85+
);
86+
assert.deepEqual(cleanup.calls, []);
87+
});
88+
89+
test('a pre-claims lease is never preempted despite device-claim authority', async () => {
90+
// A lease without deviceClaimProtocol was written by a build that never
91+
// arbitrates through device claims, so its owner may be actively using the
92+
// runner without holding any claim.
93+
const deviceId = 'legacy-lease-sim';
94+
writeRunnerLease(liveForeignLease(deviceId));
95+
appleRunnerTestHost.update({ hasDeviceClaimAuthority: () => true });
96+
const cleanup = recordingCleanupAdapter();
97+
98+
await assert.rejects(
99+
async () => await prepareRunnerLeaseForStartup(deviceId, cleanup),
100+
/already owned by another agent-device daemon/,
101+
);
102+
assert.deepEqual(cleanup.calls, []);
103+
});

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,10 @@ export type AppleRunnerHost = {
235235
visitXmlPlistEntries(nodes: XmlNode[], visitor: (key: string, valueNode: XmlNode) => void): void;
236236
// Daemon-owned lease owner state directory (packages/platform-apple/src/core/runner-owner-state.ts)
237237
leaseOwnerStateDir(): string | undefined;
238+
// Daemon-owned device-claim arbitration probe (packages/platform-apple/src/core/runner-owner-state.ts):
239+
// true only while the embedding process holds the host-global local device
240+
// claim for the device. Embedders without a claim store answer false.
241+
hasDeviceClaimAuthority(deviceId: string): boolean;
238242
};
239243

240244
let boundHost: AppleRunnerHost | undefined;
@@ -350,6 +354,8 @@ export const visitXmlPlistEntries: AppleRunnerHost['visitXmlPlistEntries'] = (no
350354
requireHost().visitXmlPlistEntries(nodes, visitor);
351355
export const leaseOwnerStateDir: AppleRunnerHost['leaseOwnerStateDir'] = () =>
352356
requireHost().leaseOwnerStateDir();
357+
export const hasDeviceClaimAuthority: AppleRunnerHost['hasDeviceClaimAuthority'] = (deviceId) =>
358+
requireHost().hasDeviceClaimAuthority(deviceId);
353359

354360
/**
355361
* Deadline keeps its root call-site shape (`Deadline.fromTimeoutMs(...)`);

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { waitForRunner } from './runner-startup-transport.ts';
1515
import { withRunnerCommandId, type RunnerCommand } from './runner-contract.ts';
1616
import {
1717
cleanupOwnedRunnerLease,
18+
currentRunnerLeaseOwnerToken,
1819
releaseRunnerLease,
1920
type RunnerLeaseCleanupAdapter,
2021
} from './runner-lease.ts';
@@ -172,7 +173,13 @@ async function runnerSessionsStillAlive(
172173
}
173174

174175
async function cleanupRunnerSessionResources(session: RunnerSession): Promise<void> {
175-
await terminateRunnerSimulatorApps(session.device);
176+
// Terminating the runner container bundles acts on the whole device, so it is
177+
// only this session's to do while the on-disk lease is still its own. After a
178+
// takeover (device-claim or logical-lease) the successor's runner lives in the
179+
// same bundles; killing them here would stop the new owner's runner.
180+
if (!runnerLeaseOwnedElsewhere(session)) {
181+
await terminateRunnerSimulatorApps(session.device);
182+
}
176183
cleanupTempFile(session.xctestrunPath);
177184
cleanupTempFile(session.jsonPath);
178185
try {
@@ -182,6 +189,11 @@ async function cleanupRunnerSessionResources(session: RunnerSession): Promise<vo
182189
}
183190
}
184191

192+
function runnerLeaseOwnedElsewhere(session: RunnerSession): boolean {
193+
const onDiskToken = currentRunnerLeaseOwnerToken(session.deviceId);
194+
return onDiskToken !== null && onDiskToken !== session.lease?.ownerToken;
195+
}
196+
185197
async function terminateRunnerSimulatorApps(device: DeviceInfo): Promise<void> {
186198
if (device.kind !== 'simulator') return;
187199

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

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
emitDiagnostic,
77
publishFileSync,
88
acquireProcessLock,
9+
hasDeviceClaimAuthority,
910
isProcessAlive,
1011
readProcessCommand,
1112
readProcessStartTime,
@@ -48,6 +49,14 @@ export type RunnerLease = {
4849
xctestrunPath: string;
4950
jsonPath: string;
5051
createdAtMs: number;
52+
/**
53+
* The owner arbitrates device ownership through host-global device claims
54+
* (#1320): while it wants the device it holds the claim, so a daemon that
55+
* holds the claim instead may stop and replace this runner. Absent on leases
56+
* written before claim arbitration existed — those owners never signal
57+
* ownership through claims, so claim authority must not preempt them.
58+
*/
59+
deviceClaimProtocol?: 1;
5160
};
5261

5362
// Why a foreign lease classifies as stale (reclaimable). The distinction is
@@ -99,6 +108,7 @@ export function buildRunnerLease(params: {
99108
xctestrunPath: params.xctestrunPath,
100109
jsonPath: params.jsonPath,
101110
createdAtMs: Date.now(),
111+
deviceClaimProtocol: 1,
102112
};
103113
}
104114

@@ -159,6 +169,10 @@ export async function prepareRunnerLeaseForStartup(
159169
await cleanupLeasedRunnerProcesses(state.lease, 'same-state-dir', cleanup);
160170
return;
161171
}
172+
if (canDeviceClaimReclaimRunner(state.lease)) {
173+
await cleanupLeasedRunnerProcesses(state.lease, 'device-claim-takeover', cleanup);
174+
return;
175+
}
162176
if (canLogicalLeaseReclaimRunner(state.lease, logicalLeaseContext)) {
163177
await cleanupLeasedRunnerProcesses(state.lease, 'logical-lease-takeover', cleanup);
164178
return;
@@ -191,6 +205,18 @@ function isSameStateDirRunnerLease(lease: RunnerLease): boolean {
191205
return path.resolve(currentStateDir) === path.resolve(lease.ownerStateDir);
192206
}
193207

208+
/**
209+
* #1320 retained-runner rule: device claims are exclusive per device, so this
210+
* process holding the claim proves the lease owner released or lost it — a
211+
* retained warm runner, not an active one. Stop-and-recreate is safe because
212+
* every owner-side cleanup path no-ops once the lease token changes. Gated on
213+
* the lease declaring claim arbitration, so owners from builds that predate
214+
* device claims (and therefore never hold one) keep today's refusal.
215+
*/
216+
function canDeviceClaimReclaimRunner(lease: RunnerLease): boolean {
217+
return lease.deviceClaimProtocol === 1 && hasDeviceClaimAuthority(lease.deviceId);
218+
}
219+
194220
function canLogicalLeaseReclaimRunner(
195221
lease: RunnerLease,
196222
logicalLeaseContext: RunnerLogicalLeaseContext | undefined,
@@ -305,6 +331,15 @@ export async function cleanupRunnerLeasesForOwner(
305331
);
306332
}
307333

334+
/**
335+
* The owner token of the lease currently on disk for this device, or null when
336+
* none is readable. Lets disposal recognize that a foreign owner (a device-claim
337+
* or logical-lease takeover) has replaced the runner it is cleaning up after.
338+
*/
339+
export function currentRunnerLeaseOwnerToken(deviceId: string): string | null {
340+
return readRunnerLease(deviceId)?.ownerToken ?? null;
341+
}
342+
308343
export function releaseRunnerLease(lease: RunnerLease | undefined): void {
309344
if (!lease) return;
310345
removeRunnerLease({
@@ -395,6 +430,7 @@ function normalizeRunnerLease(value: unknown, deviceId: string): RunnerLease | n
395430
ownerStateDir: readOptionalString(raw.ownerStateDir) ?? undefined,
396431
runnerPid: readPositiveInteger(raw.runnerPid),
397432
runnerStartTime: readOptionalString(raw.runnerStartTime),
433+
...(raw.deviceClaimProtocol === 1 ? { deviceClaimProtocol: 1 as const } : {}),
398434
};
399435
}
400436

@@ -441,14 +477,11 @@ function readFiniteNumber(value: unknown): number | null {
441477
// liveness only.
442478
async function cleanupLeasedRunnerProcesses(
443479
lease: RunnerLease,
444-
reason: 'owned' | 'stale' | 'same-state-dir' | 'logical-lease-takeover',
480+
reason: 'owned' | 'stale' | 'same-state-dir' | 'logical-lease-takeover' | 'device-claim-takeover',
445481
cleanup: RunnerLeaseCleanupAdapter,
446482
): Promise<void> {
447483
emitDiagnostic({
448-
level:
449-
reason === 'stale' || reason === 'same-state-dir' || reason === 'logical-lease-takeover'
450-
? 'warn'
451-
: 'debug',
484+
level: reason === 'owned' ? 'debug' : 'warn',
452485
phase: 'ios_runner_lease_cleanup',
453486
data: {
454487
deviceId: lease.deviceId,

0 commit comments

Comments
 (0)