diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index d80d6991af..cdc8d6af52 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -27,7 +27,7 @@ runs helper features around provider requests. | `codexAutoStart?` | `boolean` | `true` | Let the Codex shim run `ocx ensure` before launching Codex. False makes ensure a no-op. | | `codexShimAutoRestore?` | `boolean` | `true` | Restore an installed shim after a completed external Codex update replaces it. Environment opt-out: `OPENCODEX_CODEX_SHIM_AUTO_RESTORE=0`. | | `codexDesktopAuthless?` | `boolean` | `false` | Opt-in authless Codex Desktop routing on a loopback bind: inject the dedicated `opencodex` provider with `requires_openai_auth = false` so Desktop opens without a ChatGPT login. Ignored on non-loopback binds. `ocx system settings --desktop-authless on`. See [Codex integration](/guides/codex-integration/#authless-codex-desktop-opt-in). | -| `resetCreditAutoRedeem?` | `{ enabled?: boolean; leadTimeMinutes?: number }` | off | Opt-in: redeem the main Codex account's soonest-expiring reset credit `leadTimeMinutes` (1–60, default 10) before it expires. Every attempt re-reads the upstream credit list first and skips when the credit is gone (for example, redeemed by hand); the `redeem_request_id` is journaled in `$OPENCODEX_HOME/reset-credit-auto-redeem.json` before the call so a crash replays the same idempotent request instead of spending a second credit. Logs carry a hashed account key only. | +| `resetCreditAutoRedeem?` | `{ enabled?: boolean; leadTimeMinutes?: number }` | off | Opt-in: redeem the main Codex account's soonest-expiring reset credit `leadTimeMinutes` (1–60, default 10) before it expires. Every attempt re-reads the upstream credit list first and skips when the credit is gone (for example, redeemed by hand); the `redeem_request_id` is journaled in `$OPENCODEX_HOME/reset-credit-auto-redeem.json` before the call so a crash replays the same idempotent request instead of spending a second credit. Servers sharing this configuration directory coordinate reservations and settlements so one process does not replace another's request record. Logs carry a hashed account key only. | | `syncResumeHistory?` | `boolean` | `true` | Reversible Codex App history compatibility. Original metadata is backed up and restored by `ocx stop` / `ocx restore`. | | `shadowCallIntercept?` | `{ enabled?: boolean; model?: string; sourceModels?: string[] }` | off | Redirect recognized Codex helper/shadow calls to a chosen model while preserving the request's configured reasoning effort. The default source prefix is `gpt-5.6-luna`; older clients through 0.144.x used `gpt-5.4-mini`, which `sourceModels` can restore. | | `webSearchSidecar?` | `OcxWebSearchSidecarConfig` | on when usable | Web-search sidecar options. | diff --git a/src/codex/reset-credit-auto-redeem.ts b/src/codex/reset-credit-auto-redeem.ts index 19b6d3ae0e..0d445ae106 100644 --- a/src/codex/reset-credit-auto-redeem.ts +++ b/src/codex/reset-credit-auto-redeem.ts @@ -1,6 +1,7 @@ import { createHash, randomUUID } from "node:crypto"; import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; +import { ConfigMutationLockError, withConfigMutationLockSync } from "../config"; import { atomicWriteFile } from "../config/atomic-write"; import { getConfigDir } from "../config/paths"; import { registerOptionalShutdownHook } from "../lib/optional-shutdown-hooks"; @@ -91,9 +92,9 @@ function readJournal(path: string): Journal { } } -function writeJournal(path: string, journal: Journal): void { +function writeJournal(path: string, journal: Journal, now: number): void { // Keep only entries whose credit could still matter: settled ones older than a week are noise. - const cutoff = Date.now() - 7 * 24 * 60 * 60_000; + const cutoff = now - 7 * 24 * 60 * 60_000; journal.entries = journal.entries.filter(e => e.state !== "settled" || e.updatedAt > cutoff); atomicWriteFile(path, JSON.stringify(journal, null, 2)); } @@ -112,6 +113,7 @@ export interface AutoRedeemDeps { now?: () => number; setTimer?: (fn: () => void, ms: number) => unknown; clearTimer?: (handle: unknown) => void; + /** Callers sharing an overridden journal must also share the OPENCODEX_HOME mutation coordinator. */ journalFile?: string; log?: (line: string) => void; /** Upper bound on one sleep so a laptop sleep or clock jump re-checks rather than trusting a stale plan. */ @@ -155,15 +157,38 @@ export function createResetCreditAutoRedeemer(deps: AutoRedeemDeps): ResetCredit handle = setTimer(() => { handle = null; void tick(); }, Math.max(0, Math.min(ms, maxSleepMs))); }; + const retryJournal = (error: unknown): void => { + const cause = error instanceof ConfigMutationLockError ? error.cause : undefined; + const code = cause && typeof cause === "object" && "code" in cause ? String(cause.code) : ""; + const busy = code === "SQLITE_BUSY" || code === "SQLITE_LOCKED" + || (cause instanceof Error && /database (?:is|table is) locked/i.test(cause.message)); + schedule(busy ? 1_000 : idleRecheckMs); + }; + const dispatch = async (plan: AutoRedeemPlan): Promise => { - const journal = readJournal(path); - let entry = journal.entries.find(e => e.accountKey === accountKey && e.grantedAt === plan.grantedAt && e.expiresAt === plan.expiresAt); - if (entry?.state === "settled") return { kind: "skipped", reason: "credit-gone" }; - if (!entry) { - entry = { accountKey, grantedAt: plan.grantedAt, expiresAt: plan.expiresAt, redeemRequestId: randomUUID(), state: "dispatched", updatedAt: now() }; - journal.entries.push(entry); - // Journal BEFORE the network call: a crash after this line replays the same request id. - writeJournal(path, journal); + // Reserve under the shared config-mutation lock. `inFlight` only serializes ticks inside + // ONE process; two servers on the same config dir would otherwise both read a journal with + // no entry, each mint a different `redeem_request_id`, and spend two credits for one plan. + let entry: JournalEntry; + try { + entry = withConfigMutationLockSync(() => { + const journal = readJournal(path); + const existing = journal.entries.find(e => e.accountKey === accountKey && e.grantedAt === plan.grantedAt && e.expiresAt === plan.expiresAt); + if (existing) return existing; + const created: JournalEntry = { accountKey, grantedAt: plan.grantedAt, expiresAt: plan.expiresAt, redeemRequestId: randomUUID(), state: "dispatched", updatedAt: now() }; + journal.entries.push(created); + // Journal BEFORE the network call: a crash after this line replays the same request id. + writeJournal(path, journal, created.updatedAt); + return created; + }); + } catch (error) { + // Only contention gets a short retry; persistent storage failures must not spin. + retryJournal(error); + return { kind: "error", message: error instanceof Error ? error.message : "journal reservation failed" }; + } + if (entry.state === "settled") { + schedule(idleRecheckMs); + return { kind: "skipped", reason: "credit-gone" }; } log(`[opencodex] reset-credit auto-redeem: dispatching for account ${accountKey} (credit expires ${plan.expiresAt})`); let result: { code: string }; @@ -174,9 +199,25 @@ export function createResetCreditAutoRedeemer(deps: AutoRedeemDeps): ResetCredit schedule(60_000); return { kind: "ambiguous", redeemRequestId: entry.redeemRequestId }; } - entry.state = "settled"; - entry.updatedAt = now(); - writeJournal(path, journal); + // Re-read under the lock: a peer may have appended its own entries since the reservation, + // and writing a stale in-memory journal would drop them. + try { + withConfigMutationLockSync(() => { + const journal = readJournal(path); + const current = journal.entries.find(e => e.accountKey === accountKey && e.grantedAt === plan.grantedAt && e.expiresAt === plan.expiresAt); + if (!current || current.redeemRequestId !== entry.redeemRequestId) { + throw new Error("auto-redeem journal reservation changed before settlement"); + } + current.state = "settled"; + current.updatedAt = now(); + writeJournal(path, journal, current.updatedAt); + }); + } catch (error) { + // Upstream answered, but settlement could not be committed. Preserve any reservation; + // a later dispatch must reuse its request id. A vanished credit may never dispatch again. + retryJournal(error); + return { kind: "error", message: error instanceof Error ? error.message : "journal settlement failed" }; + } log(`[opencodex] reset-credit auto-redeem: upstream answered ${result.code} for account ${accountKey}`); schedule(idleRecheckMs); return { kind: "dispatched", code: result.code, redeemRequestId: entry.redeemRequestId }; diff --git a/tests/codex-integration/codex-reset-credit-auto-redeem.test.ts b/tests/codex-integration/codex-reset-credit-auto-redeem.test.ts index f19f208cc1..d7656e2193 100644 --- a/tests/codex-integration/codex-reset-credit-auto-redeem.test.ts +++ b/tests/codex-integration/codex-reset-credit-auto-redeem.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, readFileSync} from "node:fs"; +import { Database } from "bun:sqlite"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -9,6 +10,7 @@ import { type ResetCredit, } from "../../src/codex/reset-credit-auto-redeem"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { readConfigGeneration } from "../../src/config"; const T0 = Date.parse("2026-09-02T10:00:00Z"); const MIN = 60_000; @@ -18,19 +20,20 @@ const credit = (expiresInMin: number, grantedAt = "2026-09-01T00:00:00Z"): Reset }); /** Fake clock + manual timer: fire() runs the pending timer at its due time. */ -function harness(opts: { credits: () => ResetCredit[]; enabled?: () => boolean; lead?: number; journalFile: string; consumeCode?: string; consumeThrows?: boolean }) { +function harness(opts: { credits: () => ResetCredit[]; enabled?: () => boolean; lead?: number; journalFile: string; accountId?: string; consumeCode?: string; consumeThrows?: boolean; consume?: (id: string) => Promise<{ code: string }> }) { let now = T0; let pending: { fn: () => void; at: number } | null = null; const consumed: string[] = []; const logs: string[] = []; let inspects = 0; const redeemer = createResetCreditAutoRedeemer({ - accountId: "acct-main", + accountId: opts.accountId ?? "acct-main", settings: () => ({ enabled: opts.enabled ? opts.enabled() : true, leadTimeMinutes: opts.lead ?? 10 }), inspect: async () => { inspects += 1; return { credits: opts.credits() }; }, consume: async id => { if (opts.consumeThrows) throw new Error("socket hangup"); consumed.push(id); + if (opts.consume) return opts.consume(id); return { code: opts.consumeCode ?? "reset" }; }, now: () => now, @@ -49,8 +52,17 @@ function harness(opts: { credits: () => ResetCredit[]; enabled?: () => boolean; } let dir = ""; -beforeEach(() => { dir = mkdtempSync(join(tmpdir(), "ocx-auto-redeem-")); }); -afterEach(() => { removeTreeWithRetry(dir); }); +let oldHome: string | undefined; +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "ocx-auto-redeem-")); + oldHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = dir; +}); +afterEach(() => { + if (oldHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = oldHome; + removeTreeWithRetry(dir); +}); describe("reset-credit auto-redeem settings + plan (#822)", () => { test("default off; malformed reads as off; lead time clamped", () => { @@ -150,6 +162,151 @@ describe("reset-credit auto-redeemer runtime (#822)", () => { expect(h.consumed).toHaveLength(0); }); + test("settling a delayed consume preserves a peer's settled journal entry", async () => { + const journalFile = join(dir, "j.json"); + let entered!: () => void; + let release!: () => void; + const started = new Promise(resolve => { entered = resolve; }); + const gate = new Promise(resolve => { release = resolve; }); + const a = harness({ credits: () => [credit(30)], journalFile, accountId: "acct-a", consume: async () => { + entered(); + await gate; + return { code: "reset" }; + } }); + const b = harness({ credits: () => [credit(30)], journalFile, accountId: "acct-b" }); + a.setNow(T0 + 20 * MIN); + b.setNow(T0 + 20 * MIN); + const first = a.redeemer.tick(); + try { + await Promise.race([started, first.then(() => { throw new Error("first consume was not entered"); })]); + expect((await b.redeemer.tick()).kind).toBe("dispatched"); + } finally { + release(); + await first; + } + expect((await first).kind).toBe("dispatched"); + const entries = JSON.parse(readFileSync(journalFile, "utf8")).entries as Array<{ redeemRequestId: string; state: string }>; + expect(entries).toHaveLength(2); + expect(entries.map(entry => entry.redeemRequestId).sort()).toEqual([...a.consumed, ...b.consumed].sort()); + expect(entries.every(entry => entry.state === "settled")).toBe(true); + expect((await b.redeemer.tick()).kind).toBe("skipped"); + expect(b.consumed).toHaveLength(1); + }); + + test("a separate SQLite writer blocks reservation before any consume", async () => { + const journalFile = join(dir, "j.json"); + const h = harness({ credits: () => [credit(30)], journalFile }); + h.setNow(T0 + 20 * MIN); + expect(readConfigGeneration().kind).toBe("ready"); + const holder = new Database(join(dir, "config-mutation.sqlite"), { readwrite: true, create: false }); + holder.exec("PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); + try { + expect((await h.redeemer.tick()).kind).toBe("error"); + expect(h.consumed).toHaveLength(0); + expect(existsSync(journalFile)).toBe(false); + expect(h.pendingAt()).toBe(T0 + 20 * MIN + 1_000); + } finally { + holder.exec("ROLLBACK"); + holder.close(); + } + h.setNow(T0 + 20 * MIN + 1_000); + expect((await h.redeemer.tick()).kind).toBe("dispatched"); + expect(h.consumed).toHaveLength(1); + const entries = JSON.parse(readFileSync(journalFile, "utf8")).entries; + expect(entries).toHaveLength(1); + expect(entries[0].redeemRequestId).toBe(h.consumed[0]); + expect(entries[0].state).toBe("settled"); + }); + + test("a peer that observes a settled credit keeps checking for future credits", async () => { + const journalFile = join(dir, "j.json"); + const first = harness({ credits: () => [credit(30)], journalFile }); + const peer = harness({ credits: () => [credit(30)], journalFile }); + first.setNow(T0 + 20 * MIN); + peer.setNow(T0 + 20 * MIN); + expect((await first.redeemer.tick()).kind).toBe("dispatched"); + expect((await peer.redeemer.tick()).kind).toBe("skipped"); + expect(peer.consumed).toHaveLength(0); + expect(peer.pendingAt()).toBe(T0 + 35 * MIN); + }); + + test("settlement contention keeps the reserved request id for a later retry", async () => { + const journalFile = join(dir, "j.json"); + let holder: Database | null = null; + let attempts = 0; + const h = harness({ credits: () => [credit(30)], journalFile, consume: async () => { + if (attempts++ === 0) { + holder = new Database(join(dir, "config-mutation.sqlite"), { readwrite: true, create: false }); + holder.exec("PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); + } + return { code: "reset" }; + } }); + h.setNow(T0 + 20 * MIN); + try { + expect((await h.redeemer.tick()).kind).toBe("error"); + const entries = JSON.parse(readFileSync(journalFile, "utf8")).entries; + expect(entries).toHaveLength(1); + expect(entries[0].state).toBe("dispatched"); + expect(entries[0].redeemRequestId).toBe(h.consumed[0]); + expect(h.pendingAt()).toBe(T0 + 20 * MIN + 1_000); + } finally { + if (holder) { + (holder as Database).exec("ROLLBACK"); + (holder as Database).close(); + } + } + h.setNow(T0 + 20 * MIN + 1_000); + expect((await h.redeemer.tick()).kind).toBe("dispatched"); + expect(h.consumed).toHaveLength(2); + expect(h.consumed[0]).toBe(h.consumed[1]); + expect(JSON.parse(readFileSync(journalFile, "utf8")).entries[0].state).toBe("settled"); + }); + + test("journal retention uses the redeemer's injected clock", async () => { + const start = Date.parse("2000-01-01T00:00:00Z"); + const journalFile = join(dir, "j.json"); + const h = harness({ journalFile, credits: () => [{ + granted_at: "1999-12-31T00:00:00Z", + expires_at: new Date(start + 30 * MIN).toISOString(), + }] }); + h.setNow(start + 20 * MIN); + expect((await h.redeemer.tick()).kind).toBe("dispatched"); + const entries = JSON.parse(readFileSync(journalFile, "utf8")).entries; + expect(entries).toHaveLength(1); + expect(entries[0].updatedAt).toBe(start + 20 * MIN); + }); + + test("a persistent reservation write failure uses the idle retry interval", async () => { + const journalFile = join(dir, "journal-directory"); + mkdirSync(journalFile); + const h = harness({ credits: () => [credit(30)], journalFile }); + h.setNow(T0 + 20 * MIN); + expect((await h.redeemer.tick()).kind).toBe("error"); + expect(h.consumed).toHaveLength(0); + expect(h.pendingAt()).toBe(T0 + 35 * MIN); + }); + + for (const changedReservation of ["missing", "replaced"]) { + test(`settlement rejects a ${changedReservation} reservation without overwriting it`, async () => { + const journalFile = join(dir, "j.json"); + let replacement = ""; + const h = harness({ credits: () => [credit(30)], journalFile, consume: async () => { + const journal = JSON.parse(readFileSync(journalFile, "utf8")); + if (changedReservation === "missing") journal.entries = []; + else journal.entries[0].redeemRequestId = "replacement-request"; + replacement = JSON.stringify(journal); + writeFileSync(journalFile, replacement); + return { code: "reset" }; + } }); + h.setNow(T0 + 20 * MIN); + const outcome = await h.redeemer.tick(); + expect(outcome).toEqual({ kind: "error", message: "auto-redeem journal reservation changed before settlement" }); + expect(h.consumed).toHaveLength(1); + expect(readFileSync(journalFile, "utf8")).toBe(replacement); + expect(h.pendingAt()).toBe(T0 + 35 * MIN); + }); + } + test("stop clears the timer", async () => { const h = harness({ credits: () => [credit(30)], journalFile: join(dir, "j.json") }); await h.redeemer.tick();