Skip to content

Commit 47efaf1

Browse files
committed
test(ios): route the recycled-pid test through exec.ts and off the real lease tree (#1644 review)
Two fixes to the same test: - Process execution goes through utils/exec.ts's runCmdBackground, per AGENTS.md's hard rule, instead of node:child_process spawn. The kill-induced `wait` rejection is owned deliberately. - The lease root defaults to the developer's REAL ~/.agent-device tree, so the test was writing leases there. It now redirects AGENT_DEVICE_IOS_RUNNER_LEASE_DIR to a temp dir per test and restores it, writing nothing outside the sandbox. (Verified by inspecting and cleaning the leases the earlier revision left behind.)
1 parent 151be49 commit 47efaf1

1 file changed

Lines changed: 25 additions & 7 deletions

File tree

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

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import assert from 'node:assert/strict';
2-
import { spawn, type ChildProcess } from 'node:child_process';
3-
import { afterEach, test } from 'vitest';
2+
import { afterEach, beforeEach, test } from 'vitest';
43
import { mkdtempForTestSync } from '../../../../__tests__/test-utils/tmp-dir.ts';
4+
import { runCmdBackground, type ExecBackgroundResult } from '../../../../utils/exec.ts';
55
import { readProcessStartTime } from '../../../../utils/host-process.ts';
66
import {
77
RUNNER_OWNER_TOKEN,
@@ -42,11 +42,26 @@ import {
4242
* pinning the read is the point rather than a workaround.
4343
*/
4444

45-
let child: ChildProcess | undefined;
45+
// Process execution goes through utils/exec.ts, never node:child_process
46+
// directly (AGENTS.md hard rule) — including in tests.
47+
let child: ExecBackgroundResult | undefined;
48+
let previousLeaseDir: string | undefined;
49+
50+
beforeEach(() => {
51+
// Leases default to the REAL ~/.agent-device tree; redirect them so the
52+
// suite never writes into the developer's own runner state.
53+
previousLeaseDir = process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR;
54+
process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR = mkdtempForTestSync('agent-device-lease-root-');
55+
});
4656

4757
afterEach(() => {
48-
child?.kill('SIGKILL');
58+
child?.child.kill('SIGKILL');
4959
child = undefined;
60+
if (previousLeaseDir === undefined) {
61+
delete process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR;
62+
} else {
63+
process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR = previousLeaseDir;
64+
}
5065
});
5166

5267
function recordingCleanup(): {
@@ -67,8 +82,11 @@ function recordingCleanup(): {
6782
}
6883

6984
async function spawnRealChild(): Promise<{ pid: number; startTime: string | null }> {
70-
child = spawn('sleep', ['30'], { stdio: 'ignore' });
71-
const pid = child.pid;
85+
child = runCmdBackground('sleep', ['30'], { stdio: 'ignore', captureOutput: false });
86+
// The cleanup SIGKILL below makes `wait` reject; own that here so it is a
87+
// deliberate no-op rather than an unhandled rejection.
88+
child.wait.catch(() => {});
89+
const pid = child.child.pid;
7290
assert.ok(pid, 'the OS gave the child a pid');
7391
// `readProcessStartTime` shells out to `ps`, which under full-suite CPU
7492
// contention can miss its own timeout and return null. The budget is
@@ -130,7 +148,7 @@ test('a lease whose runner pid was recycled is never signalled', async () => {
130148
[undefined, undefined],
131149
'both the SIGTERM and SIGKILL passes refuse the unverified pid',
132150
);
133-
assert.equal(child?.killed, false, 'the real process was left alone');
151+
assert.equal(child?.child.killed, false, 'the real process was left alone');
134152
// Guards the assertion above against passing for the wrong reason: cleanup
135153
// must actually have run (a lease that classified as anything but `owned`
136154
// would also produce no signals).

0 commit comments

Comments
 (0)