From c012322fb62237711158ce59818e693393461ace Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 17 Sep 2026 04:26:38 +0900 Subject: [PATCH 1/3] test(ci): make two fail-open environment skips hard preconditions in CI model-metadata-sync skipped its entire drift gate when scripts/model-metadata.source.json was absent. That snapshot is tracked in this repository, so an absent input is a broken checkout, not an environment variation, and the skip silently removed the only check that the committed src/generated/model-metadata.ts still matches its source. The precondition is now asserted. server-startup-reconcile-resilience probed Bun.serve and skipped four startup cases whenever the probe failed. That is right in a sandboxed agent environment that denies Bun.serve outright, and wrong in hosted CI, where a runner that cannot bind loopback is a broken runner and four assertions disappeared with no trace. The probe now suppresses cases only outside CI, and a CI-only guard case asserts the bind capability so a genuinely unbindable runner names itself. --- .../model-metadata-sync.test.ts | 12 +++++++- ...erver-startup-reconcile-resilience.test.ts | 29 ++++++++++++++----- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/tests/codex-integration/model-metadata-sync.test.ts b/tests/codex-integration/model-metadata-sync.test.ts index 47d7c5b8953..5fb05013e46 100644 --- a/tests/codex-integration/model-metadata-sync.test.ts +++ b/tests/codex-integration/model-metadata-sync.test.ts @@ -22,9 +22,19 @@ const GENERATED = repoPath("src/generated/model-metadata.ts"); const SOURCE = repoPath("scripts/model-metadata.source.json"); describe("generated model metadata stays in sync with its source", () => { - test.skipIf(!existsSync(SOURCE))( + test( "regenerating reproduces the committed file byte for byte", async () => { + // The snapshot is tracked in this repository, so a missing input is a broken checkout, + // not an environment variation. This used to be `test.skipIf(!existsSync(SOURCE))`, which + // meant the drift gate this file exists to provide disappeared without a trace the moment + // the input went missing — including in CI, where nothing else compares the generated file + // against its source. + expect( + existsSync(SOURCE), + `${SOURCE} is missing; the generator input is repository-owned, so this checkout is incomplete`, + ).toBe(true); + const outDir = mkdtempSync(join(tmpdir(), "model-metadata-sync-")); const outPath = join(outDir, "model-metadata.ts"); diff --git a/tests/server/server-startup-reconcile-resilience.test.ts b/tests/server/server-startup-reconcile-resilience.test.ts index 0969c63d5ce..1f7758956ea 100644 --- a/tests/server/server-startup-reconcile-resilience.test.ts +++ b/tests/server/server-startup-reconcile-resilience.test.ts @@ -33,7 +33,8 @@ import type { OcxConfig } from "../../src/types"; * Sandboxed agent environments deny `Bun.serve` outright ("Is port 0 in use?", EADDRINUSE on * every port), which is an environment artifact and not a regression — the same class already * documented for tests/server/server-combo-failover-e2e.test.ts. Probe once so the - * listener-bound assertion is hosted-CI-only while the boot-sequence assertions always run. + * listener-bound assertions degrade gracefully there while the boot-sequence assertions always + * run. The probe result only suppresses a case outside CI (see `SKIP_LISTENER`). * * The probe has to be `Bun.serve` itself: a `node:net` listener still binds in an environment * where Bun's does not, so probing with the wrong API reports a false green and the skip never @@ -49,9 +50,22 @@ function canBindLoopback(): boolean { } } +const IS_CI = process.env.CI === "true"; const CAN_BIND = canBindLoopback(); -test.skipIf(!CAN_BIND)("startServer persists the Astra-first legacy roster upgrade", async () => { +/** + * The graceful skip is for a restricted local sandbox only. In hosted CI a runner that cannot + * bind loopback is a broken runner, not an environment variation, and the unconditional + * `skipIf(!CAN_BIND)` deleted four startup assertions there with no trace in the summary. Under + * CI the cases run and fail on the real bind error instead. + */ +const SKIP_LISTENER = !CAN_BIND && !IS_CI; + +test.skipIf(!IS_CI)("hosted CI can bind a loopback listener for the startup cases", () => { + expect(CAN_BIND).toBe(true); +}); + +test.skipIf(SKIP_LISTENER)("startServer persists the Astra-first legacy roster upgrade", async () => { saveConfig({ ...staleConfig(), subagentModels: ["gpt-5.5", "gpt-5.6-sol", "gpt-5.6-terra", "gpt-5.6-luna", "gpt-5.4-mini"], @@ -66,7 +80,7 @@ test.skipIf(!CAN_BIND)("startServer persists the Astra-first legacy roster upgra } }); -test.skipIf(!CAN_BIND)("startServer migrates old Grok Chat choices once and preserves later opt-in", async () => { +test.skipIf(SKIP_LISTENER)("startServer migrates old Grok Chat choices once and preserves later opt-in", async () => { saveConfig({ ...staleConfig(), defaultProvider: "xai", providers: { xai: { @@ -93,7 +107,7 @@ test.skipIf(!CAN_BIND)("startServer migrates old Grok Chat choices once and pres } finally { await restarted.stop(true); } }); -test.skipIf(!CAN_BIND)("preset reconciliation cannot undo an in-memory Grok migration after its write fails", async () => { +test.skipIf(SKIP_LISTENER)("preset reconciliation cannot undo an in-memory Grok migration after its write fails", async () => { saveConfig({ ...staleConfig(), defaultProvider: "xai", providers: { xai: { @@ -205,9 +219,10 @@ test("a fresh install with no config file at all does not throw on the boot path } }); -// Hosted-CI-only: binds a listener, which sandboxed agent environments refuse (see -// canBindLoopback above). The three assertions above cover the same claim without a port. -test.skipIf(!CAN_BIND)( +// Binds a listener, which a sandboxed agent environment refuses (see canBindLoopback above); +// outside CI that skips this case and the three assertions above still cover the boot sequence +// without a port. In CI it always runs. +test.skipIf(SKIP_LISTENER)( "startServer completes and serves /healthz when the config disappears before reconcile", async () => { saveConfig(staleConfig()); From 0248a203cad556d7f1112cbd0a3b2a313612e538 Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 17 Sep 2026 04:30:10 +0900 Subject: [PATCH 2/3] test(storage): re-enable the isolate worker-teardown cases on Linux and macOS Four Worker-spawning cases were skipped everywhere except win32. The stated cause was real: Bun 1.3.14 segfaulted at 0xFFFFFFFFFFFFFFF8 mid-file with a balanced workers_spawned/workers_terminated count (exit 133 on macOS Silicon in run 30691129351, exit 132 on ubuntu GHA in run 30700011812), which is a runtime defect our JavaScript teardown cannot close. Bun 1.4.0, the version this repository pins, contains the upstream fix: worker threads are parent-owned and joined before the parent VM is destroyed, bun:sqlite and other native resources are torn down before JSC, and a termination gate keeps native callbacks out of a stopping worker (oven-sh/bun#37075, #38299). oven-sh/bun#38519 reproduces this exact class and records 3/3 crashes on 1.3.14 against 3 x 400 clean terminate cycles on 1.4.0. The skip is deleted rather than re-scoped, and the churn count is a single 8 on every platform: the one-cycle macOS and two-cycle Linux caps were crash avoidance, and a one-cycle 'repeated spawn/reset' case does not test what its name claims. The meta-test that pinned those per-platform caps goes with them. The OS-join settle in src/storage/worker-lifecycle.ts is unchanged. --- .../storage-worker-teardown-isolate.test.ts | 74 +++++++++---------- 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/tests/storage/storage-worker-teardown-isolate.test.ts b/tests/storage/storage-worker-teardown-isolate.test.ts index f5b6309cf15..82bf5e50045 100644 --- a/tests/storage/storage-worker-teardown-isolate.test.ts +++ b/tests/storage/storage-worker-teardown-isolate.test.ts @@ -1,22 +1,36 @@ /** - * Windows-style isolate teardown regression. + * Isolate teardown regression for the storage Bun Workers. * * Bun's `bun test --isolate` reclaims the file realm at the boundary. A storage * Bun Worker that is still exiting then trips * `panic: Internal assertion failure` with `workers_spawned(N) - * workers_terminated(N-1)` on Windows and kills the whole run. + * workers_terminated(N-1)` and kills the whole run (first seen on Windows: + * run 30613324981, Bun 1.3.14). * - * A second Bun 1.3.14 failure mode is a mid-file / post-suite segfault with a - * *balanced* `workers_spawned === workers_terminated` count (exit 132/133). - * Seen on macOS Silicon and ubuntu GHA even after the first green assertion in - * this file. Churn caps and short settles were not enough. Keep isolate - * everywhere; skip Worker-spawning hammers on non-Windows (platform-cap - * meta-test still runs); win32 keeps the regression for the original panic. - * OS-join settle stays in `worker-lifecycle` for win32/darwin. + * These four cases spent months quarantined off Linux and macOS because Bun + * 1.3.14 had a second failure mode our teardown could not close from + * JavaScript: a mid-file segfault at 0xFFFFFFFFFFFFFFF8 with a *balanced* + * `workers_spawned === workers_terminated` count (exit 133 on macOS Silicon, + * run 30691129351; exit 132 on ubuntu GHA, run 30700011812). The balanced count + * is what ruled out an unjoined worker of ours: Bun destroyed the VM while + * native work that had left the thread was still outstanding. + * + * Bun 1.4.0 — the version this repository pins (package.json `dependencies.bun`, + * consumed by .github/actions/setup-project-bun) — rewrote that lifetime model: + * worker threads are parent-owned and joined before the parent VM disappears, + * native resources including bun:sqlite are torn down before JSC is destroyed, + * and a termination gate stops native callbacks entering a stopping worker + * (oven-sh/bun#37075, #38299). oven-sh/bun#38519 is the matching reproduction: + * it crashed 3/3 on 1.3.14 and survived 3 × 400 terminate cycles on 1.4.0. The + * skip is therefore gone rather than re-scoped, and the churn count is one + * number on every platform again — the shrunken per-platform caps existed only + * to dodge the 1.3.14 crash, and a one-cycle "repeated spawn/reset" case does + * not test what its name claims. * * These cases hammer the exact failure window: fire-and-forget terminate must * still be joinable by drain, and repeated spawn → reset cycles must leave the - * registry empty before the next isolate boundary. + * registry empty before the next isolate boundary. The OS-join settle in + * `worker-lifecycle` stays: Bun's `close` event is not a thread-exit proof. */ import { afterAll, afterEach, beforeEach, expect, test } from "bun:test"; import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; @@ -43,23 +57,14 @@ let testDir = ""; let previousHome: string | undefined; /** - * Bun 1.3.14: Worker spawn in this file still segfaults the isolate process - * after green assertions with balanced counts on darwin and linux GHA. Skip - * hammers off Windows; win32 keeps full coverage for the original panic. + * Spawn/reset iterations for the heavy churn case. + * + * Eight is the count that originally reproduced `workers_spawned(9) + * workers_terminated(8)` on Windows, so it is the number that proves the + * registry drains between cycles. It is no longer platform-scaled: the smaller + * Linux and macOS caps were Bun 1.3.14 crash avoidance, not a cost decision. */ -const skipNonWindowsWorkerSpawn = process.platform !== "win32"; - -/** Spawn/reset iterations for the heavy churn case — platform-stressed carefully. */ -function workerChurnCyclesForIsolate(): number { - if (process.platform === "win32") return 8; - // Non-Windows hammers are skipped; keep caps documented for the meta-test. - // Two cycles still segfaulted Bun 1.3.14 on macOS Silicon under `--isolate` - // after a green suite (balanced worker counts, exit 133). One cycle keeps a - // real spawn/reset proof without the churn that trips the runtime. - if (process.platform === "darwin") return 1; - // Linux: short loop — eight cycles segfaulted with balanced counts on ubuntu CI. - return 2; -} +const WORKER_CHURN_CYCLES = 8; function seedArchived(codexHome: string): void { mkdirSync(join(codexHome, "archived_sessions"), { recursive: true }); @@ -103,7 +108,7 @@ async function waitForLiveWorker(timeoutMs = INTERNAL_DEADLINE_MS): Promise { +test("drain joins a fire-and-forget terminate before the isolate boundary", async () => { // Reproduces the old race: sync reset void-terminates (and used to deregister // immediately), then drain returned on an empty set while the thread exited. setStorageCleanupPolicyJobTestHooks({ blockMs: 800 }); @@ -121,8 +126,8 @@ test.skipIf(skipNonWindowsWorkerSpawn)("drain joins a fire-and-forget terminate expect(liveStorageWorkerCount()).toBe(0); }, { timeout: 30_000 }); -test.skipIf(skipNonWindowsWorkerSpawn)("repeated Windows-style spawn/reset cycles leave no live workers", async () => { - const cycles = workerChurnCyclesForIsolate(); +test("repeated spawn/reset cycles leave no live workers", async () => { + const cycles = WORKER_CHURN_CYCLES; for (let i = 0; i < cycles; i++) { // Fresh CODEX_HOME each cycle so a prior worker's SQLite handle cannot // leave the seed DB locked/EBUSY on Windows after terminate. @@ -142,7 +147,7 @@ test.skipIf(skipNonWindowsWorkerSpawn)("repeated Windows-style spawn/reset cycle } }, { timeout: 60_000 }); -test.skipIf(skipNonWindowsWorkerSpawn)("async beforeEach-style join between cycles leaves no live workers", async () => { +test("async beforeEach-style join between cycles leaves no live workers", async () => { // Mirrors storage-mutation-race: each case must await join before the next // spawn. A sync beforeEach reset used to fire-and-forget terminate and leave // workers_spawned(N) workers_terminated(N-1) for the next isolate reclaim. @@ -168,14 +173,7 @@ test.skipIf(skipNonWindowsWorkerSpawn)("async beforeEach-style join between cycl } }, { timeout: 60_000 }); -test("isolate worker churn stays platform-capped", () => { - const cycles = workerChurnCyclesForIsolate(); - if (process.platform === "win32") expect(cycles).toBe(8); - else if (process.platform === "darwin") expect(cycles).toBe(1); - else expect(cycles).toBe(2); -}); - -test.skipIf(skipNonWindowsWorkerSpawn)("terminateStorageWorker is joinable and idempotent across callers", async () => { +test("terminateStorageWorker is joinable and idempotent across callers", async () => { setStorageCleanupPolicyJobTestHooks({ blockMs: 500 }); seedArchived(isolatedCodexHome!.path); const started = requestStorageCleanupPolicyRun({ From 6d417ce6ead7428a14bac32b2aae5ef5ad3c5cfc Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 17 Sep 2026 04:34:13 +0900 Subject: [PATCH 3/3] fix(codex): carry the history busy timeout into the Worker and unskip the restore-busy case tests/codex-integration/codex-composed-acceptance.test.ts declared the restore-busy envelope a platform-independent contract and then skipped it on win32. The comment was right and the skip was wrong. Run 32344670867 shows what actually happened: 'CLI watchdog: ocx restore --json' at 45197 ms on a shard where neighbouring cases took 54-106 s. No envelope, no SQLite error, no failed assertion - the child was still waiting out production's own busy budget (5 s per attempt, two attempts, 500 ms apart) inside a real CLI process. That wait is not the assertion, so it is shortened rather than budgeted for. In-process history tests already do this with setHistoryDbBusyTimeoutForTests; a child process could not be reached that way, and neither could the history Worker, which is a separate module realm that starts from the provider's default. The run message now carries the parent realm's busy timeout the same way it already carries the homes, validated and refused when malformed, and the Worker adopts it before its first state_5.sqlite open. Production sends the same codex-rs-matching 5 s the Worker would have used on its own, so the happy path is unchanged. The test spawns that one child with a --preload that applies the knob only when OCX_TEST_HISTORY_BUSY_TIMEOUT_MS is set on its environment; no production module reads that variable. The lock, the two-attempt retry, the exit code, the exact JSON envelope, the byte-identity check, the release, and the convergence assertion are untouched. --- src/codex/history-job.ts | 6 ++- src/codex/history-provider.ts | 16 ++++++ src/codex/history-worker.ts | 15 +++++- .../codex-composed-acceptance.test.ts | 52 ++++++++++++++----- .../codex-history-worker.test.ts | 31 ++++++++++- tests/helpers/history-busy-timeout-preload.ts | 29 +++++++++++ 6 files changed, 132 insertions(+), 17 deletions(-) create mode 100644 tests/helpers/history-busy-timeout-preload.ts diff --git a/src/codex/history-job.ts b/src/codex/history-job.ts index 1f4e174c514..cfbabb5b62a 100644 --- a/src/codex/history-job.ts +++ b/src/codex/history-job.ts @@ -23,7 +23,7 @@ import type { CodexHistoryWorkerOperation, HistoryWorkerResult, } from "./history-worker"; -import { historyBackupPathFor } from "./history-provider"; +import { currentHistoryDbBusyTimeoutMs, historyBackupPathFor } from "./history-provider"; import type { CodexHistoryFailureReason, CodexHistoryVerifiedNoopProof } from "./history-provider"; import { getCodexHome, resolveCodexStateDbPath } from "./paths"; @@ -437,6 +437,10 @@ export async function runCodexHistoryJob( canonicalStateDbPath: request.canonicalStateDbPath, canonicalBackupPath: request.canonicalBackupPath, ...(request.expectedDesiredEnabled === undefined ? {} : { expectedDesiredEnabled: request.expectedDesiredEnabled }), + // A Worker is a fresh module realm: it would otherwise open state_5.sqlite with this + // module's default rather than the timeout this process resolved. Production sends the + // same codex-rs-matching 5s the Worker would have used on its own. + busyTimeoutMs: currentHistoryDbBusyTimeoutMs(), env: { ...(process.env.CODEX_HOME ? { CODEX_HOME: process.env.CODEX_HOME } : {}), ...(process.env.OPENCODEX_HOME ? { OPENCODEX_HOME: process.env.OPENCODEX_HOME } : {}), diff --git a/src/codex/history-provider.ts b/src/codex/history-provider.ts index d29e5b95345..33f79da7ab2 100644 --- a/src/codex/history-provider.ts +++ b/src/codex/history-provider.ts @@ -76,6 +76,22 @@ export function setHistoryDbBusyTimeoutForTests(ms: number): void { historyDbBusyTimeoutMs = ms; } +/** + * Carry that timeout across a realm boundary. A Worker starts from the default above and cannot + * observe a parent that shortened the window — the same reason its run message carries the homes + * explicitly — so `history-job.ts` sends this value and `history-worker.ts` adopts it. In + * production both sides already hold the codex-rs-matching 5s. A non-finite or negative value is + * refused rather than allowed to disable the wait the app expects. + */ +export function currentHistoryDbBusyTimeoutMs(): number { + return historyDbBusyTimeoutMs; +} + +export function adoptHistoryDbBusyTimeout(ms: number): void { + if (!Number.isFinite(ms) || ms < 0) return; + historyDbBusyTimeoutMs = Math.floor(ms); +} + function openStateDb(stateDbPath: string): Database { const db = new Database(stateDbPath); try { diff --git a/src/codex/history-worker.ts b/src/codex/history-worker.ts index 81718b73d22..964510b08a0 100644 --- a/src/codex/history-worker.ts +++ b/src/codex/history-worker.ts @@ -33,6 +33,7 @@ import { } from "./internal/history-writer"; import { snapshotCodexHistoryNoop, + adoptHistoryDbBusyTimeout, type CodexHistoryFailureReason, type CodexHistoryVerifiedNoopProof, } from "./history-provider"; @@ -62,6 +63,11 @@ export interface HistoryWorkerRunMessage { readonly canonicalBackupPath: string; /** When set, prove this transition's desired direction while H is held. */ readonly expectedDesiredEnabled?: boolean; + /** + * The parent realm's `state_5.sqlite` busy timeout. A Worker cannot observe a parent that + * resolved a different window, for the same reason the homes below are explicit. + */ + readonly busyTimeoutMs?: number; /** Env snapshot: a Worker may not observe parent mutations on every platform. */ readonly env?: { readonly CODEX_HOME?: string; readonly OPENCODEX_HOME?: string }; } @@ -107,7 +113,11 @@ export function isHistoryWorkerRunMessage(data: unknown): data is HistoryWorkerR && nonEmpty(message.canonicalCodexHome) && nonEmpty(message.canonicalStateDbPath) && nonEmpty(message.canonicalBackupPath) - && (message.expectedDesiredEnabled === undefined || typeof message.expectedDesiredEnabled === "boolean"); + && (message.expectedDesiredEnabled === undefined || typeof message.expectedDesiredEnabled === "boolean") + && (message.busyTimeoutMs === undefined + || (typeof message.busyTimeoutMs === "number" + && Number.isFinite(message.busyTimeoutMs) + && message.busyTimeoutMs >= 0)); } /** @@ -207,6 +217,9 @@ if (typeof self !== "undefined" && typeof (self as { onmessage?: unknown }) === try { if (message.env?.CODEX_HOME) process.env.CODEX_HOME = message.env.CODEX_HOME; if (message.env?.OPENCODEX_HOME) process.env.OPENCODEX_HOME = message.env.OPENCODEX_HOME; + // Before any DB open: the timeout has to be in force for the first `openStateDb`, not + // after the writer has already waited out this realm's default. + if (message.busyTimeoutMs !== undefined) adoptHistoryDbBusyTimeout(message.busyTimeoutMs); self.postMessage(runHistoryUnitUnderLock(message)); } catch (error) { self.postMessage({ diff --git a/tests/codex-integration/codex-composed-acceptance.test.ts b/tests/codex-integration/codex-composed-acceptance.test.ts index 4806b43019d..8274ca9be2f 100644 --- a/tests/codex-integration/codex-composed-acceptance.test.ts +++ b/tests/codex-integration/codex-composed-acceptance.test.ts @@ -65,6 +65,7 @@ import { resolveEffectiveUserIdentity, } from "../../src/codex/user-identity"; import { claimOwnedServiceHome, withOwnedServiceHomePreload } from "../helpers/owned-service-home"; +import { HISTORY_BUSY_TIMEOUT_ENV } from "../helpers/history-busy-timeout-preload"; import { INTERNAL_DEADLINE_MS, SERVER_BUDGET_MS } from "../helpers/test-budget"; import { repoRoot as resolveRepoRoot } from "../helpers/repo-root"; @@ -80,6 +81,8 @@ const HELD_REQUEST_BUDGET_MS = SERVER_BUDGET_MS + INTERNAL_DEADLINE_MS; const repoRoot = resolveRepoRoot(); const cliPath = resolve(repoRoot, "src/cli/index.ts"); +/** Preload that shortens only a spawned child's SQLite busy wait; see the helper's header. */ +const historyBusyTimeoutPreload = resolve(repoRoot, "tests/helpers/history-busy-timeout-preload.ts"); const lockChildPath = resolve(repoRoot, "tests/helpers/codex-write-lock-child.ts"); const roots: Fixture[] = []; @@ -245,6 +248,7 @@ class Fixture { home = this.homeA, userprofile = this.userprofileA, includeServiceProbe = false, + extra: Record = {}, ): Record { // Do not inherit ambient homes or proxy configuration. `process.execPath` // is absolute, so a PATH is intentionally unnecessary for CLI children. @@ -270,6 +274,7 @@ class Fixture { // lookup timed out" while powershell.exe is still starting. ...(process.env.CI === "true" ? { CI: "true" } : {}), ...(includeServiceProbe ? this.serviceManagerEnv : {}), + ...extra, }; } @@ -294,10 +299,18 @@ class Fixture { }, null, 2)); } - spawnCli(argv: string[], home = this.homeA, userprofile = this.userprofileA) { - const child = Bun.spawn([process.execPath, ...withOwnedServiceHomePreload([cliPath, ...argv], this.serviceManagerPreloadPath)], { + spawnCli( + argv: string[], + home = this.homeA, + userprofile = this.userprofileA, + options: { readonly preloadPaths?: readonly string[]; readonly env?: Record } = {}, + ) { + // Extra preloads go ahead of the service-probe wiring so each stays a separate argv pair, + // which is what keeps a checkout path containing spaces safe on Windows. + const preloadArgs = (options.preloadPaths ?? []).flatMap(path => ["--preload", path]); + const child = Bun.spawn([process.execPath, ...preloadArgs, ...withOwnedServiceHomePreload([cliPath, ...argv], this.serviceManagerPreloadPath)], { cwd: this.root, - env: this.env(home, userprofile, true), + env: this.env(home, userprofile, true, options.env ?? {}), stdout: "pipe", stderr: "pipe", }); @@ -310,8 +323,9 @@ class Fixture { home = this.homeA, userprofile = this.userprofileA, timeoutMs = watchdogMs(15_000), + options: { readonly preloadPaths?: readonly string[]; readonly env?: Record } = {}, ): Promise { - const child = this.spawnCli(argv, home, userprofile); + const child = this.spawnCli(argv, home, userprofile, options); const completed = await Promise.race([ Promise.all([new Response(child.stdout).text(), new Response(child.stderr).text(), child.exited]), new Promise((_, reject) => setTimeout(() => reject(new Error(`CLI watchdog: ocx ${argv.join(" ")}`)), timeoutMs)), @@ -822,9 +836,16 @@ describe("WP13 composed toggle acceptance", () => { }, CASE_TIMEOUT_MS); /** RED: report restore success after a blocked history worker; config recovery must not hide history contention. */ - // This verifies a platform-independent busy-envelope contract. Its deliberate SQLite - // contention plus real CLI startup is not a Windows latency assertion. - test.skipIf(process.platform === "win32")("Restore truth: JSON distinguishes a busy history restore from native artifact recovery", async () => { + // This verifies a platform-independent busy-envelope contract, and it now runs everywhere. + // It was skipped on win32 after run 32344670867 killed it at the 45 s CLI watchdog + // (45197 ms, "CLI watchdog: ocx restore --json") on a shard where neighbouring cases took + // 54-106 s. Nothing about the contract failed there: no envelope, no SQLite error, no + // assertion — the child was still waiting. The waiting was production's own busy budget + // (5 s per attempt, two attempts, 500 ms apart) paid inside a real CLI child, and that wait + // is not the assertion. The child now gets the same shortened busy timeout the in-process + // history tests use, so the contended phase costs ~1 s instead of ~10.5 s while the lock, + // the retry count, and every assertion below stay exactly as they were. + test("Restore truth: JSON distinguishes a busy history restore from native artifact recovery", async () => { const fx = fixture(); fx.writeConfig({ clientIntegrations: { codex: false } }); const original = 'model = "gpt-5"\n'; @@ -879,13 +900,16 @@ describe("WP13 composed toggle acceptance", () => { `], { cwd: repoRoot, env: fx.env(), stdout: "pipe", stderr: "pipe" }); fx.children.push(holder); await waitFor(() => existsSync(held) ? true : null, "history BEGIN IMMEDIATE"); - // The contended restore deliberately waits out PRODUCTION's retry budget: - // a 5 s SQLite busy timeout per attempt, two attempts, plus the delay - // between them — ~11 s of intentional waiting before it can report `busy`. - // A 15 s watchdog left almost no margin and fired on a loaded macOS runner - // (dev CI run 31105071651). Give the wait its budget plus real headroom; - // the case's own 45 s test timeout still bounds it. - const blocked = await fx.runCli(["restore", "--json"], fx.homeA, fx.userprofileA, watchdogMs(30_000)); + // The contended restore still exhausts PRODUCTION's retry budget — two attempts against a + // lock that never releases — but each attempt's SQLite busy timeout is shortened from 5 s + // to 250 ms in this child only. What is being proven is the envelope, not the length of + // the wait, and the full-length wait is what fired the watchdog on Windows (run + // 32344670867) and earlier on a loaded macOS runner (run 31105071651). The child's history + // Worker inherits the value through its run message, since a Worker is a separate realm. + const blocked = await fx.runCli(["restore", "--json"], fx.homeA, fx.userprofileA, watchdogMs(30_000), { + preloadPaths: [historyBusyTimeoutPreload], + env: { [HISTORY_BUSY_TIMEOUT_ENV]: "250" }, + }); expect(blocked.exitCode, JSON.stringify(blocked)).toBe(1); const envelope = JSON.parse(blocked.stdout) as { success: boolean; artifacts: { history: { state: string; reason?: string } } }; expect(envelope).toMatchObject({ success: false, artifacts: { history: { state: "failed", reason: "busy" } } }); diff --git a/tests/codex-integration/codex-history-worker.test.ts b/tests/codex-integration/codex-history-worker.test.ts index 90a0a04caf6..e216d4c4c51 100644 --- a/tests/codex-integration/codex-history-worker.test.ts +++ b/tests/codex-integration/codex-history-worker.test.ts @@ -5,7 +5,7 @@ import { dirname, join, resolve } from "node:path"; import { Database } from "bun:sqlite"; -import { historyBackupPathFor, setBeforeHistoryBackupConsumeForTests, setHistoryDbBusyTimeoutForTests } from "../../src/codex/history-provider"; +import { adoptHistoryDbBusyTimeout, currentHistoryDbBusyTimeoutMs, historyBackupPathFor, setBeforeHistoryBackupConsumeForTests, setHistoryDbBusyTimeoutForTests } from "../../src/codex/history-provider"; import { isHistoryWorkerRunMessage, runHistoryUnitUnderLock, @@ -129,6 +129,35 @@ test("the run message is structured-clone safe and fully explicit", () => { expect(isHistoryWorkerRunMessage({ ...message, operation: "delete-everything" })).toBe(false); }); +/** + * The busy timeout travels with the message because a Worker is a separate realm: without it the + * Worker opens `state_5.sqlite` with its own module default and ignores a parent that resolved a + * shorter window, which is what forced a composed acceptance case to skip on Windows. + */ +test("the run message carries the parent's busy timeout and refuses a malformed one", () => { + const fixture = makeFixture("ocx-history-worker-busy-timeout-"); + const message = runMessage(fixture); + const inherited = currentHistoryDbBusyTimeoutMs(); + + expect(isHistoryWorkerRunMessage({ ...message, busyTimeoutMs: 0 })).toBe(true); + expect(isHistoryWorkerRunMessage({ ...message, busyTimeoutMs: inherited })).toBe(true); + for (const bad of [-1, Number.NaN, Number.POSITIVE_INFINITY, "250", null]) { + expect(isHistoryWorkerRunMessage({ ...message, busyTimeoutMs: bad })).toBe(false); + } + + // Adoption refuses the same values rather than disabling the wait the app expects. + try { + for (const bad of [-1, Number.NaN, Number.POSITIVE_INFINITY]) { + adoptHistoryDbBusyTimeout(bad); + expect(currentHistoryDbBusyTimeoutMs()).toBe(inherited); + } + adoptHistoryDbBusyTimeout(1_234); + expect(currentHistoryDbBusyTimeoutMs()).toBe(1_234); + } finally { + adoptHistoryDbBusyTimeout(inherited); + } +}); + test("skip is a recorded outcome, not an absence, and writes nothing", () => { const fixture = makeFixture("ocx-history-worker-skip-"); const before = readFileSync(fixture.rollout, "utf8"); diff --git a/tests/helpers/history-busy-timeout-preload.ts b/tests/helpers/history-busy-timeout-preload.ts new file mode 100644 index 00000000000..fe1925bd6e7 --- /dev/null +++ b/tests/helpers/history-busy-timeout-preload.ts @@ -0,0 +1,29 @@ +/** + * Test-only seam for a spawned `ocx` child: shorten the `state_5.sqlite` busy timeout. + * + * A test that proves cross-process SQLite contention has to hold a real lock against a real CLI + * child, and the child then pays the production busy timeout twice over — 5 s per attempt plus + * the retry delay, about 10.5 s of pure waiting. That wait is not the assertion; the JSON + * envelope is. On a contended windows-latest shard the waiting alone pushed the child past its + * watchdog, which is how the composed restore-busy case came to be skipped on Windows. + * + * In-process tests already shrink this window with `setHistoryDbBusyTimeoutForTests`. A child + * process cannot be reached that way, so it gets the same knob through `--preload`. Inert unless + * `OCX_TEST_HISTORY_BUSY_TIMEOUT_MS` is set on that child's environment, and no production + * module reads that variable or imports this file. The retry count, the lock, the failure + * classification, and the reported envelope are all untouched: only the length of a sleep changes. + */ +import { setHistoryDbBusyTimeoutForTests } from "../../src/codex/history-provider"; + +export const HISTORY_BUSY_TIMEOUT_ENV = "OCX_TEST_HISTORY_BUSY_TIMEOUT_MS"; + +const raw = process.env[HISTORY_BUSY_TIMEOUT_ENV]; +if (raw !== undefined) { + const ms = Number(raw); + // A malformed value must not silently leave the production 5 s in place while the test + // believes it was shortened, so it fails the child loudly instead. + if (!Number.isFinite(ms) || ms < 0) { + throw new Error(`${HISTORY_BUSY_TIMEOUT_ENV} must be a non-negative number, received: ${raw}`); + } + setHistoryDbBusyTimeoutForTests(ms); +}