Skip to content

Commit 58531a0

Browse files
lifraryclaude
andcommitted
test: probe for a free pid instead of assuming 4242 is dead
Nine sites across three suites stood in for an exited process with a hardcoded pid: const deadPid = process.pid === 4242 ? 4243 : 4242; The code under test asks the kernel whether that owner is still alive, so the pid is only dead until an unrelated process happens to hold it. Then production answers correctly, the test reads that as a miss, and the failure looks like a defect in the feature rather than in the fixture. That is not hypothetical. On the macOS host where this was found, pid 4242 was `liveactivitiesd`, and five tests failed together on a clean `dev` checkout: `periodic reclaim frees abandoned temps without any continuation access`, both `doctor reclaim wiring (end to end)` cases, and both `status reports stale process records end to end` cases. The three files together went 186 pass / 5 fail before this change and 191 pass / 0 fail after it, with pid 4242 still held by `liveactivitiesd` across both runs. The probe already existed. `tests/responses-state.test.ts` did it inline for one test, with a comment naming this exact hazard on a shared CI runner, while four sites in the same file and four more in `tests/cli-status-json.test.ts` and `tests/doctor.test.ts` kept the assumption. This lifts that probe into `tests/helpers/dead-pid.ts` and uses it at every site, so the knowledge lives in one place rather than in a comment beside one of nine copies. The helper throws rather than returning a sentinel: the inline version needed `expect(deadPid).toBeGreaterThan(0)` at its call site, and a throw gives every caller that guarantee without repeating the assertion. ESRCH is the only accepted answer — a successful `kill(pid, 0)` means alive and EPERM means alive but owned by somebody else. Other `4242` literals in the suite are injected fixture data read through mocked accessors, never probed against the kernel, and are left alone. Verified on macOS: bun run typecheck clean, bun run privacy:scan passed, bun run test 16514 pass / 0 fail across 998 files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4180067 commit 58531a0

4 files changed

Lines changed: 44 additions & 23 deletions

File tree

tests/cli-status-json.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { tmpdir } from "node:os";
77
import { dirname, join } from "node:path";
88
import { fileURLToPath } from "node:url";
99
import { isConnectionRefused, isUncleanExitEvidence, proxyHealthFailureReason, resolveStatusPid, selectListenTarget } from "../src/cli/status";
10+
import { findDeadPid } from "./helpers/dead-pid";
1011

1112
const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url)));
1213
const cliPath = join(repoRoot, "src", "cli", "index.ts");
@@ -431,7 +432,7 @@ describe("unclean prior exit evidence", () => {
431432
describe("status reports stale process records end to end", () => {
432433
const seed = (home: string, opts: { pid?: number; runtime?: boolean; port: number }): void => {
433434
writeFileSync(join(home, "config.json"), JSON.stringify({ port: opts.port, codexAutoStart: false }), "utf8");
434-
const pid = opts.pid ?? (process.pid === 4242 ? 4243 : 4242);
435+
const pid = opts.pid ?? findDeadPid();
435436
if (opts.pid !== 0) writeFileSync(join(home, "ocx.pid"), String(pid), "utf8");
436437
if (opts.runtime) {
437438
writeFileSync(join(home, "runtime-port.json"), JSON.stringify({ pid, port: opts.port, hostname: "127.0.0.1" }), "utf8");
@@ -519,7 +520,7 @@ describe("status reports stale process records end to end", () => {
519520
await new Promise<void>(resolve => { occupied.listen(0, "127.0.0.1", () => resolve()); });
520521
const occupiedPort = (occupied.address() as AddressInfo).port;
521522
try {
522-
const pid = process.pid === 4242 ? 4243 : 4242;
523+
const pid = findDeadPid();
523524
writeFileSync(join(home, "config.json"), JSON.stringify({ port: occupiedPort, codexAutoStart: false }), "utf8");
524525
writeFileSync(join(home, "ocx.pid"), String(pid), "utf8");
525526
writeFileSync(join(home, "runtime-port.json"), JSON.stringify({ pid, port: freePort, hostname: "127.0.0.1" }), "utf8");

tests/doctor.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
LOCAL_MANAGEMENT_READ_PATHS,
3131
verifyLocalManagementReadCapability,
3232
} from "../src/lib/local-management-capability";
33+
import { findDeadPid } from "./helpers/dead-pid";
3334

3435
const TEST_DIR = join(import.meta.dir, ".tmp-doctor-test");
3536
const TEST_CODEX_HOME = join(TEST_DIR, "codex");
@@ -802,7 +803,7 @@ describe("doctor reclaim wiring (end to end)", () => {
802803
});
803804

804805
const seedStaleTemp = (): string => {
805-
const deadPid = process.pid === 4242 ? 4243 : 4242;
806+
const deadPid = findDeadPid();
806807
const path = join(tempHome, `responses-state.json.ocx.${deadPid}.1.tmp`);
807808
writeFileSync(path, "abandoned snapshot");
808809
const old = new Date(Date.now() - 48 * 60 * 60 * 1_000);
@@ -867,7 +868,7 @@ describe("doctor reports an unclean prior proxy exit", () => {
867868
const deadPid = (): number => {
868869
const spawned = spawnSync(process.execPath, ["-e", ""], { encoding: "utf8" });
869870
const pid = spawned.pid;
870-
return typeof pid === "number" && pid > 0 ? pid : (process.pid === 4242 ? 4243 : 4242);
871+
return typeof pid === "number" && pid > 0 ? pid : findDeadPid();
871872
};
872873

873874
// Port 9 is the discard port: nothing listens, so the health probe is refused rather

tests/helpers/dead-pid.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* A pid that is genuinely free, probed rather than assumed.
3+
*
4+
* Several suites need a pid that stands in for a process that has exited: stale
5+
* `ocx.pid` records, abandoned response-state temps, doctor's reclaim paths. The
6+
* code under test asks the kernel whether that owner is still alive, so a
7+
* hardcoded "dead" pid is only dead until some unrelated process happens to hold
8+
* it — and then the production code answers correctly, the test reads that as a
9+
* miss, and the failure looks like a defect in the feature.
10+
*
11+
* That is not hypothetical. On the macOS host where this helper was written, pid
12+
* 4242 was `liveactivitiesd`, and every suite that assumed it dead failed at once:
13+
* `periodic reclaim frees abandoned temps`, both `doctor reclaim wiring` cases and
14+
* both `status reports stale process records` cases. `tests/responses-state.test.ts`
15+
* already probed for a free pid inline, with a comment naming this exact hazard;
16+
* this helper is that probe, shared instead of copied.
17+
*
18+
* ESRCH is the only answer that proves absence. A successful `kill(pid, 0)` means
19+
* the process is alive, and EPERM means it is alive but owned by somebody else —
20+
* both disqualify the candidate.
21+
*/
22+
export function findDeadPid(): number {
23+
for (let candidate = 4242; candidate < 5242; candidate += 1) {
24+
if (candidate === process.pid) continue;
25+
try {
26+
process.kill(candidate, 0);
27+
} catch (error) {
28+
if ((error as NodeJS.ErrnoException).code === "ESRCH") return candidate;
29+
}
30+
}
31+
throw new Error("no free pid in [4242, 5242) to stand in for a dead owner");
32+
}

tests/responses-state.test.ts

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
22
import { BULK_DURABLE_IO_BUDGET_MS } from "./helpers/test-budget";
3+
import { findDeadPid } from "./helpers/dead-pid";
34
import {
45
closeSync,
56
existsSync,
@@ -1586,7 +1587,7 @@ describe("Responses previous_response_id state", () => {
15861587

15871588
test("recovers only old response-state temps owned by dead processes", () => {
15881589
const old = new Date(Date.now() - 60 * 60 * 1_000);
1589-
const deadPid = process.pid === 4242 ? 4243 : 4242;
1590+
const deadPid = findDeadPid();
15901591
const stale = join(home, `responses-state.json.ocx.${deadPid}.1.tmp`);
15911592
const live = join(home, "responses-state.json.ocx.5252.2.tmp");
15921593
const current = join(home, `responses-state.json.ocx.${process.pid}.3.tmp`);
@@ -1620,21 +1621,7 @@ describe("Responses previous_response_id state", () => {
16201621
symlinkSync(realSnapshot, join(home, "responses-state.json"));
16211622

16221623
// This test drives the REAL load path, whose sweep probes live pids with kill(pid, 0).
1623-
// A hardcoded "dead" pid can collide with a live process on a shared CI runner, so
1624-
// probe for a genuinely dead one instead (ESRCH). EPERM means alive-but-not-ours.
1625-
let deadPid = -1;
1626-
for (let candidate = 4242; candidate < 5242; candidate++) {
1627-
if (candidate === process.pid) continue;
1628-
try {
1629-
process.kill(candidate, 0);
1630-
} catch (error) {
1631-
if ((error as NodeJS.ErrnoException).code === "ESRCH") {
1632-
deadPid = candidate;
1633-
break;
1634-
}
1635-
}
1636-
}
1637-
expect(deadPid).toBeGreaterThan(0);
1624+
const deadPid = findDeadPid();
16381625
const stranded = join(realDir, `responses-state.json.ocx.${deadPid}.1.tmp`);
16391626
writeFileSync(stranded, "private state");
16401627
const old = new Date(Date.now() - 60 * 60 * 1_000);
@@ -1648,7 +1635,7 @@ describe("Responses previous_response_id state", () => {
16481635
});
16491636

16501637
test("stale temp recovery is best-effort when unlink fails", () => {
1651-
const deadPid = process.pid === 4242 ? 4243 : 4242;
1638+
const deadPid = findDeadPid();
16521639
const path = join(home, `responses-state.json.ocx.${deadPid}.1.tmp`);
16531640
writeFileSync(path, "private state");
16541641
const old = new Date(Date.now() - 60 * 60 * 1_000);
@@ -1713,7 +1700,7 @@ describe("Responses previous_response_id state", () => {
17131700
// schedulePersist site sits downstream of, so a process had its only look BEFORE it
17141701
// wrote anything. Here nothing touches the continuation store at all.
17151702
const old = new Date(Date.now() - 60 * 60 * 1_000);
1716-
const deadPid = process.pid === 4242 ? 4243 : 4242;
1703+
const deadPid = findDeadPid();
17171704
const stale = join(home, `responses-state.json.ocx.${deadPid}.1.tmp`);
17181705
const young = join(home, "responses-state.json.ocx.6262.4.tmp");
17191706
for (const path of [stale, young]) writeFileSync(path, "private state");
@@ -1811,7 +1798,7 @@ describe("Responses previous_response_id state", () => {
18111798
// Report and reclaim must share one predicate. If they drift, doctor tells an operator
18121799
// to reclaim files it will then refuse to touch (or vice versa).
18131800
const old = new Date(Date.now() - 60 * 60 * 1_000);
1814-
const deadPid = process.pid === 4242 ? 4243 : 4242;
1801+
const deadPid = findDeadPid();
18151802
const stale = join(home, `responses-state.json.ocx.${deadPid}.1.tmp`);
18161803
const live = join(home, "responses-state.json.ocx.5252.2.tmp");
18171804
const young = join(home, "responses-state.json.ocx.6262.3.tmp");

0 commit comments

Comments
 (0)