diff --git a/.github/scripts/pr-quality-state.test.cjs b/.github/scripts/pr-quality-state.test.cjs index 131205e4b5..cf68abc827 100644 --- a/.github/scripts/pr-quality-state.test.cjs +++ b/.github/scripts/pr-quality-state.test.cjs @@ -795,6 +795,41 @@ describe("durable readiness re-attestation", () => { assert.equal(result.pending.checkpointAt, null); }); + it("accepts a delayed author event when the live head and body remain unchanged", () => { + const pending = { version: 1, headSha: HEAD_A, baseRef: "dev", generation: 2, phase: "await-clear", checkpointAt: CHECKPOINT }; + const result = advanceReattestation({ + pending, + legacy: false, + current: true, + readiness: readiness(0), + live: live(body0, { updatedAt: "2026-09-22T09:00:01.000Z" }), + event: authorEdit(body0, body4), + }); + assert.equal(result.pending.phase, "await-check"); + assert.equal(result.pending.checkpointAt, null); + }); + + it("rejects future author events and missing or invalid live timestamps", () => { + const pending = { version: 1, headSha: HEAD_A, baseRef: "dev", generation: 2, phase: "await-clear", checkpointAt: CHECKPOINT }; + for (const [name, liveUpdatedAt, eventUpdatedAt] of [ + ["future author event", LIVE_TIME, "2026-09-22T01:00:02.000Z"], + ["missing live timestamp", undefined, LIVE_TIME], + ["invalid live timestamp", "not-a-time", LIVE_TIME], + ]) { + const result = advanceReattestation({ + pending, + legacy: false, + current: true, + readiness: readiness(0), + live: live(body0, { updatedAt: liveUpdatedAt }), + event: authorEdit(body0, body4, { updatedAt: eventUpdatedAt }), + }); + assert.equal(result.pending.phase, "await-clear", name); + assert.equal(result.changed, false, name); + assert.equal(result.canComplete, false, name); + } + }); + it("rejects equal timestamps, title-only edits, and stale or reordered payloads", () => { const pending = { version: 1, headSha: HEAD_A, baseRef: "dev", generation: 2, phase: "await-clear", checkpointAt: CHECKPOINT }; const cases = [ diff --git a/.github/scripts/pr-readiness-reattest.cjs b/.github/scripts/pr-readiness-reattest.cjs index 59576cfb32..776eaa1ce6 100644 --- a/.github/scripts/pr-readiness-reattest.cjs +++ b/.github/scripts/pr-readiness-reattest.cjs @@ -115,6 +115,10 @@ function samePending(left, right) { function qualifyingAuthorBodyEdit({ live, event, checkpointAt }) { const checkpointMs = Date.parse(checkpointAt); const eventMs = Date.parse(event?.updatedAt ?? ""); + // GitHub can advance the live PR timestamp after the author event arrives. + // Do not cap the lag: a delayed event still proves this author's post-checkpoint + // edit when the exact body and head are unchanged at the live read. + const liveMs = Date.parse(live?.updatedAt ?? ""); return Boolean( event?.name === "pull_request_target" && event.action === "edited" && @@ -123,9 +127,8 @@ function qualifyingAuthorBodyEdit({ live, event, checkpointAt }) { event.headSha === live.headSha && typeof event.body === "string" && event.body === live.body && typeof event.previousBody === "string" && event.previousBody !== event.body && - event.updatedAt === live.updatedAt && - Number.isFinite(checkpointMs) && Number.isFinite(eventMs) && - eventMs > checkpointMs + Number.isFinite(checkpointMs) && Number.isFinite(eventMs) && Number.isFinite(liveMs) && + eventMs > checkpointMs && eventMs <= liveMs ); } diff --git a/tests/ci-workflows/pr-readiness-reattest.test.ts b/tests/ci-workflows/pr-readiness-reattest.test.ts index e7afad46d6..d6da586dc9 100644 --- a/tests/ci-workflows/pr-readiness-reattest.test.ts +++ b/tests/ci-workflows/pr-readiness-reattest.test.ts @@ -90,6 +90,19 @@ describe("author-applied policy migration with durable re-attestation", () => { expect(pending(saved(retick, T5))).toBeNull(); }); + test("clear edit survives a live PR timestamp eight hours later", async () => { + const first = await initialize(); + const laterComment = "2026-09-22T08:00:08Z"; + const result = await runEnforcePrTarget(script, { + pr: { body: body(CURRENT, 0), draft: true, head: { sha: HEAD }, updated_at: "2026-09-22T08:00:06Z" }, + eventPayload: { body: body(CURRENT, 0), draft: true, head: { sha: HEAD }, updated_at: T3 }, + eventAction: "edited", previousBody: body(OLD), comments: [first], commentUpdatedAt: laterComment, + }); + expect(pending(saved(result, laterComment)).phase).toBe("await-check"); + expect(promotions(result)).toEqual([]); + expect(bodyWrites(result)).toEqual([]); + }); + test("identical pending replay does not duplicate notices or mutate author content", async () => { const first = await initialize(); const replay = await runEnforcePrTarget(script, { diff --git a/tests/images/download-connect-deadline-default.test.ts b/tests/images/download-connect-deadline-default.test.ts index 959e787d3e..0774e9ed94 100644 --- a/tests/images/download-connect-deadline-default.test.ts +++ b/tests/images/download-connect-deadline-default.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, mock, test } from "bun:test"; +import { afterAll, describe, expect, mock, test } from "bun:test"; // The default downloader inside connectPublicHttps is the production path for // provider-returned image/video URLs (downloadImageToArtifact and @@ -6,6 +6,17 @@ import { describe, expect, mock, test } from "bun:test"; // production callers. A connect deadline wired only into pinnedHttpsGet would // therefore never arm in production — this suite pins the default path. +// `mock.module` outlives this file: Bun keeps both overrides below for every file that +// runs after this one in the same process, including download-cap-default's own capture of +// the "real" modules and tests/lib's pinned-http suites (#5439). Keep the real modules, +// captured before anything here is mocked, and put them back. +const realDns = { ...(await import("node:dns/promises")) }; +const realPinnedHttp = { ...(await import("../../src/lib/pinned-http")) }; +afterAll(() => { + mock.module("node:dns/promises", () => realDns); + mock.module("../../src/lib/pinned-http", () => realPinnedHttp); +}); + const lookupMock = mock(async (): Promise<{ address: string; family: number }[]> => [ { address: "93.184.216.34", family: 4 }, ]); diff --git a/tests/server/cancel-body-on-abort.test.ts b/tests/server/cancel-body-on-abort.test.ts index 24f1df143c..19b7e032dd 100644 --- a/tests/server/cancel-body-on-abort.test.ts +++ b/tests/server/cancel-body-on-abort.test.ts @@ -5,11 +5,17 @@ import { handleResponses } from "../../src/server/responses"; import type { OcxConfig } from "../../src/types"; import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; +// Captured once so a test that never finishes (Bun's per-test timeout) cannot leave the stubbed +// fetch installed for every later file in the same process. +const REAL_FETCH = globalThis.fetch; + let releaseSpendHome: (() => void) | undefined; afterEach(() => { // Release the lease before later teardown can replace the preload sandbox home. releaseSpendHome?.(); releaseSpendHome = undefined; + // Bun runs afterEach even when a test times out, so this is the restore that survives a hang. + globalThis.fetch = REAL_FETCH; }); function bodyWithCancelSpy(): { body: ReadableStream; cancelled: () => boolean } { @@ -79,8 +85,14 @@ describe("readBodyCapped settles the stream when a read throws", () => { const requestAbort = new AbortController(); const events: string[] = []; let rejectRead!: (reason: unknown) => void; + let readReached = false; let markReadStarted!: () => void; - const readStarted = new Promise(resolve => { markReadStarted = resolve; }); + const readStarted = new Promise(resolve => { + markReadStarted = () => { + readReached = true; + resolve(); + }; + }); const reader = { read(): Promise> { @@ -139,7 +151,23 @@ describe("readBodyCapped settles the stream when a read throws", () => { body: "offer", signal: requestAbort.signal, }), config, { model: "", provider: "" }); - await readStarted; + // Race the start signal against the handler settling: a handler that returns an error + // response without ever fetching would otherwise hang here until the harness kills the test. + await Promise.race([ + readStarted, + pending.then( + async settled => { + if (readReached) return; + throw new Error( + `handleLive settled before reaching fetch: status ${settled.status} ${(await settled.text()).slice(0, 300)}`, + ); + }, + (reason: unknown) => { + if (readReached) return; + throw new Error(`handleLive settled before reaching fetch: ${reason instanceof Error ? reason.message : String(reason)}`); + }, + ), + ]); requestAbort.abort(new DOMException("client closed request", "AbortError")); expect((await pending).status).toBe(499); diff --git a/tests/server/companion-settings.test.ts b/tests/server/companion-settings.test.ts index cdda72200b..7798790935 100644 --- a/tests/server/companion-settings.test.ts +++ b/tests/server/companion-settings.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, mock, test } from "bun:test"; +import { afterAll, describe, expect, mock, test } from "bun:test"; import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -10,12 +10,18 @@ import { } from "../../src/companion/settings"; import type { OcxConfig } from "../../src/types"; +// `mock.module` outlives this file: Bun keeps the override below for every file that runs after +// this one in the same process. This is a spread snapshot of the real module, taken before it. +const realOpenUrl = { ...(await import("../../src/lib/open-url")) }; const opened: string[] = []; mock.module("../../src/lib/open-url", () => ({ openUrl: (url: string) => { opened.push(url); }, })); +afterAll(() => { // Put the real module back for every later file in the same process. + mock.module("../../src/lib/open-url", () => realOpenUrl); +}); const { resetCompanionPresenceForTests } = await import("../../src/server/management/companion-routes"); const { handleManagementAPI } = await import("../../src/server/management-api"); diff --git a/tests/server/context-history.test.ts b/tests/server/context-history.test.ts index b44c144ab9..ece8d07291 100644 --- a/tests/server/context-history.test.ts +++ b/tests/server/context-history.test.ts @@ -1,4 +1,3 @@ -// mock.module replacements require file isolation (bun test --isolate). import { describe, test, expect, mock, beforeEach, afterAll } from "bun:test"; import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -33,6 +32,15 @@ let outgoingAccount = "test-only"; let accountMode = "pool"; let validated=0; let probe=false;let released=0;let directError=false;let duringSelection:(()=>void)|undefined; +// `mock.module` outlives this file: Bun keeps all six overrides below for every file that runs +// after this one in the same process. Keep the real modules, captured before anything here is +// mocked, and put them back. +const realAuthContext = { ...(await import("../../src/codex/auth-context")) }; +const realRouting = { ...(await import("../../src/codex/routing")) }; +const realSidecar = { ...(await import("../../src/providers/openai-sidecar")) }; +const realAuthCors = { ...(await import("../../src/server/auth-cors")) }; +const realResponses = { ...(await import("../../src/server/responses")) }; +const realLifecycle = { ...(await import("../../src/server/lifecycle")) }; const errors = { CodexAccountCooldownError: class extends Error {}, CodexMainSubstitutionUnavailableError: class extends Error {}, @@ -61,7 +69,6 @@ mock.module("../../src/codex/auth-context",()=>({ cooldownErrorResponse:()=>new Response("cooldown",{status:429}), codexMainProfileDrainingResponse:()=>new Response("draining",{status:503}), })); -const realRouting = await import("../../src/codex/routing"); mock.module("../../src/codex/routing",()=>({...realRouting, formatCodexProviderForLog:()=>"openai-test"})); mock.module("../../src/providers/openai-sidecar",()=>({listOpenAiForwardSidecarCandidates:()=>[{providerName:"openai",provider:{baseUrl:"https://chatgpt.com/backend-api/codex"},accountMode}]})); class ForwardAdmissionCredentialError extends Error {} @@ -92,7 +99,14 @@ const originalFetch=globalThis.fetch; function setFetch(handler: (input: string | URL | Request, init?: RequestInit) => Promise): void { globalThis.fetch = Object.assign(handler, { preconnect: originalFetch.preconnect }); } -afterAll(()=>{if(previousCodexHome===undefined)delete process.env.CODEX_HOME;else process.env.CODEX_HOME=previousCodexHome;resetContextRelayActivationForTests();clearContextSessionOwnersForTests();globalThis.fetch=originalFetch;mock.restore();}); +afterAll(()=>{try{if(previousCodexHome===undefined)delete process.env.CODEX_HOME;else process.env.CODEX_HOME=previousCodexHome;resetContextRelayActivationForTests();clearContextSessionOwnersForTests();globalThis.fetch=originalFetch;mock.restore();}finally{ + mock.module("../../src/codex/auth-context",()=>realAuthContext); + mock.module("../../src/codex/routing",()=>realRouting); + mock.module("../../src/providers/openai-sidecar",()=>realSidecar); + mock.module("../../src/server/auth-cors",()=>realAuthCors); + mock.module("../../src/server/responses",()=>realResponses); + mock.module("../../src/server/lifecycle",()=>realLifecycle); +}}); beforeEach(()=>{clearContextSessionOwnersForTests();for (const id of ["root", "root-test", "s"]) seedOwner(id);outgoingAccount="test-only";globalThis.fetch=originalFetch;materialized=undefined;materializationError=undefined;selection=undefined;validated=0;materializationOptions=undefined;outgoingBearer="test-only";accountMode="pool";probe=false;released=0;directError=false;duringSelection=undefined;setContextFeature(true);}); describe("context relay contract",()=>{ diff --git a/tests/server/management-api-logs-metrics.test.ts b/tests/server/management-api-logs-metrics.test.ts index 27f9091557..46335170bc 100644 --- a/tests/server/management-api-logs-metrics.test.ts +++ b/tests/server/management-api-logs-metrics.test.ts @@ -46,6 +46,9 @@ let testDir = ""; let previousHome: string | undefined; beforeEach(() => { + // The request log is process-wide: start empty so the first case does not read a row an + // earlier file left behind (a one-process tests/server run handed it a Kiro entry). + clearRequestLogsForTests(); // addRequestLog persists to usage.jsonl; without a scratch OPENCODEX_HOME a bare // `bun test ` run from outside the repo (no bunfig preload) writes these // fixture rows into the real ~/.opencodex log and poisons the GUI Usage page. diff --git a/tests/server/server-combo-failover-e2e.test.ts b/tests/server/server-combo-failover-e2e.test.ts index f4904cb38d..1fd65f1030 100644 --- a/tests/server/server-combo-failover-e2e.test.ts +++ b/tests/server/server-combo-failover-e2e.test.ts @@ -4,7 +4,7 @@ import { comboProviderFactory } from "../helpers/combo-provider"; import { registerComboContextOverflowCases } from "../helpers/combo-context-overflow-cases"; import { registerComboContextHeadroomCases } from "../helpers/combo-context-headroom-cases"; import { sessionLaneIdFromRequest } from "../../src/server/request-log-conversation"; -import { afterEach, beforeEach, describe, expect, mock, setDefaultTimeout, test } from "bun:test"; +import { afterAll, afterEach, beforeEach, describe, expect, mock, setDefaultTimeout, test } from "bun:test"; import { logsFromApiBody } from "../helpers/logs-api"; import { managementFetch as fetch, ManagementRequest as Request } from "../helpers/management-auth"; import { mkdtempSync } from "node:fs"; @@ -58,9 +58,11 @@ import { captureConfigGeneration } from "../../src/lib/state-store-sweeper"; // default 5s per-test budget (same flake class as 810fa115 / claude-management-api). setDefaultTimeout(30_000); -const actualResolver = await import("../../src/server/adapter-resolve"); +// `mock.module` outlives this file: Bun keeps both overrides below for every file that runs after +// this one in the same process. These are spread snapshots of the real modules, taken before them. +const actualResolver = { ...(await import("../../src/server/adapter-resolve")) }; const actualResolveAdapter = actualResolver.resolveAdapter; -const actualRetry = await import("../../src/lib/upstream-retry"); +const actualRetry = { ...(await import("../../src/lib/upstream-retry")) }; const actualFetchWithTransientRetry = actualRetry.fetchWithTransientRetry; const { createCursorAdapter } = await import("../../src/adapters/cursor"); import type { CursorTransportFactory } from "../../src/adapters/cursor/transport"; @@ -130,6 +132,11 @@ mock.module("../../src/lib/upstream-retry", () => ({ }, })); +afterAll(() => { // Put the real modules back for every later file in the same process. + mock.module("../../src/server/adapter-resolve", () => actualResolver); + mock.module("../../src/lib/upstream-retry", () => actualRetry); +}); + const { handleResponses } = await import("../../src/server/responses"); const { handleResponsesCompact } = await import("../../src/server/responses/compact"); type HandleOptions = NonNullable[3]>; diff --git a/tests/server/server-combo-zero-output-failover.test.ts b/tests/server/server-combo-zero-output-failover.test.ts index b4056f6920..72afd27a81 100644 --- a/tests/server/server-combo-zero-output-failover.test.ts +++ b/tests/server/server-combo-zero-output-failover.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, mock, setDefaultTimeout, test } from "bun:test"; +import { afterAll, afterEach, beforeEach, describe, expect, mock, setDefaultTimeout, test } from "bun:test"; import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -20,7 +20,9 @@ import { import type { ProviderAdapter } from "../../src/adapters/base"; import type { AdapterEvent, OcxConfig, OcxProviderConfig } from "../../src/types"; -const actualResolver = await import("../../src/server/adapter-resolve"); +// `mock.module` outlives this file: Bun keeps the override below for every file that runs after +// this one in the same process. This is a spread snapshot of the real module, taken before it. +const actualResolver = { ...(await import("../../src/server/adapter-resolve")) }; const actualResolveAdapter = actualResolver.resolveAdapter; let customRunTurn: NonNullable | undefined; @@ -44,6 +46,10 @@ mock.module("../../src/server/adapter-resolve", () => ({ }, })); +afterAll(() => { // Put the real module back for every later file in the same process. + mock.module("../../src/server/adapter-resolve", () => actualResolver); +}); + const { handleResponses } = await import("../../src/server/responses"); /** diff --git a/tests/server/server-key-failover-e2e.test.ts b/tests/server/server-key-failover-e2e.test.ts index aedce1306b..ea964c0eb0 100644 --- a/tests/server/server-key-failover-e2e.test.ts +++ b/tests/server/server-key-failover-e2e.test.ts @@ -37,7 +37,22 @@ let previousHome: string | undefined; let isolatedCodexHome: IsolatedCodexHome | null = null; let upstream: ReturnType | null = null; +// A test that hangs until Bun's per-test timeout never runs its own `finally`, so a server +// started here would outlive the test still holding the process-wide spend-ledger lease — every +// later file in the same process then fails to start its own server. Track them for the sweep. +const trackedServers = new Set>(); +// Same reason for fetch: several cases stub it and restore it only in their own `finally`. +const REAL_FETCH = globalThis.fetch; + +function startTrackedServer(port = 0): ReturnType { + const server = startServer(port); + trackedServers.add(server); + return server; +} + beforeEach(() => { + // Do not inherit a pacing runtime swapped in by an earlier file or an earlier timed-out test. + resetProviderRequestPacingForTest(); previousHome = process.env.OPENCODEX_HOME; isolatedCodexHome = installIsolatedCodexHome("ocx-keyfail-e2e-codex-"); testDir = mkdtempSync(join(tmpdir(), "ocx-keyfail-e2e-")); @@ -48,6 +63,16 @@ beforeEach(() => { }); afterEach(async () => { + // Reap before anything else: a server that survives this file keeps the spend-ledger lease. + // Every stop is attempted even when an earlier one rejects, and the failure is raised only + // after the rest of this cleanup has run. Re-stopping a server a test already stopped is safe. + const stopFailures: unknown[] = []; + for (const server of trackedServers) { + trackedServers.delete(server); + try { await server.stop(true); } catch (error) { stopFailures.push(error); } + } + resetProviderRequestPacingForTest(); + globalThis.fetch = REAL_FETCH; await upstream?.stop(true); upstream = null; await flushNativeMainStartupReleases(); @@ -63,6 +88,7 @@ afterEach(async () => { clearKeyCooldowns(); clearReasoningReplayCacheForTests(); clearBridgeSearchReplayCacheForTests(); + if (stopFailures.length > 0) throw new AggregateError(stopFailures, "tracked server stop failed in afterEach"); }); describe("server 429 key failover (end-to-end)", () => { @@ -133,7 +159,7 @@ describe("server 429 key failover (end-to-end)", () => { requestPacing: { enabled: true, minIntervalMs: 100 }, } } } as OcxConfig; saveConfig(config); - const server = startServer(0); + const server = startTrackedServer(); const abort = new AbortController(); try { await waitForProviderRequestSlot("paced", config.providers.paced); @@ -185,7 +211,7 @@ describe("server 429 key failover (end-to-end)", () => { { id: "first", key: "${OCX_SELECTION_E2E_KEY}" }, { id: "second", key: "synthetic-second" }, ], } } } as OcxConfig); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL(`/v1/${inbound}`, server.url), { method: "POST", headers: { "content-type": "application/json" }, @@ -249,7 +275,7 @@ describe("server 429 key failover (end-to-end)", () => { }, } as OcxConfig; saveConfig(config); - server = startServer(0); + server = startTrackedServer(); const res = await originalFetch(new URL("/v1/responses", server.url), { method: "POST", headers: { "content-type": "application/json" }, @@ -336,7 +362,7 @@ describe("server 429 key failover (end-to-end)", () => { }, } as OcxConfig; saveConfig(config); - server = startServer(0); + server = startTrackedServer(); const res = await originalFetch(new URL("/v1/responses", server.url), { method: "POST", headers: { "content-type": "application/json" }, @@ -412,7 +438,7 @@ describe("server 429 key failover (end-to-end)", () => { }, } as OcxConfig; saveConfig(config); - const server = startServer(0); + const server = startTrackedServer(); try { const res = await fetch(new URL(surface === "chat" ? "/v1/chat/completions" : "/v1/responses", server.url), { method: "POST", @@ -466,7 +492,7 @@ describe("server 429 key failover (end-to-end)", () => { baseUrl: `http://127.0.0.1:${upstream.port}/v1`, allowPrivateNetwork: true, apiKey: "synthetic-first", apiKeyPool: [{ id: "first", key: "synthetic-first" }, { id: "second", key: "synthetic-second" }] } }, } as OcxConfig); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/responses", server.url), { method: "POST", headers: { "content-type": "application/json" }, @@ -502,7 +528,7 @@ describe("server 429 key failover (end-to-end)", () => { metered: { adapter, authMode: "key", apiKey: "synthetic-key", allowPrivateNetwork: true, baseUrl: `http://127.0.0.1:${upstream.port}` }, } } as OcxConfig); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/responses", server.url), { method: "POST", headers: { "content-type": "application/json" }, @@ -558,7 +584,7 @@ describe("server 429 key failover (end-to-end)", () => { }, } as OcxConfig; saveConfig(config); - const server = startServer(0); + const server = startTrackedServer(); try { const res = await fetch(new URL("/v1/responses", server.url), { method: "POST", @@ -707,7 +733,7 @@ describe("server 429 key failover (end-to-end)", () => { }, } as OcxConfig; saveConfig(config); - const server = startServer(0); + const server = startTrackedServer(); const headers = { "content-type": "application/json", "x-codex-parent-thread-id": "thread-key-rotation", @@ -790,7 +816,7 @@ describe("server 429 key failover (end-to-end)", () => { }, } as OcxConfig; saveConfig(config); - const server = startServer(0); + const server = startTrackedServer(); try { const res = await fetch(new URL("/v1/responses", server.url), { method: "POST", @@ -836,7 +862,7 @@ describe("server 429 key failover (end-to-end)", () => { }, } as OcxConfig; saveConfig(config); - const server = startServer(0); + const server = startTrackedServer(); try { const res = await fetch(new URL("/v1/responses", server.url), { method: "POST", @@ -901,7 +927,7 @@ describe("server 429 key failover (end-to-end)", () => { }, } as OcxConfig; saveConfig(config); - const server = startServer(0); + const server = startTrackedServer(); try { const res = await fetch(new URL("/v1/responses", server.url), { method: "POST", @@ -951,7 +977,7 @@ describe("server 429 key failover (end-to-end)", () => { test("a cooled committed key is replaced before the first attempt", async () => { const seen = await cooledCommittedKeySetup("round-robin"); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/chat/completions", server.url), { method: "POST", headers: { "content-type": "application/json" }, @@ -969,7 +995,7 @@ describe("server 429 key failover (end-to-end)", () => { test("without a configured strategy the cooled key is still used", async () => { const seen = await cooledCommittedKeySetup(); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/chat/completions", server.url), { method: "POST", headers: { "content-type": "application/json" }, @@ -1027,7 +1053,7 @@ describe("server 429 key failover (end-to-end)", () => { const restored = loadConfig(); restored.providers["env-pooled"]!.apiKey = "\${OCX_KEYFAIL_COOLED}"; saveConfig(restored); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/responses", server.url), { method: "POST", headers: { "content-type": "application/json" }, @@ -1086,7 +1112,7 @@ test.each([false, true])("chat-native attributes same-key 429 usage then the rot apiKeyPool: [{ id: "one", key: "key-one" }, { id: "two", key: "key-two" }], retryOn429: { attempts: 1, intervalMs: 100, maxIntervalMs: 100, respectRetryAfter: false }, } } } as OcxConfig); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/chat/completions", server.url), { method: "POST", @@ -1139,7 +1165,7 @@ test("chat-native preserves same-key retry, key rotation, usage, and request log apiKeyPool: [{ id: "one", key: "key-one" }, { id: "two", key: "key-two" }], retryOn429: { attempts: 1, intervalMs: 100, maxIntervalMs: 100, respectRetryAfter: false }, } } } as OcxConfig); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/chat/completions", server.url), { method: "POST", @@ -1188,7 +1214,7 @@ test.each([false, true])("key refetch retains transient recovery metadata (strea apiKey: "synthetic-refetch-a", apiKeyPool: [{ id: "a", key: "synthetic-refetch-a" }, { id: "b", key: "synthetic-refetch-b" }], transientRetryOn5xx: { attempts: 3 }, retryOn429: { attempts: 0 }, } } } as OcxConfig); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/responses", server.url), { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ model: "refetch/test", input: "hello", stream }) }); @@ -1307,7 +1333,7 @@ test("a keyed caller's bridged search is restored on its next turn", async () => const config = bridgedReplayConfig(baseUrl, false); saveConfig(config); seedBridgedSearch(baseUrl, bridgeCallerPrincipal(config)); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/responses", server.url), bridgedReplayRequest(BRIDGE_CALLER_KEY)); expect(response.status).toBe(200); @@ -1329,7 +1355,7 @@ test("a keyless loopback caller neither restores nor shares a bridged search", a saveConfig(config); seedBridgedSearch(baseUrl, bridgeCallerPrincipal(config)); seedBridgedSearch(baseUrl, "loopback"); - const server = startServer(0); + const server = startTrackedServer(); try { const response = await fetch(new URL("/v1/responses", server.url), bridgedReplayRequest(undefined)); expect(response.status).toBe(200); @@ -1370,7 +1396,7 @@ test("a dispatch-time key switch rebuilds the bridged-search restore under the n // the request below carries, but the credential whose selection is about to lapse. seedBridgedSearch(baseUrl, bridgeCallerPrincipal(config)); - const server = startServer(0); + const server = startTrackedServer(); const abort = new AbortController(); try { await waitForProviderRequestSlot("pooled", config.providers.pooled); diff --git a/tests/server/server-live.test.ts b/tests/server/server-live.test.ts index 85f100ec3c..161f2fa349 100644 --- a/tests/server/server-live.test.ts +++ b/tests/server/server-live.test.ts @@ -2,7 +2,7 @@ * /v1/live relay: Codex App / ChatGPT voice POSTs call-create against the injected base_url, * so the proxy must relay it to an OpenAI upstream instead of the /v1/* JSON-404 guard. */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, onTestFinished, test } from "bun:test"; import { existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { saveCodexAccountCredential } from "../../src/codex/account-store"; @@ -609,83 +609,28 @@ test("call-create and its sideband join bind to the same pool account (openai/co }, { timeout: 20_000 }); test("sideband GET /v1/live/{callId} relays the exact frame ceiling bidirectionally", async () => { - // The peer is a helper so it can report what it saw: this case's only symptom on failure is - // its own deadline, which names neither the slow leg nor whether the reply was ever sent. + // The peer is a helper so it can report what it saw: this case's failure arrives as a timeout, + // which names neither the slow leg nor whether the reply was ever sent. const { server: upstream, seenPaths, seenUpgradeHeaders, probe } = sidebandRelayUpstream(MAX_WS_FRAME_BYTES); - // Created inside the try: a startServer throw used to leak the peer and the socket override. + // No inner deadline: the 50MiB transfer is the thing the assertions are about, so a wall clock + // over it would fail the contract for being the runner rather than the relay. The harness budget + // below is the only bound left, and a case it kills emits no failure message of its own, so + // cleanup lives on `onTestFinished` -- which runs however this case ends -- and reports the stall. let restoreWebSocket: (() => void) | undefined; let live: ReturnType | undefined; - try { - saveConfig(forwardConfig()); - // Redirect ChatGPT sideband targets to the local mock; the config stays canonical. A sibling - // helper because this case is at its size cap and the repo answer is not to compress. - const { OriginalWebSocket, restore } = redirectSidebandWebSocket(upstream.port); - restoreWebSocket = restore; - const server = live = startServer(0); - const client = openSidebandClient(OriginalWebSocket, server.url, "/v1/live/rtc_sideband", DIRECT_CHATGPT_TOKEN); - // The try opens here, one statement after the client exists, so the timer and the phase - // recorder are both created inside the block that closes them. - let phase: ReturnType | undefined; - let timer: ReturnType | undefined; - try { - // Each leg is its own segment and the timer's ticks read the peer's event count, so a - // segment that reaches no further milestone is visible as such. That is weaker than proof - // of a stall: a tick without movement says no milestone was reached, not that nothing moved. - phase = phaseTimer("sideband 50MiB exact frame ceiling", probe.progress); - const leg = phase; - await new Promise((resolve, reject) => { - const fail = (reason: string): void => reject(new Error(`${reason}; peer: ${probe.summary()}`)); - timer = setTimeout(() => fail("sideband timeout"), 15_000); - let stage: "echo-roundtrip" | "await-ceiling-echo" | "done" = "echo-roundtrip"; - leg.split("upgrade"); - client.addEventListener("open", () => { - probe.noteClient("open"); - leg.split("echo-roundtrip"); - client.send("ping-sideband"); - }); - client.addEventListener("close", event => { - probe.noteClient("close=" + event.code); - // ANY close before this case settles is its own outcome, not a deadline. Keying that - // on the first echo left the harder half unreported: a close after the ping and - // before the ceiling echo -- the disconnect a 50MiB frame is most likely to cause -- - // fell through to the 15s timeout and read as a slow peer. - if (stage !== "done") fail(`sideband closed during ${stage}`); - }); - client.addEventListener("message", (event) => { - try { - probe.noteClient("message"); - if (stage === "echo-roundtrip") { - expect(String(event.data)).toBe("echo:ping-sideband"); - leg.split("allocate-ceiling-frame"); - const frame = Buffer.alloc(MAX_WS_FRAME_BYTES); - // The send is synchronous, so allocating the frame, handing it to the socket and - // waiting for the acknowledgement are three separate costs. Timing them as one - // segment reported allocation time as wait time. - leg.split("send-ceiling-frame"); - client.send(frame); - stage = "await-ceiling-echo"; - leg.split("await-ceiling-echo"); - return; - } - expect(String(event.data)).toBe(`bytes:${MAX_WS_FRAME_BYTES}`); - expectSidebandUpgrade({ seenPaths, seenUpgradeHeaders }, "/v1/live/rtc_sideband", DIRECT_CHATGPT_TOKEN); - stage = "done"; - resolve(); - } catch (err) { - reject(err); - } - }); - client.addEventListener("error", () => fail("client websocket error")); - }); - } finally { - // The case owns both: leaving them for the runner to collect is a resource leak whatever - // it did or did not contribute to any particular deadline. - phase?.end(); - clearTimeout(timer); - client.close(); - } - } finally { + let client: WebSocket | undefined; + let phase: ReturnType | undefined; + // Read by the hook, so it outlives the promise whose handlers advance it. + let stage: "echo-roundtrip" | "await-ceiling-echo" | "done" = "echo-roundtrip"; + let cleanedUp = false; + const cleanup = async (): Promise => { + if (cleanedUp) return; + cleanedUp = true; + // What a timed-out case cannot otherwise print: the stage it stalled in and what the peer saw. + if (stage !== "done") console.info(`[sideband ceiling] ended during ${stage}; peer: ${probe.summary()}`); + phase?.end(); + client?.close(); restoreWebSocket?.(); // Nested so the peer is stopped even when stopping the proxy throws: one failed shutdown // must not leave the other listener running for every case after this one. @@ -694,7 +639,69 @@ test("sideband GET /v1/live/{callId} relays the exact frame ceiling bidirectiona } finally { await upstream.stop(true); } - } + }; + onTestFinished(cleanup); + + saveConfig(forwardConfig()); + // Redirect ChatGPT sideband targets to the local mock; the config stays canonical. A sibling + // helper because this case is at its size cap and the repo answer is not to compress. + const { OriginalWebSocket, restore } = redirectSidebandWebSocket(upstream.port); + restoreWebSocket = restore; + const server = live = startServer(0); + const ws = openSidebandClient(OriginalWebSocket, server.url, "/v1/live/rtc_sideband", DIRECT_CHATGPT_TOKEN); + client = ws; + // Each leg is its own segment and the timer's ticks read the peer's event count, so a + // segment that reaches no further milestone is visible as such. That is weaker than proof + // of a stall: a tick without movement says no milestone was reached, not that nothing moved. + phase = phaseTimer("sideband 50MiB exact frame ceiling", probe.progress); + const leg = phase; + // Settles on events only: the two messages, a close before `done`, a client error, or a throw. + await new Promise((resolve, reject) => { + // Silent once the hook owns teardown: a later rejection would be unhandled, not a result. + const fail = (reason: string): void => { + if (cleanedUp) return; + reject(new Error(`${reason}; peer: ${probe.summary()}`)); + }; + leg.split("upgrade"); + ws.addEventListener("open", () => { + probe.noteClient("open"); + leg.split("echo-roundtrip"); + ws.send("ping-sideband"); + }); + ws.addEventListener("close", event => { + probe.noteClient("close=" + event.code); + // ANY close before this case settles is its own outcome, not a deadline. Keying that + // on the first echo left the harder half unreported: a close after the ping and + // before the ceiling echo -- the disconnect a 50MiB frame is most likely to cause -- + // reached no message of its own and waited out the harness budget. + if (stage !== "done") fail(`sideband closed during ${stage}`); + }); + ws.addEventListener("message", (event) => { + try { + probe.noteClient("message"); + if (stage === "echo-roundtrip") { + expect(String(event.data)).toBe("echo:ping-sideband"); + leg.split("allocate-ceiling-frame"); + const frame = Buffer.alloc(MAX_WS_FRAME_BYTES); + // The send is synchronous, so allocating the frame, handing it to the socket and + // waiting for the acknowledgement are three separate costs. Timing them as one + // segment reported allocation time as wait time. + leg.split("send-ceiling-frame"); + ws.send(frame); + stage = "await-ceiling-echo"; + leg.split("await-ceiling-echo"); + return; + } + expect(String(event.data)).toBe(`bytes:${MAX_WS_FRAME_BYTES}`); + expectSidebandUpgrade({ seenPaths, seenUpgradeHeaders }, "/v1/live/rtc_sideband", DIRECT_CHATGPT_TOKEN); + stage = "done"; + resolve(); + } catch (err) { + if (!cleanedUp) reject(err); + } + }); + ws.addEventListener("error", () => fail("client websocket error")); + }); }, { timeout: 20_000 }); test("standalone GET /v1/realtime?intent=quicksilver&model= upgrades and relays bidirectionally", async () => { diff --git a/tests/server/server-stop-config-hardening.test.ts b/tests/server/server-stop-config-hardening.test.ts index 102292cc61..131decd40b 100644 --- a/tests/server/server-stop-config-hardening.test.ts +++ b/tests/server/server-stop-config-hardening.test.ts @@ -114,8 +114,12 @@ test("a rejected native-lifecycle release still drains the ACL flight before sto throw new Error("native release exploded"); }); let server: ReturnType | null = null; + // Kept separate and never nulled: the body nulls `server` to show stop() has settled, but + // the real native-lifecycle release still has to run against the object startServer returned. + let startedServer: ReturnType | null = null; try { server = startServer(0); + startedServer = server; let settled: "pending" | "rejected" | "resolved" = "pending"; let rejection: unknown; const stopping = server.stop(true).then(() => { settled = "resolved"; }, (error: unknown) => { settled = "rejected"; rejection = error; }); @@ -136,6 +140,11 @@ test("a rejected native-lifecycle release still drains the ACL flight before sto releaseSpy.mockRestore(); aclSpy.mockRestore(); if (server) await server.stop(true).catch(() => undefined); + if (startedServer) await nativeStartup.releaseNativeMainStartupLifecycle(startedServer); + // The spoofed win32 platform makes startServer take a process-wide ownership block, and the + // throwing spy meant it was never dropped: this case must not leave that gate blocked for + // every later test file sharing the process. + expect(nativeStartup.isNativeMainTrafficBlocked()).toBe(false); } }); diff --git a/tests/server/startup-action-control-elevation.test.ts b/tests/server/startup-action-control-elevation.test.ts index 66fdbff3e4..3b8129c9e5 100644 --- a/tests/server/startup-action-control-elevation.test.ts +++ b/tests/server/startup-action-control-elevation.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { afterAll, afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; import * as childProcess from "node:child_process"; import { WINDOWS_SCHTASKS_CREATE_ACCESS_DENIED_MARKER } from "../../src/lib/windows-elevation"; @@ -13,11 +13,19 @@ const execFileMock = mock(( const finalizeMock = mock(async () => ({ kind: "done" as const })); +// `mock.module` outlives this file: Bun keeps the override below for every file that runs after +// this one in the same process. This is a spread snapshot of the real module, taken before it. +const realChildProcess = { ...(await import("node:child_process")) }; + mock.module("node:child_process", () => ({ ...childProcess, execFile: execFileMock, })); +afterAll(() => { // Put the real module back for every later file in the same process. + mock.module("node:child_process", () => realChildProcess); +}); + const { classifyCliInstallFailure, clearStartupInstallPartialBlock, diff --git a/tests/server/system-routes.test.ts b/tests/server/system-routes.test.ts index 45907fb6ab..78b72f05c2 100644 --- a/tests/server/system-routes.test.ts +++ b/tests/server/system-routes.test.ts @@ -11,7 +11,7 @@ * zero across the suite would need a finalizer that aggregates many short-lived * sharded processes, which does not exist. This file covers the route. */ -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { handleManagementAPI } from "../../src/server/management-api"; import { @@ -57,6 +57,11 @@ function flakyIo(failures: number, code = "EBUSY", platform: NodeJS.Platform = " }; } +// The counters are process-wide: start from zero so an earlier file's retries are not read here. +beforeEach(() => { + resetWindowsReplaceRetryCountersForTests(); +}); + afterEach(() => { resetWindowsReplaceRetryCountersForTests(); });