test(windows): verify sibling recycle without POSIX cleanup assumption

[lidge-jun]

Summary

The connected-sibling recycle test already observed a replacement with the expected port, sibling marker, and healthy PID on Windows. Its cleanup then used process.kill(pid, "SIGTERM") and waited for runtime-port.json to disappear. Windows terminates that process without running the record-removal handler, so the test timed out after the product behavior had passed.

Wait for the replacement process to exit on every platform. Keep the runtime-record cleanup assertion on POSIX, where the signal handler runs. The Windows test still drives the full link-ended supervisor path and asserts the replacement's port, siblingOfPort, and /healthz PID. No product runtime code changes.

Verification

• Failed dev run, windows 8/9 job 108474557729: 61 pass / 1 fail in the six-file batch. The sole failure is the replacement shutdown wait in the test's finally, after the replacement assertions.
• Windows mini scratch checkout, Bun 1.4.0: original focused file 1 pass / 1 fail at replacement shutdown; patched focused file 2 pass / 0 fail. The exact six-file CI batch, including client-link-runtime.test.ts, passed 62 / 0. All remote writes stayed under the temporary ocx-recycle-win-9d scratch directory.
• macOS at this branch head: bun test tests/clients/client-link-runtime.test.ts — 2 pass / 0 fail; bun x tsc --noEmit, bun run structure:check, bun run privacy:scan, and git diff --check origin/dev..HEAD — pass.
• bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts — 27 pass / 0 fail. The full local suite was not run. Manual ci.yml lane=all run 36268938863 was dispatched for exact head 5277122d61ab363c383d8726d69b7a3d4910f804; its result is pending.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed. No user-facing behavior changed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. This diff changes only test cleanup.

[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-09-26T20:17:45.394616Z	5277122	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]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 109304b6-8bbc-431e-a4bf-9cdd5dbfb0bb

📥 Commits

Reviewing files that changed from the base of the PR and between e772bdb and 5277122.

📒 Files selected for processing (1)

• tests/clients/client-link-runtime.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

---

📝 Walkthrough

Walkthrough

The sibling-recycle test now waits for the replacement process to exit before checking runtime cleanup. On non-Windows platforms, it also waits for runtime-port.json removal. On Windows, it skips that check.

Changes

Runtime cleanup test

Layer / File(s)	Summary
Platform-specific cleanup assertions tests/clients/client-link-runtime.test.ts	The test waits for the replacement process to exit before checking cleanup. It waits for runtime-port.json removal on non-Windows platforms and skips that check on Windows.

Priority: ⬇️ Low

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

Change: Other

Merge Risk: ⚪ Minimal · up to 52771

This test-only change accounts for Windows cleanup behavior and leaves no material issue that needs resolution before merging.

Architecture Summary

Architecture risk: 🔵 Low · up to 52771

The change affects 1 system.

Changed systems: tests

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — tests (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in tests/clients/client-link-runtime.test.ts: The test now confirms the replacement process has exited before checking cleanup. It waits for runtime-port.json removal only on non-Windows platforms; the previous unconditional shutdown check is removed.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies a Windows-focused test change and accurately describes the removal of the POSIX cleanup assumption from the sibling-recycle test.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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]

리뷰 · 우선순위 36 / 80

이 PR은 프로그램 본체를 고치지 않습니다. Windows에서 깨진 테스트의 뒷정리만 바꿉니다.

테스트가 확인하는 일은 이미 통과하고 있었습니다. 연결이 끝나면 새 프로세스가 같은 포트로 뜨고, 형제 표시가 붙고, /healthz가 그 프로세스 번호를 돌려줍니다. 그 확인이 끝난 뒤 finally에서 그 프로세스를 SIGTERM으로 끄고, runtime-port.json이 사라질 때까지 기다렸습니다.

Windows에서는 이 끄기가 파일을 지우는 코드를 실행하지 않습니다. 프로세스만 바로 없어집니다. 그래서 제품 동작은 맞았는데, 파일 삭제를 기다리는 부분이 시간을 넘겼습니다. 실패한 곳은 dev 실행 36267382255의 windows 8/9입니다.

이제는 모든 OS에서 프로세스가 사라졌는지만 먼저 기다립니다. 살아 있는지는 죽이지 않는 확인(kill에 신호 0)으로 봅니다. 맥과 리눅스에서는 그 다음에도 기록 파일이 지워지는지 확인합니다. Windows에서는 그 확인을 뺍니다. 파일은 테스트용 임시 폴더를 지울 때 같이 사라집니다. 기준 브랜치는 dev입니다.

tests/clients/client-link-runtime.test.ts:135 - process.kill(pid, 0)이 성공하면 결과는 항상 참입니다. ? null : true의 거짓 쪽은 실행되지 않습니다. 프로세스가 없으면 예외가 나고, 그때 대기가 끝납니다. 이 테스트가 띄운 프로세스라 그렇게 봐도 됩니다. 권한 오류가 나도 "이미 끝남"으로 처리합니다.

메인테이너의 판단이 필요한 지점
Windows 테스트가 runtime-port.json 삭제를 안 봐도 되는지입니다. 이번 실패는 교체가 성공한 뒤의 테스트 정리입니다. Windows의 SIGTERM은 정리 함수를 타지 않고, 강제 종료가 기록 파일을 남기는 것은 src/cli/status-probes.ts의 기존 설명과 같습니다. 맥과 리눅스 확인은 그대로입니다.

너의 추천
이 수정으로 머지하면 됩니다. 제품을 따로 고칠 필요는 없습니다. 헤드 5277122로 돌아가고 있는 전체 CI(실행 36268938863)에서 Windows 잡이 이 테스트를 통과하는지만 보고 넣으면 됩니다.

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