fix(codex): activate quota from saved deadlines - #6020
terrytan95 wants to merge 1 commit into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (14)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughQuota activation now waits locally for retained reset deadlines instead of querying metadata solely because quota snapshots are stale. Missing-deadline discovery and failed activations use bounded backoff. Regression tests and documentation cover restart behavior, deadline recovery, and the separation from dashboard quota queries. ChangesQuota activation scheduling
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant MinuteSweep as Local minute sweep
participant AutoRefresh as runCodexQuotaAutoRefresh
participant WHAM
participant Warmup as Quota warmup
MinuteSweep->>AutoRefresh: Check retained reset deadlines
AutoRefresh->>WHAM: Query when an enabled deadline is missing and retry is due
WHAM-->>AutoRefresh: Return quota metadata and reset deadlines
AutoRefresh->>Warmup: Activate due windows
Warmup-->>AutoRefresh: Return response headers with next deadlines
Merge Risk: ⚪ Minimal · up to No confirmed issue blocks merging. Discovery after a completed deadline remains unverified and merits a focused check when the relevant source is available. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change reduces background quota lookups without widening account access or bypassing the existing sign-in controls. A retry from an earlier credential failure may, however, delay activation after sign-in is restored by up to an hour. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
리뷰 · 우선순위 62 / 80이 PR은 5시간 창과 일주일 창이 다시 열릴 때 계정을 살짝 깨우는 기능은 그대로 둡니다. 빼는 것은, 다음 리셋 시각을 이미 아는데도 사용량을 자꾸 물어보던 요청입니다. 지금은 스냅샷이 5분만 지나도 그 요청이 나갑니다. 저장된 마감이 있어도 그렇습니다. 바꾼 뒤에는 켜 둔 창의 마감이 있으면 1분마다 이 컴퓨터에서만 시각을 봅니다. 프록시를 껐다 켜서 메모리에 스냅샷이 없어도, 설정에 남은 마감으로 기다립니다. 마감이 비어 있을 때만 사용량을 조회합니다. 조회가 안 되거나 깨우기가 실패하면 다음 시도는 5분, 10분, 20분, 40분, 그 다음부터는 60분입니다. 성공한 응답에 다음 시각이 있으면 그 값으로 이어 가고, 사용량을 한 번 더 묻지 않습니다. 대시보드 새로고침과 리셋 알림은 이 일정과 따로 돕니다. 베이스는 라인 - 라인 - 같은 파일 340행과 351행. 401이면 재로그인이 필요하다고 표시합니다. 메인테이너의 판단이 필요한 지점 저장된 마감이 틀려도, 스냅샷이 오래됐다는 이유만으로 사용량을 다시 묻지 않습니다. 상류가 리셋 시각을 바꾸면, 일반 요청이나 직접 새로고침이 오기 전까지 모릅니다. 작성자는 이 선택을 본문에 적었습니다. 로그인 강제 종료를 막는다는 측정은 없고, 불필요한 요청만 줄인다고 했습니다. 대기 시간은 프로세스 메모리에만 있습니다. 프록시를 재시작하면 5분부터 다시 셉니다. 작성자는 활성화 테스트 46개가 통과했다고 했습니다. 너의 추천 합치기 전에 351행 대기를 나누세요. 메인이 바쁜 경우와 재로그인이 필요한 경우는 5분으로 두세요. 재로그인 표시가 풀리면 그 계정 대기를 지우세요. 상류 깨우기가 실제로 실패한 경우만 5분에서 60분으로 늘리세요. 베이스 이 댓글은 grok-bot이 작성했습니다 |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Blocking exact head 11c92862ad8a3c1b13eb18ee7668fdb0bfae3753 on two P2 scheduler defects:
-
activationRetryByAccountIdis account-keyed rather than credential-generation-keyed. A 401/failure from the old generation can leave a 60-minute retry, then replacement credentials clear generation-scoped reauth evidence but remain suppressed by line 340 until that old retry expires. A late old-generation failure can also reintroduce the cooldown after replacement. Scope/reset retry evidence to the credential generation without clearing durable deadline/completion state, and add the replacement-during-await regression. -
warmAccount()throws when native-main admission is locally unavailable, and the outer catch grows the same 5→10→20→40→60 upstream-failure backoff. Startup/recovery gates, drains, shutdown, or exhausted admission can therefore defer activation for an hour even though no upstream request ran. Return a distinct local-busy/skipped outcome with bounded short retry, and test repeated refusal followed by release. Ordinary concurrent main use alone is not the refusal condition.
Readiness also needs reconciliation: the PR is non-draft with all boxes checked, but its body says the broad run was interrupted with exit 130/840 untriaged reports, and exact-head Cross-platform CI 36283299907 is action_required rather than semantic green. Please obtain clean exact-head CI after fixing the two cases.
The known-deadline scheduling and cleanup paths otherwise look coherent; this review does not attribute the 840 reports to this patch.
|
Landed on |
Carried from lidge-jun#6020 into merge train round 3. Co-authored-by: Terry Tan <tmy1995hflc@gmail.com>
Follow-up to lidge-jun#6020, from the review on that PR. Retry records carry the credential generation, so a replaced or reauthenticated credential no longer waits out its predecessor backoff, and a failure that raced a replacement is not recorded. A local native-main admission refusal retries after one minute without doubling the upstream backoff. main account unavailable stays in the growing backoff; generation keying already lets a later token start clean.
3 participants
Summary
Closes #6018.
Automatic quota-window activation currently queries upstream usage again when a snapshot is five minutes old, even when the next reset deadline is already persisted. This change waits on those saved deadlines locally and queries only when an enabled window lacks an uncompleted deadline, preserving automatic activation while reducing avoidable authenticated background traffic.
The issue records the user's observation of forced sign-outs and concern about regular background traffic. Causation, OpenAI detection thresholds, and improved session lifetime are not established or measured; this PR claims reduced unnecessary requests only.
Key Changes
Verification
bun test tests/codex-integration/codex-quota-auto-refresh.test.ts tests/codex-integration/codex-quota-auto-refresh-main-admission.test.ts— 46 tests, 274 assertions. Includes five hours of simulated local ticks, restart without quota cache, zero pre-deadline metadata queries, exactly one activation, missing-window discovery/backoff, response-header continuation, and expired native-main credential handling with/without metadata discovery.bun run typecheck.bun run structure:check,bun run privacy:scan, andgit diff --check.cd docs-site && bun install --frozen-lockfile && bun run build— 537 pages, 73,478 internal links checked.bun run test:changedexpanded broadly and was interrupted with SIGINT (exit 130) after roughly eight minutes as local command responsiveness degraded. Its partial output contains 840 failure reports, including repository-local fixture cleanup rejected by the real-Codex-home guard because this managed checkout is under.codex/worktrees, plus CLI subprocess and network timing failures. All 46 activation tests also passed within that run. The other failures have not all been triaged or reproduced on the base revision, so they are not asserted to be unrelated. A clean broader run in a suitable checkout/CI remains required before readiness.bun run testhas not been run; this contribution remains a draft, not a review-ready attestation. Cross-platform CI and independent maintainer review remain outstanding. No live account probes or activation requests were used for validation.Impact & Risk
No breaking configuration changes, migrations, new dependencies, or environment variables. Known idle deadlines can remain stale after an out-of-band upstream schedule change until normal traffic, explicit refresh, or recovery observes it. Retry backoff is process-local and resets on restart; actual activation may occur on the next minute sweep. This affects scheduling only and makes no guarantee about upstream sign-out prevention.
Checklist
Author inspection and regression tests preserve the existing authentication/ownership protections; independent security review is 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