diff --git a/docs-site/src/content/docs/reference/architecture.md b/docs-site/src/content/docs/reference/architecture.md index ba797a4a561..d9a323ae3c5 100644 --- a/docs-site/src/content/docs/reference/architecture.md +++ b/docs-site/src/content/docs/reference/architecture.md @@ -47,8 +47,9 @@ full owner inventory for the Responses surface. `server/index/serve-options.ts` owns the HTTP boundary and delegates the Responses data plane to the `server/responses.ts` facade and its `server/responses/*.ts` modules: -1. `server/index/serve-options.ts` applies CORS and API authentication, rejects new work while draining, and - records request lifecycle metadata. It serves `GET /v1/models`, `POST /v1/responses`, +1. `server/index/serve-options.ts` applies CORS and API authentication, rejects new work while draining + with HTTP 503, JSON `error.code: "server_restarting"`, and `Retry-After: 5`, and + records request lifecycle metadata. Drain responses use the receiving listener's CORS policy. It serves `GET /v1/models`, `POST /v1/responses`, `POST /v1/responses/compact`, `POST /v1/images/generations` / `POST /v1/images/edits` (relayed to an OpenAI-family upstream by `server/images.ts` for codex's built-in `image_gen` tool), `POST /v1/live` / `POST /v1/realtime/calls` (ChatGPT / Codex App voice and OpenAI diff --git a/docs-site/src/content/docs/reference/management-api.md b/docs-site/src/content/docs/reference/management-api.md index b78cbf6087e..3bd3700766c 100644 --- a/docs-site/src/content/docs/reference/management-api.md +++ b/docs-site/src/content/docs/reference/management-api.md @@ -662,6 +662,12 @@ whether to star the repository. ### System lifecycle +During a restart drain, new data-plane requests receive HTTP 503 with JSON +`error.type: "server_error"`, `error.code: "server_restarting"`, and the message +"OpenCodex is restarting; retry this request." Responses retain `Retry-After: 5` +and the receiving listener's CORS policy. This code lets every Codex version retry +the 503 without reporting model capacity; provider overload errors retain their separate mapping. + | Method and path | Purpose | Notable errors | | --- | --- | --- | | `GET /api/system/memory` | Return scalar process, heap, stream, response-state, watchdog, and active-turn metrics. Response-state diagnostics include spill-write status, consecutive failures, fixed privacy-safe failure class, and last failure/success timestamps. `spillLastWriteFailureOrigin` is `retry_returned_timeout`, `timeout_memo_refusal`, or null; cumulative `spillAclRetryReturnedTimeouts` and `spillAclTimeoutMemoRefusals` count terminal failed publications. See [Windows spill diagnostics](/troubleshooting/windows-memory/) for process-local semantics. Raw errors and paths are never returned. | — | diff --git a/docs-site/src/content/docs/troubleshooting/windows-memory.md b/docs-site/src/content/docs/troubleshooting/windows-memory.md index 10bc3799cc8..e65b9c74cfe 100644 --- a/docs-site/src/content/docs/troubleshooting/windows-memory.md +++ b/docs-site/src/content/docs/troubleshooting/windows-memory.md @@ -78,7 +78,8 @@ runtime the leak itself remains an upstream problem: continuation-size fields from this endpoint and offers a confirm-gated **Drain & restart** action: it shows the current active-turn count, waits up to 60s for active turns (reusing - the existing 503 + `Retry-After` drain), then aborts any remaining turns. + the existing HTTP 503 JSON `server_restarting` + `Retry-After: 5` drain), + then aborts any remaining turns. The running proxy owns restart authorization and drain coordination, then exits; an installed service manager launches the replacement when applicable. The action reports success only after a different, identity-verified process diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index f331f7c3c5c..f7f670e8a89 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -603,7 +603,7 @@ "codex-inject.test.ts": "codex-integration", "codex-injected-marker.test.ts": "codex-integration", "codex-integration-record.test.ts": "codex-integration", - "codex-journal.test.ts": "codex-integration", + "codex-journal-recovery.test.ts": "codex-integration", "codex-journal.test.ts": "codex-integration", "codex-legacy-config-keys.test.ts": "codex-integration", "codex-lineage-placement.test.ts": "codex-integration", "codex-log-guard-coderabbit.test.ts": "codex-integration", diff --git a/src/codex/journal.ts b/src/codex/journal.ts index b05f10476e9..384211a07d3 100644 --- a/src/codex/journal.ts +++ b/src/codex/journal.ts @@ -85,6 +85,9 @@ export interface RestoreJournalResult { ownershipRefusal?: CodexHomeOwnerRefusalReason; configRestored: boolean; profileRestored: boolean; + /** True only when restore rewrote or removed a file (not when state was already original). */ + configRewritten: boolean; + profileRewritten: boolean; configChanged: boolean; profileChanged: boolean; complete: boolean; @@ -299,12 +302,13 @@ export function restoreJournalState(): RestoreJournalResult { try { assertCodexHomeOwner(CODEX_HOME); } catch (error) { if (!(error instanceof CodexHomeOwnerRefusal)) throw error; - return { configRestored: false, profileRestored: false, configChanged: false, profileChanged: false, - complete: false, unverified: true, ownershipRefusal: error.reason }; + return { configRestored: false, profileRestored: false, configRewritten: false, profileRewritten: false, + configChanged: false, profileChanged: false, complete: false, unverified: true, ownershipRefusal: error.reason }; } const journal = readJournal(); if (!journal) { - return { configRestored: false, profileRestored: false, configChanged: false, profileChanged: false, complete: false, unverified: false }; + return { configRestored: false, profileRestored: false, configRewritten: false, profileRewritten: false, + configChanged: false, profileChanged: false, complete: false, unverified: false }; } const currentConfig = existsSync(CODEX_CONFIG_PATH) ? readFileSync(CODEX_CONFIG_PATH, "utf-8") : null; const currentProfile = existsSync(CODEX_PROFILE_PATH) ? readFileSync(CODEX_PROFILE_PATH, "utf-8") : null; @@ -316,6 +320,8 @@ export function restoreJournalState(): RestoreJournalResult { return { configRestored: comparison.configAlreadyOriginal, profileRestored: comparison.profileAlreadyOriginal, + configRewritten: false, + profileRewritten: false, configChanged: !configUnchanged, profileChanged: !profileUnchanged, complete: false, @@ -326,14 +332,18 @@ export function restoreJournalState(): RestoreJournalResult { assertCodexHomeOwner(CODEX_HOME); let configRestored = comparison.configAlreadyOriginal; let profileRestored = comparison.profileAlreadyOriginal; + let configRewritten = false; + let profileRewritten = false; if (configUnchanged && !configRestored) { atomicWriteFile(CODEX_CONFIG_PATH, comparison.originalConfig); configRestored = true; + configRewritten = true; } if (profileUnchanged && !profileRestored) { if (comparison.originalProfile !== null) { atomicWriteFile(CODEX_PROFILE_PATH, comparison.originalProfile); profileRestored = true; + profileRewritten = true; } else if (existsSync(CODEX_PROFILE_PATH)) { // "There was no profile before, so remove the one we generated." Claiming success // without checking is how a caller ends up deleting the journal, reporting a clean @@ -343,6 +353,7 @@ export function restoreJournalState(): RestoreJournalResult { try { unlinkSync(CODEX_PROFILE_PATH); profileRestored = true; + profileRewritten = true; } catch (error) { profileRestored = (error as NodeJS.ErrnoException).code === "ENOENT"; } @@ -355,6 +366,8 @@ export function restoreJournalState(): RestoreJournalResult { return { configRestored, profileRestored, + configRewritten, + profileRewritten, configChanged: !configUnchanged, profileChanged: !profileUnchanged, complete, @@ -367,6 +380,11 @@ export function restoreJournal(): boolean { return restoreJournalState().complete; } +/** A no-rewrite recovery is quiet only when the stale journal is actually gone. */ +function warnIfJournalRetained(): void { + if (existsSync(JOURNAL_PATH)) console.error("⚠️ Codex journal recovery found nothing to restore, but the journal could not be removed; it will be retried on the next start."); +} + export interface ReconcileJournalOptions { activeClientApiKeyId?: string; } @@ -388,7 +406,13 @@ export function reconcileJournal(options: ReconcileJournalOptions = {}): boolean console.error("⚠️ Codex journal recovery was not verified; current configuration files and the journal were preserved."); return false; } - if (!restored.configRestored && !restored.profileRestored) return false; + if (!restored.complete) { + console.error("⚠️ Codex journal recovery was incomplete; the journal was preserved — check the Codex files."); + return false; + } + // Warn only when restore actually rewrote state; an already-original + // snapshot still cleans itself up but has nothing to report. + if (!restored.configRewritten && !restored.profileRewritten) { warnIfJournalRetained(); return false; } console.error(`⚠️ Uncommitted or mismatched client routing (${owner.apiKeyId}) was restored from the Codex journal.`); return true; } @@ -406,7 +430,14 @@ export function reconcileJournal(options: ReconcileJournalOptions = {}): boolean console.error("⚠️ Codex journal recovery was not verified; current configuration files and the journal were preserved."); return false; } - if (!restored.configRestored && !restored.profileRestored) return false; + if (!restored.complete) { + console.error("⚠️ Codex journal recovery was incomplete; the journal was preserved — check the Codex files."); + return false; + } + // Warn only when restore actually rewrote state. A stale journal whose + // artifacts are already original still cleans itself up (complete removes + // it) but has nothing to report — claiming a restore then cries wolf. + if (!restored.configRewritten && !restored.profileRewritten) { warnIfJournalRetained(); return false; } console.error(`⚠️ Previous session (PID ${pid}) did not shut down cleanly. Codex state restored from journal.`); return true; } diff --git a/src/server/index.ts b/src/server/index.ts index 02d8e03c196..525edba5978 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -463,21 +463,19 @@ function startServerWithSpendLedgerOwner(port: number | undefined, deps: StartSe return serveGuiFile(rawPath) !== null; } - // Codex treats empty / non-JSON 503 bodies as "Unknown error" (#452). Keep Retry-After and - // the server_is_overloaded code so clients can back off, but always return a JSON envelope. - // These two run BEFORE the auth/origin checks, so they need the receiving listener's policy - // explicitly (#1102). Reaching for the shared `config` here would attach public-policy CORS - // headers to a 503 on the loopback listener — no model runs and no credential is spent, but - // it is the one error path that would answer a rebinding origin with its own origin echoed - // back. + // Codex maps 503 + server_is_overloaded to ServerOverloaded ("model at capacity"), retried + // only with retry advice that older clients ignore (#6642); server_restarting is a + // retryable UnexpectedStatus in every version and never reads as model capacity. + // Keep JSON (empty / non-JSON 503 means "Unknown error", #452) and Retry-After. + // Drain and busy run BEFORE auth/origin checks: use the receiving listener's policy + // (#1102), because shared public `config` could echo a rebinding origin on loopback. function drainingResponse(req: Request, policy: RequestPolicyView): Response { - const response = formatErrorResponse(503, "server_error", "Service shutting down"); - const headers = new Headers(response.headers); - for (const [name, value] of Object.entries(corsHeaders(req, policy))) { - headers.set(name, value); - } - headers.set("Retry-After", "5"); - return new Response(response.body, { status: 503, headers }); + return withCors(new Response(JSON.stringify({ + error: { type: "server_error", code: "server_restarting", message: "OpenCodex is restarting; retry this request." }, + }), { + status: 503, + headers: { "Content-Type": "application/json", "Retry-After": "5" }, + }), req, policy); } function serverBusyResponse(req: Request, resource: string, policy: RequestPolicyView): Response { diff --git a/structure/codex-home.md b/structure/codex-home.md index 5fd0211942f..1997cdf2382 100644 --- a/structure/codex-home.md +++ b/structure/codex-home.md @@ -80,6 +80,17 @@ injected hash. Native content can establish a fresh snapshot; routed content can unverified older original. Existing hash-backed edit preservation and external-provider opt-out remain separate paths. +`src/codex/journal.ts` distinguishes restored state from actual writes with `configRewritten` +and `profileRewritten`; removing a generated profile counts as a write, while already-original +state and an ENOENT removal outcome do not. `reconcileJournal` checks unverified recovery first, +then incomplete recovery, before its silent no-write return. Both dead-process and mismatched-client +recovery warn without file contents or client identity when either artifact remains unrestored, +return false and retain the journal, including when config was rewritten but profile removal failed. +Only complete recovery with an actual write reports a restore; complete already-original recovery +removes the stale journal silently, and warns instead when the journal file is still present +afterwards (`removeJournal` ignores unlink errors such as a Windows lock). Ownership/hash checks and `profileRestoreFailed` remain intact. +`tests/codex-integration/codex-journal-recovery.test.ts` covers these diagnostics and write flags. + The source-built Docker image explicitly keeps `CODEX_HOME=/home/bun/.codex` separate from `OPENCODEX_HOME=/home/bun/.opencodex`. Compose persists them in `codex-state` and `ocx-state` respectively, retaining a read-only root. The image creates owner-only diff --git a/structure/ops/service-and-sidecars.md b/structure/ops/service-and-sidecars.md index 764e0e5a811..8262fd53c13 100644 --- a/structure/ops/service-and-sidecars.md +++ b/structure/ops/service-and-sidecars.md @@ -454,6 +454,18 @@ endpoint can never restart. Focused coverage is `tests/update/update-stop-classi ## Restart handoff +During drain, `src/server/index.ts` rejects new data-plane work with HTTP 503 and +an explicit JSON envelope: `error.type` is `server_error`, `error.code` is +`server_restarting`, and `error.message` is "OpenCodex is restarting; retry this request." +The response keeps `Content-Type: application/json`, `Retry-After: 5`, and the +receiving listener's CORS policy, including on the unauthenticated loopback listener. +Codex maps a 503 with `server_is_overloaded` to `ServerOverloaded` ("Selected model is at +capacity"); current Codex retries it only when retry advice survives mapping, and older +clients do not retry it at all. `server_restarting` falls through to retryable +`UnexpectedStatus` in either case and never reports a restart as model capacity. This drain-only +response bypasses the shared provider-overload mapping in `src/lib/errors.ts`. +`tests/codex-integration/issue-452-empty-503.test.ts` pins its body and both listeners' CORS. + A dashboard drain-and-restart (`src/server/management/system-restart.ts`, which is also the restart after a join into a Child) and the client runtime's standalone recycle (`src/client/runtime.ts`) replace their process through `src/server/restart-replacement.ts`. Every replacement `ocx start` diff --git a/structure/runtime.md b/structure/runtime.md index a4ba9577f82..bc2ea888970 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -193,7 +193,7 @@ until shutdown. Normal shutdown restores native Codex; a sibling instance beside `OCX_SERVICE=1`, so managed restarts do not repeatedly restore/reinject; explicit service stop and uninstall still restore. `src/service/cli.ts` removes the service token on uninstall only when persisted client state is disconnected and no pending connect marker owns the newly issued key. `src/client/connect.ts` publishes that fingerprint marker before the key, then clears it with the connection commit or rollback under the client lifecycle and config mutation locks. Connected, invalid, or mismatched client state retains an existing token. A valid pending marker retains only its matching fingerprint; an older marker does not own a replacement service key. An absent token is reported as absent; unsafe, malformed, or unreadable markers and lock, state-read, or deletion failures leave cleanup unverified. `src/cli/tray-proxy.ts` coordinates `ocx restart` and tray restart. A second start requires proof the prior attempt never launched or its child exited, plus confirmed absence under a fresh bounded discovery window; a missing health response alone is insufficient. After an accepted restart, polling continues through uncertain observations until its deadline and requires a new runtime PID on the same port. If the old proxy then disappears, explicit recovery starts it regardless of the Codex autostart preference; a skipped recovery fails rather than reporting success. -The package-tree integrity fence for live package replacement follows the [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence). After client journal reconciliation, `ocx ensure` in `src/cli/dispatch.ts` treats validated connected-client state as a successful no-op without local startup or a remote health probe; invalid or mismatched state still fails, and disconnected state keeps local ensure. `tests/cli/cli-ensure-client-state.test.ts` pins this dispatch contract so the Codex shim does not warn merely because its proxy is delegated to a hub. +The package-tree integrity fence for live package replacement follows the [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence). After client journal reconciliation, `ocx ensure` in `src/cli/dispatch.ts` treats validated connected-client state as a successful no-op without local startup or a remote health probe; invalid or mismatched state still fails, and disconnected state keeps local ensure. `tests/cli/cli-ensure-client-state.test.ts` pins this dispatch contract so the Codex shim does not warn merely because its proxy is delegated to a hub. Startup journal reconciliation, including replacement after restart/drain, follows the [Codex-home recovery contract](codex-home.md#codex-home): already-original recovery is silent, incomplete recovery retains its journal and warns without claiming restoration, and only complete recovery that rewrites or removes an artifact reports a restore; restart admission, drain deadlines and replacement ownership are unchanged. A busy preferred port is never resolved by starting somewhere else. Both questions a start asks about an existing proxy — the pre-bind owner check and the port-is-busy check in `src/cli/index.ts` @@ -206,7 +206,7 @@ holder is refused with the same message the owner check prints (intentional stay `OCX_SERVICE=1`, using the [Windows wrapper protocol](ops/docs-and-release.md#windows-service-wrapper-and-incomplete-updates)), and a holder that does not identify as opencodex is reported as such rather than called foreign, because an identity probe cannot distinguish a foreign server from an unreachable one. An explicit `--port` still never hops — it waits for the -pin through `src/server/port-reclaim.ts` — and a configured `port: 0` still means "ask the OS". A restart replacement that finds its own draining parent waits for it instead ([restart handoff](ops/service-and-sidecars.md#restart-handoff)). +pin through `src/server/port-reclaim.ts` — and a configured `port: 0` still means "ask the OS". A restart replacement that finds its own draining parent waits for it instead ([restart handoff](ops/service-and-sidecars.md#restart-handoff)). During drain, `src/server/index.ts` answers new data-plane work with HTTP 503 JSON `server_restarting`, `Retry-After: 5`, and receiving-listener CORS; the same handoff section owns that response contract. Every `startServer` invocation acquires the `src/lib/spend-ledger-owner.ts` SQLite writer lease for its resolved OpenCodex state directory before loading configuration or binding a listener. diff --git a/tests/codex-integration/codex-journal-recovery.test.ts b/tests/codex-integration/codex-journal-recovery.test.ts new file mode 100644 index 00000000000..4f3a5846add --- /dev/null +++ b/tests/codex-integration/codex-journal-recovery.test.ts @@ -0,0 +1,202 @@ +import { afterEach, beforeEach, describe, expect, setDefaultTimeout, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import { existsSync, mkdtempSync, readFileSync, realpathSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { repoRoot } from "../helpers/repo-root"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; + +setDefaultTimeout(SPAWN_BUDGET_MS); + +const original = '# original config\nmodel_provider = "openai"\n'; +const injected = '# modified\nmodel_provider = "opencodex"\n'; +const profile = '# generated profile\nmodel_provider = "opencodex"\n'; +const deadPid = 1234567; +const incompleteWarning = "Codex journal recovery was incomplete; the journal was preserved — check the Codex files."; +const hash = (value: string) => createHash("sha256").update(value).digest("hex"); + +function runScript(codexHome: string, script: string) { + const result = spawnSync(process.execPath, ["--eval", script], { + cwd: repoRoot(), + env: { ...process.env, CODEX_HOME: codexHome }, + encoding: "utf8", + timeout: SPAWN_BUDGET_MS - 5_000, + }); + return { stdout: result.stdout?.trim() ?? "", stderr: result.stderr?.trim() ?? "", status: result.status ?? 1 }; +} + +// The fixture PID is dead by construction, independent of the host's process table. +const deadOwnerScript = ` + const { spyOn } = require("bun:test"); + const originalKill = process.kill; + let probes = 0; + const killSpy = spyOn(process, "kill").mockImplementation((pid, signal) => { + if (pid === ${deadPid} && signal === 0) { + probes += 1; + throw Object.assign(new Error("fixture owner exited"), { code: "ESRCH" }); + } + return originalKill.call(process, pid, signal); + }); +`; + +describe("codex-journal recovery diagnostics", () => { + let testDir: string; + beforeEach(() => { + testDir = realpathSync.native(mkdtempSync(join(tmpdir(), "ocx-journal-recovery-"))); + writeFileSync(join(testDir, "config.toml"), original); + }); + afterEach(() => removeTreeWithRetry(testDir)); + + function seed(owner: "process" | "client", config: string, generatedProfile: boolean) { + writeFileSync(join(testDir, "config.toml"), config); + if (generatedProfile) writeFileSync(join(testDir, "opencodex.config.toml"), profile); + const journal = JSON.stringify({ + version: 1, + originalConfig: Buffer.from(original).toString("base64"), + originalProfile: null, + ...(generatedProfile ? { injectedConfigHash: hash(config), injectedProfileHash: hash(profile) } : {}), + pid: deadPid, + ...(owner === "client" ? { owner: { kind: "client", apiKeyId: "fixture-client-owner" } } : {}), + timestamp: new Date().toISOString(), + }); + writeFileSync(join(testDir, "opencodex-journal.json"), journal); + return journal; + } + + for (const owner of ["process", "client"] as const) { + test(`reconcileJournal stays silent when a ${owner === "process" ? "dead-owner" : "client-owner"} journal needs no rewrite`, () => { + seed(owner, original, false); + const r = runScript(testDir, `${deadOwnerScript} + const config = require("./src/config"); + const originalWrite = config.atomicWriteFile; + let writes = 0; + const writeSpy = spyOn(config, "atomicWriteFile").mockImplementation((...args) => { + writes += 1; + return originalWrite(...args); + }); + const { reconcileJournal } = require("./src/codex/journal"); + try { console.log(JSON.stringify({ restored: reconcileJournal(), probes, writes })); } + finally { writeSpy.mockRestore(); killSpy.mockRestore(); } + `); + expect(r.status).toBe(0); + expect(JSON.parse(r.stdout)).toEqual({ restored: false, probes: owner === "process" ? 1 : 0, writes: 0 }); + expect(readFileSync(join(testDir, "config.toml"), "utf8")).toBe(original); + expect(existsSync(join(testDir, "opencodex-journal.json"))).toBe(false); + expect(r.stderr).toBe(""); + }); + + test(`${owner}-owner no-rewrite recovery warns when the journal cannot be removed`, () => { + const journal = seed(owner, original, false); + const r = runScript(testDir, `${deadOwnerScript} + const fs = require("node:fs"); + const { syncBuiltinESMExports } = require("node:module"); + const journalPath = require("node:path").join(process.env.CODEX_HOME, "opencodex-journal.json"); + const originalUnlink = fs.unlinkSync; + let denied = 0; + const unlinkSpy = spyOn(fs, "unlinkSync").mockImplementation((path) => { + if (path === journalPath) { + denied += 1; + throw Object.assign(new Error("fixture journal locked"), { code: "EBUSY" }); + } + return originalUnlink(path); + }); + syncBuiltinESMExports(); + const { reconcileJournal } = require("./src/codex/journal"); + try { console.log(JSON.stringify({ restored: reconcileJournal(), denied })); } + finally { unlinkSpy.mockRestore(); syncBuiltinESMExports(); killSpy.mockRestore(); } + `); + expect(r.status).toBe(0); + expect(JSON.parse(r.stdout)).toEqual({ restored: false, denied: 1 }); + expect(readFileSync(join(testDir, "config.toml"), "utf8")).toBe(original); + expect(readFileSync(join(testDir, "opencodex-journal.json"), "utf8")).toBe(journal); + expect(r.stderr).toContain("the journal could not be removed"); + expect(r.stderr).not.toContain("restored"); + }); + + for (const configRewritten of [false, true]) { + test(`${owner}-owner incomplete recovery warns when ${configRewritten ? "config was rewritten" : "config is already original"} and profile unlink fails`, () => { + const journal = seed(owner, configRewritten ? injected : original, true); + const r = runScript(testDir, `${deadOwnerScript} + const fs = require("node:fs"); + const { syncBuiltinESMExports } = require("node:module"); + const profilePath = require("node:path").join(process.env.CODEX_HOME, "opencodex.config.toml"); + const originalUnlink = fs.unlinkSync; + let denied = 0; + const unlinkSpy = spyOn(fs, "unlinkSync").mockImplementation((path) => { + if (path === profilePath) { + denied += 1; + throw Object.assign(new Error("fixture profile unlink denied"), { code: "EPERM" }); + } + return originalUnlink(path); + }); + syncBuiltinESMExports(); + const { reconcileJournal } = require("./src/codex/journal"); + try { console.log(JSON.stringify({ restored: reconcileJournal(), denied, probes })); } + finally { unlinkSpy.mockRestore(); syncBuiltinESMExports(); killSpy.mockRestore(); } + `); + expect(r.status).toBe(0); + const out = JSON.parse(r.stdout); + expect(out.denied).toBe(1); + expect(out.probes).toBe(owner === "process" ? 1 : 0); + expect(readFileSync(join(testDir, "config.toml"), "utf8")).toBe(original); + expect(readFileSync(join(testDir, "opencodex.config.toml"), "utf8")).toBe(profile); + expect(readFileSync(join(testDir, "opencodex-journal.json"), "utf8")).toBe(journal); + expect(out.restored).toBe(false); + expect(r.stderr).toBe(`⚠️ ${incompleteWarning}`); + expect(r.stderr).not.toContain("restored"); + expect(r.stderr).not.toContain("fixture-client-owner"); + expect(r.stderr).not.toContain(testDir); + }); + } + } + + for (const mode of ["already-original", "rewrite-profile", "remove-profile", "unlink-ENOENT", "unlink-EPERM"] as const) { + test(`restoreJournalState reports actual writes for ${mode}`, () => { + seed("process", mode === "already-original" ? original : injected, mode !== "already-original"); + if (mode === "rewrite-profile") { + const journalPath = join(testDir, "opencodex-journal.json"); + const journal = JSON.parse(readFileSync(journalPath, "utf8")); + journal.originalProfile = Buffer.from("# original profile\n").toString("base64"); + writeFileSync(journalPath, JSON.stringify(journal)); + } + const r = runScript(testDir, ` + const { spyOn } = require("bun:test"); + const fs = require("node:fs"); + const { syncBuiltinESMExports } = require("node:module"); + const profilePath = require("node:path").join(process.env.CODEX_HOME, "opencodex.config.toml"); + const originalUnlink = fs.unlinkSync; + let denied = 0; + const unlinkSpy = spyOn(fs, "unlinkSync").mockImplementation((path) => { + if (path === profilePath && ${mode === "unlink-ENOENT" || mode === "unlink-EPERM"}) { + denied += 1; + if (${mode === "unlink-ENOENT"}) originalUnlink(path); + throw Object.assign(new Error("fixture unlink result"), { code: ${JSON.stringify(mode === "unlink-ENOENT" ? "ENOENT" : "EPERM")} }); + } + return originalUnlink(path); + }); + syncBuiltinESMExports(); + const { restoreJournalState } = require("./src/codex/journal"); + try { console.log(JSON.stringify({ result: restoreJournalState(), denied })); } + finally { unlinkSpy.mockRestore(); syncBuiltinESMExports(); } + `); + expect(r.status).toBe(0); + const out = JSON.parse(r.stdout); + expect(out.result).toMatchObject({ + configRestored: true, + profileRestored: mode !== "unlink-EPERM", + configRewritten: mode !== "already-original", + profileRewritten: mode === "rewrite-profile" || mode === "remove-profile", + complete: mode !== "unlink-EPERM", + unverified: false, + }); + expect(out.result.profileRestoreFailed).toBe(mode === "unlink-EPERM" ? true : undefined); + expect(out.denied).toBe(mode.startsWith("unlink-") ? 1 : 0); + expect(readFileSync(join(testDir, "config.toml"), "utf8")).toBe(original); + expect(existsSync(join(testDir, "opencodex-journal.json"))).toBe(mode === "unlink-EPERM"); + if (mode === "rewrite-profile") expect(readFileSync(join(testDir, "opencodex.config.toml"), "utf8")).toBe("# original profile\n"); + else expect(existsSync(join(testDir, "opencodex.config.toml"))).toBe(mode === "unlink-EPERM"); + }); + } +}); diff --git a/tests/codex-integration/issue-452-empty-503.test.ts b/tests/codex-integration/issue-452-empty-503.test.ts index c04b6a8fdfb..d8aa8049535 100644 --- a/tests/codex-integration/issue-452-empty-503.test.ts +++ b/tests/codex-integration/issue-452-empty-503.test.ts @@ -11,6 +11,7 @@ import { getDebugLogEntries, resetDebugLogBufferForTests } from "../../src/lib/d import { resetDebugSettingsForTests } from "../../src/lib/debug-settings"; import { setDraining } from "../../src/server/lifecycle"; import { startServer } from "../../src/server"; +import { findAvailablePort } from "../../src/server/ports"; import { readDisplaySafeErrorText } from "../../src/server/responses/core"; import { formatPassthroughUpstreamError } from "../../src/server/responses/passthrough-error"; import type { OcxConfig, OcxParsedRequest } from "../../src/types"; @@ -348,13 +349,17 @@ describe("passthrough empty 503 (#452)", () => { }); describe("drain 503 JSON (#452)", () => { - test("POST /v1/responses while draining returns JSON error body", async () => { + test("POST /v1/responses while draining returns retryable JSON with listener-policy CORS (#6642)", async () => { if (existsSync(TEST_DIR)) removeTreeWithRetry(TEST_DIR); mkdirSync(TEST_DIR, { recursive: true }); process.env.OPENCODEX_HOME = TEST_DIR; - delete process.env.OPENCODEX_API_AUTH_TOKEN; + process.env.OPENCODEX_API_AUTH_TOKEN = "drain-test-token"; + const loopbackPort = await findAvailablePort(0, "127.0.0.1"); + const publicPort = await findAvailablePort(0, "0.0.0.0", { reservedPort: loopbackPort }); saveConfig({ - port: 0, + port: publicPort, + hostname: "0.0.0.0", + unauthenticatedLoopbackListener: { enabled: true, port: loopbackPort }, defaultProvider: "xiaomi", providers: { xiaomi: { @@ -366,24 +371,35 @@ describe("drain 503 JSON (#452)", () => { }, } as OcxConfig); - const server = startServer(0); + const server = startServer(publicPort); try { setDraining(true); - const response = await originalGlobalFetch(new URL("/v1/responses", server.url), { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ model: "mimo-v2.5-pro", input: "hi" }), - }); - expect(response.status).toBe(503); - expect(response.headers.get("retry-after")).toBe("5"); - const json = await response.json() as { error?: { message?: string; code?: string | null } }; - expect(json.error?.message).toContain("shutting down"); - expect(json.error?.code).toBe("server_is_overloaded"); + for (const port of [server.port, loopbackPort]) { + for (const origin of ["http://localhost:20100", "http://rebinding.example.test"]) { + const allowed = origin === "http://localhost:20100"; + // The denied loopback request is same-origin by Host: public policy would echo it. + const host = !allowed && port === loopbackPort ? "rebinding.example.test" : `127.0.0.1:${port}`; + const response = await originalGlobalFetch(`http://127.0.0.1:${port}/v1/responses`, { + method: "POST", + headers: { "content-type": "application/json", Origin: origin, Host: host }, + body: JSON.stringify({ model: "mimo-v2.5-pro", input: "hi" }), + }); + expect(response.status).toBe(503); + expect(response.headers.get("content-type")).toContain("application/json"); + expect(response.headers.get("retry-after")).toBe("5"); + expect(response.headers.get("access-control-allow-origin")) + .toBe(allowed ? origin : `http://localhost:${server.port}`); + const json = await response.json() as { error?: { type?: string; message?: string; code?: string | null } }; + expect(json.error?.code).toBe("server_restarting"); + expect(json.error?.type).toBe("server_error"); + expect(json.error?.message).toBe("OpenCodex is restarting; retry this request."); + } + } } finally { setDraining(false); await server.stop(true); } - }); + }, SERVER_BUDGET_MS); }); describe("openai-chat provider debug (#452)", () => { diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 71e304011d0..0490b533fff 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -383,7 +383,7 @@ "codex-inject-retained-table.test.ts": "codex-integration", "codex-inject-v1-reconcile.test.ts": "codex-integration", "codex-inject-write-lock.test.ts": "codex-integration", "codex-inject.test.ts": "codex-integration", "codex-injected-marker.test.ts": "codex-integration", "codex-integration-record.test.ts": "codex-integration", - "codex-journal.test.ts": "codex-integration", "codex-legacy-config-keys.test.ts": "codex-integration", + "codex-journal-recovery.test.ts": "codex-integration", "codex-journal.test.ts": "codex-integration", "codex-legacy-config-keys.test.ts": "codex-integration", "codex-lineage-placement.test.ts": "codex-integration", "codex-log-guard-coderabbit.test.ts": "codex-integration", "codex-log-guard-doctor-coderabbit.test.ts": "codex-integration", "codex-log-guard-doctor-protection.test.ts": "codex-integration",