test(routing): observe the quorum-cache store read with a spy, not atime (Windows stack 4/4) - #3555
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR records Windows CI stabilization results and documents an NTFS atime failure in quorum-cache tests. The test now counts pass-through ChangesWindows Suite Stabilization
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: ✨ 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. |
Ingwannu
left a comment
There was a problem hiding this comment.
The test change fixes a real Windows-observer defect without weakening the quorum-cache behavior assertions. NTFS atime can be disabled, so it cannot prove a read; the pass-through, exact-auth.json path-filtered readFileSync spy observes the operation directly. The cache-hit ablation makes the burst assertion non-vacuous, and no product source is changed.
I am requesting changes only because this PR targets codex/win-3-k-owner-budget, not an accepted integration branch, and its parent chain is still open. After #3548/#3549/#3550 land in order, retarget/rebase this layer onto the then-current dev and run exact-head CI there. I will approve the final one-file test diff if the spy remains isolated/restored and the Windows shard is green. Do not merge it into the temporary stack branch.
리뷰 · 우선순위 64 / 80이 PR은 Windows 스위트 안정화 스택의 네 번째(그리고 지금 기준으로 마지막) 조각입니다. 앞선 #3548→#3549→#3550이 계획한 세 파일 고정을 끝낸 뒤, 문제의 핵심은 “auth.json을 읽었는가”를 어떻게 증명하느냐입니다. 지금 고친 관측기는 버스트 케이스 주석도 같이 고쳤습니다. 예전 이름(“shares one store read”)은 채움(fill) 때 라인 30-45 (beforeEach/afterEach) - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
60425ed to
ffa6881
Compare
…rver on Windows (060)
…asured on windows-latest
…ed spy, not atime Three cases in anthropic-quorum-cache failed on windows 2/4 as soon as #3533 landed: the observer pinned auth.json's atime into the past and asserted that readFileSync moved it. Measured on windows-latest (run 33929916059): fsutil DisableLastAccess = 3, and readFileSync left atimeMs unchanged. So the three 'invalidates immediately' cases could never see the read they assert on, and the 'shares one read' case passed vacuously on the same platform. The observer now counts pass-through spyOn(fs, 'readFileSync') calls against this home's auth.json - the same instrument claude-system-env-auto.test.ts already uses, filtered so refresh-intent files, lock snapshots and peekAuthStore never enter the count. All four call sites and every assertion keep their shape. No src/ change. Ablation: disabling the cache-hit return at anthropic-routing.ts:291 turns the burst case red on the spy (Expected false, Received true), which atime could not do on Windows. A comment records that the burst case proves zero reads during hits, not one read per fill - a fill is up to three reads through getAccountCredential, and tightening to ===1 would be wrong.
fd786be to
70d9041
Compare
Summary
Windows suite stabilization, PR 4 of 4 (stacked on #3550). Fixes three
anthropic-quorum-cachefailures on windows 2/4 that arrived ondevwith #3533, after the first three PRs were planned.The test observed "was the store read" by pinning
auth.json's atime into the past and assertingreadFileSyncmoved it. Measured on windows-latest (run 33929916059):fsutil behavior query DisableLastAccessreports3 (System Managed, Last Access Time Updates DISABLED), andreadFileSyncleavesatimeMsunchanged. So the three "invalidates immediately" cases could never see the read they assert on, and the "shares one read" case passed vacuously on the same platform — green, but unable to fail.The observer now counts pass-through
spyOn(fs, "readFileSync")calls against this home'sauth.json, the instrumentclaude-system-env-auto.test.tsalready uses. The path filter keeps refresh-intent files, lock snapshots andpeekAuthStoreout of the count. Every call site and assertion keeps its shape. Nosrc/change.Ablation before the fix: disabling the cache-hit return at
anthropic-routing.ts:291turns the burst case red on the spy (Expected false, Received true) — atime could not do that on Windows. The burst case's comment now records that it proves zero reads during hits, not one read per fill (a fill is up to three reads viagetAccountCredential).Verification
bun test tests/routing/anthropic-quorum-cache.test.ts(macOS): 7 passgit diff --statshows only the test filebun run typecheckcleansrc/counter proposed in the first draft was dropped for this spyChecklist
Summary by CodeRabbit
Tests
Documentation