fix: gate worker bootstrap on explicit flag instead of process.send - #176
Merged
Conversation
The main worker module self-initialized whenever process.send was defined, which is true for any process forked over IPC. A Vitest pool:'forks' test worker that transitively imports the engine therefore ran the bootstrap block and emitted a "ready" message the host IPC layer can't handle, breaking the test run. Pass an explicit --sidequest-worker argv flag when the engine forks the worker and gate the bootstrap on it, so the module stays inert when merely imported. Closes #175
The setup action ran `yarn set version berry`, which downloads the latest Yarn (4.17.0) and ignores the `packageManager: [email protected]` pin. The newer Yarn migrates the lockfile metadata from version 9 to 10, so `yarn install --frozen-lockfile` fails with YN0028 (lockfile would be modified). Corepack is already enabled and provisions the pinned 4.14.1 from package.json, so the explicit version step is redundant and harmful. Drop it to keep installs deterministic.
GiovaniGuizzo
approved these changes
Jun 18, 2026
Contributor
|
🎉 This PR is included in version 1.15.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Closes #175.
Importing Sidequest (even without calling
start()) inside a Vitest test running withpool: 'forks'makes Vitest throw.The main worker module (
packages/engine/src/workers/main.ts) ran its bootstrap block as a top-level import side effect, gated on!!process.send. That heuristic is meant to answer "am I the worker the engine forked?", butprocess.sendis defined in any process forked over IPC, including a Vitestpool: 'forks'test worker. So when such a worker transitively imports the engine, the bootstrap ran and:process.on("message")listener,process.on("disconnect")handler that callsprocess.exit(),process.send("ready"), which the host IPC layer can't handle.Fix
Pass an explicit
--sidequest-workerargv flag when the engine forks the worker, and gate the bootstrap onprocess.argv.includes(WORKER_PROCESS_FLAG)instead of!!process.send. The whole block (all three side effects) now only runs when the engine actually forks the worker.argvis used rather than an env var on purpose: piscina worker threads inheritprocess.env, so an env var would leak into the job execution environment; the fork'sargvdoes not.The
ready→starthandshake is unchanged, so the normal start path is unaffected.Tests
main.jsand asserts: no"ready"without the flag (reproduces the issue),"ready"with the flag.