Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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. |
리뷰 · 우선순위 66 / 80이 PR은 메인 계정의 옛 5시간 잠금이 새 사용량 조회 뒤에도 안 풀리는 경우를 고쳐요. WHAM이 3차 창을 빼 보내고, 2차 창은
이 가지는 초안이에요. 바탕은 라인 - 메인테이너의 판단이 필요한 지점 본문도 공급자 확인 전에는 머지하지 말라고 해요. 3차를 뺀 응답이 짧은 창이 없다는 완전한 증거인지는 WHAM을 보내는 쪽이 확인해야 해요. 테스트로 그 약속을 증명할 수 없어요. 이 머리의 CI는 아직 줄 서 있어요. 본문은 로컬 테스트를 일부러 건너뛰었다고 해요. 체크리스트의 보안 항목은 비어 있어요. 가정이 틀리면 진짜 차단이 풀려요. 너의 추천 WHAM 쪽이 이 응답을 짧은 창 없음의 증거로 확인한 뒤에 835행을 머지하세요. #6179와 #6183이 이 댓글은 grok-bot이 작성했습니다 |
09a7564 to
06d83fa
Compare
9262140 to
384535b
Compare
73aec7b to
b8df28e
Compare
384535b to
d39cd0e
Compare
1a7ce3e to
8e5a46c
Compare
|
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. |
8e5a46c to
4bce789
Compare
|
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 This PR is now rebased onto |
Ingwannu
left a comment
There was a problem hiding this comment.
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.
|
@Ingwannu Agreed; this stays held. 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. |
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.parseMainPolicyUsageQuotaaccepts an omitted own tertiary window only in this exact shape:null;allowedistrueandlimit_reachedisfalse, both as booleans;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
devdirectly.Refs #5831
Co-authored-by: 정우철 oocheol@naver.com
Verification
bun run structure:check(exit 0),bun run privacy:scan(exit 0),git diff --check(exit 0).main-quota-evidence-validation.test.ts(positive 64/97.99, blocked 98/100, omitted/malformed/contradictory fields, unknown duration).Checklist