fix: gate worker bootstrap on explicit flag instead of process.send (#176)

* fix: gate worker bootstrap on explicit flag instead of process.send

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

* ci: pin Yarn via corepack instead of floating to latest

The setup action ran `yarn set version berry`, which downloads the latest Yarn
(4.17.0) and ignores the `packageManager: yarn@4.14.1` 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.

Author: merencia

.github/actions/setup/action.yml

@@ -30,10 +30,6 @@ runs:
       shell: bash
       run: corepack enable
 
-    - name: Install Yarn Berry
-      shell: bash
-      run: yarn set version berry
-
     - name: Install Dependencies
       if: steps.cache.outputs.cache-hit != 'true'
       shell: bash

packages/engine/src/constants.ts

@@ -2,3 +2,10 @@ import path from "path";
 
 export const DEFAULT_WORKER_PATH = path.resolve(import.meta.dirname, "workers", "main.js");
 export const DEFAULT_RUNNER_PATH = path.resolve(import.meta.dirname, "shared-runner", "runner.js");
+
+/**
+ * argv flag the engine passes when it forks the main worker. The worker module gates its
+ * bootstrap on this flag instead of `!!process.send`, so it stays inert when merely imported
+ * inside another forked process (e.g. a Vitest `pool: 'forks'` test worker).
+ */
+export const WORKER_PROCESS_FLAG = "--sidequest-worker";

packages/engine/src/engine.ts

@@ -5,7 +5,7 @@ import { existsSync } from "fs";
 import { cpus } from "os";
 import { fileURLToPath } from "url";
 import { inspect } from "util";
-import { DEFAULT_WORKER_PATH } from "./constants";
+import { DEFAULT_WORKER_PATH, WORKER_PROCESS_FLAG } from "./constants";
 import { Dependency, dependencyRegistry } from "./dependency-registry";
 import { JOB_BUILDER_FALLBACK } from "./job/constants";
 import { ScheduledJobRegistry } from "./job/cron-registry";
@@ -264,7 +264,7 @@ export class Engine {
       if (!this.mainWorker) {
         const runWorker = () => {
           logger("Engine").debug("Starting main worker...");
-          this.mainWorker = fork(DEFAULT_WORKER_PATH);
+          this.mainWorker = fork(DEFAULT_WORKER_PATH, [WORKER_PROCESS_FLAG]);
           logger("Engine").debug(`Worker PID: ${this.mainWorker.pid}`);
           this.mainWorker.on("message", (msg) => {
             if (msg === "ready") {

packages/engine/src/workers/main.ts

@@ -1,6 +1,7 @@
 import { Backend } from "@sidequest/backend";
 import { logger } from "@sidequest/core";
 import cron from "node-cron";
+import { WORKER_PROCESS_FLAG } from "../constants";
 import { Engine, EngineConfig, NonNullableEngineConfig } from "../engine";
 import { Dispatcher } from "../execution/dispatcher";
 import { ExecutorManager } from "../execution/executor-manager";
@@ -142,7 +143,11 @@ export class MainWorker {
   }
 }
 
-const isChildProcess = !!process.send;
+// Gate the bootstrap on the explicit flag the engine passes when forking, not on `!!process.send`.
+// Any process forked over IPC (including a Vitest `pool: 'forks'` test worker that transitively
+// imports this module) has `process.send`, so the old heuristic would self-initialize there and
+// emit a "ready" message the host IPC layer can't handle. See issue #175.
+const isChildProcess = process.argv.includes(WORKER_PROCESS_FLAG);
 
 if (isChildProcess) {
   const worker = new MainWorker();

tests/integration/worker-bootstrap.integration.test.mjs

@@ -0,0 +1,54 @@
+import { fork } from "node:child_process";
+import { dirname, resolve } from "node:path";
+import { fileURLToPath } from "node:url";
+import { afterEach, describe, expect, it } from "vitest";
+
+const here = dirname(fileURLToPath(import.meta.url));
+const WORKER_PATH = resolve(here, "../../packages/engine/dist/workers/main.js");
+const WORKER_FLAG = "--sidequest-worker";
+
+/**
+ * Forks the built worker module with the given argv and resolves with the first IPC message it
+ * emits, or `null` if none arrives within the timeout. The worker is always killed afterwards.
+ */
+function forkWorkerAndCaptureFirstMessage(argv, timeoutMs = 2000) {
+  return new Promise((resolvePromise, rejectPromise) => {
+    const child = fork(WORKER_PATH, argv, { stdio: "ignore" });
+    const timer = setTimeout(() => {
+      child.kill();
+      resolvePromise(null);
+    }, timeoutMs);
+
+    child.once("message", (msg) => {
+      clearTimeout(timer);
+      child.kill();
+      resolvePromise(msg);
+    });
+    child.once("error", (err) => {
+      clearTimeout(timer);
+      child.kill();
+      rejectPromise(err);
+    });
+  });
+}
+
+describe("worker bootstrap gating (issue #175)", () => {
+  let lastChild;
+
+  afterEach(() => {
+    lastChild?.kill();
+    lastChild = undefined;
+  });
+
+  it("does NOT emit a 'ready' message when forked without the sentinel flag", async () => {
+    // Reproduces a Vitest `pool: 'forks'` worker that transitively imports the engine: the process
+    // has an IPC channel (process.send) but is not a Sidequest worker, so the module must stay inert.
+    const msg = await forkWorkerAndCaptureFirstMessage([]);
+    expect(msg).toBeNull();
+  });
+
+  it("emits 'ready' when forked with the sentinel flag", async () => {
+    const msg = await forkWorkerAndCaptureFirstMessage([WORKER_FLAG]);
+    expect(msg).toBe("ready");
+  });
+});