Skip to content

test(routing): observe the quorum-cache store read with a spy, not atime (Windows stack 4/4) - #3555

Merged
lidge-jun merged 5 commits into
devfrom
codex/win-4-quorum-observer
Sep 5, 2026
Merged

test(routing): observe the quorum-cache store read with a spy, not atime (Windows stack 4/4)#3555
lidge-jun merged 5 commits into
devfrom
codex/win-4-quorum-observer

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

Windows suite stabilization, PR 4 of 4 (stacked on #3550). Fixes three anthropic-quorum-cache failures on windows 2/4 that arrived on dev with #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 asserting readFileSync moved it. Measured on windows-latest (run 33929916059): fsutil behavior query DisableLastAccess reports 3 (System Managed, Last Access Time Updates DISABLED), and readFileSync leaves 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 — green, but unable to fail.

The observer now counts pass-through spyOn(fs, "readFileSync") calls against this home's auth.json, the instrument claude-system-env-auto.test.ts already uses. The path filter keeps refresh-intent files, lock snapshots and peekAuthStore out of the count. Every call site and assertion keeps its shape. No src/ change.

Ablation before the fix: disabling the cache-hit return at anthropic-routing.ts:291 turns 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 via getAccountCredential).

Verification

  • bun test tests/routing/anthropic-quorum-cache.test.ts (macOS): 7 pass
  • Ablation run and reverted; git diff --stat shows only the test file
  • bun run typecheck clean
  • CI run 33930757649 dispatched on this head (windows 2/4 expected green)
  • Plan audit (read-only, sol/high): GO-WITH-FIXES (3), all folded — the guarded src/ counter proposed in the first draft was dropped for this spy

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.

Summary by CodeRabbit

  • Tests

    • Improved cross-platform reliability for Anthropic quorum-cache tests, including Windows environments where filesystem access-time tracking is unavailable.
    • Added reliable read-count verification to confirm cache hits avoid unnecessary file reads.
    • Ensured test instrumentation is cleaned up between cases.
  • Documentation

    • Recorded Windows CI stabilization results, timing considerations, and remaining platform-specific test findings.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 4, 2026 23:47
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: fb426843-a098-41f5-8206-fcbd80665833

📥 Commits

Reviewing files that changed from the base of the PR and between 8860964 and 70d9041.

📒 Files selected for processing (5)
  • devlog/_plan/260905_windows_suite_stabilization/000_plan.md
  • devlog/_plan/260905_windows_suite_stabilization/050_ci_residual_retained_root.md
  • devlog/_plan/260905_windows_suite_stabilization/060_dev_drift_atime.md
  • devlog/_plan/260905_windows_suite_stabilization/070_quorum_cache_observer.md
  • tests/routing/anthropic-quorum-cache.test.ts

📝 Walkthrough

Walkthrough

The PR records Windows CI stabilization results and documents an NTFS atime failure in quorum-cache tests. The test now counts pass-through readFileSync calls for the sandbox’s auth.json and verifies zero additional reads on warm-cache hits.

Changes

Windows Suite Stabilization

Layer / File(s) Summary
CI stabilization results
devlog/_plan/260905_windows_suite_stabilization/000_plan.md, devlog/_plan/260905_windows_suite_stabilization/050_ci_residual_retained_root.md
Records touched-file results, unrelated Windows failures, successful Windows shards, timing variance, child-process cleanup, and follow-up publication status.
Quorum-cache read observation
devlog/_plan/260905_windows_suite_stabilization/060_dev_drift_atime.md, devlog/_plan/260905_windows_suite_stabilization/070_quorum_cache_observer.md, tests/routing/anthropic-quorum-cache.test.ts
Documents that Windows NTFS does not reliably update atime. Replaces atime checks with a restored, pass-through readFileSync spy filtered to auth.json. The warm-cache test now checks for zero additional reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: ingwannu

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/win-4-quorum-observer

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 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Sep 4, 2026

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

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.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 64 / 80

이 PR은 Windows 스위트 안정화 스택의 네 번째(그리고 지금 기준으로 마지막) 조각입니다. 앞선 #3548#3549→#3550이 계획한 세 파일 고정을 끝낸 뒤, dev에 #3533이 들어오면서 windows 2/4의 tests/routing/anthropic-quorum-cache.test.ts 세 케이스가 새로 빨개진 드리프트를 고칩니다. 지금 dev HEAD는 445742966(패키지 2.43.0, 최근 #3544 Google LaTeX 유도 등)이고, 이 브랜치 베이스는 dev가 아니라 #3550 헤드(codex/win-3-k-owner-budget)입니다. 제품 src/는 건드리지 않고 관측 방식만 바꿉니다.

문제의 핵심은 “auth.json을 읽었는가”를 어떻게 증명하느냐입니다. 지금 dev의 테스트는 utimesSync로 atime을 과거로 밀어 두고, readFileSync가 atime을 올렸는지로 읽기를 판정합니다. 그런데 windows-latest NTFS는 fsutil DisableLastAccess = 3(시스템 관리, Last Access 갱신 OFF)이라 readFileSync가 atimeMs를 안 바꿉니다. 그래서 “invalidates immediately” 세 케이스는 읽기를 절대 못 보고, “shares one read” 버스트 케이스는 같은 이유로 항상 통과(허위 양성)할 수 있었습니다. 캐시가 깨져도 Windows에서는 빨간불이 안 켜지는 상태였습니다.

고친 관측기는 bun:testspyOn(fs, "readFileSync") 패스스루입니다. mockImplementation으로 가로채지 않고 호출만 세며, 실제 읽기는 그대로 갑니다. 세는 대상은 이 테스트가 만든 OPENCODEX_HOME 아래 auth.json 경로와 문자열이 정확히 같은 호출뿐입니다. refresh-intent 파일·락 스냅샷처럼 다른 경로의 읽기는 자동으로 빠집니다. claude-system-env-auto.test.ts가 이미 쓰는 스파이 패턴과 같고, 070 계획 초안의 authStoreReadCountForTestsOnly 제품 카운터 시암은 버렸습니다. src/ 변경이 없어서 privacy:scan·제품 계약 리스크가 거의 없습니다.

버스트 케이스 주석도 같이 고쳤습니다. 예전 이름(“shares one store read”)은 채움(fill) 때 getAccountSet + 최대 두 번의 getAccountCredential로 읽기가 여러 번일 수 있다는 사실과 어긋날 수 있습니다. 이제 주석은 “히트 구간에 읽기 0회”를 증명한다고 못 박아서, 나중에 누군가 === 1로 조이다가 깨는 일을 막습니다. 작성자 ablation(캐시 히트 return을 끄면 스파이로는 빨개지고 atime으로는 Windows에서 못 잡음)도 방향이 맞습니다. 로컬 macOS 7/7·typecheck 깨끗하다고 적혀 있고, 코드 변경량은 테스트 한 파일 + 계획/증적 문서입니다.

라인 30-45 (beforeEach/afterEach) - spyOn(fs, "readFileSync")를 매 케이스에 걸고 mockRestore합니다. 패스스루라 실제 I/O는 유지되고, afterEach에서 스파이부터 복구한 뒤 홈을 지워서 다른 파일로 스파이 누수가 안 갑니다. 형태는 안전합니다.
라인 authReadCount 경로 필터 - String(path) === join(home, "auth.json")만 셉니다. 락 파일·refresh-intent는 경로가 달라서 자연히 제외됩니다. 다만 PR/070 문구의 “peekAuthStore를 필터로 뺀다”는 표현은 조금 과장입니다. peekAuthStore도 같은 auth.jsonreadFileSync하므로 경로 필터만으로는 구분되지 않습니다. 이 스위트가 그 API를 안 부르는 것이 실제 보호막입니다. 주석을 “이 테스트는 peek를 안 부른다” 쪽으로 고치면 나중에 덜 헷갈립니다.
라인 버스트 케이스 주석 - “0 reads during hits, not one read per fill”은 맞습니다. 케이스 제목은 아직 “shares one store read”라서, 제목까지 “zero store reads on cache hits”처럼 맞추면 문서·로그가 더 일치합니다(필수 변경은 아님).
경로 스택 베이스 - base가 codex/win-3-k-owner-budget(#3550)이라 지금 dev에 단독 머지할 수 없습니다. #3548#3549→#3550이 먼저 들어가거나, 스택 전체를 순서대로 랜딩해야 합니다. 제목은 4/4인데 앞 PR 제목은 여전히 1/3·2/3·3/3이라 번호만 어긋나 있습니다(동작 문제는 아님).
경로 CI - 이 PR 목적 자체가 windows 2/4의 quorum-cache 세 케이스 복구입니다. 체크가 아직 pending/일부 windows skip 상태면, windows 2/4 로그에서 해당 파일 7건이 초록인지 확인한 뒤에만 머지하세요. macOS만 초록이고 Windows 관측이 안 보이면 이 PR의 존재 이유가 비검증으로 남습니다.
경로 devlog/_plan/260905_windows_suite_stabilization/070_*.md - 초안에 있던 제품 카운터 시암을 스파이로 바꾼 이유가 잘 적혀 있습니다. 랜딩 후 corpus/fuck-powershell ntfs-atime 케이스는 후속으로 둬도 됩니다.

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

너의 추천
우선순위 64로 두고, #3548#3549→#3550이 dev에 들어간 뒤 이 PR을 이어서 머지하세요. src/ 없는 테스트 관측 교체라 제품 리스크는 낮고, Windows atime 허위 양성을 실제로 끊는 마지막 조각입니다. 머지 전 조건은 하나뿐입니다. hosted windows 2/4에서 anthropic-quorum-cache가 초록인지 로그로 확인하세요. peek 주석·케이스 제목은 원하면 한 줄 follow-up으로 충분합니다.

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

jun added 5 commits September 5, 2026 11:39
…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.
@lidge-jun
lidge-jun force-pushed the codex/win-4-quorum-observer branch from fd786be to 70d9041 Compare September 5, 2026 02:39
@lidge-jun
lidge-jun merged commit e6ed7a4 into dev Sep 5, 2026
5 of 6 checks passed
@lidge-jun
lidge-jun deleted the codex/win-4-quorum-observer branch September 5, 2026 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants