Repository navigation
fix(proxy): ignore stale quota cooldowns - #1323
Conversation
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change adds active overage parsing and availability handling. Persisted quota snapshots older than 15 minutes no longer affect cooldown admission or active routing evidence. Routing candidates now expose normalized quota staleness. ChangesQuota routing reliability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Startup
participant PersistedQuota
participant CooldownState
participant RoutingMetrics
participant RoutingCandidate
Startup->>PersistedQuota: load quota snapshot
Startup->>PersistedQuota: calculate snapshot age
Startup->>CooldownState: preserve active cooldown state
RoutingMetrics->>RoutingCandidate: include quotaStale and quota age
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/server/routes/claudeProxyRoutes.ts`:
- Around line 987-993: Update the quota freshness logic around quotaLastUpdated,
quotaAgeMs, and quotaStale so any finite persisted timestamp greater than now is
treated as stale rather than clamped to zero age; preserve the existing
freshness threshold for non-future timestamps, and add coverage for a future
persisted timestamp.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 75e560fb-4303-47bc-8bb7-5393c1a63d1c
📒 Files selected for processing (5)
src/lib/proxy/accountQuota.tssrc/lib/proxy/proxyAnalysis.tssrc/lib/server/routes/claudeProxyRoutes.tssrc/lib/types/proxy.tstest/proxyReliabilityHardening.test.ts
| const quotaLastUpdated = | ||
| q && Number.isFinite(q.lastUpdated) ? q.lastUpdated : null; | ||
| const quotaAgeMs = | ||
| quotaLastUpdated === null ? null : Math.max(0, now - quotaLastUpdated); | ||
| const quotaStale = | ||
| quotaAgeMs !== null && quotaAgeMs > QUOTA_SNAPSHOT_FRESHNESS_MS; | ||
| const routingQuota = quotaStale ? undefined : q; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Treat future persisted timestamps as stale.
Line 990 clamps a future lastUpdated value to zero age. A snapshot saved by a clock ahead of the current process remains fresh for up to its future offset plus 15 minutes. Its rejected status can then make an account unusable after restart.
Mark quotaLastUpdated > now as stale. Add a test with a future persisted timestamp.
Proposed fix
const quotaAgeMs =
quotaLastUpdated === null ? null : Math.max(0, now - quotaLastUpdated);
const quotaStale =
- quotaAgeMs !== null && quotaAgeMs > QUOTA_SNAPSHOT_FRESHNESS_MS;
+ quotaLastUpdated !== null &&
+ (quotaLastUpdated > now ||
+ quotaAgeMs! > QUOTA_SNAPSHOT_FRESHNESS_MS);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const quotaLastUpdated = | |
| q && Number.isFinite(q.lastUpdated) ? q.lastUpdated : null; | |
| const quotaAgeMs = | |
| quotaLastUpdated === null ? null : Math.max(0, now - quotaLastUpdated); | |
| const quotaStale = | |
| quotaAgeMs !== null && quotaAgeMs > QUOTA_SNAPSHOT_FRESHNESS_MS; | |
| const routingQuota = quotaStale ? undefined : q; | |
| const quotaLastUpdated = | |
| q && Number.isFinite(q.lastUpdated) ? q.lastUpdated : null; | |
| const quotaAgeMs = | |
| quotaLastUpdated === null ? null : Math.max(0, now - quotaLastUpdated); | |
| const quotaStale = | |
| quotaLastUpdated !== null && | |
| (quotaLastUpdated > now || | |
| quotaAgeMs! > QUOTA_SNAPSHOT_FRESHNESS_MS); | |
| const routingQuota = quotaStale ? undefined : q; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/server/routes/claudeProxyRoutes.ts` around lines 987 - 993, Update
the quota freshness logic around quotaLastUpdated, quotaAgeMs, and quotaStale so
any finite persisted timestamp greater than now is treated as stale rather than
clamped to zero age; preserve the existing freshness threshold for non-future
timestamps, and add coverage for a future persisted timestamp.
4bd609a to
e0f598a
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens the Claude proxy’s quota/cooldown logic after restarts by preventing stale persisted quota snapshots from affecting routing eligibility or creating/clearing cooldowns, while also expanding Anthropic unified header support for overage.
Changes:
- Stop applying persisted quota snapshots to mutate cooldown admission state during startup hydration; only persisted cooldowns and fresh upstream headers can change cooldowns.
- Treat quota snapshots older than 15 minutes as stale and exclude them from routing/saturation/usability decisions while still retaining them for diagnostics (via
quotaStaleevidence). - Parse Anthropic
unified-overage-in-useand treat it as an authoritative overage eligibility signal whenoverage-status: allowed.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/proxyReliabilityHardening.test.ts | Adds coverage for unified-overage-in-use and for ignoring stale quota snapshots when rehydrating runtime state. |
| src/lib/types/proxy.ts | Extends proxy quota/routing evidence types with quotaStale and overageInUse. |
| src/lib/server/routes/claudeProxyRoutes.ts | Implements freshness gating for quota snapshots and prevents persisted quota hydration from creating/clearing cooldowns. |
| src/lib/proxy/proxyAnalysis.ts | Makes offline routing evidence parsing tolerant of missing quotaStale while recording it when present. |
| src/lib/proxy/accountQuota.ts | Implements overageInUse support and updates overage-eligibility logic accordingly. |
Suppressed comments (1)
src/lib/types/proxy.ts:574
ProxyAccountSortMetricsis exported, andquotaStalebeing required forces any external construction/mocking of this type to update immediately. If the intent is to allow older tooling to keep working (similar to routing evidence), consider making this field optional as well and treating missing asfalse.
export type ProxyAccountSortMetrics = {
usable: boolean;
saturated: boolean;
hasQuota: boolean;
quotaStale: boolean;
quotaLastUpdated: number | null;
quotaAgeMs: number | null;
coolingActive: boolean;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| usable: boolean; | ||
| saturated: boolean; | ||
| quotaObserved: boolean; | ||
| quotaStale: boolean; | ||
| quotaLastUpdated: number | null; | ||
| quotaAgeMs: number | null; |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
Thanks for the thorough review! The changes look good - I've verified that:
The implementation is solid and addresses the identified issues without introducing new risks. Ready to merge once you approve! 🚀 |
| @@ -52,10 +52,11 @@ export function getUnifiedRateLimitStatus( | |||
|
|
|||
There was a problem hiding this comment.
💡 MINOR: JSDoc updated to clarify quota overage logic - documentation update correctly reflects that active overage signals are authoritative while older snapshots use fallback percentage + allowed status
|
|
||
| const accountRuntimeState = new Map<string, RuntimeAccountState>(); | ||
|
|
||
| /** Persisted quota is advisory after a restart. Older snapshots cannot reject |
There was a problem hiding this comment.
💡 MINOR: Quota freshness tracking implemented - Added QUOTA_SNAPSHOT_FRESHNESS_MS = 15 minute threshold to track when quota snapshots become stale, correctly implementing documented behavior
e0f598a to
6d114b7
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
🎉 This PR is included in version 10.12.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Root cause
The worker rehydrated
account-quotas.jsonafter a restart, then treated persistedrejectedquota statuses as current upstream evidence. This could create fresh cooldowns from stale snapshots without sending a request. In the observed incident, the configured NeuroLink account snapshot was five days old and was immediately cooled despite having current availability.Changes
quotaStale: true; offline analysis accepts historical records that do not contain the new field.unified-overage-in-use; when paired withoverage-status: allowed, it keeps the account eligible even when the fallback header is absent.Validation
pnpm exec vitest run test/proxyReliabilityHardening.test.ts test/proxyAnalysis.test.ts test/proxyUpdaterFallback.test.ts test/proxyUsageStatsPersistence.test.ts(163 passing)pnpm run test:proxy-limit-headers(18 passing)pnpm exec prettier --check ...pnpm exec tsc --noEmitgit diff --checkNo live proxy files, processes, configuration, or traffic were touched for this PR.
Summary by CodeRabbit
Bug Fixes
Improvements