From 94b609abf6f6d3289681bd22997e3555bfdee248 Mon Sep 17 00:00:00 2001 From: JUN Date: Sat, 12 Sep 2026 21:30:15 +0900 Subject: [PATCH] test(cli): stop judging a stale record on a port another test can take The fallback-port fixture records a port from allocateFreePort, which reports the port it has already released. On a four-shard runner every other test binding an ephemeral port is a candidate to take it, and when that happens status finds a listener on the recorded port, reports the record as live, and the assertion fails against something the test never set up. It failed exactly that way on the preview promotion run while the same commit passed on dev. Confirm the recorded port refuses immediately before and immediately after the status probe, and re-allocate when something took it in between. The assertion is unchanged and no weaker: a run only counts when the endpoint demonstrably refused across the whole probe, and exhausting the attempts fails with that reason rather than silently passing. --- tests/cli/cli-status-json.test.ts | 44 +++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 8 deletions(-) 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);