test(windows): close the four residual shard-2 Windows failures

[lidge-jun]

Summary

The last four Windows-only failures from the dispatched leg. After #2968 and #2972, shards 1, 3 and 4
pass on run 33289201339; shard 2 held
these four. platform-windows runs only on workflow_dispatch, so none of them was visible to the CI
that gates release.yml.

Four failures, three distinct mechanisms

launchd service plist > the generated launcher command follows a retargeted shim… — a false
platform assumption the earlier serialization fix in #2968 did not reach. That commit corrected the path
assertions, but the test also executes an extensionless #!/bin/sh fixture, and Windows cannot run
that as a native executable. The fixture is now a cross-platform Bun redirect, keeping the version
retargeting, old-version deletion, spaces-and-apostrophes and path-serialization coverage it had.

ocx init piped stdin (#754) > exits cleanly when stdin closes before the first prompt answer — the
8-second race started at process spawn, so Windows startup and import time was being charged against
EOF handling. It now waits for the first prompt to appear, closes stdin while readline is genuinely
awaiting an answer, and waits on exit directly. The budget no longer measures interpreter startup.

Two storage mutation coordinator cases — both were sleep-based assumptions that a loaded Windows
runner invalidates:

• renameNoReplace() is linkSync then unlinkSync, so there is a real interval where both names
  exist. The test treated destination existence as move completion and could observe that interval.
• The other slept 80ms and assumed the cleanup lease had been acquired. A preceding failed restore also
  left an unresolved request, which matches the ECONNRESET-then-401 cascade in the Windows log.

Both now use an explicit handshake — the worker publishes only after every move completes, and restore is
attempted only once the cleanup lease is confirmed held. Teardown always releases the worker and drains
the pending request. No timeout was raised and nothing was skipped: a longer sleep would have hidden the
linkSync/unlinkSync window rather than closing it.

Verification

Based on dev@aa16a71e0.

• bun test tests/service.test.ts tests/init-eof.test.ts tests/storage-mutation-race.test.ts → 154 pass, 0 fail
• bun x tsc --noEmit → clean

Two of the four could not be reproduced exactly on POSIX — the Windows scheduling delay and the 401
cascade specifically — so what each fix removes is the incidental dependency (spawn time, wall-clock
sleeps, an ambiguous filesystem observation) rather than a simulated repro. The dispatched Windows run on
this branch is the real gate and is what I will check before merging.

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

• Bug Fixes

  • Improved reliability of initialization handling when input reaches end-of-file.
  • Improved cross-platform behavior for generated launcher commands after version retargeting.

• Tests

  • Strengthened storage race-condition coverage using deterministic synchronization checkpoints.
  • Improved cleanup and process teardown handling to reduce flaky or leaking tests.

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-30T03:29:47.819063Z	5138944	PR opened

ℹ️ 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.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bffacced-77ca-46cd-8582-7076744afcb1

📥 Commits

Reviewing files that changed from the base of the PR and between aa16a71 and 5138944.

📒 Files selected for processing (5)

• src/storage/cleanup.ts
• src/storage/storage-mutation-coordinator.ts
• tests/init-eof.test.ts
• tests/service.test.ts
• tests/storage-mutation-race.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The changes add deterministic filesystem handshakes for storage race tests, synchronize the initialization EOF test with its prompt, and make the shim-retarget test use cross-platform Bun fixtures.

Changes

Storage race synchronization

Layer / File(s)	Summary
Storage handshake hooks src/storage/cleanup.ts, src/storage/storage-mutation-coordinator.ts	Test hooks can publish readiness markers and wait for release markers after restore file moves or mutation-slot acquisition.
Deterministic storage race tests tests/storage-mutation-race.test.ts	Race tests use polling and explicit ready/release checkpoints. Teardown releases paused workers and consumes pending requests.

Process test stability

Layer / File(s)	Summary
Process test synchronization tests/init-eof.test.ts	The test waits for the initialization prompt before sending EOF and cleans up the process in finally.
Cross-platform shim fixtures tests/service.test.ts	The shim-retarget test uses Bun entry scripts and pathToFileURL imports executed through process.execPath.

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

Merge Risk: ⚪ Minimal · up to 51389

This PR makes Windows and storage race tests deterministic without changing normal application behavior. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: ingwannu, wibias

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies a Windows-specific test fix and accurately summarizes the PR objective of resolving the four remaining shard-2 failures. It is concise and specific.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/windows-shard2-green-260830

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[lidge-jun]

리뷰 · 우선순위 60 / 80

설명

지금 dev는 #2968(Windows dispatch 테스트 시임)과 #2972(프롬프트 경로 mkdirSync)까지 올린 뒤에도, workflow_dispatch platform-windows shard 2에 실패 네 개가 남아 있다. 이 PR은 그 네 개를 닫는다. release.yml 게이트 CI에는 안 보이지만, Windows 레그 녹색을 목표로 하는 마무리 열차와 맞는다. 제품 런타임 기능 추가는 거의 없고, 테스트와 테스트 전용 훅이 본체다.

실패 메커니즘은 세 종류다. (1) launchd service plist 테스트가 extensionless #!/bin/sh 픽스처를 실행한다. #2968은 경로 단언만 고쳤고, Windows는 그 셸 스크립트를 네이티브로 못 돌린다. 이 PR은 버전 타깃을 Bun import(pathToFileURL(...)) 리다이렉트 픽스처로 바꿔, 리타깃·구버전 삭제·공백/아포스트로피·경로 직렬화 커버는 유지한 채 호스트를 가리지 않게 했다. (2) ocx init EOF 테스트는 8초 예산을 프로세스 spawn부터 재서, Windows 기동/임포트 시간이 EOF 처리로 잘못 청구됐다. 이제 첫 프롬프트 문자열이 나온 뒤 stdin을 닫고 exit를 기다린다. (3) storage mutation coordinator 두 케이스는 sleep 가정이었다. renameNoReplace()는 linkSync+unlinkSync라 잠깐 두 이름이 동시에 존재할 수 있고, 80ms sleep은 cleanup 리스 획득을 보장하지 못한다. ready/release 파일 핸드셰이크로 “이동 끝”과 “슬롯 획득”을 명시한다.

프로덕션 쪽 변경은 src/storage/cleanup.ts의 pauseAfterFileMoves와 src/storage/storage-mutation-coordinator.ts의 pauseAfterAcquire뿐이다. 둘 다 테스트 훅이고, 기본 경로에서는 설정되지 않으면 동작이 같다. finally에서 release 파일을 쓰고 pending fetch를 drain해서, 어설션 실패 후에도 워커/요청이 다음 테스트로 새지 않게 했다. 타임아웃을 늘리거나 스킵하지 않은 점이 좋다. 로컬 검증은 service/init-eof/storage-mutation-race 154 pass와 tsc clean을 본문에 적었다. 다만 네 개 중 일부는 POSIX에서 완전 재현이 안 되고, 최종 확인은 다시 돌린 Windows dispatch에 달려 있다.

라인 수준

tests/service.test.ts launchd shim - #!/bin/sh 실행 가정 제거, Bun 리다이렉트로 교체. Windows false-red의 직접 원인이다.
tests/init-eof.test.ts - 첫 프롬프트 동기화 후 stdin.end. 예산을 기동이 아니라 EOF 처리에 쓴다.
src/storage/cleanup.ts pauseAfterFileMoves - 이동 후 ready, release 대기. link/unlink 중간 상태를 “완료”로 오인하지 않게 한다.
src/storage/storage-mutation-coordinator.ts pauseAfterAcquire - kind별 슬롯 획득 핸드셰이크. 80ms sleep 제거.
tests/storage-mutation-race.test.ts finally release+drain - 실패 시 워커 잔존/요청 누수 방지.

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

• merge 전에 platform-windows shard 2를 한 번 더 workflow_dispatch로 확인할지, 아니면 리눅스 녹색 + 설명만으로 올릴지.
• 테스트 훅이 src/storage/*에 남는 것이 장기적으로 괜찮은지. 지금은 기존 holdAfterFileMovesMs/blockMs와 같은 패턴이라 일관되지만, 훅 표면이 계속 늘면 test-only 모듈로 모을지 결정이 필요하다.
• release.yml에 Windows를 게이트로 넣을 계획은 여전히 없는지(기존 스냅샷 방향: push CI Windows는 dispatch-only).

너의 추천

merge 하라. 가능하면 merge 직전/직후 platform-windows shard 2 dispatch로 네 실패가 사라졌는지 한 번 확인하고, 결과를 이 PR이나 release-readiness 기록에 한 줄 남겨라. 제품 코드 회귀 걱정은 테스트 훅 기본 off라 낮다.

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