Skip to content

fix(codex): recover stale main locks from two-window WHAM usage - #6188

Draft
lidge-jun wants to merge 1 commit into
devfrom
codex/rt5-account-pool-main-lock-two-window
Draft

lidge-jun wants to merge 1 commit into
devfrom
codex/rt5-account-pool-main-lock-two-window

Conversation

@lidge-jun

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

Copy link
Copy Markdown
Owner

Summary

A stale main-account short-window lock could survive a newer authenticated usage reading that showed capacity was available. This happened when WHAM returned the tertiary window omitted, the secondary window explicit null, and a measured primary window of at least 24 h. This carries the parser part of #5831 by @oocheol. parseMainPolicyUsageQuota accepts an omitted own tertiary window only in this exact shape:

  • the secondary window is explicit null;
  • allowed is true and limit_reached is false, both as booleans;
  • the primary window is measured with an explicit duration of at least 86,400 s.

Anything malformed, partial or contradictory keeps the existing strict rule. The 98% threshold is unchanged: a qualifying reading below 98% retires the stale short evidence, and one at 98% or above keeps the lock.

Held, do not merge yet. As the owner review on #6183 says, the code cannot tell whether those flags reflect only the weekly window or whether no five-hour window truly remains. Merging needs provider/owner confirmation that this response is complete evidence of no governing short window. Tests cannot prove that contract.

It builds on the credential publication fence from #6183, which is merged, and now targets dev directly.

Refs #5831

Co-authored-by: 정우철 oocheol@naver.com

Verification

  • Local tests were skipped on maintainer instruction (release train 5). The PR CI on this head is the test evidence.
  • Run locally at the head: bun run structure:check (exit 0), bun run privacy:scan (exit 0), git diff --check (exit 0).
  • Tests: main-quota-evidence-validation.test.ts (positive 64/97.99, blocked 98/100, omitted/malformed/contradictory fields, unknown duration).

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 quota admission evidence, and a wrong upstream assumption could release a real block. It needs maintainer review plus the provider confirmation.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

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

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.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 66 / 80

이 PR은 메인 계정의 옛 5시간 잠금이 새 사용량 조회 뒤에도 안 풀리는 경우를 고쳐요. WHAM이 3차 창을 빼 보내고, 2차 창은 null, 1차 창은 24시간 이상일 때예요. 화면의 사용량은 낮은데 정책 캐시는 예전 5시간 100%를 붙잡고 있었어요.

parseMainPolicyUsageQuota는 그 모양만 받아요. 2차가 정확히 null이고, 3차 키가 응답에 없고, allowed가 불리언 true, limit_reached가 불리언 false이며, 1차 창이 86400초 이상이고 사용량 숫자가 있을 때만 옛 짧은 창을 지워요. 숫자가 깨졌거나 칸이 비었거나 5시간 창이 아직 있으면 잠금을 유지해요. 98% 기준은 그대로예요. 64%와 97.99%면 잠금이 풀리고, 98%와 100%면 짧은 창만 지우고 잠금은 남아요.

이 가지는 초안이에요. 바탕은 dev가 아니라 #6183 가지 codex/rt5-account-pool-main-lock이에요. #6183은 #6179 위에 쌓여 있어요. #5831은 제목이 같고 아직 열려 있어요. 본문은 이 파서가 #5831의 파서 부분이라고 해요.

라인 - src/codex/quota.ts 835행 allowedTwoWindow. 3차 키가 없고 위 두 플래그만 맞으면 짧은 창이 없다고 봐요. 그 플래그가 주간 창만 보고 켜진 것인지, 5시간 창이 정말 없는 것인지는 이 코드가 구분하지 못해요. 테스트는 그 JSON 모양만 확인해요. WHAM이 다른 이유로 3차를 빼면, 아직 남은 5시간 잠금이 풀려요.

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

본문도 공급자 확인 전에는 머지하지 말라고 해요. 3차를 뺀 응답이 짧은 창이 없다는 완전한 증거인지는 WHAM을 보내는 쪽이 확인해야 해요. 테스트로 그 약속을 증명할 수 없어요.

이 머리의 CI는 아직 줄 서 있어요. 본문은 로컬 테스트를 일부러 건너뛰었다고 해요. 체크리스트의 보안 항목은 비어 있어요. 가정이 틀리면 진짜 차단이 풀려요.

너의 추천

WHAM 쪽이 이 응답을 짧은 창 없음의 증거로 확인한 뒤에 835행을 머지하세요. #6179와 #6183이 dev에 들어간 뒤에 이 PR의 바탕을 dev로 바꾸세요. #5831은 같은 제목으로 아직 열려 있으니 닫으세요.

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

@lidge-jun
lidge-jun force-pushed the codex/rt5-account-pool-main-lock-two-window branch from 09a7564 to 06d83fa Compare September 28, 2026 11:33
@lidge-jun
lidge-jun force-pushed the codex/rt5-account-pool-main-lock branch 2 times, most recently from 9262140 to 384535b Compare September 28, 2026 11:35
@lidge-jun
lidge-jun force-pushed the codex/rt5-account-pool-main-lock-two-window branch 2 times, most recently from 73aec7b to b8df28e Compare September 28, 2026 12:01
@lidge-jun
lidge-jun force-pushed the codex/rt5-account-pool-main-lock branch from 384535b to d39cd0e Compare September 28, 2026 12:01
@lidge-jun
lidge-jun force-pushed the codex/rt5-account-pool-main-lock-two-window branch 2 times, most recently from 1a7ce3e to 8e5a46c Compare September 28, 2026 13:34
@Ingwannu

Copy link
Copy Markdown
Owner

Maintainer hold confirmed on exact head 8e5a46c. The implementation is deliberately narrow, but the decisive premise is still external: omitted tertiary + explicit null secondary + allowed=true + limit_reached=false + a measured >=24h primary must mean that no governing short window exists. Source tests can validate shape handling, not that provider contract. Keep this draft unmergeable until that contract is confirmed, #6183 lands, the branch is rebased onto current dev, and exact-head CI is green.

Base automatically changed from codex/rt5-account-pool-main-lock to dev September 28, 2026 14:17
@lidge-jun
lidge-jun force-pushed the codex/rt5-account-pool-main-lock-two-window branch from 8e5a46c to 4bce789 Compare September 28, 2026 14:17
@lidge-jun

Copy link
Copy Markdown
Owner Author

Coordinator question: is #5831 fully covered? Not yet. #6183 (merged as 59c222d) landed #5831's credential-publication fence. The rule #5831 is named for, which accepts the two-window WHAM shape (omitted tertiary, explicit-null secondary, measured primary of at least 24 h, exact allowed/limit_reached) as evidence that clears a stale short-window block, exists only in this draft. It stays held until the provider/owner confirms that shape is complete evidence of no governing short window. Please keep #5831 open until this PR lands or is dropped.

This PR is now rebased onto dev (4bce789, a single commit by 정우철) and retargeted to dev.

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

Reviewed exact head 4bce789. The branch is current and mechanically clean, but the admission contract remains unproven. The implementation still treats long primary + secondary:null + omitted tertiary + allowed:true + limit_reached:false as authoritative proof that no short window exists. Those booleans do not establish response-topology completeness or that an omitted short window is below the local 98% lock threshold; a still-99% omitted short window can therefore be erased and weekly 64% can move the local hard lock to ready.

The only provenance is a test comment describing a sanitized observed shape, while the structure doc explicitly says completeness is not independently confirmed. The observed fixture is also plan_type=prolite but the implementation applies to every plan. Keep this held until the producer/provider contract confirms omitted tertiary means no governing short window (and whether that is plan-scoped), then encode that scope with a captured fixture/regression.

@lidge-jun

Copy link
Copy Markdown
Owner Author

@Ingwannu Agreed; this stays held. allowed: true and limit_reached: false do not prove the response topology is complete, and they cannot bound an omitted short window below the local 98% lock. As written, a still-99% short window could be erased and a 64% weekly reading could move the hard lock to ready. The only provenance is one sanitized prolite observation, but the rule applies to every plan. No code or test change can close that gap. It needs the producer/provider contract (does an omitted tertiary mean no governing short window, and is that plan-scoped?) plus a captured fixture encoding that scope.

Lane decision for release train 5: this PR does not land this train. It stays a draft with the branch kept. #6183 already landed the credential-publication part of #5831, and the two-window admission rule here is the only remaining part. For the coordinator: please leave #5831 open with a note pointing at this PR until the contract is confirmed; then this rule can be re-scoped (per plan if needed) with a captured fixture and a regression.

This branch has not been deployed

No deployments
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