-
Notifications
You must be signed in to change notification settings - Fork 91
fix(ci): avoid vitest thread RPC timeouts on Windows #1225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughUpdate Windows CI and local Vitest configuration to use fork-based worker pools with Windows-specific min/max fork settings and coverage reporting; adjust GitHub Actions env vars accordingly and change a test's token-store path used in unit tests. Changes
Sequence Diagram(s)(Skipped — changes are configuration and small test-path update without multi-component sequential flow requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-13T16:04:07.079ZApplied to files:
⏰ Context from checks skipped due to timeout of 270000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting 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 |
LLxprt PR Review – PR #1225Issue AlignmentEvidence: The PR directly addresses issue #1224 (Windows vitest RPC timeouts). Changes include:
Conclusion: Fully aligned with issue requirements. Side Effects
Code Quality
Tests and Coverage
VerdictReady The PR is ready for merge. The changes are targeted, well-structured, and directly resolve the Windows RPC timeout issue. Minor note: the token-store.spec.ts change may warrant a quick sanity check to ensure the implementation supports the explicit path being passed. |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-24.x-ubuntu-latest' artifact from the main CI run. |
|
All checks are green now. Summary of changes:
Local validation: format/lint/typecheck/build passed. Full test suite failed only on existing prompt-loader watchFiles flake (same failure as before). Please verify if you want to keep the unhandled-error ignore on Windows, or if you'd prefer a custom reporter to only ignore this specific timeout. |
TLDR
Switch core vitest runs to the forks pool on Windows and align CI to limit forks instead of threads so Windows runs stay parallel without worker_threads RPC stalls.
Dive Deeper
Windows failures are triggered by onTaskUpdate RPC timeouts in the worker_threads pool (default 60s). Core tests now explicitly use forks on win32 to avoid those stalls, and CI sets VITEST_MAX_FORKS/VITEST_MIN_FORKS to keep concurrency bounded.
Reviewer Test Plan
Testing Matrix
Linked issues / bugs
Closes #1224