Skip to content

fix(codex): pace quota recovery queries - #6154

Closed
terrytan95 wants to merge 2 commits into
lidge-jun:devfrom
terrytan95:terry-quota-query-backoff
Closed

terrytan95 wants to merge 2 commits into
lidge-jun:devfrom
terrytan95:terry-quota-query-backoff

Conversation

@terrytan95

@terrytan95 terrytan95 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #6153.

Main-account hard-lock recovery currently forces usage reads on every minute sweep even before a known reset, and failed ordinary quota reads can be retried by subsequent dashboard/background requests despite an upstream Retry-After. Wait locally for known blocking resets and share credential-scoped HTTP/transport failure pacing at the ordinary WHAM dispatch boundary, preserving fresh-evidence requirements and authentication recovery.

Key Changes

  • Wait until the latest known blocking reset before background main recovery; when the reset is unknown or the account remains blocked, use the shared capped 5/10/20/40/60-minute delay calculation. Reconcile the physical bearer before consulting retry state; fence late results to their captured credential generation. Passing a deadline never unlocks the account.
  • Share 5/10/20/40/60-minute failure backoff between main, pool, and 401-replay usage reads; respect longer Retry-After under the existing bounded parser. Forced refresh bypasses successful cache, not failure pacing.
  • Scope retry state to configuration home/config generation/credential generation; bound retained entries and prevent a late response from erasing a newer dispatch's failure.
  • Keep 401/403 authentication handling unchanged; deferred reads publish no fresh quota or dispatch evidence. Update English/Chinese documentation and structure contracts.

Verification

  • PASS: bun test tests/codex-integration/codex-quota-query-backoff.test.ts tests/codex-integration/main-account-hard-lock-recovery.test.ts tests/codex-integration/codex-auth-api.test.ts tests/codex-integration/codex-quota-auto-refresh.test.ts tests/codex-integration/codex-quota-auto-refresh-main-admission.test.ts tests/codex-integration/codex-quota-auto-refresh-generation.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/lab/core-lab-boundary.test.ts — 462 tests, 2,784 assertions, zero failures.
  • PASS: bun run typecheck, bun run structure:check, bun run privacy:scan, git diff --check.
  • PASS: cd docs-site && bun install --frozen-lockfile && bun run build — 537 pages; 73,620 internal links checked.
  • Regression cases additionally cover successful-but-blocked capped recovery, credential replacement during a wait, the 24-hour Retry-After ceiling, and both late-success/newer-failure and late-failure/newer-success ordering. Existing cases include one hour of pre-reset local ticks with zero upstream reads, confirmation before unlock, Retry-After seconds/date across forced and background reads, transport backoff, successful recovery, replacement credentials, and concurrent response ordering. Existing 341 authentication API tests cover 401 recovery and manual reset behavior.
  • No live account requests were used. Full bun run test and import-graph-expanded test:changed were not run: the earlier change's graph run selected broadly and degraded this host, with protected-home fixture cleanup failures in this managed checkout. This PR stays draft; full-suite/cross-platform validation and independent security review remain outstanding. The targeted final run is green; no broader pass is claimed.

Impact & Risk

No configuration migration, dependency, environment variable, or deployment change. Early out-of-band quota recovery is no longer discovered before a known reset unless ordinary traffic or explicit reads observe it. Retry state is process-local and bounded to 256 keys; restart, configuration/credential generation changes, or eviction can permit an earlier retry. Successful HTTP responses with malformed bodies are outside the new transport/status backoff, and 401/403 remain with existing authentication logic. Reserve/login queries are separate. Reduced requests do not establish or guarantee improved account/session lifetime.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Author inspection and regression verification are complete; independent security review and review-readiness confirmation are not claimed.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • New Features
    • Quota checks now pause after network or non-authentication failures, with delays increasing from 5 to 60 minutes. Valid Retry-After instructions can extend the wait; successful checks clear it.
    • Hard-lock recovery waits for a known quota reset before checking again. Without a future reset, background checks are spaced at least five minutes apart.
    • Deferred checks do not report stale quota data as fresh. Waiting periods reset when the proxy restarts.
  • Documentation
    • Updated English and Chinese guides to explain quota-check pacing and recovery behavior.

Maintainer feedback follow-up

Addresses #6153 (comment) in commit b3e68de. The earlier fixed five-minute successful-but-blocked recovery delay now uses the shared capped calculation, and retry admission reconciles the physical bearer first. The PR has been returned to draft. Focused recovery/interleaving tests pass; exact-head upstream test CI and independent review are still required. The author account has read-only upstream repository permission and cannot approve or dispatch maintainer-only CI.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Sep 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 63 / 80

이 PR은 메인 계정이 하드락에 걸린 동안, 리셋 시각을 이미 아는데도 1분마다 사용량 주소를 두드리는 일을 멈춥니다. 리셋 시각이 미래이면 그 시각까지 로컬에서 기다립니다. 시각이 지나도 잠금을 풀지 않고, 사용량을 한 번 더 읽어 확인한 뒤에만 풉니다. 리셋 시각이 없거나 이미 지났는데도 아직 막혀 있으면, 확인 읽기 사이를 5분으로 벌립니다.

메인 계정, 풀 계정, 401 이후 다시 읽기가 같은 실패 간격을 씁니다. 간격은 5분, 10분, 20분, 40분, 그다음 60분입니다. 전송 실패와 인증이 아닌 HTTP 실패에 적용됩니다. Retry-After가 더 길면 그 길이를 따릅니다. 기존 파서는 하루를 넘기지 않습니다. 강제 새로고침도 이 간격을 피하지 못합니다. 성공하면 간격이 지워집니다. 401과 403은 지금처럼 인증 복구가 처리하고, 이 간격은 지우기만 합니다. 자격 증명이 바뀌면 키가 달라서 바로 다시 읽을 수 있습니다. 늦게 도착한 성공 응답이 더 새로운 실패의 간격을 지우지 않습니다. 읽기를 미루면 새 사용량도, 새 조회 증거도 올리지 않습니다. 이 상태는 프로세스 안에만 있고, 키는 설정 디렉터리와 세대 번호로 나뉩니다. 최대 256개입니다.

이슈 #6153을 닫습니다. 베이스는 dev입니다. 드래프트이고, 준비 체크는 0/4입니다. 지정한 테스트 458개는 통과했다고 적혀 있습니다. 전체 bun run test는 돌리지 않았습니다.

라인 - fetchCodexUsage의 after: 0. 요청을 보내기 전에 맵에 넣는 표시의 after가 0입니다. 0은 지금보다 크지 않아서, 첫 응답이 오기 전에 같은 키의 다른 호출도 통과합니다. 대시보드 새로고침과 분 스윕이 겹치면 사용량 주소로 여러 번 나갑니다. 나중에 끝난 호출만 실패 간격을 남깁니다.

라인 - runMainAccountHardLockRecovery의 result.quotaRefresh, fetchMainAccountInfoAttempt의 if (!resp). 5분 대기는 조회 결과에 quotaRefresh가 있을 때만 기록됩니다. 공유 백오프가 조회를 생략하면 226행은 quotaRefresh 없이 돌아옵니다. Retry-After가 5분보다 길면, 5분이 지난 뒤 매분 스윕이 프로필 잠금을 잡고 getValidMainAccountToken까지 들어갑니다. 액세스 토큰이 아직 유효하면 갱신 요청은 나가지 않습니다. 잠금과 진입은 남습니다. 사용량 주소는 fetchCodexUsage가 null로 막습니다.

메인테이너의 판단이 필요한 지점

본문이 깨진 200 응답은 성공으로 보고 실패 간격을 지웁니다. 다음 조회가 바로 다시 나갑니다. 작성자는 이 경우를 새 간격 밖으로 두었습니다. 깨진 본문도 5분부터 기다릴지.

401/403은 간격을 지우고 기존 인증 복구에 맡깁니다. 인증 실패가 반복되면 사용량 조회가 다시 붙을 수 있습니다. 그 반복을 이번 범위 밖에 둘지.

프로세스 재시작이나 256개를 넘는 삭제는 간격을 잊습니다. 작성자도 그렇게 적었습니다. 그 범위를 그대로 둘지.

너의 추천

리셋 전에는 안 읽고, 시간만으로 잠금을 풀지 않으며, 실패 간격을 메인과 풀이 같이 쓰는 방향은 이슈와 같습니다. 응답이 오기 전에는 같은 키를 막지 못하고, 긴 Retry-After의 뒷부분에서 하드락 스윕이 매분 잠금을 잡습니다. 그 두 곳을 고친 뒤에 드래프트를 푸세요. 전체 테스트와 보안 리뷰는 작성자가 아직 안 했다고 적었습니다. 체크리스트 4칸이 채워지기 전에는 머지하지 마세요.

이 댓글은 grok-bot이 작성했습니다

@terrytan95
terrytan95 marked this pull request as ready for review September 28, 2026 07:14
@terrytan95
terrytan95 marked this pull request as draft September 28, 2026 07:16

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head review of b3e68de2928ba72b9ffe57c8fbd13034ef5168b6: the accepted reset-window recovery behavior is mostly covered, but the shared pacing boundary still permits the retry storm #6153 reports.

src/codex/quota-query-backoff.ts:21-30 records a per-key marker with after: 0, so another same-key caller always dispatches while the first request is pending and replaces marker ownership. If A then returns 429/Retry-After, A's failed() update is discarded; while B is slow, C/D can continue dispatching. The new tests explicitly admit two concurrent dispatches, so this is not cross-owner pacing at the dispatch boundary.

Use in-flight admission/coalescing (or a pending marker that blocks later callers) while preserving ordered completion and generation semantics. Add A pending → B/C arrive → A reports Retry-After coverage proving only one upstream query and durable backoff.

Also repair/narrow the structure changes: the new WHAM sentence was inserted mid-sentence in structure/dashboard-and-usage.md and structure/providers/openai-accounts.md, leaving both documents grammatically corrupted, with unrelated structure files blanket-edited. After those fixes, rebase onto current dev and obtain executable exact-head CI before undrafting.

@lidge-jun lidge-jun mentioned this pull request Sep 28, 2026
2 of 3 tasks
@lidge-jun

Copy link
Copy Markdown
Owner

Thanks @terrytan95. This change landed on dev through the carry in #6179 (merged as 34de606), which keeps your authorship with a Co-authored-by trailer and adds shared-deadline and backoff fixes from review. Closing this PR as superseded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants