Skip to content

fix(codex): retire main hard lock on authoritative absent 5h window - #6287

Merged
lidge-jun merged 1 commit into
devfrom
codex/6244-hard-lock-retirement
Sep 30, 2026
Merged

lidge-jun merged 1 commit into
devfrom
codex/6244-hard-lock-retirement

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fix main-account 98% protection retaining a retired 5h block when a fresh authenticated WHAM response explicitly reports primary_window: null and 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

  • Red regression before the fix: 18 passed, 5 failed because the 19-day-old short tuple remained after explicit-null primary observations.
  • 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.
  • Required 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.ts yielded 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.
  • The shared host was running three other wrapped test suites, and the first focused attempt waited behind the global user test lock. Focused validation and the required changed run use the supported intentional-overlap option with isolated homes. A second full local run is deferred to Cross-platform CI to avoid multiplying broad suites on this shared host; all jobs requested by the Cross-platform CI pull-request event must succeed at this PR's exact head before readiness. The workflow intentionally does not request the full macOS/Windows suites for this source-only PR; their skipped placeholders are not claimed as passing evidence. Windows npm/keyring smoke remains selected.
  • Independent plan audit and final adversarial implementation review passed with no blockers. Cross-platform CI run 36662774692 succeeded for head 370beb9ea6c297d722859c384bdd3a66bab3ff2b (pull_request, attempt 1): all 17 selected jobs completed successfully, including all four Linux test shards, the gates and aggregate ci, 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

  • 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.

This changes main-account admission. Explicit coordinator/maintainer security review remains required before merge; this task must leave the PR unmerged.

Summary by CodeRabbit

  • Bug Fixes
    • Main-account blocking status can now recover when a complete, validated quota reading confirms that the short-term usage window is absent and provides valid weekly usage. It can also recover from a measured long primary window.
    • Recovery still requires explicit, valid evidence for the other quota windows. Missing or incomplete readings, monthly-only usage, and elapsed reset times alone do not clear a block.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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 configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: cd50edab-1d77-4980-8ace-b7222fa6bc2a

📥 Commits

Reviewing files that changed from the base of the PR and between 1b97c6f and 370beb9.

📒 Files selected for processing (10)
  • docs-site/src/content/docs/ko/reference/cli/providers-accounts.md
  • docs-site/src/content/docs/reference/cli/providers-accounts.md
  • scripts/test-layout/layout.json
  • src/codex/main-account-hard-lock.ts
  • src/codex/quota.ts
  • structure/providers/openai-tiers.md
  • tests/codex-integration/main-account-hard-lock-recovery.test.ts
  • tests/codex-integration/main-account-hard-lock-retirement.test.ts
  • tests/codex-integration/main-quota-provenance.test.ts
  • tests/fixtures/test-layout-expected.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The 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.

Changes

Main-account hard-lock recovery

Layer / File(s) Summary
Accept validated primary-window absence
src/codex/quota.ts, src/codex/main-account-hard-lock.ts, structure/providers/openai-tiers.md, docs-site/src/content/docs/reference/cli/providers-accounts.md, docs-site/src/content/docs/ko/reference/cli/providers-accounts.md
The quota parser accepts a null primary window with a measured long secondary window and parsed weekly usage as replacement evidence. The other windows must still meet the explicit validation rules. Comments and documentation describe the recovery conditions, including that a predicted reset alone does not establish recovery.
Test retirement and recovery cases
tests/codex-integration/main-account-hard-lock-retirement.test.ts, tests/codex-integration/main-account-hard-lock-recovery.test.ts, tests/codex-integration/main-quota-provenance.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
Integration tests cover primary-window absence, weekly usage below and at the 98% threshold, incomplete or invalid observations, stale writers, and fresh short-window readings. Existing weekly-usage fixtures now place the value in the secondary window. Test-layout mappings include the new test file.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

  • lidge-jun/opencodex#5620: Introduced the shortWindowAbsent observation and replacement of retained 5-hour evidence after a validated long primary window.
  • lidge-jun/opencodex#5870: Defines the 98% hard-lock behavior and retention of blocking short-window evidence until valid replacement evidence arrives.
  • lidge-jun/opencodex#5831: Extends WHAM topology evidence and checks credentials before publishing recovery state.

Suggested labels: review-ready

Merge Risk: ⚪ Minimal · up to 370be

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 Review

Security architecture risk: 🔵 Low · up to 370be

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The changed decision concerns protection of the installation's main account. Policy writers bind to the observed account identity and identity generation, and policy reads reject records belonging to another observed identity.

Trust Boundaries and Controls

  • observed — On the inspected production path, the response cannot publish policy evidence unless the owned physical account selection, identity generation, credential generation, and bearer remain current. A dispatch fence prevents an older response from overwriting a newer published result. The parser validates evidence shape; authentication and freshness authority are enforced by the surrounding publication path.

Resilience and Maintainability Implications

  • observed — Background hard-lock recovery coalesces concurrent work, respects reauthentication state and query pacing, and obtains native-main ownership before fetching. Token-preparation or fetch failures do not explicitly clear policy protection; the recovery lease and in-flight state are released in the terminal cleanup path.
  • inferred — Policy retirement and ordinary quota presentation need not be identical. A nonelapsed short tuple can remain in the legacy map, which the raw-quota API and routing scores consume; elapsed legacy tuples are dropped, and the main account card uses freshly parsed window values. This separation can affect secondary observations or routing choices, but it does not demonstrate a bypass of the hard-lock evaluator's identity-bound policy projection.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: retiring the main hard lock when an authoritative response confirms that the 5-hour window is absent.
Linked Issues check ✅ Passed Issue #6244 requires that a stale blocking 5h tuple cannot hold the main-account lock indefinitely when new observations prove that the 5h window is absent. The reviewed head implements the authoritat…
Out of Scope Changes check ✅ Passed The changed files remain connected to issue #6244. src/codex/quota.ts and src/codex/main-account-hard-lock.ts document the policy and release rule. The English and Korean provider-account document…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files. (5 skipped: 5 …
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • 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

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

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

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 58 / 80

이 PR은 메인 계정의 98% 하드락이 풀리지 않던 버그를 고칩니다. 예전에 5시간 창이 98% 이상으로 잡혀 잠겼는데, 그 뒤 WHAM이 5시간 창을 더 이상 안 주고 primary_window: null에 주간 사용량만 주는 경우가 있습니다. 예전 규칙은 “1차 창이 24시간 이상이고 수치가 있어야”만 옛 5시간 증거를 지울 수 있어서, null 1차 + 주간 2차 모양으로는 영원히 잠긴 채로 남았습니다. 지금은 그 모양도 “권위 있는 부재”로 받아들여, 주간이 98% 미만이면 잠금을 풀고, 98% 이상이면 주간 기준으로 계속 막습니다. 필드가 빠지거나, 숫자만 깨져 있거나, 크레딧만 있거나, 헤더만 오거나, 오래된 identity writer면 여전히 안 풉니다. 문서·구조 설명·회귀 테스트도 같이 맞춰 두었습니다. base는 dev입니다.

라인 - src/codex/quota.ts parseMainPolicyUsageQuota: 해제의 실제 스위치는 여기의 shortWindowAbsent입니다. main-account-hard-lock.ts는 주석만 “부재면 해제”로 바뀌었고, 함수 자체는 short 필드가 지워진 뒤 결과를 볼 뿐입니다. 동작은 맞지만, 주석만 보면 hard-lock 쪽이 부재를 직접 검사하는 것처럼 읽힐 수 있습니다.
라인 - parseMainPolicyUsageQuota 새 조건 primary === null && isMeasuredLongWindow(secondary) && quota.weeklyPercent !== undefined: secondary가 측정된 장기 창이면 보통 weeklyPercent도 같이 채워집니다. 이중 조건이 파서 매핑 변경에 대한 방어인지, 아니면 한쪽에만 기대도 되는지 한 줄로 남기면 이후 수정이 안전해집니다.
라인 - 로컬 test:changed는 869실패로 적혀 있고, 원인으로 실제 Codex home·프록시 충돌을 듭니다. 포커스 스위트 통과만으로는 머지 근거가 부족합니다. exact-head Cross-platform CI가 녹색인지가 게이트입니다.
라인 - 메인 계정 입장(admission) 정책 변경인데 PR 체크리스트의 security review는 비어 있습니다. 비밀/토큰 유출은 없어 보이지만, “언제 잠금을 푸느냐”는 보안·남용 경계에 가깝습니다.

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

WHAM이 실제로 primary_window: null + secondary 주간을 “5시간 창이 없다”는 권위 신호로 쓰는지, 이 한 스냅샷만으로 19일짜리 차단 튜플을 지워도 되는지. 운영자 수동 해제 API를 계속 안 넣는 선택도 이슈 #6244 기대(TTL/unknown/운영 액션)와 얼마나 맞는지. exact-head CI 전 머지 여부.

너의 추천

방향은 맞고, 회귀 테스트 범위도 좋습니다. Cross-platform CI가 이 head에서 통과하고, admission 관점의 짧은 보안 확인이 끝나면 머지해도 됩니다. CI 전이거나 WHAM null-primary 의미를 팀에서 다르게 보면 보류하세요. types/config 분할·중복 PR 이슈는 이 변경과 무관합니다.

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

@lidge-jun
lidge-jun marked this pull request as ready for review September 30, 2026 03:16
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 30, 2026 03:16
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T03:19:30.755050Z 370beb9 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread src/codex/quota.ts
Comment on lines +843 to +844
if (quota && (isMeasuredLongWindow(primary)
|| (primary === null && isMeasuredLongWindow(secondary) && quota.weeklyPercent !== undefined))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration into dev with security review (owner-authorized backlog loop).

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 parseMainPolicyUsageQuota accepting an explicit primary_window: null with a measured long secondary window as proof that the 5h window is absent. This holds only when every other window is an explicit null or a measured long window, so it keeps the complete-topology requirement already used for a long primary. Rejected, with tests: omitted windows, unmeasured or out-of-range percentages, a short tertiary, tertiary-only monthly plans, header-only readings, elapsed resets, and stale identity writers. Weekly usage at 98% or above still blocks. The residual risk is an upstream response that reports primary_window: null while a 5h limit still applies. That would release the lock early, and the proxy would recover on the next blocking reading, as the rearm test shows. I accept that trade-off against the 19-day permanent exclusion in #6244.

Evidence. Exact head 370beb9ea6: Cross-platform CI passed all selected jobs, and scripts/ci/assert-mergeable-review.sh --maintainer-integration 6287 passed. Related: #6188 also works on the quota.ts admission contract and remains under Ingwannu's review. This PR does not resolve that objection.

@lidge-jun
lidge-jun merged commit f69918c into dev Sep 30, 2026
48 of 49 checks passed
@lidge-jun
lidge-jun deleted the codex/6244-hard-lock-retirement branch September 30, 2026 03:27
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.

1 participant