From 0caa45b90070ba339b1103ec38f1e274192f09d3 Mon Sep 17 00:00:00 2001 From: ndycode Date: Mon, 15 Jun 2026 04:18:09 +0800 Subject: [PATCH 1/3] fix(rotation): break stale-recovery deadlock on transient account state Issue #606: the runtime rotation proxy returns a permanent 503 ("All managed Codex accounts are temporarily unavailable") even when accounts are healthy and `doctor` passes, because stale-runtime recovery is deadlocked against the very transient state it is meant to clear. Per-account cooldowns (`coolingDownUntil`/`cooldownReason`) and `rateLimitResetTimes` are serialized to disk by `buildStorageSnapshot`, so `recoverStaleRuntimeState`'s `loadFromDisk()` restores the same state that wedged the pool. `resetVolatileRuntimeState()` only clears global singletons (trackers, circuit breakers), not this per-account state. The recovery guard then refused to reload when any account was "rate-limited" or "cooling-down*", so the only path that could clear the state never fired. The two halves are coupled; neither fixes the deadlock alone: - relax the recovery guard so only "policy-blocked" (external, won't change across a reload) still suppresses recovery; transient reasons now let it through. - add `AccountManager.clearAccountTransientState()` (and a `clearAllRateLimits` helper) and call it in the recovery path right after `loadFromDisk()`, before the reloaded manager is published, so recovery starts from a real clean slate. Regression tests pin both halves (each fails if either change is reverted): an all-cooling-down pool now recovers to 200, policy-blocked pools still do not trigger a reload, and unit tests cover the new clearing method. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/accounts.ts | 26 +++++ lib/accounts/rate-limits.ts | 14 +++ lib/runtime-rotation-proxy.ts | 13 ++- lib/runtime/rotation-proxy-state.ts | 17 ++++ test/accounts.test.ts | 85 +++++++++++++++++ test/runtime-rotation-proxy.test.ts | 143 ++++++++++++++++++++++++---- 6 files changed, 277 insertions(+), 21 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index 5d57ea1cd..2e4f642f3 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -65,6 +65,7 @@ export { getQuotaKey, clampNonNegativeInt, clearExpiredRateLimits, + clearAllRateLimits, isRateLimitedForQuotaKey, isRateLimitedForFamily, formatWaitTime, @@ -91,6 +92,7 @@ import { clampNonNegativeInt, getQuotaKey, clearExpiredRateLimits, + clearAllRateLimits, isRateLimitedForFamily, formatWaitTime, type RateLimitReason, @@ -668,6 +670,30 @@ export class AccountManager { resetAllCircuitBreakers(); } + /** + * Wipe per-account transient state — active cooldowns and all rate-limit + * reset windows — across every managed account, then persist the cleared + * pool so the next reload does not restore it. + * + * `resetVolatileRuntimeState` only clears process-global singletons (rotation + * trackers, circuit breakers); the cooldown timestamps and `rateLimitResetTimes` + * maps live on the `ManagedAccount` objects and are serialized to disk by + * `buildStorageSnapshot`. The stale-runtime recovery path reloads accounts + * from that snapshot, so without this the same transient state that wedged the + * pool is restored verbatim and recovery never escapes it (issue #606). + */ + clearAccountTransientState(): void { + if (this.accounts.length === 0) return; + // Snapshot the reference list so a concurrent mutation of `this.accounts` + // (e.g. removeAccount) cannot reshape the array mid-iteration. + for (const account of [...this.accounts]) { + this.clearAccountCooldown(account); + clearAllRateLimits(account); + account.lastRateLimitReason = undefined; + } + this.saveToDiskDebounced(); + } + setActiveIndex(index: number): ManagedAccount | null { if (!Number.isFinite(index)) return null; if (index < 0 || index >= this.accounts.length) return null; diff --git a/lib/accounts/rate-limits.ts b/lib/accounts/rate-limits.ts index e71b42d4e..95350b556 100644 --- a/lib/accounts/rate-limits.ts +++ b/lib/accounts/rate-limits.ts @@ -53,6 +53,20 @@ export function clearExpiredRateLimits(entity: RateLimitedEntity): void { } } +/** + * Remove every rate-limit reset entry from `entity`, regardless of whether the + * window has expired. Unlike {@link clearExpiredRateLimits}, this drops + * still-future entries too. Used by the stale-runtime recovery path to give a + * reloaded account pool a clean slate so transient state persisted to disk + * cannot keep the pool wedged (issue #606). + */ +export function clearAllRateLimits(entity: RateLimitedEntity): void { + const keys = Object.keys(entity.rateLimitResetTimes); + for (const key of keys) { + delete entity.rateLimitResetTimes[key]; + } +} + export function isRateLimitedForQuotaKey(entity: RateLimitedEntity, key: QuotaKey): boolean { const resetTime = entity.rateLimitResetTimes[key]; return resetTime !== undefined && nowMs() < resetTime; diff --git a/lib/runtime-rotation-proxy.ts b/lib/runtime-rotation-proxy.ts index 17ddbfed8..29671ebc8 100644 --- a/lib/runtime-rotation-proxy.ts +++ b/lib/runtime-rotation-proxy.ts @@ -982,10 +982,15 @@ async function handleRequestInner( exhaustionReason === "no-account" && (policyDecision?.blockedAccountIndexes.size ?? 0) === 0 && ![...accountSkipReasons.values()].some( - (reason) => - reason === "rate-limited" || - reason.startsWith("cooling-down") || - reason === "policy-blocked", + // Only policy blocks still suppress stale-state recovery: a policy + // decision is external and won't change across a disk reload, so + // reloading cannot help. "rate-limited" and "cooling-down*" are + // transient states that recovery is *designed* to escape — they are + // persisted to disk (buildStorageSnapshot) and so survive a reload, + // which previously deadlocked the pool against the very recovery that + // would clear them. recoverStaleRuntimeState now wipes that transient + // state after reloading, so let those reasons through (issue #606). + (reason) => reason === "policy-blocked", ) ) { reloadedAfterNoAccount = true; diff --git a/lib/runtime/rotation-proxy-state.ts b/lib/runtime/rotation-proxy-state.ts index 5ea9122b1..8fb60cb4d 100644 --- a/lib/runtime/rotation-proxy-state.ts +++ b/lib/runtime/rotation-proxy-state.ts @@ -99,6 +99,23 @@ export async function recoverStaleRuntimeState( recordRuntimeReset("pool-exhausted-no-account"); const reloaded = await AccountManager.loadFromDisk(); reloaded.setRoutingMutexMode(state.routingMutexMode); + // Wipe per-account cooldowns and rate-limit windows on the freshly + // reloaded pool. `resetVolatileRuntimeState` above only cleared global + // singletons (trackers, circuit breakers); the per-account transient + // state is serialized to disk, so loadFromDisk restores the same state + // that wedged the pool. Clearing it here gives recovery a real clean + // slate before any request can pick up the manager (issue #606). Runs + // before the `state.activeAccountManager` assignment so no concurrent + // request can observe the reloaded manager with stale state. + // + // This also drops still-future rate-limit windows from genuine upstream + // 429s, not just stale ones from a dead prior process. That is the + // intended trade-off: recovery only runs at full pool exhaustion, where + // the alternative is a hard 503 anyway, and the 1s reload dedupe bounds + // re-probing to ~1/sec — the upstream simply re-429s and re-populates the + // window until the next exhaustion. Availability is preferred over + // honoring backoff in this already-degraded state. + reloaded.clearAccountTransientState(); state.activeAccountManager = reloaded; state.knownAccountManagers.add(reloaded); state.lastStaleRuntimeReloadAt = Date.now(); diff --git a/test/accounts.test.ts b/test/accounts.test.ts index de33dee32..4a948ca15 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1447,6 +1447,91 @@ describe("AccountManager", () => { }); }); + describe("clearAccountTransientState", () => { + it("clears active cooldowns on every account", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + { refreshToken: "token-2", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const first = manager.getAccountByIndex(0)!; + const second = manager.getAccountByIndex(1)!; + manager.markAccountCoolingDown(first, 60_000, "network-error"); + manager.markAccountCoolingDown(second, 60_000, "server-error"); + + manager.clearAccountTransientState(); + + expect(manager.isAccountCoolingDown(first)).toBe(false); + expect(manager.isAccountCoolingDown(second)).toBe(false); + expect(first.coolingDownUntil).toBeUndefined(); + expect(first.cooldownReason).toBeUndefined(); + expect(second.coolingDownUntil).toBeUndefined(); + expect(second.cooldownReason).toBeUndefined(); + }); + + it("clears all rate-limit reset windows, including ones still in the future", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [{ refreshToken: "token-1", addedAt: now, lastUsed: now }], + }; + + const manager = new AccountManager(undefined, stored); + const account = manager.getCurrentAccount()!; + manager.markRateLimitedWithReason(account, 60_000, "codex", "quota"); + manager.markRateLimitedWithReason( + account, + 60_000, + "codex", + "tokens", + "gpt-5.2", + ); + expect(Object.keys(account.rateLimitResetTimes).length).toBeGreaterThan(0); + + manager.clearAccountTransientState(); + + expect(account.rateLimitResetTimes).toEqual({}); + expect(account.lastRateLimitReason).toBeUndefined(); + }); + + it("persists the cleared pool so a reload does not restore stale state", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [{ refreshToken: "token-1", addedAt: now, lastUsed: now }], + }; + + const manager = new AccountManager(undefined, stored); + const account = manager.getCurrentAccount()!; + manager.markAccountCoolingDown(account, 60_000, "server-error"); + const saveSpy = vi.spyOn(manager, "saveToDiskDebounced"); + + manager.clearAccountTransientState(); + + expect(saveSpy).toHaveBeenCalledTimes(1); + }); + + it("is a no-op when no accounts are loaded", () => { + const manager = new AccountManager(undefined, { + version: 3 as const, + activeIndex: 0, + accounts: [], + }); + const saveSpy = vi.spyOn(manager, "saveToDiskDebounced"); + + expect(() => manager.clearAccountTransientState()).not.toThrow(); + expect(saveSpy).not.toHaveBeenCalled(); + }); + }); + describe("auth failure tracking", () => { it("increments consecutive auth failures", () => { const now = Date.now(); diff --git a/test/runtime-rotation-proxy.test.ts b/test/runtime-rotation-proxy.test.ts index 2d5aab4dc..59b47a515 100644 --- a/test/runtime-rotation-proxy.test.ts +++ b/test/runtime-rotation-proxy.test.ts @@ -1840,30 +1840,139 @@ describe("runtime rotation proxy", () => { "server-error", ); } + // With every account cooling down, the proxy now *attempts* stale-runtime + // recovery (issue #606): cooling-down is transient and no longer suppresses + // the reload. Force that reload to fail so this test still exercises the + // final-exhaustion path and its skip-reason reporting deterministically. + const loadSpy = vi + .spyOn(AccountManager, "loadFromDisk") + .mockRejectedValue(new Error("reload unavailable")); const { calls, fetchImpl } = createRecordingFetch(() => new Response("should not be called", { status: HTTP_STATUS.OK }), ); const proxy = await startProxy({ accountManager, fetchImpl }); - const response = await postResponses(proxy, { model: "gpt-5-codex" }); - const payload = (await response.json()) as { - error: { - code: string; - reason: string; - account_skip_reasons: Record; - hint: string; + try { + const response = await postResponses(proxy, { model: "gpt-5-codex" }); + const payload = (await response.json()) as { + error: { + code: string; + reason: string; + account_skip_reasons: Record; + hint: string; + }; }; - }; - expect(response.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); - expect(payload.error.code).toBe("codex_runtime_rotation_pool_exhausted"); - expect(payload.error.reason).toBe("no-account"); - expect(payload.error.account_skip_reasons).toMatchObject({ - "0": "cooling-down:network-error", - "1": "cooling-down:server-error", - }); - expect(payload.error.hint).toContain("rotation reset-runtime"); - expect(calls).toHaveLength(0); + expect(loadSpy).toHaveBeenCalledTimes(1); + expect(response.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); + expect(payload.error.code).toBe("codex_runtime_rotation_pool_exhausted"); + expect(payload.error.reason).toBe("no-account"); + expect(payload.error.account_skip_reasons).toMatchObject({ + "0": "cooling-down:network-error", + "1": "cooling-down:server-error", + }); + expect(payload.error.hint).toContain("rotation reset-runtime"); + expect(calls).toHaveLength(0); + } finally { + loadSpy.mockRestore(); + } + }); + + it("recovers from an all-cooling-down pool by reloading and clearing transient state (issue #606)", async () => { + const now = Date.now(); + // Persisted cooldown/rate-limit state survives a reload, so the reloaded + // pool carries the same wedged transient state. recoverStaleRuntimeState + // must clear it before the manager is used, otherwise selection deadlocks + // against the very recovery meant to escape it. + const staleStorage = createStorage(now, 2); + for (const account of staleStorage.accounts) { + account.coolingDownUntil = now + 60_000; + account.cooldownReason = "server-error"; + account.rateLimitResetTimes = { codex: now + 60_000 }; + } + const staleManager = new AccountManager(undefined, staleStorage); + const reloadedStorage = createStorage(now, 2); + for (const account of reloadedStorage.accounts) { + account.coolingDownUntil = now + 60_000; + account.cooldownReason = "server-error"; + account.rateLimitResetTimes = { codex: now + 60_000 }; + } + const reloadedManager = new AccountManager(undefined, reloadedStorage); + const clearSpy = vi.spyOn(reloadedManager, "clearAccountTransientState"); + const loadSpy = vi + .spyOn(AccountManager, "loadFromDisk") + .mockResolvedValueOnce(reloadedManager); + const resetSpy = vi.spyOn(AccountManager, "resetVolatileRuntimeState"); + const { calls, fetchImpl } = createRecordingFetch(() => textEventStream()); + try { + const proxy = await startProxy({ + accountManager: staleManager, + fetchImpl, + }); + + const response = await postResponses(proxy, { model: "gpt-5-codex" }); + await response.text(); + + expect(response.status).toBe(HTTP_STATUS.OK); + expect(loadSpy).toHaveBeenCalledTimes(1); + expect(resetSpy).toHaveBeenCalledTimes(1); + expect(clearSpy).toHaveBeenCalledTimes(1); + // After clearing, the reloaded accounts are selectable again. + expect(reloadedManager.getAccountByIndex(0)?.coolingDownUntil).toBeUndefined(); + expect(reloadedManager.getAccountByIndex(0)?.rateLimitResetTimes).toEqual({}); + expect(calls).toHaveLength(1); + } finally { + loadSpy.mockRestore(); + resetSpy.mockRestore(); + clearSpy.mockRestore(); + } + }); + + it("does not attempt stale-runtime recovery when accounts are policy-blocked (issue #606)", async () => { + const now = Date.now(); + const accountManager = new AccountManager(undefined, createStorage(now, 2)); + // A policy decision is external and will not change across a disk reload, + // so a policy-blocked pool must continue to suppress stale-runtime + // recovery. `allowed` stays true so the request proceeds into account + // selection, where every account is rejected as policy-blocked and + // selection returns null. Recovery is then gated off by the guard's + // `blockedAccountIndexes.size === 0` precondition — the path that keeps a + // policy block from being papered over by a reload. + const policySpy = vi + .spyOn(runtimePolicy, "evaluateRuntimePolicy") + .mockResolvedValue({ + allowed: true, + statusCode: 200, + errorCode: null, + reasons: [], + projectKey: null, + blockedAccountIndexes: new Set([0, 1]), + scoreBoostByAccount: {}, + budgetEvaluations: [], + }); + const loadSpy = vi.spyOn(AccountManager, "loadFromDisk"); + const { calls, fetchImpl } = createRecordingFetch(() => textEventStream()); + try { + const proxy = await startProxy({ accountManager, fetchImpl }); + + const response = await postResponses(proxy, { model: "gpt-5-codex" }); + const payload = (await response.json()) as { + error: { code: string; account_skip_reasons: Record }; + }; + + expect(response.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); + expect(payload.error.code).toBe("codex_runtime_rotation_pool_exhausted"); + expect(payload.error.account_skip_reasons).toMatchObject({ + "0": "policy-blocked", + "1": "policy-blocked", + }); + // Recovery must be suppressed: loadFromDisk is never called. + expect(loadSpy).not.toHaveBeenCalled(); + expect(calls).toHaveLength(0); + } finally { + policySpy.mockRestore(); + loadSpy.mockRestore(); + } }); it("deduplicates concurrent stale-runtime reload recovery", async () => { From c7a755d3acbb6084ce611d668b4b871c2e5b80dc Mon Sep 17 00:00:00 2001 From: ndycode Date: Mon, 15 Jun 2026 04:29:48 +0800 Subject: [PATCH 2/3] fix(rotation): flush cleared transient state to disk during recovery Addresses Greptile P2 on #607: `clearAccountTransientState()` only schedules a 500ms debounced write, so a process exit within that window would let the next startup reload the wedged snapshot. The in-memory clear already unblocks the live pool; this makes the "next reload starts clean" guarantee durable across a restart too. - await `flushPendingSave()` after the clear in the recovery path so the cleared snapshot reaches disk synchronously. Recovery is rare (full-pool exhaustion), so the extra write is negligible. - soften the `clearAccountTransientState` jsdoc to state the disk write is debounced (best-effort) and durability requires a flush. - assert `flushPendingSave` is called in the all-cooling-down recovery test. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/accounts.ts | 9 +++++++-- lib/runtime/rotation-proxy-state.ts | 7 +++++++ test/runtime-rotation-proxy.test.ts | 7 +++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index 2e4f642f3..f1b73ac39 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -672,8 +672,8 @@ export class AccountManager { /** * Wipe per-account transient state — active cooldowns and all rate-limit - * reset windows — across every managed account, then persist the cleared - * pool so the next reload does not restore it. + * reset windows — across every managed account, then schedule a debounced + * persist of the cleared pool. * * `resetVolatileRuntimeState` only clears process-global singletons (rotation * trackers, circuit breakers); the cooldown timestamps and `rateLimitResetTimes` @@ -681,6 +681,11 @@ export class AccountManager { * `buildStorageSnapshot`. The stale-runtime recovery path reloads accounts * from that snapshot, so without this the same transient state that wedged the * pool is restored verbatim and recovery never escapes it (issue #606). + * + * The in-memory clear is immediate (it is what unblocks the live pool). The + * disk write is debounced via `saveToDiskDebounced`, so a caller that needs + * the cleared state to survive a restart should `await flushPendingSave()` + * afterwards rather than rely on the debounce window completing. */ clearAccountTransientState(): void { if (this.accounts.length === 0) return; diff --git a/lib/runtime/rotation-proxy-state.ts b/lib/runtime/rotation-proxy-state.ts index 8fb60cb4d..7500df341 100644 --- a/lib/runtime/rotation-proxy-state.ts +++ b/lib/runtime/rotation-proxy-state.ts @@ -116,6 +116,13 @@ export async function recoverStaleRuntimeState( // window until the next exhaustion. Availability is preferred over // honoring backoff in this already-degraded state. reloaded.clearAccountTransientState(); + // Force the cleared snapshot to disk now rather than waiting out the + // debounce window inside clearAccountTransientState. If the process + // exited during that window the next startup would reload the wedged + // snapshot; flushing here makes the "next reload starts clean" guarantee + // durable across a restart, not just best-effort. Recovery is rare + // (full-pool exhaustion), so the extra synchronous write is cheap. + await reloaded.flushPendingSave(); state.activeAccountManager = reloaded; state.knownAccountManagers.add(reloaded); state.lastStaleRuntimeReloadAt = Date.now(); diff --git a/test/runtime-rotation-proxy.test.ts b/test/runtime-rotation-proxy.test.ts index 59b47a515..5c98f9505 100644 --- a/test/runtime-rotation-proxy.test.ts +++ b/test/runtime-rotation-proxy.test.ts @@ -1899,6 +1899,9 @@ describe("runtime rotation proxy", () => { } const reloadedManager = new AccountManager(undefined, reloadedStorage); const clearSpy = vi.spyOn(reloadedManager, "clearAccountTransientState"); + const flushSpy = vi + .spyOn(reloadedManager, "flushPendingSave") + .mockResolvedValue(); const loadSpy = vi .spyOn(AccountManager, "loadFromDisk") .mockResolvedValueOnce(reloadedManager); @@ -1917,6 +1920,9 @@ describe("runtime rotation proxy", () => { expect(loadSpy).toHaveBeenCalledTimes(1); expect(resetSpy).toHaveBeenCalledTimes(1); expect(clearSpy).toHaveBeenCalledTimes(1); + // The cleared snapshot is flushed to disk synchronously so a restart + // inside the debounce window cannot reload the wedged state. + expect(flushSpy).toHaveBeenCalledTimes(1); // After clearing, the reloaded accounts are selectable again. expect(reloadedManager.getAccountByIndex(0)?.coolingDownUntil).toBeUndefined(); expect(reloadedManager.getAccountByIndex(0)?.rateLimitResetTimes).toEqual({}); @@ -1925,6 +1931,7 @@ describe("runtime rotation proxy", () => { loadSpy.mockRestore(); resetSpy.mockRestore(); clearSpy.mockRestore(); + flushSpy.mockRestore(); } }); From 4549d31216fd1106785fe1df69c3c4d0a02e52a6 Mon Sep 17 00:00:00 2001 From: ndycode Date: Mon, 15 Jun 2026 04:32:16 +0800 Subject: [PATCH 3/3] test(accounts): harden clearAccountTransientState coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address CodeRabbit review on #607: - assert lastRateLimitReason before-state so the cleared assertion proves a present field was removed - upgrade the persistence test to flush and inspect the actual persisted snapshot (coolingDownUntil/cooldownReason/rateLimitResetTimes cleared), not just that a debounced save was scheduled — this is the #606 durability path - add a combined-state case (one account with both cooldown and rate-limit) proving the method clears both in one pass - add a mixed-state case (cooldown / rate-limited / clean accounts) proving iteration handles a clean account without throwing Co-Authored-By: Claude Opus 4.8 (1M context) --- test/accounts.test.ts | 70 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 3 deletions(-) diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 4a948ca15..c87b5c379 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1494,6 +1494,7 @@ describe("AccountManager", () => { "gpt-5.2", ); expect(Object.keys(account.rateLimitResetTimes).length).toBeGreaterThan(0); + expect(account.lastRateLimitReason).toBe("tokens"); manager.clearAccountTransientState(); @@ -1501,7 +1502,7 @@ describe("AccountManager", () => { expect(account.lastRateLimitReason).toBeUndefined(); }); - it("persists the cleared pool so a reload does not restore stale state", () => { + it("clears cooldown and rate-limit state together on the same account", () => { const now = Date.now(); const stored = { version: 3 as const, @@ -1511,12 +1512,75 @@ describe("AccountManager", () => { const manager = new AccountManager(undefined, stored); const account = manager.getCurrentAccount()!; + // A real stuck account from #606 carries both states at once; prove the + // method clears both in one pass rather than short-circuiting. manager.markAccountCoolingDown(account, 60_000, "server-error"); - const saveSpy = vi.spyOn(manager, "saveToDiskDebounced"); + manager.markRateLimitedWithReason(account, 60_000, "codex", "quota"); + expect(manager.isAccountCoolingDown(account)).toBe(true); + expect(Object.keys(account.rateLimitResetTimes).length).toBeGreaterThan(0); + + manager.clearAccountTransientState(); + + expect(account.coolingDownUntil).toBeUndefined(); + expect(account.cooldownReason).toBeUndefined(); + expect(account.rateLimitResetTimes).toEqual({}); + expect(account.lastRateLimitReason).toBeUndefined(); + }); + + it("clears mixed per-account state without throwing on clean accounts", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + { refreshToken: "token-2", addedAt: now, lastUsed: now }, + { refreshToken: "token-3", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const coolingDown = manager.getAccountByIndex(0)!; + const rateLimited = manager.getAccountByIndex(1)!; + const clean = manager.getAccountByIndex(2)!; + manager.markAccountCoolingDown(coolingDown, 60_000, "network-error"); + manager.markRateLimitedWithReason(rateLimited, 60_000, "codex", "quota"); + + expect(() => manager.clearAccountTransientState()).not.toThrow(); + + expect(coolingDown.coolingDownUntil).toBeUndefined(); + expect(rateLimited.rateLimitResetTimes).toEqual({}); + expect(clean.coolingDownUntil).toBeUndefined(); + expect(clean.rateLimitResetTimes).toEqual({}); + }); + + it("persists the cleared pool so a reload does not restore stale state", async () => { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [{ refreshToken: "token-1", addedAt: now, lastUsed: now }], + }; + + const manager = new AccountManager(undefined, stored); + const account = manager.getCurrentAccount()!; + manager.markAccountCoolingDown(account, 60_000, "server-error"); + manager.markRateLimitedWithReason(account, 60_000, "codex", "quota"); manager.clearAccountTransientState(); + // Flush so we assert against the actual persisted snapshot, not just + // that a save was scheduled — this is the #606 durability guarantee. + await manager.flushPendingSave(); - expect(saveSpy).toHaveBeenCalledTimes(1); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + const persisted = mockSaveAccounts.mock.calls[0]?.[0]; + expect(persisted?.accounts[0]?.coolingDownUntil).toBeUndefined(); + expect(persisted?.accounts[0]?.cooldownReason).toBeUndefined(); + expect(persisted?.accounts[0]?.rateLimitResetTimes ?? {}).toEqual({}); }); it("is a no-op when no accounts are loaded", () => {