From 9e5a917eddb8d1f1ab32dc0421e415ddb71ff3e9 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 21:25:44 +0900 Subject: [PATCH 1/5] fix(test): promote Bun 1.4.4 with independent runner role verification --- .github/actions/setup-project-bun/action.yml | 4 +-- .github/workflows/ci.yml | 10 +++--- CONTRIBUTING.md | 6 ++-- docs-site/src/content/docs/contributing.md | 6 ++-- package.json | 2 +- structure/runtime.md | 4 +-- tests/ci-workflows/ci-scope-gaps.test.ts | 2 +- tests/ci-workflows/install-scripts.test.ts | 6 ++-- tests/test-layout-tooling.test.ts | 34 ++++++++++++++++---- 9 files changed, 48 insertions(+), 26 deletions(-) diff --git a/.github/actions/setup-project-bun/action.yml b/.github/actions/setup-project-bun/action.yml index 1ab22dd1dc9..cf28f6f9d43 100644 --- a/.github/actions/setup-project-bun/action.yml +++ b/.github/actions/setup-project-bun/action.yml @@ -8,8 +8,8 @@ description: >- # (#6713 run 37631558263: Linux 1/4, macOS 2/2, Windows 9/9). It moved from 1.4.0 to # 1.4.3 for oven-sh/bun#44581: 1.4.0's Bun.spawnSync could lose a child's exit after # finalizers ran inside an earlier call and spin past its own timeout (#4956). -# Rejoin only after the upstream runner crash is fixed and a green lane=all -# dispatch proves the unified version; then delete testRunnerBun and this role input. +# Both roles now default to the package's 1.4.4 security transition, retaining role validation. +# Promotion requires a green full cross-platform lane=all run on the exact candidate head. inputs: role: description: Bun role to install (runtime or test-runner). diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 028c28ca318..7bcd0eaa3f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -578,12 +578,10 @@ jobs: printf 'Scheduled run: workflow from %s, testing dev at %s\n' "$GITHUB_SHA" "$sha" >> "$GITHUB_STEP_SUMMARY" printf 'scheduled run tests dev at %s (workflow %s)\n' "$sha" "$GITHUB_SHA" - # Bun roles (#6713): runtime 1.4.2 fixes #6684's Windows Bun.fetch streaming; - # the test runner skipped 1.4.2's multi-file --isolate preload segfault (#4821 - # reverted #4064; run 37631558263: Linux 1/4, macOS 2/2, Windows 9/9) and runs - # 1.4.3, whose oven-sh/bun#44581 stops Bun.spawnSync losing a child's exit (#4956). - # Rejoin after the upstream runner crash is fixed and a green lane=all dispatch - # proves the unified version, then delete testRunnerBun and the action's role input. + # Bun runtime and test-runner roles both read their 1.4.4 defaults from package.json. + # Keep role validation after the historical isolate preload and spawnSync failures + # (#6713, #4956). Promotion requires a green full cross-platform lane=all run on + # the exact candidate head; focused local results do not replace that proof. # The suite, split by file across four Linux runners. # # `scripts/ci/run-bun-test-batches.sh` assigns files to shards by the per-file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7d4aa94f580..bbcd248f8f8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,9 +69,11 @@ A ready-for-review PR is the author's claim that the change is complete, underst ## Local validation and hooks -The suite uses Bun 1.4.3 (`testRunnerBun`) while the shipped runtime is 1.4.4. -`scripts/test.ts` finds a 1.4.3 binary on PATH or in `~/.bun/bin`, or uses +The suite uses Bun 1.4.4 (`testRunnerBun`) while the shipped runtime is 1.4.4. +`scripts/test.ts` finds a 1.4.4 binary on PATH or in `~/.bun/bin`, or uses `OCX_TEST_RUNNER_BUN`; install that version locally if it is missing. +The roles remain independently checked even when their versions match. Promotion requires +a successful full cross-platform `lane=all` run on the candidate head. Run `bun run test` before review readiness. If the full local suite is too costly for the task or available resources, run at least focused regression tests for diff --git a/docs-site/src/content/docs/contributing.md b/docs-site/src/content/docs/contributing.md index 7fe0b09831b..d0021b7f12a 100644 --- a/docs-site/src/content/docs/contributing.md +++ b/docs-site/src/content/docs/contributing.md @@ -45,9 +45,11 @@ bun run privacy:scan # credential/privacy scan used by CI bun run prepare:package # refresh package launchers/assets ``` -The suite uses Bun 1.4.3 (`package.json`'s `testRunnerBun`) while the shipped runtime is 1.4.4. -`scripts/test.ts` finds a 1.4.3 binary on PATH or in `~/.bun/bin`, or uses `OCX_TEST_RUNNER_BUN`. +The suite uses Bun 1.4.4 (`package.json`'s `testRunnerBun`) while the shipped runtime is 1.4.4. +`scripts/test.ts` finds a 1.4.4 binary on PATH or in `~/.bun/bin`, or uses `OCX_TEST_RUNNER_BUN`. Install that version locally if it is missing; the resolver never downloads it. +The roles remain independently checked even when their versions match. Promotion requires +a successful full cross-platform `lane=all` run on the candidate head. Run `bun run test` by default. If a full run is disproportionately expensive for the task size, available machine resources, or concurrent worktrees, you must at least run focused regression tests diff --git a/package.json b/package.json index 89176da2851..a270427faf2 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "trustedDependencies": [ "bun" ], - "testRunnerBun": "1.4.3", + "testRunnerBun": "1.4.4", "scripts": { "dev": "bun run src/cli/index.ts start", "dev:proxy": "bun run src/cli/index.ts start", diff --git a/structure/runtime.md b/structure/runtime.md index 43e071d457f..763f3e90c3d 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -4,7 +4,7 @@ The minute sweep checks persisted activation deadlines locally; only missing dea ## Bun runtime and test runner -`package.json` pins the shipped npm runtime (`dependencies.bun`) to 1.4.4 and the test runner (`testRunnerBun`) to 1.4.3; `@types/bun` remains unchanged. Runtime 1.4.2 fixed #6684's Windows Bun.fetch truncation after the first cleartext upstream body chunk; 1.4.4 keeps that fix and ships NSS 3.131.1 roots, which drop the compromised eMudhra CAs. +`package.json` pins both the shipped npm runtime (`dependencies.bun`) and the test runner (`testRunnerBun`) to 1.4.4; the roles remain independently validated and `@types/bun` remains unchanged. Runtime 1.4.2 fixed #6684's Windows Bun.fetch truncation after the first cleartext upstream body chunk; 1.4.4 keeps that fix and ships NSS 3.131.1 roots, which drop the compromised eMudhra CAs. The runner pin skipped 1.4.2's multi-file `bun test --isolate` preload reload segfault (`load_preloads -> JSModuleLoader::loadModule -> JSPromise::status`, address 0x10): #4064 introduced 1.4.2 on 2026-09-09 and #4821 reverted it on 2026-09-17. It left 1.4.0 for 1.4.3 because of oven-sh/bun#44581 (#4956). Bun 1.4.0's `Bun.spawnSync` pointed the VM at its private wait loop, so a finalizer that ran during one call (an earlier file's stdout/stderr sink, a stream reader) released its poll on that loop; the cached loop's poll count drifted to zero and a later call stopped polling for its child, spinning past its own `timeout` until the 120-second batch deadline. In a multi-file `--isolate` process that depends on what earlier files left to finalize, so single-file attribution stayed clean. `tests/ci-workflows/test-runner-spawnsync-finalizer.test.ts` reproduces the mechanism in a child under a bounded parent and fails on 1.4.0. #6713 run 37631558263 reproduces that 1.4.2 preload segfault on Linux 1/4, macOS 2/2 and Windows 9/9; single-file success does not prove the multi-file runner works. @@ -12,7 +12,7 @@ The runner pin skipped 1.4.2's multi-file `bun test --isolate` preload reload se Test jobs, including single-file development-version validation, select `test-runner`. They also build dashboard fixtures; `gates` uploads a dashboard preview and checks release-helper syntax on that pin. Release packaging, Docker, installed-artifact and service smoke jobs, desktop shell and widget preparation use `runtime`. Shipped binaries compile through `process.execPath` in `scripts/build-standalone.ts` and `desktop/scripts/prepare-sidecar.ts` on the runtime pin. -`scripts/lib/test-runner-bun.ts` selects the test pin for `scripts/test.ts`, `scripts/release.ts` preflight, `scripts/test-layout/verify.ts`, provider-option gates and GUI package tests via `scripts/test-with-pinned-bun.ts`: matching current Bun first, then a version-checked `OCX_TEST_RUNNER_BUN`, then PATH outside node_modules and `~/.bun/bin`. Missing or mismatched binaries fail with install guidance, without downloading; package-script PATH shadowing cannot select the shipped runtime for tests. Rejoin only after an upstream fix and green unified `lane=all` proof; then remove the runner pin, resolver and CI role split. +`scripts/lib/test-runner-bun.ts` selects the test pin for `scripts/test.ts`, `scripts/release.ts` preflight, `scripts/test-layout/verify.ts`, provider-option gates and GUI package tests via `scripts/test-with-pinned-bun.ts`: matching current Bun first, then a version-checked `OCX_TEST_RUNNER_BUN`, then PATH outside node_modules and `~/.bun/bin`. Missing or mismatched binaries fail with install guidance, without downloading. The caller need not already run the test pin; only a version match permits reusing its executable. `tests/test-layout-tooling.test.ts` verifies matching/different caller fixtures, the selected isolate command, and refusal before spawning tests for a wrong-version override. The 1.4.4 security transition aligns the default versions while retaining the role fields, resolver and CI role checks. Promotion requires successful full cross-platform `lane=all` proof on the exact candidate head; focused local tests do not establish that proof. ## Resolved static model policy diff --git a/tests/ci-workflows/ci-scope-gaps.test.ts b/tests/ci-workflows/ci-scope-gaps.test.ts index f780d71aa90..0729ab6945d 100644 --- a/tests/ci-workflows/ci-scope-gaps.test.ts +++ b/tests/ci-workflows/ci-scope-gaps.test.ts @@ -65,7 +65,7 @@ describe("an edit to the setup action alone", () => { }); }); -// #6713: user runtimes and compiled artifacts need 1.4.2; isolate runners need the testRunnerBun pin (1.4.3, #4956). +// Keep runtime and test-runner role selection distinct even when package.json pins both to 1.4.4. // Check the active setup at each test command, so a later role switch cannot evade the guard. const runsBunTests = (step: Step): boolean => /\bbun\s+test\b|\bbun\s+run\s+test(?::changed)?(?:\s|$)|\bbun\s+(?:run\s+)?scripts\/test\.ts\b|scripts\/ci\/run-bun-test-batches\.sh/.test( diff --git a/tests/ci-workflows/install-scripts.test.ts b/tests/ci-workflows/install-scripts.test.ts index 51e62b0b6fc..23ed7b7461e 100644 --- a/tests/ci-workflows/install-scripts.test.ts +++ b/tests/ci-workflows/install-scripts.test.ts @@ -94,10 +94,10 @@ describe("install scripts", () => { // and spun past its own timeout. Scratch run 38213764818 stalled 8 of 252 Linux shard 1/4 // batch-5 runs that way (CLI child a zombie, its pidfd armed but never polled). // tests/ci-workflows/test-runner-spawnsync-finalizer.test.ts keeps the runner above it. - // Rejoin after the upstream runner crash is fixed and a green `lane=all` dispatch - // proves the unified version; then delete testRunnerBun and the setup action's role. + // The security transition now defaults both roles to 1.4.4. Keep their independent + // validation; promotion still requires a green full cross-platform lane=all candidate run. expect(pkg.dependencies?.bun).toBe("1.4.4"); - expect(pkg.testRunnerBun).toBe("1.4.3"); + expect(pkg.testRunnerBun).toBe("1.4.4"); expect(pkg.dependencies?.zod).toBe("4.4.3"); expect(pkg.devDependencies?.typescript).toBe("7.0.2"); expect(pkg.devDependencies?.["@types/bun"]).toBe("1.4.0"); diff --git a/tests/test-layout-tooling.test.ts b/tests/test-layout-tooling.test.ts index 354e3943fe0..cb4a7482819 100644 --- a/tests/test-layout-tooling.test.ts +++ b/tests/test-layout-tooling.test.ts @@ -7,6 +7,7 @@ import { repoPath, repoRoot } from "./helpers/repo-root"; import { listTestFiles, planMoves } from "../scripts/test-layout/plan"; import { runMove } from "../scripts/test-layout/move"; import { runVerify } from "../scripts/test-layout/verify"; +import * as runnerBun from "../scripts/lib/test-runner-bun"; import { testRunnerBun } from "../package.json"; import { anchors, @@ -469,24 +470,43 @@ describe("move end to end", () => { } }); - test("domain verification launches isolate tests through the pinned runner", () => { + const otherBunVersion = testRunnerBun === "1.4.3" ? "1.4.4" : "1.4.3"; + test.each([ + ["matching caller", testRunnerBun, testRunnerBun, false], + ["different caller", otherBunVersion, testRunnerBun, false], + ["wrong override version", otherBunVersion, otherBunVersion, true], + ] as const)("domain verification preserves the configured runner role: %s", (_name, currentVersion, overrideVersion, refused) => { const { root, cleanup } = scratchRepo(); const layoutPath = join(root, "scripts", "test-layout", "layout.json"); try { runMove({ root, domains: ["server", "providers"], dryRun: false, layoutPath, skipVerify: true, log: () => {} }); + const binaryName = process.platform === "win32" ? "bun.exe" : "bun"; + const pinnedRunner = join(root, "test-runner", binaryName); + const caller = currentVersion === testRunnerBun ? pinnedRunner : join(root, "runtime", binaryName); + const probes: string[] = []; + // Keep role resolution real, but version probes independent of the host and the spawn mock. + const select = spyOn(runnerBun, "getTestRunnerBun").mockImplementation(() => runnerBun.resolveTestRunnerBun({ + pin: testRunnerBun, currentVersion, execPath: caller, + env: { OCX_TEST_RUNNER_BUN: pinnedRunner }, pathEntries: [], homeDir: root, platform: process.platform, + probeVersion: binary => { probes.push(binary); return binary === pinnedRunner ? overrideVersion : undefined; }, + })); const spawn = spyOn(Bun, "spawnSync").mockReturnValue({ exitCode: 0, stdout: Buffer.from(""), stderr: Buffer.from(""), } as ReturnType); try { - const report = runVerify({ root, domains: ["server", "providers"], layoutPath, log: () => {} }); - expect(report.testExit).toBe(0); - // The test itself runs on the required pin, so its executable is the resolver's first choice. - expect(Bun.version).toBe(testRunnerBun); - expect(spawn.mock.calls.at(-1)?.[0]).toEqual([ - process.execPath, "test", "--isolate", "tests/server", "tests/providers", + const verify = () => runVerify({ root, domains: ["server", "providers"], layoutPath, log: () => {} }); + if (refused) expect(verify).toThrow(`OCX_TEST_RUNNER_BUN reports ${overrideVersion}`); + else expect(verify()).toMatchObject({ ok: true, testExit: 0 }); + expect(select).toHaveBeenCalledTimes(1); + expect(probes).toEqual(currentVersion === testRunnerBun ? [] : [pinnedRunner]); + const testSpawns = spawn.mock.calls.filter(([command]) => Array.isArray(command) && command[1] === "test"); + expect(testSpawns.map(([command]) => command)).toEqual(refused ? [] : [ + [pinnedRunner, "test", "--isolate", "tests/server", "tests/providers"], ]); + if (!refused) expect(testSpawns[0]?.[1]).toMatchObject({ cwd: root, stdout: "inherit", stderr: "inherit" }); } finally { spawn.mockRestore(); + select.mockRestore(); } } finally { cleanup(); From 1deb67d274c8246df9ad539bff93f958782032cb Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 21:38:09 +0900 Subject: [PATCH 2/5] docs: align Docker Bun comments with unified role defaults --- Dockerfile | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index 82fe86f4cf1..54e0153e0f1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,12 +2,12 @@ # Keep the runtime aligned with package.json and pin the multi-platform image index. # Runtime 1.4.4 keeps 1.4.2's #6684 Windows Bun.fetch streaming fix and ships NSS -# 3.131.1 roots without the compromised eMudhra CAs; CI tests run on 1.4.3 -# because 1.4.2 segfaults reloading preloads in multi-file --isolate (#4821; -# #6713 run 37631558263: Linux 1/4, macOS 2/2, Windows 9/9) and 1.4.0's spawnSync -# can lose a child's exit (#4956, oven-sh/bun#44581). Rejoin after the -# upstream crash fix passes a unified-version lane=all dispatch, then delete -# package.json's testRunnerBun pin and setup-project-bun's role input. +# 3.131.1 roots without the compromised eMudhra CAs. Runtime and test-runner +# defaults are both 1.4.4; keep package.json's testRunnerBun pin and +# setup-project-bun's role input independently validated. Promotion requires +# an exact-head cross-platform lane=all dispatch, including multi-file --isolate: +# historical 1.4.2 preload crashes (#4821; #6713 run 37631558263) and 1.4.0 +# spawnSync exit loss (#4956, oven-sh/bun#44581) remain regression risks. ARG BUN_IMAGE=oven/bun:1.4.4@sha256:1d22548f177f4ff38e0c15baa3c2c46c6c2060ba2d1bc573bb4720b407548f1d FROM ${BUN_IMAGE} AS manifest From 41f08fffd3e7dea64a6eed79af918aed74a153dd Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 22:07:35 +0900 Subject: [PATCH 3/5] test: use a draining TLS origin for early upload replies --- .../claude-picker-upload.test.ts | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/tests/claude-integration/claude-picker-upload.test.ts b/tests/claude-integration/claude-picker-upload.test.ts index 8d4cd0d1859..95c6351f04d 100644 --- a/tests/claude-integration/claude-picker-upload.test.ts +++ b/tests/claude-integration/claude-picker-upload.test.ts @@ -3,14 +3,39 @@ import { Agent, createServer, request } from "node:https"; import { connect, constants, type ClientHttp2Session, type ClientHttp2Stream } from "node:http2"; import type { ClientRequest, IncomingMessage, ServerResponse } from "node:http"; import type { Duplex } from "node:stream"; +import { createServer as createTlsServer } from "node:tls"; import { createLocalInterceptCa, issueLocalInterceptLeaf } from "../../src/claude/intercept/local-ca"; import { startPickerListener, type PickerListenerOptions } from "../../src/claude/intercept/picker-listener"; -async function fixture(handler: (req: IncomingMessage, res: ServerResponse) => void, +async function fixture(handler: ((req: IncomingMessage, res: ServerResponse) => void) | "early-fin", options: Partial = {}) { const ca = createLocalInterceptCa(); const leaf = issueLocalInterceptLeaf(ca, ["claude.ai"]); - const upstream = createServer({ cert: leaf.certPem, key: leaf.keyPem }, handler); + const earlyReplies: Array<{ requestComplete: boolean; flushed: boolean }> = []; + // A wire-level origin sends the reply and FIN while continuing to drain input. Bun's HTTP + // server also tears down unfinished requests; overriding destroySoon alone does not prevent + // that reset racing ahead of the reply on Windows with a buffered upload. + const upstream = handler === "early-fin" ? createTlsServer({ cert: leaf.certPem, key: leaf.keyPem, allowHalfOpen: true }, socket => { + socket.on("error", () => socket.destroy()); + let headers = ""; + let replied = false; + socket.on("data", chunk => { + if (replied) return; + headers += chunk.toString("latin1"); + const end = headers.indexOf("\r\n\r\n"); + if (end < 0) return; + replied = true; + const early = headers.startsWith("POST /early "); + const body = early ? "early" : "ordinary"; + const length = /\r\ncontent-length: (\d+)\r\n/i.exec(headers.slice(0, end + 2)); + const requestBody = headers.slice(end + 4); + const observation = { requestComplete: length ? requestBody.length >= Number(length[1]) + : requestBody.endsWith("0\r\n\r\n"), flushed: false }; + if (early) earlyReplies.push(observation); + socket.once("finish", () => { observation.flushed = true; }); + socket.end(`HTTP/1.1 ${early ? "413 Payload Too Large" : "200 OK"}\r\nContent-Length: ${body.length}\r\nConnection: close\r\n\r\n${body}`); + }); + }) : createServer({ cert: leaf.certPem, key: leaf.keyPem }, handler); const sockets = new Set(); for (const event of ["connection", "secureConnection"] as const) upstream.on(event, socket => { sockets.add(socket); socket.once("close", () => sockets.delete(socket)); @@ -34,7 +59,7 @@ async function fixture(handler: (req: IncomingMessage, res: ServerResponse) => v rejectUnauthorized: true, agent: h1Agent, path, method, headers: { Host: "claude.ai", ...headers } }); clients.push(req); return req; }; - return { logs, h2, h1, relay, + return { logs, h2, h1, relay, earlyReplies, async close() { const closed = clients.map(client => client.destroyed ? Promise.resolve() : observeClose(client)); await relay.close(); @@ -353,16 +378,7 @@ for (const protocol of ["h1", "h2"] as const) { // "buffered" answers early while 4 MiB of an advertised 32 MiB body is still queued. for (const input of protocol === "h1" ? ["small", "buffered"] as const : ["small"] as const) bounded(`${protocol} early upstream response closes ${input === "buffered" ? "a buffered" : "an"} unfinished upload without losing the reply`, async wait => { - const f = await fixture((req, res) => { - // Answer like a real origin that replies early: keep reading the body and close with a FIN. - // Bun's own server would close this unfinished request with destroySoon(), a reset that - // Windows delivers before the reply bytes, so the relay could never read what it should relay. - req.on("error", () => res.destroy()); req.resume(); - const socket = req.socket; - socket.destroySoon = () => { socket.end(); return socket; }; - if (req.url === "/early") { res.writeHead(413, { "Content-Length": "5" }); res.end("early"); } - else res.end("ordinary"); - }); + const f = await fixture("early-fin"); try { const session = f.h2(); const upload = protocol === "h2" @@ -373,6 +389,7 @@ for (const protocol of ["h1", "h2"] as const) { upload.write(input === "buffered" ? Buffer.alloc(4 * 1024 * 1024, 0x61) : "x"); expect(await wait(result)).toEqual({ status: 413, text: "early" }); await wait(closed); + expect(f.earlyReplies).toEqual([{ requestComplete: false, flushed: true }]); if (protocol === "h2") expect((upload as ClientHttp2Stream).rstCode).toBe(constants.NGHTTP2_NO_ERROR); expect(await wait(normalH2(session))).toEqual({ status: 200, text: "ordinary" }); } finally { await wait(f.close()); } From a9c3d127d3728f50533df32504b787823b5220ee Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 22:09:36 +0900 Subject: [PATCH 4/5] test: confine draining origin to buffered HTTP/1 upload --- .../claude-integration/claude-picker-upload.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/claude-integration/claude-picker-upload.test.ts b/tests/claude-integration/claude-picker-upload.test.ts index 95c6351f04d..d0bb998b873 100644 --- a/tests/claude-integration/claude-picker-upload.test.ts +++ b/tests/claude-integration/claude-picker-upload.test.ts @@ -378,7 +378,16 @@ for (const protocol of ["h1", "h2"] as const) { // "buffered" answers early while 4 MiB of an advertised 32 MiB body is still queued. for (const input of protocol === "h1" ? ["small", "buffered"] as const : ["small"] as const) bounded(`${protocol} early upstream response closes ${input === "buffered" ? "a buffered" : "an"} unfinished upload without losing the reply`, async wait => { - const f = await fixture("early-fin"); + const f = await fixture(input === "buffered" ? "early-fin" : (req, res) => { + // Answer like a real origin that replies early: keep reading the body and close with a FIN. + // Bun's own server would close this unfinished request with destroySoon(), a reset that + // Windows delivers before the reply bytes, so the relay could never read what it should relay. + req.on("error", () => res.destroy()); req.resume(); + const socket = req.socket; + socket.destroySoon = () => { socket.end(); return socket; }; + if (req.url === "/early") { res.writeHead(413, { "Content-Length": "5" }); res.end("early"); } + else res.end("ordinary"); + }); try { const session = f.h2(); const upload = protocol === "h2" @@ -389,7 +398,7 @@ for (const protocol of ["h1", "h2"] as const) { upload.write(input === "buffered" ? Buffer.alloc(4 * 1024 * 1024, 0x61) : "x"); expect(await wait(result)).toEqual({ status: 413, text: "early" }); await wait(closed); - expect(f.earlyReplies).toEqual([{ requestComplete: false, flushed: true }]); + if (input === "buffered") expect(f.earlyReplies).toEqual([{ requestComplete: false, flushed: true }]); if (protocol === "h2") expect((upload as ClientHttp2Stream).rstCode).toBe(constants.NGHTTP2_NO_ERROR); expect(await wait(normalH2(session))).toEqual({ status: 200, text: "ordinary" }); } finally { await wait(f.close()); } From c938424b8d84e2a27756ff6198d512594ffef37b Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 14:55:02 +0000 Subject: [PATCH 5/5] fix(quota): bound snapshot persistence under sustained updates Preserve idle debouncing while bounding scheduled deferral with a monotonic clock. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- .../test-layout/layout-codex-collision.json | 1 + src/codex/quota.ts | 11 +- src/providers/account-quota-disk.ts | 16 ++- structure/INDEX.md | 3 + structure/manifest.json | 7 ++ structure/quota-snapshots.md | 25 ++++ .../codex-quota-persistence.test.ts | 109 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 1 + ...provider-account-quota-persistence.test.ts | 87 +++++++++++++- 9 files changed, 253 insertions(+), 7 deletions(-) create mode 100644 structure/quota-snapshots.md create mode 100644 tests/codex-integration/codex-quota-persistence.test.ts diff --git a/scripts/test-layout/layout-codex-collision.json b/scripts/test-layout/layout-codex-collision.json index c4709d021d7..098bf6f518b 100644 --- a/scripts/test-layout/layout-codex-collision.json +++ b/scripts/test-layout/layout-codex-collision.json @@ -1,4 +1,5 @@ { + "codex-quota-persistence.test.ts": "codex-integration", "codex-stored-main-duplicate.test.ts": "codex-integration", "codex-pool-caller-fallback-policy.test.ts": "codex-integration", "codex-windows-current-sid.test.ts": "codex-integration" diff --git a/src/codex/quota.ts b/src/codex/quota.ts index 0088c4968d6..ebcd30372aa 100644 --- a/src/codex/quota.ts +++ b/src/codex/quota.ts @@ -25,6 +25,8 @@ const QUOTA_CACHE_FILENAME = "codex-quota-cache.json"; /** Keep last-known bars across restarts; WHAM still refreshes on TTL in live/prime paths. */ const QUOTA_DISK_MAX_AGE_MS = 6 * 60 * 60_000; const QUOTA_PERSIST_DEBOUNCE_MS = 250; +/** Match the reset ledger's maximum deferral under sustained quota observations. */ +const QUOTA_PERSIST_MAX_DEFERRAL_MS = 1_000; type QuotaDiskFile = { version: 1; @@ -39,6 +41,7 @@ type MainPolicyQuotaObservation = Omit & { shor let mainPolicyQuota: MainPolicyQuota | null = null; let diskHydrated = false; let persistTimer: ReturnType | null = null; +let persistStartedAt: number | null = null; const MONTHLY_WINDOW_MIN_SECONDS = 28 * 24 * 60 * 60; /** @@ -778,9 +781,14 @@ function hydrateAccountQuotasFromDisk(): void { } function schedulePersistAccountQuotas(): void { + const now = performance.now(); + persistStartedAt ??= now; + const delay = Math.max(0, Math.min(QUOTA_PERSIST_DEBOUNCE_MS, + QUOTA_PERSIST_MAX_DEFERRAL_MS - (now - persistStartedAt))); if (persistTimer) clearTimeout(persistTimer); persistTimer = setTimeout(() => { persistTimer = null; + persistStartedAt = null; try { const quotas: Record = {}; for (const [accountId, quota] of accountQuota.entries()) { @@ -796,7 +804,7 @@ function schedulePersistAccountQuotas(): void { } catch { // Best-effort persistence only. } - }, QUOTA_PERSIST_DEBOUNCE_MS); + }, delay); } export function getAccountQuota(accountId: string): StoredAccountQuota | null { @@ -861,6 +869,7 @@ export function clearAccountQuota(accountId?: string): void { clearTimeout(persistTimer); persistTimer = null; } + persistStartedAt = null; try { const path = join(getConfigDir(), QUOTA_CACHE_FILENAME); if (existsSync(path)) unlinkSync(path); diff --git a/src/providers/account-quota-disk.ts b/src/providers/account-quota-disk.ts index 55dc711fe56..bf0aa1e5160 100644 --- a/src/providers/account-quota-disk.ts +++ b/src/providers/account-quota-disk.ts @@ -29,6 +29,8 @@ const FILENAME = "provider-account-quota-cache.json"; */ const DISK_MAX_AGE_MS = 6 * 60 * 60_000; const PERSIST_DEBOUNCE_MS = 250; +/** Match the reset ledger's maximum deferral under sustained quota observations. */ +const MAX_PERSIST_DEFERRAL_MS = 1_000; type DiskFile = { version: 1; @@ -38,6 +40,7 @@ type DiskFile = { }; let persistTimer: ReturnType | null = null; +let persistStartedAt: number | null = null; /** Read the snapshot. Returns an empty map for a missing, corrupt or stale file. */ export function readPersistedAccountQuotas(now = Date.now()): Map { @@ -86,14 +89,19 @@ export function readPersistedKiroVerdicts(now = Date.now()): Map Iterable<[string, ProviderQuota | KiroPersistedQuota]>, verdicts: () => Iterable<[string, KiroPersistedVerdict]> = () => [], ): void { + const now = performance.now(); + persistStartedAt ??= now; + const delay = Math.max(0, Math.min(PERSIST_DEBOUNCE_MS, + MAX_PERSIST_DEFERRAL_MS - (now - persistStartedAt))); if (persistTimer) clearTimeout(persistTimer); persistTimer = setTimeout(() => { persistTimer = null; + persistStartedAt = null; try { const out: Record = {}; for (const [key, quota] of rows()) out[key] = quota; @@ -106,12 +114,12 @@ export function schedulePersistAccountQuotas( } catch { // Best-effort persistence only. } - }, PERSIST_DEBOUNCE_MS); + }, delay); } /** Test seam: drop any pending write so a suite cannot leak one into the next file. */ export function cancelPendingAccountQuotaPersist(): void { - if (!persistTimer) return; - clearTimeout(persistTimer); + if (persistTimer) clearTimeout(persistTimer); persistTimer = null; + persistStartedAt = null; } diff --git a/structure/INDEX.md b/structure/INDEX.md index e4982e65274..c296253766e 100644 --- a/structure/INDEX.md +++ b/structure/INDEX.md @@ -73,6 +73,7 @@ Per-vendor contracts and the adapter authority that constructs them. | Doc | Scope | | --- | --- | +| [`quota-snapshots.md`](quota-snapshots.md) | Bounded write deferral and cancellation for Codex and provider account quota snapshots. | | [`providers/generic-account-pool.md`](providers/generic-account-pool.md) | Opt-in conversation ownership, request-local selection receipts and bounded release for generic OAuth pools. | | [`providers/anthropic-progress.md`](providers/anthropic-progress.md) | Opus 5.5 progress display, visible commentary, and exact signed-block replay. | | [`providers/anthropic-account-thresholds.md`](providers/anthropic-account-thresholds.md) | Account-owned usage thresholds, inheritance, routing boundaries and durable policy changes. | @@ -214,6 +215,7 @@ A source area can be described by more than one doc, because these docs are orga | `src/codex/catalog/vendor-context.ts` | [`catalog-vendor-context.md`](catalog-vendor-context.md) | | `src/codex/cli-update-plan.ts` | [`codex-cli-update.md`](codex-cli-update.md) | | `src/codex/provider-routing.ts` | [`codex-provider-routing.md`](codex-provider-routing.md) | +| `src/codex/quota.ts` | [`quota-snapshots.md`](quota-snapshots.md) | | `src/codex/runtime.ts` | [`catalog-effort-clamp.md`](catalog-effort-clamp.md) | | `src/codex/signed-in-identity.ts` | [`providers/openai-signed-in-preservation.md`](providers/openai-signed-in-preservation.md) | | `src/combos/` | [`runtime.md`](runtime.md)
[`providers-and-adapters.md`](providers-and-adapters.md)
[`providers/jev-decision.md`](providers/jev-decision.md)
[`fallback-policy.md`](fallback-policy.md) | @@ -254,6 +256,7 @@ A source area can be described by more than one doc, because these docs are orga | `src/plugins/` | [`ops/plugins.md`](ops/plugins.md) | | `src/protocols/` | [`data-planes/protocol-paths.md`](data-planes/protocol-paths.md) | | `src/providers/` | [`runtime.md`](runtime.md)
[`subagents.md`](subagents.md)
[`transports/inventory.md`](transports/inventory.md)
[`providers-and-adapters.md`](providers-and-adapters.md)
[`providers/anthropic-account-pool.md`](providers/anthropic-account-pool.md)
[`providers/xai-grok.md`](providers/xai-grok.md) | +| `src/providers/account-quota-disk.ts` | [`quota-snapshots.md`](quota-snapshots.md) | | `src/providers/anthropic-billing-ownership.ts` | [`providers/anthropic-key-billing.md`](providers/anthropic-key-billing.md) | | `src/providers/anthropic-billing-recovery.ts` | [`providers/anthropic-key-billing.md`](providers/anthropic-key-billing.md) | | `src/providers/anthropic-billing-refusal.ts` | [`providers/anthropic-key-billing.md`](providers/anthropic-key-billing.md) | diff --git a/structure/manifest.json b/structure/manifest.json index 3f6079e5566..48118211dab 100644 --- a/structure/manifest.json +++ b/structure/manifest.json @@ -47,6 +47,13 @@ } ], "docs": [ + { + "path": "quota-snapshots.md", + "tier": 4, + "title": "Account Quota Snapshots", + "scope": "Bounded write deferral and cancellation for Codex and provider account quota snapshots.", + "documents": ["src/codex/quota.ts", "src/providers/account-quota-disk.ts"] + }, { "path": "providers/generic-account-pool.md", "tier": 4, diff --git a/structure/quota-snapshots.md b/structure/quota-snapshots.md new file mode 100644 index 00000000000..04031e3a814 --- /dev/null +++ b/structure/quota-snapshots.md @@ -0,0 +1,25 @@ +# Account Quota Snapshots + +`src/codex/quota.ts` and `src/providers/account-quota-disk.ts` persist last-known account +quotas for restart recovery. The Codex snapshot also carries bounded observation history +and separately identity-bound main-policy evidence; provider snapshots include Kiro's +identity-bound exhaustion verdicts. Their existing hydration and expiry rules govern readers. + +## Bounded write deferral + +Both writers coalesce updates with a 250 ms idle debounce and a one-second maximum +deferral from the first pending update, measured on the monotonic clock. Continued +observations cannot postpone the scheduled write beyond that deadline; event-loop delays +and disk failures remain best-effort. This uses the same maximum deferral as the quota +reset ledger in `src/quota/reset-seen-store.ts` without making producer calls write synchronously. + +The timer snapshots the latest accepted state and resolves the configured home when it +runs. Completion, failure, and cancellation reset the deadline. Clearing all Codex quotas +cancels its pending snapshot; clearing one account persists the remaining rows with the +existing deadline. Provider cancellation discards the pending row and verdict callbacks. +Atomic replacement and account identity checks stay with the existing writer and producer. + +`tests/codex-integration/codex-quota-persistence.test.ts` and +`tests/providers/provider-account-quota-persistence.test.ts` check real disk contents under +controlled timers: idle debounce, consecutive sustained-update windows, cancellation and +home reset, removed rows, latest provider callbacks, and recovery after a failed write. diff --git a/tests/codex-integration/codex-quota-persistence.test.ts b/tests/codex-integration/codex-quota-persistence.test.ts new file mode 100644 index 00000000000..989d7f4d328 --- /dev/null +++ b/tests/codex-integration/codex-quota-persistence.test.ts @@ -0,0 +1,109 @@ +import { afterEach, beforeEach, describe, expect, jest, spyOn, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmdirSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { clearAccountQuota, flushQuotaObservationsForTests, setAccountQuotaFromParsed } from "../../src/codex/quota"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const previousHome = process.env.OPENCODEX_HOME; +const FILE = "codex-quota-cache.json"; +let homes: string[]; +let home: string; +let elapsed: number; +let restoreClock: () => void; + +function advance(ms: number): void { + elapsed += ms; + jest.advanceTimersByTime(ms); +} + +function snapshot(): Record { + return JSON.parse(readFileSync(join(home, FILE), "utf8")).quotas; +} + +beforeEach(async () => { + home = mkdtempSync(join(tmpdir(), "ocx-codex-quota-persist-")); + homes = [home]; + process.env.OPENCODEX_HOME = home; + clearAccountQuota(); + await flushQuotaObservationsForTests(); + jest.useFakeTimers(); + elapsed = 0; + const clock = spyOn(performance, "now").mockImplementation(() => elapsed); + restoreClock = () => clock.mockRestore(); +}); + +afterEach(async () => { + clearAccountQuota(); + await flushQuotaObservationsForTests(); + restoreClock(); + jest.clearAllTimers(); + jest.useRealTimers(); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + for (const path of homes) removeTreeWithRetry(path); +}); + +describe("Codex quota snapshot persistence", () => { + test("a short burst keeps the 250 ms idle debounce and writes the latest quota", () => { + setAccountQuotaFromParsed("pool-a", { weeklyPercent: 10 }); + advance(100); + setAccountQuotaFromParsed("pool-a", { weeklyPercent: 20 }); + advance(249); + expect(existsSync(join(home, FILE))).toBe(false); + advance(1); + expect(snapshot()["pool-a"]?.weeklyPercent).toBe(20); + }); + + test("sustained updates persist the latest quota at each one-second deadline", () => { + for (let batch = 0; batch < 2; batch++) { + for (let update = 1; update <= 10; update++) { + setAccountQuotaFromParsed("pool-a", { weeklyPercent: batch * 10 + update }); + advance(100); + } + expect(snapshot()["pool-a"]?.weeklyPercent).toBe((batch + 1) * 10); + } + }); + + test("clearing one account keeps the pending deadline and excludes its row", () => { + setAccountQuotaFromParsed("removed", { weeklyPercent: 75 }); + for (let update = 1; update <= 9; update++) { + setAccountQuotaFromParsed("kept", { weeklyPercent: update }); + advance(100); + } + clearAccountQuota("removed"); + advance(100); + expect(snapshot()).toEqual({ kept: expect.objectContaining({ weeklyPercent: 9 }) }); + }); + + test("clearing all accounts cancels the old write and resets the deadline for a new home", () => { + const oldHome = home; + for (let update = 0; update < 9; update++) { + setAccountQuotaFromParsed("old-account", { weeklyPercent: update }); + advance(100); + } + clearAccountQuota(); + home = mkdtempSync(join(tmpdir(), "ocx-codex-quota-next-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + setAccountQuotaFromParsed("new-account", { weeklyPercent: 15 }); + advance(249); + expect(existsSync(join(home, FILE))).toBe(false); + expect(existsSync(join(oldHome, FILE))).toBe(false); + advance(1); + expect(snapshot()).toEqual({ "new-account": expect.objectContaining({ weeklyPercent: 15 }) }); + }); + + test("a failed write remains best-effort and the next update gets a fresh debounce", () => { + mkdirSync(join(home, FILE)); + setAccountQuotaFromParsed("pool-a", { weeklyPercent: 10 }); + expect(() => advance(250)).not.toThrow(); + rmdirSync(join(home, FILE)); + advance(1_000); + setAccountQuotaFromParsed("pool-a", { weeklyPercent: 20 }); + advance(249); + expect(existsSync(join(home, FILE))).toBe(false); + advance(1); + expect(snapshot()["pool-a"]?.weeklyPercent).toBe(20); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index fed4bb7ebc7..3b529f6a3de 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1,5 +1,6 @@ { "workbuddy-quota.test.ts": "providers", "workbuddy-login.test.ts": "oauth", "workbuddy-adapter.test.ts": "adapters", "workbuddy-provider.test.ts": "providers", "link-tunnel-history.test.ts": "clients", "link-readiness.test.ts": "clients", "link-diagnostics.test.ts": "clients", "combo-description.test.ts": "routing", "combo-description-catalog.test.ts": "codex-integration", "catalog-auto-review-override.test.ts": "codex-integration", "management-auto-review-settings.test.ts": "server", "management-auto-review-auth.test.ts": "server", "prefer-model-alias-ids.test.ts": "server", "responses-mimo-envelope.test.ts": "responses", "responses-call-id-linkage.test.ts": "responses", "responses-phase-inference.test.ts": "responses", "service-proxy-env.test.ts": "service", + "codex-quota-persistence.test.ts": "codex-integration", "bridge-terminal-refusal.test.ts": "adapters", "responses-replay-metadata-principal.test.ts": "responses", "responses-combo-replay-principal.test.ts": "responses", diff --git a/tests/providers/provider-account-quota-persistence.test.ts b/tests/providers/provider-account-quota-persistence.test.ts index c91208ff9d0..22c4ccf2618 100644 --- a/tests/providers/provider-account-quota-persistence.test.ts +++ b/tests/providers/provider-account-quota-persistence.test.ts @@ -1,10 +1,11 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { afterEach, beforeEach, describe, expect, jest, spyOn, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmdirSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { cancelPendingAccountQuotaPersist, readPersistedAccountQuotas, + readPersistedKiroVerdicts, schedulePersistAccountQuotas, } from "../../src/providers/account-quota-disk"; import type { ProviderQuota } from "../../src/providers/quota-types"; @@ -105,3 +106,85 @@ describe("provider account quota persistence", () => { expect(existsSync(join(home, FILE))).toBe(false); }); }); + +describe("provider quota snapshot deadlines", () => { + let elapsed: number; + let restoreClock: () => void; + const advance = (ms: number) => { + elapsed += ms; + jest.advanceTimersByTime(ms); + }; + + beforeEach(() => { + jest.useFakeTimers(); + elapsed = 0; + const clock = spyOn(performance, "now").mockImplementation(() => elapsed); + restoreClock = () => clock.mockRestore(); + }); + + afterEach(() => { + cancelPendingAccountQuotaPersist(); + restoreClock(); + jest.clearAllTimers(); + jest.useRealTimers(); + }); + + test("a short burst keeps the 250 ms idle debounce", () => { + schedulePersistAccountQuotas(() => [[KEY, quota(10)]]); + advance(100); + schedulePersistAccountQuotas(() => [[KEY, quota(20)]]); + advance(249); + expect(existsSync(join(home, FILE))).toBe(false); + advance(1); + expect(readPersistedAccountQuotas().get(KEY)?.monthlyPercent).toBe(20); + }); + + test("sustained updates persist the latest row and verdict callbacks every second", () => { + const key = "kiro\u0000acct-a"; + const identity = "a".repeat(64); + for (let batch = 0; batch < 2; batch++) { + for (let update = 1; update <= 10; update++) { + const percent = batch * 10 + update; + schedulePersistAccountQuotas(() => [[key, { ...quota(percent), identity }]], + () => [[key, { exhausted: percent >= 20, identity, observedAt: Date.now() }]]); + advance(100); + } + expect(readPersistedAccountQuotas().get(key)?.monthlyPercent).toBe((batch + 1) * 10); + expect(readPersistedKiroVerdicts().get(key)?.exhausted).toBe(batch === 1); + } + }); + + test("cancellation resets the deadline before the next home schedules a write", () => { + const oldHome = home; + for (let update = 0; update < 9; update++) { + schedulePersistAccountQuotas(() => [[KEY, quota(update)]]); + advance(100); + } + cancelPendingAccountQuotaPersist(); + home = mkdtempSync(join(tmpdir(), "ocx-quota-persist-next-")); + process.env.OPENCODEX_HOME = home; + try { + schedulePersistAccountQuotas(() => [["cursor\u0000new-account", quota(15)]]); + advance(249); + expect(existsSync(join(home, FILE))).toBe(false); + expect(existsSync(join(oldHome, FILE))).toBe(false); + advance(1); + expect([...readPersistedAccountQuotas().keys()]).toEqual(["cursor\u0000new-account"]); + } finally { + removeTreeWithRetry(oldHome); + } + }); + + test("a failed write remains best-effort and the next update gets a fresh debounce", () => { + mkdirSync(join(home, FILE)); + schedulePersistAccountQuotas(() => [[KEY, quota(10)]]); + expect(() => advance(250)).not.toThrow(); + rmdirSync(join(home, FILE)); + advance(1_000); + schedulePersistAccountQuotas(() => [[KEY, quota(20)]]); + advance(249); + expect(existsSync(join(home, FILE))).toBe(false); + advance(1); + expect(readPersistedAccountQuotas().get(KEY)?.monthlyPercent).toBe(20); + }); +});