Skip to content

Commit d2eb93d

Browse files
committed
test(server-auth): stop three tests from racing the runner instead of the clock
Three separate causes behind the windows-latest failures in this file, each of which reads as the same flake from the summary line. Two tests still built a fresh startPoolRetryHarness per case inside a loop — five and six of them. Each one wipes and recreates TEST_DIR, binds a server, and redirects global fetch, which does not fit Bun's 5s default on a Windows runner. Worse, the request still in flight when the budget expired raced the next test through that same global fetch, which is how an unrelated test failed at 412ms in an earlier run. Both now use one harness with a body cursor, the shape already applied to their sibling, and each case still proves its own original 400 and exactly one acct-pool-a dispatch. WS-REBIND-01 was not a budget overrun at all. It failed at 748ms because it carries its own 1s deadline for a two-hop WebSocket retry that stops and restarts a real server. An internal deadline dying faster than the test budget is exactly what made this look random. That 1s becomes 15s and the test gets an explicit 30s budget. The harness also never reset the WebSocket registry, which is process-global and survives teardown, while WS-REBIND-01 asserts exact per-account socket counts. Five kinds of state were cleared and this one was not. Reset it with the rest — defensive here rather than a reproduced failure, and labelled as such. Ablated: no-oping updateCodexWebSocketAuthContext fails WS-REBIND-01, so the added headroom did not hollow out the migration proof.
1 parent 2eb7499 commit d2eb93d

1 file changed

Lines changed: 41 additions & 17 deletions

File tree

tests/server-auth.test.ts

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { request as httpRequest } from "node:http";
55
import { tmpdir } from "node:os";
66
import { join } from "node:path";
77
import { saveCodexAccountCredential } from "../src/codex/account-store";
8-
import { getTrackedCodexWebSocketCountForAccount } from "../src/codex/websocket-registry";
8+
import { clearCodexWebSocketRegistry, getTrackedCodexWebSocketCountForAccount } from "../src/codex/websocket-registry";
99
import { clearAccountNeedsReauth, clearAccountQuota, getAccountQuota, isAccountNeedsReauth, markAccountNeedsReauth, updateAccountQuota } from "../src/codex/auth-api";
1010
import {
1111
CODEX_THREAD_AFFINITY_IDLE_TTL_MS,
@@ -162,6 +162,12 @@ async function startPoolRetryHarness(
162162
clearAccountQuota();
163163
clearAccountNeedsReauth("pool-a");
164164
clearAccountNeedsReauth("pool-b");
165+
// The registry is process-global and survives a harness teardown. WS-REBIND-01
166+
// asserts exact per-account socket counts, so a socket leaked by any earlier test
167+
// in this file shifts its snapshots and fails it in milliseconds — which reads as
168+
// a flake next to the timeouts, but is ordinary shared state. Reset it with the
169+
// rest rather than leaving one of six kinds of state uncleaned.
170+
clearCodexWebSocketRegistry();
165171

166172
const dispatches: string[] = [];
167173
const upstream = Bun.serve({
@@ -1984,15 +1990,18 @@ describe("server local API auth", () => {
19841990
`${exact} suffix`,
19851991
exact.replace("ChatGPT account.", "ChatGPT account"),
19861992
];
1987-
for (const detail of cases) {
1988-
const body = JSON.stringify({ detail });
1989-
const harness = await startPoolRetryHarness(() => rejectionResponse(body));
1990-
try {
1991-
await expectOriginal400(await harness.request(), body);
1992-
expect(harness.dispatches).toEqual(["acct-pool-a"]);
1993-
} finally {
1994-
await stopPoolRetryHarness(harness);
1993+
const negativeBodies = cases.map(detail => JSON.stringify({ detail }));
1994+
let nextNegative = 0;
1995+
const negative = await startPoolRetryHarness(() => rejectionResponse(negativeBodies[nextNegative++]!));
1996+
try {
1997+
for (const body of negativeBodies) {
1998+
const priorDispatches = negative.dispatches.length;
1999+
await expectOriginal400(await negative.request(), body);
2000+
expect(negative.dispatches.slice(priorDispatches)).toEqual(["acct-pool-a"]);
19952001
}
2002+
expect(nextNegative).toBe(negativeBodies.length);
2003+
} finally {
2004+
await stopPoolRetryHarness(negative);
19962005
}
19972006
const positive = await startPoolRetryHarness(accountId => accountId === "acct-pool-a"
19982007
? rejectionResponse(JSON.stringify({ detail: ` THE '${POOL_RETRY_MODEL}' MODEL IS NOT SUPPORTED\nWHEN USING CODEX WITH A CHATGPT ACCOUNT. ` }))
@@ -2006,14 +2015,23 @@ describe("server local API auth", () => {
20062015
}, 12_000);
20072016

20082017
test("valid JSON wrong top-level shape never authorizes a pool retry", async () => {
2009-
for (const body of ['"string"', "42", "true", "null", '["detail"]']) {
2010-
const harness = await startPoolRetryHarness(() => rejectionResponse(body));
2011-
try {
2018+
// One harness, five bodies — same reason as the sibling above. Each
2019+
// startPoolRetryHarness() wipes and recreates TEST_DIR, binds a server, and
2020+
// redirects global fetch; five of those did not fit Bun's 5s default on a
2021+
// Windows runner, and the request still in flight when the budget expired
2022+
// raced the next test through that same global fetch.
2023+
const bodies = ['"string"', "42", "true", "null", '["detail"]'];
2024+
let nextBody = 0;
2025+
const harness = await startPoolRetryHarness(() => rejectionResponse(bodies[nextBody++]!));
2026+
try {
2027+
for (const body of bodies) {
2028+
const priorDispatches = harness.dispatches.length;
20122029
await expectOriginal400(await harness.request(), body);
2013-
expect(harness.dispatches).toEqual(["acct-pool-a"]);
2014-
} finally {
2015-
await stopPoolRetryHarness(harness);
2030+
expect(harness.dispatches.slice(priorDispatches)).toEqual(["acct-pool-a"]);
20162031
}
2032+
expect(nextBody).toBe(bodies.length);
2033+
} finally {
2034+
await stopPoolRetryHarness(harness);
20172035
}
20182036
});
20192037

@@ -2085,7 +2103,13 @@ describe("server local API auth", () => {
20852103
if (String(event.data).includes("response.completed")) resolve();
20862104
});
20872105
ws.addEventListener("error", () => reject(new Error("websocket retry failed")), { once: true });
2088-
setTimeout(() => reject(new Error("websocket retry timed out")), 1_000);
2106+
// This 1s was the real cause of WS-REBIND-01 failing at 748ms on windows-latest:
2107+
// an internal deadline, not Bun's test budget, which is why it died far too fast
2108+
// to look like a timeout. The test stops and restarts a real server, opens a real
2109+
// WebSocket, and waits for a two-hop retry across accounts — a second of that on a
2110+
// contended runner is optimistic. The assertions below are unchanged; only the
2111+
// room to reach them grew.
2112+
setTimeout(() => reject(new Error("websocket retry timed out")), 15_000);
20892113
});
20902114
expect(harness.dispatches).toEqual(["acct-pool-a", "acct-pool-b"]);
20912115
expect(registrySnapshots).toEqual([[1, 0], [0, 1]]);
@@ -2095,7 +2119,7 @@ describe("server local API auth", () => {
20952119
ws.close();
20962120
await stopPoolRetryHarness(harness);
20972121
}
2098-
});
2122+
}, { timeout: 30_000 });
20992123

21002124
test("passthrough connect failure records selected pool account health", async () => {
21012125
if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true });

0 commit comments

Comments
 (0)