fix: prefer fresh access tokens during account selection

[ndycode]

Summary

• prefer accounts with sufficiently fresh access tokens during runtime selection, while still falling back to stale accounts when the whole pool needs refresh recovery

What Changed

• added fresh-token-aware selection helpers in lib/accounts.ts using a five-minute freshness window
• updated the standard and hybrid family selectors to prioritize fresh accounts when any are available
• kept stale accounts selectable as the fallback path when every available account is near expiry
• added regression coverage in test/accounts.test.ts and test/accounts-load-from-disk.test.ts for fresh-preference and all-stale fallback behavior

Validation

• npm run lint
• npm run typecheck
• npm test
• 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

Risk and Rollback

• Risk level: moderate; touches request-time account selection, but preserves stale-token fallback when no fresh alternative exists
• Rollback plan: revert commit 66d2d63

Additional Notes

• targeted test run: npm test -- test/accounts.test.ts test/accounts-load-from-disk.test.ts

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

this pr adds fresh-token-aware account selection across all four selection paths (isAccountAvailableForFamily, getCurrentOrNextForFamily, getNextForFamily, getCurrentOrNextForFamilyHybrid) using a 5-minute freshness window, with a stale-account fallback when the whole pool is near expiry. the refactor also extracts isAccountSelectableForFamily to deduplicate the enabled/rate-limit/cooldown checks.

• hasFreshAccessToken and hasFreshAvailableAccountForFamily correctly gate fresh preference; the stale-fallback behavior (all accounts stale → allow all) is tested and working
• P1: the accountsWithMetrics mapping in getCurrentOrNextForFamilyHybrid no longer filters enabled === false accounts — they're now included with isAvailable: false. selectHybridAccount's LRU fallback iterates the full array, so a disabled account can be returned when every other account is rate-limited/cooling down. the old if (account.enabled === false) return null; guard must be restored
• test coverage for the new paths is good; the getNextForFamily fresh-preference case (previously flagged) is now covered in accounts-load-from-disk.test.ts
• npm test is unchecked in the validation checklist — full suite should be confirmed green before merge given the scope of the change

Confidence Score: 4/5

safe to merge after fixing the disabled-account LRU regression in the hybrid selector

one confirmed P1 regression: disabled accounts are no longer filtered from the accountsWithMetrics array, making them selectable by the LRU fallback path in selectHybridAccount when all enabled accounts are unavailable; the fix is a one-line restore of the existing guard

lib/accounts.ts lines 556-570 (accountsWithMetrics map in getCurrentOrNextForFamilyHybrid)

Important Files Changed

Filename	Overview
lib/accounts.ts	adds fresh-token helpers and wires them into all four selection paths; P1 regression: disabled accounts are no longer filtered from accountsWithMetrics, making them eligible for the LRU fallback in getCurrentOrNextForFamilyHybrid
test/accounts.test.ts	adds four new test cases covering fresh-preference and all-stale fallback for isAccountAvailableForFamily, getCurrentOrNextForFamily, and getCurrentOrNextForFamilyHybrid; coverage looks solid for the new paths
test/accounts-load-from-disk.test.ts	adds stale-fallback and fresh-preference tests for getNextForFamily loaded from a disk-like storage object; addresses the previously flagged coverage gap

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[selection request] --> B[hasFreshAvailableAccountForFamily\nscan all accounts]
    B -->|any fresh+available| C[requireFresh = true]
    B -->|none fresh or available| D[requireFresh = false]
    C --> E[isAccountSelectableForFamily\nenabled? not rate-limited? not cooling?\nand hasFreshAccessToken?]
    D --> F[isAccountSelectableForFamily\nenabled? not rate-limited? not cooling?]
    E -->|pass| G[return account]
    F -->|pass| G
    E -->|fail for all| H{selection path}
    F -->|fail for all| H
    H -->|getCurrentOrNextForFamily\ngetNextForFamily| I[return null]
    H -->|getCurrentOrNextForFamilyHybrid| J[selectHybridAccount LRU fallback]
    J -->|OLD: disabled accounts filtered out| K[safe LRU pick]
    J -->|NEW: disabled accounts included with isAvailable=false| L[⚠️ may return disabled account]

Loading

Comments Outside Diff (2)

1. index.ts, line 1914-1926 (link)

   O(N²) scan in stream-failover loop

   isAccountAvailableForFamily now does a full O(N) pool scan (via hasFreshAvailableAccountForFamily) on every call. the caller at line 1914 is already a loop over streamFallbackCandidateOrder, so each failover event now scans the entire account pool once per candidate. for a large pool this degrades unnecessarily.

   pre-compute requireFresh once before the loop — or expose a batch-availability helper — to restore O(N) total cost:

   // before the loop:
   const requireFresh = accountManager.hasFreshAvailableAccountForFamily(modelFamily, model);
   // then in the loop, pass requireFresh through to the check

   this mirrors the pattern already used inside getCurrentOrNextForFamily and the other internal selectors.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: index.ts
   Line: 1914-1926
   
   Comment:
   **O(N²) scan in stream-failover loop**
   
   `isAccountAvailableForFamily` now does a full O(N) pool scan (via `hasFreshAvailableAccountForFamily`) on every call. the caller at line 1914 is already a loop over `streamFallbackCandidateOrder`, so each failover event now scans the entire account pool once per candidate. for a large pool this degrades unnecessarily.
   
   pre-compute `requireFresh` once before the loop — or expose a batch-availability helper — to restore O(N) total cost:
   
   ```typescript
   // before the loop:
   const requireFresh = accountManager.hasFreshAvailableAccountForFamily(modelFamily, model);
   // then in the loop, pass requireFresh through to the check
   ```
   
   this mirrors the pattern already used inside `getCurrentOrNextForFamily` and the other internal selectors.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

2. lib/accounts.ts, line 556-570 (link)

   disabled accounts leak into hybrid LRU fallback

   the old code had if (account.enabled === false) return null; in the accountsWithMetrics map, which filtered disabled accounts out entirely. the new code removes that guard and instead relies on isAccountSelectableForFamily returning false, but that only sets isAvailable: false — the account still stays in the accountsWithMetrics array.

   in selectHybridAccount the LRU fallback path (if (available.length === 0)) iterates the full resolvedAccounts array, not just the available slice. so when every non-disabled account is rate-limited or cooling down, a disabled account can be returned from selectHybridAccount, and getCurrentOrNextForFamilyHybrid returns it without any enabled check.

   fix: restore the early-return null for disabled accounts so they're excluded from the array:

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: lib/accounts.ts
   Line: 556-570
   
   Comment:
   **disabled accounts leak into hybrid LRU fallback**
   
   the old code had `if (account.enabled === false) return null;` in the `accountsWithMetrics` map, which filtered disabled accounts out entirely. the new code removes that guard and instead relies on `isAccountSelectableForFamily` returning `false`, but that only sets `isAvailable: false` — the account still stays in the `accountsWithMetrics` array.
   
   in `selectHybridAccount` the LRU fallback path (`if (available.length === 0)`) iterates the **full `resolvedAccounts` array**, not just the available slice. so when every non-disabled account is rate-limited or cooling down, a disabled account can be returned from `selectHybridAccount`, and `getCurrentOrNextForFamilyHybrid` returns it without any `enabled` check.
   
   fix: restore the early-return null for disabled accounts so they're excluded from the array:
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/accounts.ts
Line: 556-570

Comment:
**disabled accounts leak into hybrid LRU fallback**

the old code had `if (account.enabled === false) return null;` in the `accountsWithMetrics` map, which filtered disabled accounts out entirely. the new code removes that guard and instead relies on `isAccountSelectableForFamily` returning `false`, but that only sets `isAvailable: false` — the account still stays in the `accountsWithMetrics` array.

in `selectHybridAccount` the LRU fallback path (`if (available.length === 0)`) iterates the **full `resolvedAccounts` array**, not just the available slice. so when every non-disabled account is rate-limited or cooling down, a disabled account can be returned from `selectHybridAccount`, and `getCurrentOrNextForFamilyHybrid` returns it without any `enabled` check.

fix: restore the early-return null for disabled accounts so they're excluded from the array:

```suggestion
		const accountsWithMetrics: AccountWithMetrics[] = this.accounts
			.map((account): AccountWithMetrics | null => {
				if (!account) return null;
				if (account.enabled === false) return null;
				return {
					index: account.index,
					isAvailable: this.isAccountSelectableForFamily(
						account,
						family,
						model,
						requireFresh,
					),
					lastUsed: account.lastUsed,
				};
			})
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: lib/accounts.ts
Line: 419-424

Comment:
**O(n) scan per index check**

`isAccountAvailableForFamily` now calls `hasFreshAvailableAccountForFamily` on every invocation, which walks all accounts and calls `isAccountSelectableForFamily` (including `clearExpiredRateLimits`) for each one. callers that check every index in a loop — e.g. a UI list render — will hit O(n²) work. for small pools this is fine, but worth caching `requireFresh` at the call site or lifting the scan up to where all indexes are iterated together.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (2): Last reviewed commit: "test: cover fresh family selection happy..." | 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

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 32 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 32 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0acba5ed-7601-48f9-9dfd-89544eb8b9ec

📥 Commits

Reviewing files that changed from the base of the PR and between 53488ef and 66d2d63.

📒 Files selected for processing (3)

• lib/accounts.ts
• test/accounts-load-from-disk.test.ts
• test/accounts.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/fresh-token-selection

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/fresh-token-selection

---

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.}

[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.