Skip to content

Commit 753263f

Browse files
thymikeethymikee
authored
fix(ios): preserve simulator app accessibility after runner teardown (#4276)
* fix(ios): preserve simulator app accessibility after runner teardown * refactor(ios): share the accessibility repair step and resolve the runner pid at use * fix(ios): report teardown repair failures and keep owner cleanup going past a busy lease lock * test(ios): make the accessibility test seams honest about failure * fix(ios): keep probing runner termination after a failed or timed-out pgrep * test(ios): assert the termination probe retries
1 parent bfc08a7 commit 753263f

20 files changed

Lines changed: 757 additions & 28 deletions

‎packages/apple-runner/src/__tests__/runner-cache-retention.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,8 @@ test('owner cleanup still stops a readable lease next to an unreadable lease fil
241241
await cleanupRunnerLeasesForOwner(
242242
{ pid: 4242, startTime: null },
243243
{
244+
confirmRunnerTermination: async () => true,
245+
enableIosSimulatorAppAccessibility: async () => {},
244246
cleanupRunnerProcessTree: async () => {},
245247
cleanupRunnerXcodebuildProcesses: async () => {},
246248
cleanupTempFile,

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

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
writeRunnerLease,
3333
} from '../runner-lease.ts';
3434

35+
const mockPrepareAccessibility = vi.fn();
3536
const mockIsProcessAlive = vi.fn();
3637
const mockIsProcessGroupAlive = vi.fn();
3738
const mockRunAppleToolCommand = vi.fn();
@@ -50,12 +51,14 @@ beforeEach(() => {
5051
signalProcessGroupBestEffort: mockSignalProcessGroupBestEffort,
5152
runAppleToolCommand: mockRunAppleToolCommand,
5253
runXcrun: mockRunXcrun,
54+
enableIosSimulatorAppAccessibility: mockPrepareAccessibility,
5355
});
5456
vi.useFakeTimers();
5557
mockIsProcessAlive.mockReturnValue(true);
5658
mockIsProcessGroupAlive.mockReturnValue(false);
5759
mockRunAppleToolCommand.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' });
5860
mockRunXcrun.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' });
61+
mockPrepareAccessibility.mockResolvedValue(undefined);
5962
});
6063

6164
afterEach(() => {
@@ -148,19 +151,35 @@ test('simulator disposal terminates runner container apps while it still owns th
148151
vi.useRealTimers();
149152
mockIsProcessAlive.mockReturnValue(false);
150153
const lease = makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-disposal-own' });
151-
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), { lease });
154+
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), {
155+
lease,
156+
xctestrunPath: lease.xctestrunPath,
157+
});
158+
mockRunAppleToolCommand.mockResolvedValue({ exitCode: 1, stdout: '', stderr: '' });
159+
let tokenDuringRepair: string | null = null;
160+
let terminationsDuringRepair = 0;
161+
mockPrepareAccessibility.mockImplementation(async () => {
162+
tokenDuringRepair = currentRunnerLeaseOwnerToken(IOS_SIMULATOR.id);
163+
terminationsDuringRepair = simulatorTerminateCalls().length;
164+
});
152165
writeRunnerLease(lease);
153166

154167
await disposeRunnerSession(session, { graceful: false, waitTimeoutMs: 1 });
155168

156169
expect(simulatorTerminateCalls()).not.toEqual([]);
170+
expect(mockPrepareAccessibility).toHaveBeenCalledExactlyOnceWith(IOS_SIMULATOR);
171+
expect(tokenDuringRepair).toBe(lease.ownerToken);
172+
expect(terminationsDuringRepair).toBeGreaterThan(0);
173+
expect(currentRunnerLeaseOwnerToken(IOS_SIMULATOR.id)).toBeNull();
157174
});
158175

159176
test('simulator disposal skips container-app termination after a foreign takeover replaced the lease', async () => {
160177
vi.useRealTimers();
161178
mockIsProcessAlive.mockReturnValue(false);
179+
mockRunAppleToolCommand.mockResolvedValue({ exitCode: 1, stdout: '', stderr: '' });
162180
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), {
163181
lease: makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-disposal-loser' }),
182+
xctestrunPath: makeRunnerLease({ deviceId: IOS_SIMULATOR.id }).xctestrunPath,
164183
});
165184
// The successor's runner lives in the same container bundles on the shared
166185
// simulator; terminating them here would stop the new owner's runner.
@@ -171,18 +190,21 @@ test('simulator disposal skips container-app termination after a foreign takeove
171190
await disposeRunnerSession(session, { graceful: false, waitTimeoutMs: 1 });
172191

173192
expect(simulatorTerminateCalls()).toEqual([]);
193+
expect(mockPrepareAccessibility).not.toHaveBeenCalled();
174194
expect(mockCleanupTempFile).toHaveBeenCalledWith(session.launch.local!.xctestrunPath);
175195
});
176196

177197
test('disposal serializes behind a successor reclaim window and never terminates its runner', async () => {
178198
vi.useRealTimers();
179199
mockIsProcessAlive.mockReturnValue(false);
200+
mockRunAppleToolCommand.mockResolvedValue({ exitCode: 1, stdout: '', stderr: '' });
180201
const loserLease = makeRunnerLease({
181202
deviceId: IOS_SIMULATOR.id,
182203
ownerToken: 'owner-toctou-loser',
183204
});
184205
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), {
185206
lease: loserLease,
207+
xctestrunPath: loserLease.xctestrunPath,
186208
});
187209
writeRunnerLease(loserLease);
188210

@@ -198,6 +220,7 @@ test('disposal serializes behind a successor reclaim window and never terminates
198220
await new Promise((resolve) => setTimeout(resolve, 300));
199221
expect(disposalSettled).toBe(false);
200222
expect(simulatorTerminateCalls()).toEqual([]);
223+
expect(mockPrepareAccessibility).not.toHaveBeenCalled();
201224
releaseRunnerLease(loserLease);
202225
writeRunnerLease(
203226
makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-toctou-successor' }),
@@ -206,9 +229,89 @@ test('disposal serializes behind a successor reclaim window and never terminates
206229
await disposal;
207230

208231
expect(simulatorTerminateCalls()).toEqual([]);
232+
expect(mockPrepareAccessibility).not.toHaveBeenCalled();
209233
expect(currentRunnerLeaseOwnerToken(IOS_SIMULATOR.id)).toBe('owner-toctou-successor');
210234
});
211235

236+
test('disposal skips repair when a runner process group survives termination', async () => {
237+
const lease = makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-still-running' });
238+
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), {
239+
lease,
240+
xctestrunPath: lease.xctestrunPath,
241+
});
242+
writeRunnerLease(lease);
243+
mockIsProcessAlive.mockReturnValue(false);
244+
mockIsProcessGroupAlive.mockReturnValue(true);
245+
const disposal = disposeRunnerSession(session, { graceful: false, abort: true });
246+
await vi.advanceTimersByTimeAsync(2_000);
247+
await disposal;
248+
expect(mockPrepareAccessibility).not.toHaveBeenCalled();
249+
expect(currentRunnerLeaseOwnerToken(IOS_SIMULATOR.id)).toBeNull();
250+
});
251+
252+
test('disposal waits for the matching xcodebuild to disappear before repair', async () => {
253+
const lease = makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-wait-exit' });
254+
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), {
255+
lease,
256+
xctestrunPath: lease.xctestrunPath,
257+
});
258+
writeRunnerLease(lease);
259+
mockIsProcessAlive.mockReturnValue(false);
260+
let probes = 0;
261+
mockRunAppleToolCommand.mockImplementation(async (command) =>
262+
command === '/usr/bin/pgrep' && probes++ > 0
263+
? { exitCode: 1, stdout: '', stderr: '' }
264+
: execResult(),
265+
);
266+
const disposal = disposeRunnerSession(session, { graceful: false, abort: true });
267+
await vi.advanceTimersByTimeAsync(0);
268+
expect(mockPrepareAccessibility).not.toHaveBeenCalled();
269+
await vi.advanceTimersByTimeAsync(50);
270+
await disposal;
271+
expect(mockPrepareAccessibility).toHaveBeenCalledOnce();
272+
});
273+
274+
test('a failed or timed-out termination probe is retried, not read as a live runner', async () => {
275+
const lease = makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-probe-retry' });
276+
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), {
277+
lease,
278+
xctestrunPath: lease.xctestrunPath,
279+
});
280+
writeRunnerLease(lease);
281+
mockIsProcessAlive.mockReturnValue(false);
282+
let probes = 0;
283+
mockRunAppleToolCommand.mockImplementation(async (command) => {
284+
if (command !== '/usr/bin/pgrep') return execResult();
285+
probes += 1;
286+
if (probes === 1) throw new Error('probe timed out');
287+
return probes === 2
288+
? { exitCode: 2, stdout: '', stderr: 'probe error' }
289+
: { exitCode: 1, stdout: '', stderr: '' };
290+
});
291+
const disposal = disposeRunnerSession(session, { graceful: false, abort: true });
292+
await vi.advanceTimersByTimeAsync(200);
293+
await disposal;
294+
// The thrown probe and the exit-2 probe are both retried; only the third (exit 1) confirms.
295+
expect(probes).toBe(3);
296+
expect(mockPrepareAccessibility).toHaveBeenCalledOnce();
297+
});
298+
299+
test('disposal still releases its lease when accessibility preparation fails', async () => {
300+
vi.useRealTimers();
301+
const lease = makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-repair-failed' });
302+
const session = makeRunnerSession(IOS_SIMULATOR, Promise.resolve(execResult()), {
303+
lease,
304+
xctestrunPath: lease.xctestrunPath,
305+
});
306+
writeRunnerLease(lease);
307+
mockIsProcessAlive.mockReturnValue(false);
308+
mockRunAppleToolCommand.mockResolvedValue({ exitCode: 1, stdout: '', stderr: '' });
309+
mockPrepareAccessibility.mockRejectedValue(new Error('unavailable'));
310+
await disposeRunnerSession(session, { graceful: false, waitTimeoutMs: 1 });
311+
expect(mockPrepareAccessibility).toHaveBeenCalledOnce();
312+
expect(currentRunnerLeaseOwnerToken(IOS_SIMULATOR.id)).toBeNull();
313+
});
314+
212315
test('a leased cleanup selects the launch named by the artifact the lease recorded', async () => {
213316
// The launch is found by matching its argv, and a lease knows the artifact that launch was
214317
// started with. Detaching is why following the path rather than the token matters: it rewrites

‎packages/apple-runner/src/__tests__/runner-lease-accessibility.test.ts‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
import assert from 'node:assert/strict';
2+
import { beforeEach, test, vi } from 'vitest';
3+
import type { DeviceInfo } from '@agent-device/kernel/device';
4+
import { appleRunnerTestHost } from '../test-host.ts';
5+
import {
6+
buildRunnerLease,
7+
cleanupRunnerLeasesForOwner,
8+
currentRunnerLeaseOwnerToken,
9+
withRunnerLeaseLock,
10+
writeRunnerLease,
11+
type RunnerLeaseCleanupAdapter,
12+
} from '../runner-lease.ts';
13+
import { IOS_SIMULATOR, IOS_DEVICE, MACOS_DEVICE, TVOS_SIMULATOR } from './device-fixtures.ts';
14+
import { mkdtempForTestSync } from './tmp-dir.ts';
15+
16+
beforeEach(() => {
17+
process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR = mkdtempForTestSync('agent-device-ax-lease-');
18+
});
19+
20+
function leaseFor(device: DeviceInfo) {
21+
return buildRunnerLease({
22+
device,
23+
sessionId: 'ax-test',
24+
runnerPid: undefined,
25+
endpoint: { port: 8123, token: 'test-token' },
26+
xctestrunPath: '/tmp/ax-test.xctestrun',
27+
jsonPath: '/tmp/ax-test.json',
28+
});
29+
}
30+
31+
for (const confirmed of [true, false]) {
32+
test(`lease cleanup repairs only after confirmed termination (${confirmed}), before releasing`, async () => {
33+
const lease = leaseFor({ ...IOS_SIMULATOR, simulatorSetPath: '/tmp/ax-simlock-set' });
34+
writeRunnerLease(lease);
35+
const calls: string[] = [];
36+
const cleanup: RunnerLeaseCleanupAdapter = {
37+
cleanupRunnerProcessTree: async (_pid, signal) => {
38+
calls.push(signal);
39+
},
40+
cleanupRunnerXcodebuildProcesses: async () => {
41+
calls.push('xcodebuild');
42+
},
43+
confirmRunnerTermination: async () => {
44+
calls.push('confirm');
45+
return confirmed;
46+
},
47+
enableIosSimulatorAppAccessibility: async (device) => {
48+
assert.equal(currentRunnerLeaseOwnerToken(device.id), lease.ownerToken);
49+
assert.equal(device.appleOs, 'ios');
50+
assert.equal(device.simulatorSetPath, '/tmp/ax-simlock-set');
51+
calls.push('repair');
52+
},
53+
cleanupTempFile: () => {
54+
calls.push('temp');
55+
},
56+
};
57+
await cleanupRunnerLeasesForOwner({ pid: lease.ownerPid }, cleanup);
58+
assert.deepEqual(calls, [
59+
'SIGTERM',
60+
'xcodebuild',
61+
'SIGKILL',
62+
'confirm',
63+
...(confirmed ? ['repair'] : []),
64+
'temp',
65+
'temp',
66+
]);
67+
assert.equal(currentRunnerLeaseOwnerToken(lease.deviceId), null);
68+
});
69+
}
70+
71+
test('failed repair emits a diagnostic and still releases the lease', async () => {
72+
const lease = leaseFor(IOS_SIMULATOR);
73+
writeRunnerLease(lease);
74+
const diagnostic = vi.fn();
75+
appleRunnerTestHost.update({ emitDiagnostic: diagnostic });
76+
await cleanupRunnerLeasesForOwner(
77+
{ pid: lease.ownerPid },
78+
{
79+
cleanupRunnerProcessTree: async () => {},
80+
cleanupRunnerXcodebuildProcesses: async () => {},
81+
confirmRunnerTermination: async () => true,
82+
enableIosSimulatorAppAccessibility: async () => {
83+
throw new Error('repair unavailable');
84+
},
85+
cleanupTempFile: () => {},
86+
},
87+
);
88+
assert.ok(
89+
diagnostic.mock.calls.some(
90+
([event]) => event.phase === 'ios_runner_accessibility_repair_failed',
91+
),
92+
);
93+
assert.equal(currentRunnerLeaseOwnerToken(lease.deviceId), null);
94+
});
95+
96+
for (const device of [
97+
IOS_SIMULATOR,
98+
IOS_DEVICE,
99+
MACOS_DEVICE,
100+
TVOS_SIMULATOR,
101+
{ ...IOS_SIMULATOR, appleOs: 'visionos' },
102+
] as DeviceInfo[]) {
103+
test(`legacy or excluded ${device.kind}/${device.appleOs} lease never repairs`, async () => {
104+
const lease = leaseFor(device);
105+
if (device === IOS_SIMULATOR) {
106+
delete lease.simulatorTarget;
107+
lease.simulatorSetPath = '/tmp/legacy-set';
108+
} else assert.equal(lease.simulatorTarget, undefined);
109+
writeRunnerLease(lease);
110+
const prepare = vi.fn();
111+
await cleanupRunnerLeasesForOwner(
112+
{ pid: lease.ownerPid },
113+
{
114+
cleanupRunnerProcessTree: async () => {},
115+
cleanupRunnerXcodebuildProcesses: async () => {},
116+
confirmRunnerTermination: async () => true,
117+
enableIosSimulatorAppAccessibility: prepare,
118+
cleanupTempFile: () => {},
119+
},
120+
);
121+
assert.equal(prepare.mock.calls.length, 0);
122+
assert.equal(currentRunnerLeaseOwnerToken(lease.deviceId), null);
123+
});
124+
}
125+
126+
test('owner cleanup skips a lease replaced while waiting for the ownership fence', async () => {
127+
const lease = leaseFor(IOS_SIMULATOR);
128+
writeRunnerLease(lease);
129+
const repair = vi.fn();
130+
const terminate = vi.fn();
131+
let cleanup: Promise<void> | undefined;
132+
await withRunnerLeaseLock(lease.deviceId, async () => {
133+
cleanup = cleanupRunnerLeasesForOwner(
134+
{ pid: lease.ownerPid },
135+
{
136+
cleanupRunnerProcessTree: terminate,
137+
cleanupRunnerXcodebuildProcesses: async () => {},
138+
confirmRunnerTermination: async () => true,
139+
enableIosSimulatorAppAccessibility: repair,
140+
cleanupTempFile: () => {},
141+
},
142+
);
143+
writeRunnerLease({ ...lease, ownerToken: 'successor-token' });
144+
});
145+
await cleanup;
146+
assert.equal(terminate.mock.calls.length, 0);
147+
assert.equal(repair.mock.calls.length, 0);
148+
assert.equal(currentRunnerLeaseOwnerToken(lease.deviceId), 'successor-token');
149+
});
150+
151+
test('owner cleanup still releases other leases when one device lease lock is contended', async () => {
152+
const busy = leaseFor(IOS_SIMULATOR);
153+
const free = leaseFor({ ...IOS_SIMULATOR, id: 'second-simulator-udid' });
154+
writeRunnerLease(busy);
155+
writeRunnerLease(free);
156+
const diagnostic = vi.fn();
157+
appleRunnerTestHost.update({ emitDiagnostic: diagnostic });
158+
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] });
159+
let releaseBusyLock: () => void = () => {};
160+
const heldLock = withRunnerLeaseLock(
161+
busy.deviceId,
162+
() => new Promise<void>((resolve) => (releaseBusyLock = resolve)),
163+
);
164+
try {
165+
await vi.advanceTimersByTimeAsync(0);
166+
const cleanup = cleanupRunnerLeasesForOwner(
167+
{ pid: busy.ownerPid },
168+
{
169+
cleanupRunnerProcessTree: async () => {},
170+
cleanupRunnerXcodebuildProcesses: async () => {},
171+
confirmRunnerTermination: async () => true,
172+
enableIosSimulatorAppAccessibility: async () => {},
173+
cleanupTempFile: () => {},
174+
},
175+
);
176+
await vi.advanceTimersByTimeAsync(31_000);
177+
await cleanup;
178+
assert.equal(currentRunnerLeaseOwnerToken(free.deviceId), null);
179+
assert.ok(
180+
diagnostic.mock.calls.some(
181+
([event]) =>
182+
event.phase === 'ios_runner_owner_cleanup_lease_lock_unavailable' &&
183+
event.data.deviceId === busy.deviceId,
184+
),
185+
);
186+
} finally {
187+
releaseBusyLock();
188+
vi.useRealTimers();
189+
await heldLock.catch(() => {});
190+
}
191+
});

‎packages/apple-runner/src/__tests__/runner-lease-claim-takeover.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ function recordingCleanupAdapter(): RunnerLeaseCleanupAdapter & { calls: string[
5353
const calls: string[] = [];
5454
return {
5555
calls,
56+
confirmRunnerTermination: async () => true,
57+
enableIosSimulatorAppAccessibility: async () => {},
5658
cleanupRunnerProcessTree: async (_pid, signal) => {
5759
calls.push(`process-tree:${signal}`);
5860
},

‎packages/apple-runner/src/__tests__/runner-lease-recycled-pid.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ function recordingCleanup(): {
7272
return {
7373
signalledPids,
7474
adapter: {
75+
confirmRunnerTermination: async () => true,
76+
enableIosSimulatorAppAccessibility: async () => {},
7577
cleanupRunnerProcessTree: async (pid) => {
7678
signalledPids.push(pid);
7779
},

0 commit comments

Comments
 (0)