-
Notifications
You must be signed in to change notification settings - Fork 583
fix(codex): accept monthly-classified snapshots and probe-owned refresh generations (#955 review) #967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(codex): accept monthly-classified snapshots and probe-owned refresh generations (#955 review) #967
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -139,6 +139,8 @@ export type CodexQuotaRecoveryProbeClaim = { | |
| leaseId: string; | ||
| cooldownGeneration: number; | ||
| credentialGeneration: number; | ||
| /** Claim-time `replacedAt`; unchanged after a probe-owned refresh, stamped on external replacement. */ | ||
| credentialReplacedAt?: number; | ||
| }; | ||
|
|
||
| export type CodexQuotaRecoveryProbeProof = { | ||
|
|
@@ -439,6 +441,7 @@ export function claimDueCodexQuotaRecoveryProbes( | |
| scope?: CodexQuotaScope; | ||
| health: CodexUpstreamHealth; | ||
| credentialGeneration: number; | ||
| credentialReplacedAt?: number; | ||
| order: number; | ||
| }> = []; | ||
| for (const [order, account] of (config.codexAccounts ?? []).entries()) { | ||
|
|
@@ -467,6 +470,7 @@ export function claimDueCodexQuotaRecoveryProbes( | |
| ...(candidate.scope ? { scope: candidate.scope } : {}), | ||
| health: candidate.health, | ||
| credentialGeneration: record.generation, | ||
| ...(record.replacedAt !== undefined ? { credentialReplacedAt: record.replacedAt } : {}), | ||
| order, | ||
| }); | ||
| } | ||
|
|
@@ -491,6 +495,9 @@ export function claimDueCodexQuotaRecoveryProbes( | |
| leaseId, | ||
| cooldownGeneration: candidate.health.cooldownGeneration ?? 0, | ||
| credentialGeneration: candidate.credentialGeneration, | ||
| ...(candidate.credentialReplacedAt !== undefined | ||
| ? { credentialReplacedAt: candidate.credentialReplacedAt } | ||
| : {}), | ||
| }; | ||
| }); | ||
| } | ||
|
|
@@ -506,10 +513,21 @@ export function settleCodexQuotaRecoveryProbe( | |
| ? scopedHealthFor(claim.accountId, claim.scope) | ||
| : upstreamHealth.get(claim.accountId); | ||
| if (!health || health.probeLeaseId !== claim.leaseId) return false; | ||
| const currentRecord = readCodexAccountRecord(claim.accountId); | ||
| const proofGeneration = proof.credentialGeneration; | ||
| // A probe-owned token refresh (getValidCodexToken) advances the credential generation by | ||
| // exactly one while preserving `replacedAt`; an external credential replacement bumps the | ||
| // generation too but stamps a fresh `replacedAt`. Accept the +1 transition only when the | ||
| // claim-time lineage is intact AND the generation the fresh quota was proven under is live. | ||
| const generationFenced = proofGeneration !== undefined | ||
| && (proofGeneration === claim.credentialGeneration | ||
| ? isCodexAccountGenerationLive(claim.accountId, proofGeneration) | ||
| : proofGeneration === claim.credentialGeneration + 1 | ||
| && currentRecord?.replacedAt === claim.credentialReplacedAt | ||
| && isCodexAccountGenerationLive(claim.accountId, proofGeneration)); | ||
|
Comment on lines
+525
to
+527
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When another caller such as the token guardian is already refreshing this cooled account, the probe can join that Useful? React with 👍 / 👎. |
||
| const fenced = (health.cooldownGeneration ?? 0) === claim.cooldownGeneration | ||
| && (health.probeLeaseGeneration ?? 0) === claim.cooldownGeneration | ||
| && claim.credentialGeneration === proof.credentialGeneration | ||
| && isCodexAccountGenerationLive(claim.accountId, claim.credentialGeneration); | ||
| && generationFenced; | ||
| if (!recovered || !fenced) { | ||
| const released = withProbeLeaseReleased(health, now); | ||
| if (claim.scope) setScopedHealth(claim.accountId, claim.scope, released); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a weekly plan such as Team,
parseUsageQuota()can producemonthlyPercentfrom a tertiary window even when the weekly primary/secondary window is entirely absent;tests/codex-routing.test.ts:1199-1205explicitly preserves that tertiary-only representation. This fallback now treats such a response as complete recovery evidence, so a low optional 30-day reading can clear a reset-derived shared cooldown without any fresh reading for the weekly quota that caused it, immediately routing traffic back to an account that may still be exhausted. Preserve whether the monthly value came from an explicitly monthly primary and accept monthly-only recovery for weekly-named plans only in that case, rather than accepting every parsed monthly value.Useful? React with 👍 / 👎.