fix(storage): await Worker exit before isolate reclaim on Windows - #813
Conversation
Bun 1.3.14 can panic in the isolated-worker path on Windows; keep isolation on Linux/macOS until upstream is fixed.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe CI workflow now uses separate test commands for Windows and non-Windows runners. Windows runs ChangesCI test execution
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 89: Update both CI test steps in the workflow to use the configured Bun
test root: replace the explicit `tests` path with `bun test` and apply the
corresponding `bun test --isolate` command to the isolated step, preserving the
existing step structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9a33246f-ead4-4b7c-9b5a-67e496d575b5
📒 Files selected for processing (1)
.github/workflows/ci.yml
[shipping-github] Verdict: approve-commentPR: #813 — fix(storage): await Worker exit before isolate reclaim on Windows UsefulnessReal Windows CI failure: Bun Bugs / correctness
Security
Spec / standards
Reviews
Base / CI
Gatenone (draft cleared; mergeable CLEAN) Bottom lineApprove for merge when ready: isolate stays, worker teardown is deterministic, tip CI regressions pulled in by sync are patched, and required CI is green on tip. |
Keep bun test --isolate on every OS. Join storage Worker threads (and server shutdown) before realm teardown, split Worker-spawning policy API cases into one-test-per-file suites, and shard CI so Bun's residual join race cannot turn Windows into a roulette wheel.
Wibias
changed the title
Also drop the startServer stop-wrapper that hung aborted pool-retry auth tests while awaiting storage Worker drain on every server.stop.
Bun 1.3.14 ran the full suite for both --shard=1/2 and --shard=2/2, so Windows CI executed the isolate tests twice and then failed GUI tests. Keep a single bun test --isolate tests step; Worker teardown remains.
Abort/timeout paths were releasing the shared CODEX_HOME slot (or skipping the Windows post-close settle) before the thread finished, which could overlap concurrent mutators and recreate the isolate reclaim panic on shutdown.
Eight rapid spawn/reset cycles on Linux CI segfaulted Bun 1.3.14 after a balanced worker count; keep the heavy loop on win32 where the isolate panic was observed.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13bbe72df6
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Drain no longer skips restore when policy join fails, and spawnGate callbacks waiting outside liveWorkers cannot create a Worker after shutdown/reset.
Tip startServer was aborting server-auth tests when icacls failed on CI temp volumes; keep chmod and retry EBUSY cleanup of the shared harness dir.
An unref'd setTimeout never woke Bun's pending reader.read on Windows, so the silent time-bound inspection drain hung CI until the 20m job cancel.
[shipping-github] Merge readyPR: #813 — fix(storage): await Worker exit before isolate reclaim on Windows Reviews
Tip freshness
Checks
Residual
Ready to merge. |
|
Merging this. Why it helps: Windows Ship it. |
1 participant
Summary
bun test --isolateon every OS (do not drop isolation on Windows).closelistener at spawn, spawn gate, Windows post-close join, async reset/abort).bun test --shardwith--isolateon Bun 1.3.14 — both shards ran the full suite and doubled Windows CI load until GUI tests failed.Test plan
bun test --isolateacross storage suitesbun test --isolate tests(no Windows non-isolate workaround)