fix(rotation): persist cooldown when account has no resolvable accountId

[ndycode]

Summary

• Found via a codebase-wide bug-hunt sweep (12 subsystem hunters → adversarial verification → synthesis). One confirmed defect survived verification: a cooldown-persistence gap in the runtime rotation loop.
• The missing-accountId branch in runRotationLoop marks the account cooling down via markAccountCoolingDown but was the lone cooldown branch that never called saveToDiskDebounced(). Every other cooldown branch in the same function persists its mutation: network-error, 429, server-error, and 401 invalidation.
• Because coolingDownUntil/cooldownReason are serialized in the V3 storage snapshot, a process restart inside the 30s cooldown window dropped the cooldown and immediately re-selected the still-broken account.

What Changed

• lib/runtime-rotation-proxy.ts — add the missing accountManager.saveToDiskDebounced() after markAccountCoolingDown in the missing-accountId branch, mirroring the network-error branch directly below it. Added a comment explaining the persistence requirement.
• test/runtime-rotation-proxy.test.ts — regression test: an account with no stored accountId and a non-JWT access token drives resolveAccountId to return null, the branch fires, no upstream request is issued, and the cooldown is persisted. The test fails without the fix (verified by mutation).

Severity

Low. No upstream request is issued (token refresh already succeeded), the cooldown is only 30s, and a restart within the window degrades gracefully — the account is simply re-selected, re-cooled in memory, and rotation proceeds to the next account. This is a consistency/durability gap, not a cascading failure. Fixing it makes the branch consistent with every other cooldown path.

Validation

• npm run lint
• npm run typecheck
• npm test (4911 passed, 3 skipped, 0 failures)
• npm test -- test/documentation.test.ts
• npm run build

Docs and Governance Checklist

• README updated (if user-visible behavior changed)
• docs/getting-started.md updated (if onboarding flow changed)
• docs/features.md updated (if capability surface changed)
• relevant docs/reference/* pages updated (if commands/settings/paths changed)
• docs/upgrade.md updated (if migration behavior changed)
• SECURITY.md and CONTRIBUTING.md reviewed for alignment

No user-visible surface changed — internal rotation-loop consistency fix.

Risk and Rollback

• Risk level: low. Adds one debounced disk write on an already-degraded path that previously skipped it; matches the established pattern of every sibling branch.
• Rollback plan: revert this commit.

🤖 Generated with Claude Code

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

patches the one cooldown branch in runRotationLoop that mutated cooldown state without scheduling a disk write. every other branch (network-error, 429, server-error, 401 invalidation) already calls saveToDiskDebounced(); the no-accountId branch did not, so a process restart inside the 30s window dropped the cooldown and could immediately re-select the still-broken account.

• lib/runtime-rotation-proxy.ts — adds accountManager.saveToDiskDebounced() after markAccountCoolingDown in the missing-accountId branch, with a comment explaining the V3 serialization requirement.
• test/runtime-rotation-proxy.test.ts — adds a regression test that confirms no upstream call is issued, the account receives the expected cooldown fields, the response is 503, and saveToDiskDebounced is called.

Confidence Score: 5/5

safe to merge — one-line addition on an already-degraded path that previously skipped a debounced disk write, matching the established pattern of every sibling cooldown branch

the change is minimal and surgical: one saveToDiskDebounced() call added to close a consistency gap between the no-accountId branch and its three sibling branches; no new code paths, no new state, no behavioral change except durability of the cooldown across restarts; regression test correctly exercises the branch end-to-end and would have failed before the fix

no files require special attention

Important Files Changed

Filename	Overview
lib/runtime-rotation-proxy.ts	adds the missing saveToDiskDebounced() call after markAccountCoolingDown in the no-accountId branch, making it consistent with every other cooldown branch (network-error, 429, server-error, 401); the one-line addition with explanatory comment is correct and minimal
test/runtime-rotation-proxy.test.ts	new regression test exercises the no-accountId branch end-to-end: verifies no upstream call, 503 response status, cooldown fields, and saveToDiskDebounced invocation; test setup (non-JWT access token, no stored accountId) correctly forces the branch under test

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[runRotationLoop iteration] --> B[ensureFreshAccessToken]
    B -->|not ok, invalidated| C[return 401 to client]
    B -->|not ok, retryable| D[transientAttempts++, continue]
    B -->|ok| E[resolveAccountId]
    E -->|null: no accountId| F[markAccountCoolingDown\nauthFailureCooldownMs]
    F --> G["saveToDiskDebounced() ✅ ADDED"]
    G --> H[transientAttempts++, continue]
    E -->|resolved| I[send upstream request]
    I -->|network error| J[markAccountCoolingDown\nnetworkErrorCooldownMs]
    J --> K[saveToDiskDebounced]
    I -->|429| L[markRateLimitedWithReason]
    L --> M[saveToDiskDebounced]
    I -->|5xx| N[markAccountCoolingDown\nserverErrorCooldownMs]
    N --> O[saveToDiskDebounced]
    I -->|success| P[stream response to client]

Loading

_{Reviews (3): Last reviewed commit: "test(rotation): assert exhaustion status..." | Re-trigger Greptile}

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

Warning

Review limit reached

@ndycode, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 23 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7ffd3563-ee79-4a27-86f1-4acb3af8f6bd

📥 Commits

Reviewing files that changed from the base of the PR and between db0adb0 and 0f50e3f.

📒 Files selected for processing (2)

• lib/runtime-rotation-proxy.ts
• test/runtime-rotation-proxy.test.ts

📝 Walkthrough

Walkthrough

lib/runtime-rotation-proxy.ts adds a saveToDiskDebounced() call in the auth-failure cooldown branch so cooldown state survives restarts. A new test in test/runtime-rotation-proxy.test.ts verifies that when an account has no resolvable accountId, the cooldown fields are set and the disk-write is triggered.

Changes

Auth-failure cooldown persistence

Layer / File(s)

Summary

Auth-failure persist call and test lib/runtime-rotation-proxy.ts, test/runtime-rotation-proxy.test.ts

lib/runtime-rotation-proxy.ts:1061–1066 adds accountManager.saveToDiskDebounced() after markAccountCoolingDown(DEFAULT_AUTH_FAILURE_COOLDOWN_MS), matching behavior of other cooldown branches. test/runtime-rotation-proxy.test.ts:1665–1703 adds a test that constructs a single-account fixture with no accountId, fires a /responses request, and asserts cooldownReason, coolingDownUntil, and saveToDiskDebounced are all set.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

• ndycode/codex-multi-auth#442: directly relates — both touch how coolingDownUntil and cooldown markers are persisted to account storage, with that PR adding a CLI command to clear the same state this PR ensures is written.
• ndycode/codex-multi-auth#463: closest match — also modifies lib/runtime-rotation-proxy.ts to call accountManager.saveToDiskDebounced() after an account-state change (402/403 deactivation), establishing the pattern this PR extends to the auth-failure branch.

---

a few observations as a senior eye on this change:

the fix is correct and minimal. lib/runtime-rotation-proxy.ts:1061–1066 adds exactly one saveToDiskDebounced() call, consistent with the network-error, rate-limit, server-error, and 401 branches. good.

test coverage is solid but narrow. test/runtime-rotation-proxy.test.ts:1665–1703 covers the happy path for the new call. flag: there's no assertion on how many times saveToDiskDebounced is called — if the path is exercised twice due to a retry loop, a duplicate call would be invisible. worth a toHaveBeenCalledTimes(1) check.

concurrency risk not introduced here but worth noting. saveToDiskDebounced is inherently async-safe via debouncing, but if two accounts hit the auth-failure branch concurrently, the debounce window means only one write is guaranteed within that interval. this was already true for other branches, so no regression here, but worth documenting.

windows edge case. disk-write timing on windows under heavy i/o can cause the debounced write to be delayed or dropped at process exit. no test covers a simulated-restart scenario (write → process exit → reload) — that's the actual bug being fixed. consider an integration-level test or at least a comment in lib/runtime-rotation-proxy.ts:1064 pointing to the exit-flush mechanism if one exists.

no public api changes. confirmed — no exported types or signatures changed.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	title follows conventional commits format with type, scope, and lowercase imperative summary, matching all requirements under 72 chars.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	the pr description is thorough, well-structured, and covers all required sections with appropriate detail about the bug, fix, testing, and risk assessment.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/persist-cooldown-missing-accountid

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/persist-cooldown-missing-accountid

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}