fix(codex): retire main hard lock on authoritative absent 5h window - #6287
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 (10)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe quota parser now accepts explicit primary-window absence with measured weekly secondary usage as evidence to replace retained short-window quota. It preserves validation requirements for other windows and the 98% weekly lock threshold. Integration tests and documentation cover the expanded recovery rules. ChangesMain-account hard-lock recovery
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~15 minutes Change: Bug fix · Severity of issue fixed: Medium Possibly related PRs
Suggested labels: Merge Risk: ⚪ Minimal · up to The change permits recovery from retained short-window blocks only with validated absence and measured weekly usage, while preserving blocking at 98% or above. No actionable merge-blocking defect is established; normal checks and maintainer approval still apply. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change accepts an additional authenticated usage-response shape for releasing main-account protection. Identity and credential checks remain in place, and weekly usage at or above 98% still blocks. No introduced security bypass was established, but secondary quota consumers and the complete before-and-after comparison were only partially verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 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 |
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 58 / 80이 PR은 메인 계정의 98% 하드락이 풀리지 않던 버그를 고칩니다. 예전에 5시간 창이 98% 이상으로 잡혀 잠겼는데, 그 뒤 WHAM이 5시간 창을 더 이상 안 주고 라인 - 메인테이너의 판단이 필요한 지점 WHAM이 실제로 너의 추천 방향은 맞고, 회귀 테스트 범위도 좋습니다. Cross-platform CI가 이 head에서 통과하고, admission 관점의 짧은 보안 확인이 끝나면 머지해도 됩니다. CI 전이거나 WHAM null-primary 의미를 팀에서 다르게 보면 보류하세요. types/config 분할·중복 PR 이슈는 이 변경과 무관합니다. 이 댓글은 grok-bot이 작성했습니다 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 370beb9ea6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (quota && (isMeasuredLongWindow(primary) | ||
| || (primary === null && isMeasuredLongWindow(secondary) && quota.weeklyPercent !== undefined)) |
There was a problem hiding this comment.
Retire the legacy short tuple with the policy evidence
When the retained blocking short reading has no reset timestamp, or its reset is still in the future, this new shortWindowAbsent marker clears only mainPolicyQuota: setAccountQuotaFromParsed still merges the unmarked parseUsageQuota result into the ordinary accountQuota, whose assignCarriedShort path preserves that short tuple. Consequently getMainAccountHardLockStatus becomes ready while computeCodexUsageScore(getAccountQuota(MAIN_CODEX_ACCOUNT_ID)) continues to include the stale 98–100% short value, so Pool routing and auto-switching can keep avoiding the main account indefinitely despite the authoritative-null response. Consume the validated absence marker in the main account's ordinary merge as well, with coverage for missing and future short resets.
Useful? React with 👍 / 👎.
|
Maintainer integration into Security review (account admission). This changes when the main-account hard lock releases. It does not touch credentials, tokens, OAuth flows, or outbound destinations. The only new release path is Evidence. Exact head |
Summary
Fix main-account 98% protection retaining a retired 5h block when a fresh authenticated WHAM response explicitly reports
primary_window: nulland supplies measured weekly usage in the secondary window. The policy now accepts this shape alongside the existing measured-long-primary shape, while requiring all other ordinary windows to be explicitly null or measured with a declared duration of at least 24 hours.Only fresh valid short usage below 98%, or validated authoritative WHAM absence, can retire the blocking 5h tuple. Omitted/partial weekly updates, unreadable windows, credits-only responses, reset clocks, cache age, and stale identity writers still cannot release it. A remaining weekly reading at 98% or higher continues to block. No operator recovery API is added because the existing WHAM explicit-null signal supplies authoritative absence; installations receiving only partial observations continue to hold the lock.
Closes #6244
Verification
OCX_TEST_NO_QUEUE=1 bun run test tests/codex-integration/main-account-hard-lock-retirement.test.ts tests/codex-integration/main-account-hard-lock-policy.test.ts tests/codex-integration/main-quota-evidence-validation.test.ts tests/codex-integration/main-quota-provenance.test.ts tests/codex-integration/main-account-hard-lock-recovery.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts: 356 passed, 0 failed. This includes actual persistence/rehydration and the production background recovery consumer.bun run typecheck,bun run structure:check,bun run privacy:scan: passed.cd docs-site && bun install --frozen-lockfile && bun run build: passed, 545 pages and 74,716 internal links checked.OCX_TEST_NO_QUEUE=1 bun run test:changed: failed, 26,327 passed / 47 skipped / 869 failed across 1,339 files (444.9s). The run emitted 1,431 real-Codex-home cleanup refusals because this managed checkout is under the real Codex home; there were also launcher collisions with the already-running local proxy and other assertion failures. The new retirement tests had zero failures. The CLI/service files that showed assertion failures in the broad run passed in isolation:OCX_TEST_NO_QUEUE=1 bun run test tests/cli/cli-headless-parity.test.ts tests/service/service-ownership-state.test.tsyielded 127 passed / 0 failed. An unchanged routing test reproduces the cleanup refusal in isolation:OCX_TEST_NO_QUEUE=1 bun run test tests/routing/probe-lease-dispatch-wiring.test.ts --test-name-pattern 'production Responses dispatch records demand'failed before entering the test body. This broad local result is not claimed as passing; exact-head hosted CI is required for broad validation.370beb9ea6c297d722859c384bdd3a66bab3ff2b(pull_request, attempt 1): all 17 selected jobs completed successfully, including all four Linux test shards, the gates and aggregateci, Docker, docs/structure, and Windows/Ubuntu npm/keyring smoke. The eight unselected native/control/other job placeholders were skipped by workflow scope and are not passing-test evidence.Checklist
This changes main-account admission. Explicit coordinator/maintainer security review remains required before merge; this task must leave the PR unmerged.
Summary by CodeRabbit