Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions tests/cli-status-json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { isConnectionRefused, isUncleanExitEvidence, proxyHealthFailureReason, resolveStatusPid, selectListenTarget } from "../src/cli/status";
import { findDeadPid } from "./helpers/dead-pid";

const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url)));
const cliPath = join(repoRoot, "src", "cli", "index.ts");
Expand Down Expand Up @@ -431,7 +432,7 @@ describe("unclean prior exit evidence", () => {
describe("status reports stale process records end to end", () => {
const seed = (home: string, opts: { pid?: number; runtime?: boolean; port: number }): void => {
writeFileSync(join(home, "config.json"), JSON.stringify({ port: opts.port, codexAutoStart: false }), "utf8");
const pid = opts.pid ?? (process.pid === 4242 ? 4243 : 4242);
const pid = opts.pid ?? findDeadPid();
if (opts.pid !== 0) writeFileSync(join(home, "ocx.pid"), String(pid), "utf8");
if (opts.runtime) {
writeFileSync(join(home, "runtime-port.json"), JSON.stringify({ pid, port: opts.port, hostname: "127.0.0.1" }), "utf8");
Expand Down Expand Up @@ -519,7 +520,7 @@ describe("status reports stale process records end to end", () => {
await new Promise<void>(resolve => { occupied.listen(0, "127.0.0.1", () => resolve()); });
const occupiedPort = (occupied.address() as AddressInfo).port;
try {
const pid = process.pid === 4242 ? 4243 : 4242;
const pid = findDeadPid();
writeFileSync(join(home, "config.json"), JSON.stringify({ port: occupiedPort, codexAutoStart: false }), "utf8");
writeFileSync(join(home, "ocx.pid"), String(pid), "utf8");
writeFileSync(join(home, "runtime-port.json"), JSON.stringify({ pid, port: freePort, hostname: "127.0.0.1" }), "utf8");
Expand Down
5 changes: 3 additions & 2 deletions tests/doctor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
LOCAL_MANAGEMENT_READ_PATHS,
verifyLocalManagementReadCapability,
} from "../src/lib/local-management-capability";
import { findDeadPid } from "./helpers/dead-pid";

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

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

// Port 9 is the discard port: nothing listens, so the health probe is refused rather
Expand Down
32 changes: 32 additions & 0 deletions tests/helpers/dead-pid.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* A pid that is genuinely free, probed rather than assumed.
*
* Several suites need a pid that stands in for a process that has exited: stale
* `ocx.pid` records, abandoned response-state temps, doctor's reclaim paths. The
* code under test asks the kernel whether that owner is still alive, so a
* hardcoded "dead" pid is only dead until some unrelated process happens to hold
* it — and then the production code answers correctly, the test reads that as a
* miss, and the failure looks like a defect in the feature.
*
* That is not hypothetical. On the macOS host where this helper was written, pid
* 4242 was `liveactivitiesd`, and every suite that assumed it dead failed at once:
* `periodic reclaim frees abandoned temps`, both `doctor reclaim wiring` cases and
* both `status reports stale process records` cases. `tests/responses-state.test.ts`
* already probed for a free pid inline, with a comment naming this exact hazard;
* this helper is that probe, shared instead of copied.
*
* ESRCH is the only answer that proves absence. A successful `kill(pid, 0)` means
* the process is alive, and EPERM means it is alive but owned by somebody else —
* both disqualify the candidate.
*/
export function findDeadPid(): number {
for (let candidate = 4242; candidate < 5242; candidate += 1) {
if (candidate === process.pid) continue;
try {
process.kill(candidate, 0);
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ESRCH") return candidate;
}
}
throw new Error("no free pid in [4242, 5242) to stand in for a dead owner");
}
25 changes: 6 additions & 19 deletions tests/responses-state.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import { BULK_DURABLE_IO_BUDGET_MS } from "./helpers/test-budget";
import { findDeadPid } from "./helpers/dead-pid";
import {
closeSync,
existsSync,
Expand Down Expand Up @@ -2176,7 +2177,7 @@ describe("Responses previous_response_id state", () => {

test("recovers only old response-state temps owned by dead processes", () => {
const old = new Date(Date.now() - 60 * 60 * 1_000);
const deadPid = process.pid === 4242 ? 4243 : 4242;
const deadPid = findDeadPid();
const stale = join(home, `responses-state.json.ocx.${deadPid}.1.tmp`);
const live = join(home, "responses-state.json.ocx.5252.2.tmp");
const current = join(home, `responses-state.json.ocx.${process.pid}.3.tmp`);
Expand Down Expand Up @@ -2210,21 +2211,7 @@ describe("Responses previous_response_id state", () => {
symlinkSync(realSnapshot, join(home, "responses-state.json"));

// This test drives the REAL load path, whose sweep probes live pids with kill(pid, 0).
// A hardcoded "dead" pid can collide with a live process on a shared CI runner, so
// probe for a genuinely dead one instead (ESRCH). EPERM means alive-but-not-ours.
let deadPid = -1;
for (let candidate = 4242; candidate < 5242; candidate++) {
if (candidate === process.pid) continue;
try {
process.kill(candidate, 0);
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ESRCH") {
deadPid = candidate;
break;
}
}
}
expect(deadPid).toBeGreaterThan(0);
const deadPid = findDeadPid();
const stranded = join(realDir, `responses-state.json.ocx.${deadPid}.1.tmp`);
writeFileSync(stranded, "private state");
const old = new Date(Date.now() - 60 * 60 * 1_000);
Expand All @@ -2238,7 +2225,7 @@ describe("Responses previous_response_id state", () => {
});

test("stale temp recovery is best-effort when unlink fails", () => {
const deadPid = process.pid === 4242 ? 4243 : 4242;
const deadPid = findDeadPid();
const path = join(home, `responses-state.json.ocx.${deadPid}.1.tmp`);
writeFileSync(path, "private state");
const old = new Date(Date.now() - 60 * 60 * 1_000);
Expand Down Expand Up @@ -2303,7 +2290,7 @@ describe("Responses previous_response_id state", () => {
// schedulePersist site sits downstream of, so a process had its only look BEFORE it
// wrote anything. Here nothing touches the continuation store at all.
const old = new Date(Date.now() - 60 * 60 * 1_000);
const deadPid = process.pid === 4242 ? 4243 : 4242;
const deadPid = findDeadPid();
const stale = join(home, `responses-state.json.ocx.${deadPid}.1.tmp`);
const young = join(home, "responses-state.json.ocx.6262.4.tmp");
for (const path of [stale, young]) writeFileSync(path, "private state");
Expand Down Expand Up @@ -2401,7 +2388,7 @@ describe("Responses previous_response_id state", () => {
// Report and reclaim must share one predicate. If they drift, doctor tells an operator
// to reclaim files it will then refuse to touch (or vice versa).
const old = new Date(Date.now() - 60 * 60 * 1_000);
const deadPid = process.pid === 4242 ? 4243 : 4242;
const deadPid = findDeadPid();
const stale = join(home, `responses-state.json.ocx.${deadPid}.1.tmp`);
const live = join(home, "responses-state.json.ocx.5252.2.tmp");
const young = join(home, "responses-state.json.ocx.6262.3.tmp");
Expand Down
Loading