feat: support vitest worker process #8
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Node.js v24 to the CI test matrix, updates release workflow permissions and switches to an npm-release workflow, and refactors IPC send logic in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Sender as MessageSender
participant Child as Child/Worker
note over Sender,Child: Route message by runtime context
Caller->>Sender: send(message, child)
alt Cluster fork (child.process?.connected)
Sender->>Child: child.send(message)
note right of Child: Logged as cluster fork (cluster.isWorker)
else Child process (child.connected)
Sender->>Child: child.send(message)
note right of Child: Logged as child process
else Vitest worker (env VITEST & VITEST_WORKER_ID)
Sender-)Child: setImmediate(child.emit('sendmessage-to-self', message))
note right of Child: Vitest self-dispatch, then return
else No channel
Sender-->>Caller: warn("no channel available")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Vitest worker processes by detecting the Vitest environment and using event emission instead of the standard IPC send method. It also updates the CI configuration to include Node.js 24 testing and modernizes the release workflow.
- Add Vitest worker process detection and handling using environment variables
- Update CI to test against Node.js 24 and modernize release workflow
- Restructure child process communication logic for better separation of concerns
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/index.ts | Add Vitest worker process detection and emit-based message handling |
| .github/workflows/release.yml | Update release workflow permissions and use npm-release action |
| .github/workflows/nodejs.yml | Add Node.js 24 to test matrix |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/index.ts
Outdated
| if (process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID) { | ||
| debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit message: %j', | ||
| process.env.VITEST_WORKER_ID, message); | ||
| return setImmediate(child.emit.bind(child, 'message', message)); |
There was a problem hiding this comment.
The setImmediate call doesn't return a meaningful value. Since other branches return the result of child.send!(), this should either return a Promise or consistent value, or the function's return type should be updated to accommodate both synchronous and asynchronous returns.
| uses: node-modules/github-actions/.github/workflows/npm-release.yml@master | ||
| secrets: | ||
| NPM_TOKEN: ${{ secrets.NPM_TOKEN }} | ||
| GIT_TOKEN: ${{ secrets.GIT_TOKEN }} |
There was a problem hiding this comment.
The NPM_TOKEN secret was removed but may still be required by the npm-release workflow. Verify that the new workflow doesn't need NPM authentication or ensure the token is passed through a different mechanism.
| GIT_TOKEN: ${{ secrets.GIT_TOKEN }} | |
| GIT_TOKEN: ${{ secrets.GIT_TOKEN }} | |
| NPM_TOKEN: ${{ secrets.NPM_TOKEN }} |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8 +/- ##
==========================================
- Coverage 97.22% 92.04% -5.18%
==========================================
Files 1 1
Lines 72 88 +16
Branches 16 20 +4
==========================================
+ Hits 70 81 +11
- Misses 2 7 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary of ChangesHello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the message sending utility to properly interact with Vitest worker processes. It introduces a specialized communication method for these workers, ensuring that messages are delivered correctly within a Vitest testing environment, thereby improving compatibility and functionality. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for vitest worker processes by detecting them via environment variables and using emit for messaging. The implementation refactors the process-sending logic, separating cluster.fork() and child_process.fork() cases, which improves code clarity and debuggability. The changes are logical and well-implemented. I've provided one suggestion to further enhance readability by extracting a condition into a named variable.
| if (process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID) { | ||
| debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit message: %j', | ||
| process.env.VITEST_WORKER_ID, message); | ||
| return setImmediate(child.emit.bind(child, 'message', message)); | ||
| } |
There was a problem hiding this comment.
To improve readability, consider extracting the condition for detecting a vitest worker into a named constant. This makes the code's intent more explicit and the if statement cleaner.
| if (process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID) { | |
| debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit message: %j', | |
| process.env.VITEST_WORKER_ID, message); | |
| return setImmediate(child.emit.bind(child, 'message', message)); | |
| } | |
| const isVitestWorker = process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID; | |
| if (isVitestWorker) { | |
| debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit message: %j', | |
| process.env.VITEST_WORKER_ID, message); | |
| return setImmediate(child.emit.bind(child, 'message', message)); | |
| } |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/nodejs.yml(1 hunks).github/workflows/release.yml(1 hunks)src/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Node.js / Test (macos-latest, 18)
- GitHub Check: Node.js / Test (windows-latest, 18.19.0)
- GitHub Check: Node.js / Test (windows-latest, 24)
🔇 Additional comments (1)
.github/workflows/release.yml (1)
17-18: Restore the NPM token secret mapping.Switching to
npm-release.ymlstill requires anNPM_TOKEN; removing it will leave the reusable workflow without credentials and the publish job will fail once it tries to hit npm. Please keep passingNPM_TOKENalongsideGIT_TOKEN.uses: node-modules/github-actions/.github/workflows/npm-release.yml@master secrets: GIT_TOKEN: ${{ secrets.GIT_TOKEN }} + NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
| if (child.connected) { | ||
| if (process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID) { | ||
| debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit sendmessage-to-self: %j', | ||
| process.env.VITEST_WORKER_ID, message); | ||
| return setImmediate(child.emit.bind(child, 'sendmessage-to-self', message)); | ||
| } | ||
| debug('child is childprocess.fork() process, send: %j', message); | ||
| return child.send!(message); |
There was a problem hiding this comment.
Emit the standard message event in the Vitest branch.
All other same-process fallbacks emit 'message' so existing process.on('message') handlers fire. Switching this branch to 'sendmessage-to-self' means no listener will see the payload, so Vitest workers never receive the message—regressing the very scenario we’re trying to support. Keep emitting 'message' (or add matching listeners elsewhere).
- if (process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID) {
- debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit sendmessage-to-self: %j',
- process.env.VITEST_WORKER_ID, message);
- return setImmediate(child.emit.bind(child, 'sendmessage-to-self', message));
+ if (process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID) {
+ debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit message: %j',
+ process.env.VITEST_WORKER_ID, message);
+ return setImmediate(child.emit.bind(child, 'message', message));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (child.connected) { | |
| if (process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID) { | |
| debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit sendmessage-to-self: %j', | |
| process.env.VITEST_WORKER_ID, message); | |
| return setImmediate(child.emit.bind(child, 'sendmessage-to-self', message)); | |
| } | |
| debug('child is childprocess.fork() process, send: %j', message); | |
| return child.send!(message); | |
| if (child.connected) { | |
| if (process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID) { | |
| debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit message: %j', | |
| process.env.VITEST_WORKER_ID, message); | |
| return setImmediate(child.emit.bind(child, 'message', message)); | |
| } | |
| debug('child is childprocess.fork() process, send: %j', message); | |
| return child.send!(message); |
🤖 Prompt for AI Agents
In src/index.ts around lines 63 to 70, the Vitest branch emits a nonstandard
'sendmessage-to-self' event which prevents existing process.on('message')
handlers from receiving the payload; change the emitted event to the standard
'message' so Vitest worker processes get the same same-process fallback as other
branches (i.e., call setImmediate(child.emit.bind(child, 'message', message))
while preserving the debug call).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/index.ts (1)
4-4: Use namespace import for cluster to avoid default-import pitfallsDefault import requires esModuleInterop. Safer to use a namespace import.
-import cluster from 'node:cluster'; +import * as cluster from 'node:cluster';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/index.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Node.js / Test (macos-latest, 18.19.0)
- GitHub Check: Node.js / Test (windows-latest, 18.19.0)
- GitHub Check: Node.js / Test (windows-latest, 16)
- GitHub Check: Node.js / Test (windows-latest, 24)
- GitHub Check: Node.js / Test (windows-latest, 20)
🔇 Additional comments (2)
src/index.ts (2)
58-61: LGTM: correct handling for cluster.fork() workersChecking
child.process?.connectedand delegating tochild.sendis appropriate here.
72-76: Emit the standard 'message' event for VitestUse
'message'to preserve existingprocess.on('message')handlers; the custom'sendmessage-to-self'won’t be observed. This also aligns with other same-process fallbacks.Run this to verify nothing depends on the custom event name:
#!/bin/bash rg -n -C2 "sendmessage-to-self"
| if (child.connected) { | ||
| debug('child.connected: %s, cluster.isWorker: %s, cluster.isPrimary: %s', | ||
| child.connected, cluster.isWorker, cluster.isPrimary); | ||
| if (cluster.isWorker || cluster.isPrimary) { | ||
| debug('child is cluster.fork() process, send: %j', message); | ||
| return child.send!(message); | ||
| } |
There was a problem hiding this comment.
Fix always-true gate that makes the Vitest branch unreachable
cluster.isWorker || cluster.isPrimary is always true, so the early return prevents the Vitest handling below from ever running.
Apply this refactor to check Vitest first and fall back to childprocess send:
- if (child.connected) {
- debug('child.connected: %s, cluster.isWorker: %s, cluster.isPrimary: %s',
- child.connected, cluster.isWorker, cluster.isPrimary);
- if (cluster.isWorker || cluster.isPrimary) {
- debug('child is cluster.fork() process, send: %j', message);
- return child.send!(message);
- }
-
- if (process.env.VITEST === 'true' && process.env.VITEST_WORKER_ID) {
- debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit sendmessage-to-self: %j',
- process.env.VITEST_WORKER_ID, message);
- return setImmediate(child.emit.bind(child, 'sendmessage-to-self', message));
- }
- debug('child is childprocess.fork() process, send: %j', message);
- return child.send!(message);
- }
+ if (child.connected) {
+ const isVitestWorker = process.env.VITEST === 'true' && !!process.env.VITEST_WORKER_ID;
+ if (isVitestWorker) {
+ debug('child is vitest worker process, VITEST_WORKER_ID: %s, emit message: %j',
+ process.env.VITEST_WORKER_ID, message);
+ return setImmediate(child.emit.bind(child, 'message', message));
+ }
+ debug('child is childprocess.fork() process, send: %j', message);
+ return child.send!(message);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (child.connected) { | |
| debug('child.connected: %s, cluster.isWorker: %s, cluster.isPrimary: %s', | |
| child.connected, cluster.isWorker, cluster.isPrimary); | |
| if (cluster.isWorker || cluster.isPrimary) { | |
| debug('child is cluster.fork() process, send: %j', message); | |
| return child.send!(message); | |
| } | |
| if (child.connected) { | |
| const isVitestWorker = process.env.VITEST === 'true' && !!process.env.VITEST_WORKER_ID; | |
| if (isVitestWorker) { | |
| debug( | |
| 'child is vitest worker process, VITEST_WORKER_ID: %s, emit message: %j', | |
| process.env.VITEST_WORKER_ID, | |
| message | |
| ); | |
| return setImmediate(child.emit.bind(child, 'message', message)); | |
| } | |
| debug('child is childprocess.fork() process, send: %j', message); | |
| return child.send!(message); | |
| } |
🤖 Prompt for AI Agents
In src/index.ts around lines 64-70, the current guard uses "cluster.isWorker ||
cluster.isPrimary" which is always true and causes the Vitest-specific branch
below to never execute; change the branching order to detect the Vitest worker
first (e.g., check process.env.VITEST_WORKER_ID or appropriate Vitest indicator)
and handle sending the message to the Vitest parent/process there, and only if
not running under Vitest fall back to treating the child as a cluster.fork()
process and call child.send; remove the always-true early-return so non-cluster
child processes still reach the correct Vitest or child-process send logic and
keep debug logging for both paths.
Summary by CodeRabbit
Bug Fixes
Chores