diff --git a/tests/cli/cli-status-json.test.ts b/tests/cli/cli-status-json.test.ts index aeef546421..1c613ed376 100644 --- a/tests/cli/cli-status-json.test.ts +++ b/tests/cli/cli-status-json.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, spyOn, test } from "bun:test"; import { createHash } from "node:crypto"; import { spawnSync } from "node:child_process"; import { existsSync, mkdtempSync, readdirSync, readFileSync, writeFileSync, mkdirSync, unlinkSync } from "node:fs"; -import { createServer } from "node:net"; +import { createConnection, createServer } from "node:net"; import type { AddressInfo } from "node:net"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -879,6 +879,24 @@ describe("status reports stale process records end to end", () => { await new Promise(resolve => { probe.close(() => resolve()); }); return port; } + + /** + * Prove the endpoint refuses right now. + * + * `allocateFreePort` releases the port it reports, and on a sharded runner every other + * test binding an ephemeral port is a candidate to take it. A fixture that assumes a + * released port is still refusing asserts against whatever happened to bind it. + */ + async function refusesConnection(port: number): Promise { + return await new Promise(resolve => { + const socket = createConnection({ port, host: "127.0.0.1" }); + const settle = (refused: boolean): void => { socket.destroy(); resolve(refused); }; + socket.setTimeout(1_000); + socket.once("connect", () => settle(false)); + socket.once("timeout", () => settle(false)); + socket.once("error", () => settle(true)); + }); + } let freePort: number; beforeEach(async () => { freePort = await allocateFreePort(); }); @@ -950,17 +968,27 @@ describe("status reports stale process records end to end", () => { await new Promise(resolve => { occupied.listen(0, "127.0.0.1", () => resolve()); }); const occupiedPort = (occupied.address() as AddressInfo).port; try { - // Allocate after the listener is bound: it can reuse the port released by - // beforeEach, so that earlier number no longer proves a refused endpoint. - const recordedPort = await allocateFreePort(); - expect(recordedPort).not.toBe(occupiedPort); 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: recordedPort, hostname: "127.0.0.1" }), "utf8"); - const parsed = JSON.parse(runStatusJson(home).stdout) as { proxy?: { staleProcessState?: unknown } }; - expect(parsed.proxy?.staleProcessState).toBe(true); + // The recorded port has to refuse for this to discriminate, and `allocateFreePort` + // hands back a port it has already released. Confirm refusal immediately before and + // immediately after the probe, and re-allocate when something took it in between, so + // a stolen port retries instead of failing an assertion it never exercised. + let parsed: { proxy?: { staleProcessState?: unknown } } | undefined; + for (let attempt = 0; attempt < 5 && parsed === undefined; attempt++) { + const recordedPort = await allocateFreePort(); + if (recordedPort === occupiedPort) continue; + if (!await refusesConnection(recordedPort)) continue; + writeFileSync(join(home, "runtime-port.json"), JSON.stringify({ pid, port: recordedPort, hostname: "127.0.0.1" }), "utf8"); + const observed = JSON.parse(runStatusJson(home).stdout) as { proxy?: { staleProcessState?: unknown } }; + if (!await refusesConnection(recordedPort)) continue; + parsed = observed; + } + + expect(parsed, "no allocated port stayed refused across the status probe").toBeDefined(); + expect(parsed?.proxy?.staleProcessState).toBe(true); } finally { await new Promise(resolve => { occupied.close(() => resolve()); }); removeTreeWithRetry(home);