fix(vscode): keep worker teardown races out of notifications

Two intermittent error notifications traced to the rstest worker's
child-process 'error' handler, which treated every 'error' event as a
spawn failure the user must fix:

- "write EPIPE": a birpc message racing the worker's death. send() was
  called without a callback, so a lost race became an 'error' event and
  a notification. Pass a callback and log instead — the 'exit' handler
  already owns reporting an exit nobody asked for.
- "spawn node ENOENT": Node blames the executable when it is the spawn
  cwd that is gone (a project directory deleted under a live master by a
  branch switch or a build wiping fixtures). That is a stale-project
  state detection will reconcile, not a broken runtime: log the real
  cause, skip the notification and the crashed status.

A genuine spawn failure with the cwd intact — the wrong-nodeExecutable
case the notification exists for — keeps notifying. Post-spawn 'error'
events are absorbed, matching LanguageServerProcessOwner's shape in the
lint/fmt stacks.

Author: fi3ework

packages/vscode/src/stacks/test/master.ts

@@ -1,4 +1,5 @@
 import { type ChildProcess, spawn } from 'node:child_process';
+import { existsSync } from 'node:fs';
 import net from 'node:net';
 import path, { dirname } from 'node:path';
 import { type BirpcReturn, createBirpc } from 'birpc';
@@ -643,6 +644,12 @@ export class RstestApi {
     });
   }
 
+  // One wording for the pre-spawn guard and the 'error'-handler fallback it
+  // leaves for the delete-after-check race.
+  private missingCwdMessage(): string {
+    return `worker spawn skipped: project directory ${this.cwd} no longer exists; the project will be re-detected if it comes back`;
+  }
+
   public async createChildProcess(
     testRunReporter = new TestRunReporter(),
     startDebugging?: boolean,
@@ -689,6 +696,18 @@ export class RstestApi {
         'worker spawn aborted: this master was disposed while its Node runtime was being resolved',
       );
     }
+    // A project directory deleted under a live master (branch switch,
+    // `git clean`, a build wiping fixtures) is a stale-project state, not a
+    // runtime the user must fix: detection watches the config file and will
+    // drop or re-add the project. Refusing here, before the spawn, spares
+    // the caller a worker whose every call rejects with an opaque
+    // "[birpc] rpc is closed" — and spares the user Node's misreading of a
+    // missing cwd as "spawn node ENOENT".
+    if (!existsSync(this.cwd)) {
+      const message = this.missingCwdMessage();
+      logger.warn(message);
+      throw new Error(message);
+    }
     const nodeEnv = getConfigValue('nodeEnv', this.workspace);
     const debugNodeEnv = startDebugging
       ? getConfigValue('debugNodeEnv', this.workspace)
@@ -732,9 +751,15 @@ export class RstestApi {
 
     const worker = createBirpc<Worker, TestRunReporter>(testRunReporter, {
       // Target the local process rather than the shared field, which is
-      // reassigned on every spawn; skip once the IPC channel is gone.
+      // reassigned on every spawn; skip once the IPC channel is gone. The
+      // callback matters: without one, Node surfaces a failed write — a
+      // message losing the race against the worker's death — as a process
+      // 'error' event instead.
       post: (data) => {
-        if (rstestProcess.connected) rstestProcess.send(data);
+        if (rstestProcess.connected)
+          rstestProcess.send(data, (error) => {
+            if (error) logger.debug('IPC send to worker failed', error);
+          });
       },
       on: (fn) => rstestProcess.on('message', fn),
       bind: 'functions',
@@ -754,23 +779,38 @@ export class RstestApi {
       configFilePath: this.configFilePath,
     });
 
+    let spawned = false;
     rstestProcess.on('spawn', () => {
+      spawned = true;
       status.workerSpawned(this.statusSource);
     });
 
     rstestProcess.on('error', (error) => {
-      logger.error('Worker process error', error);
-      // The status-aggregation adaptation: a worker that never came up is the
-      // `crashed` state of the shared status bar. The notification is kept because
-      // a failed spawn is almost always a wrong `nodeExecutable` the user has
-      // to fix, and the status bar alone is easy to miss mid-run.
-      status.crashed(
-        `worker process failed: ${error.message}`,
-        this.statusSource,
-      );
-      vscode.window.showErrorMessage(
-        `Rstest worker process failed: ${error.message}`,
-      );
+      if (spawned) {
+        // Post-spawn errors (a failed kill(), an IPC write losing the race
+        // against the worker's death) are teardown noise with nothing for
+        // the user to fix; the 'exit' handler already reports an exit nobody
+        // asked for. Same shape as `LanguageServerProcessOwner`'s handler.
+        logger.debug('Worker process error after spawn', error);
+      } else if (!existsSync(this.cwd)) {
+        // The cwd was deleted between the pre-spawn guard and the spawn —
+        // Node blames the executable ("spawn node ENOENT") when it is the
+        // cwd that is gone. Same stale-project state, same quiet report.
+        logger.warn(this.missingCwdMessage());
+      } else {
+        logger.error('Worker process error', error);
+        // The status-aggregation adaptation: a worker that never came up is the
+        // `crashed` state of the shared status bar. The notification is kept because
+        // a failed spawn is almost always a wrong `nodeExecutable` the user has
+        // to fix, and the status bar alone is easy to miss mid-run.
+        status.crashed(
+          `worker process failed: ${error.message}`,
+          this.statusSource,
+        );
+        vscode.window.showErrorMessage(
+          `Rstest worker process failed: ${error.message}`,
+        );
+      }
       // Reject any in-flight birpc calls instead of letting them hang; $close
       // runs the `off` handler, which removes the process from the Set.
       if (!worker.$closed) worker.$close();

packages/vscode/tests/stacks/test/master.test.ts

@@ -1,3 +1,4 @@
+import type { ChildProcess } from 'node:child_process';
 import fs from 'node:fs';
 import { createRequire } from 'node:module';
 import os from 'node:os';
@@ -11,7 +12,7 @@ import {
   resetUserNodeCaches,
 } from '../../../src/shared/nodeResolution';
 import { status } from '../../../src/stacks/test/status';
-import type { StatusReporter } from '../../../src/types';
+import type { StackState, StatusReporter } from '../../../src/types';
 import { createStatusRecorder } from './statusRecorder';
 
 // The Rstest runner injects its own `@rstest/core` into every resolution path so
@@ -117,6 +118,18 @@ rs.mock('vscode', () => {
 // the workspace `node_modules` and `@rstest/core` is genuinely missing.
 const noCoreDir = os.tmpdir();
 
+// The opposite fixture: a cwd where `@rstest/core` resolves, for suites whose
+// case under test sits past the resolution step.
+const packageDir = path.resolve(__dirname, '../../..');
+
+// Seeding the memo is how the probe is injected: `resolveWorkerNodeCommand`
+// takes no probe option (it is called from deep inside a spawn path), and the
+// memo is keyed by executable path, so a seeded entry is the answer it gets.
+const seedNodeProbe = (executable: string, probe: NodeProbe) =>
+  configuredNodeBelowFloor(executable, {
+    probe: () => Promise.resolve(probe),
+  });
+
 // Settings are a module-level bag every suite writes into; clearing them per
 // test keeps one suite's configuration from leaking into the next.
 afterEach(() => {
@@ -357,13 +370,7 @@ describe('RstestApi with a configured nodeExecutable', () => {
     versionMismatch: (detail) => mismatches.push(detail),
   };
 
-  // Seeding the memo is how the probe is injected: `resolveWorkerNodeCommand`
-  // takes no probe option (it is called from deep inside a spawn path), and the
-  // memo is keyed by executable path, so a seeded entry is the answer it gets.
-  const seedProbe = (probe: NodeProbe) =>
-    configuredNodeBelowFloor(configuredNode, {
-      probe: () => Promise.resolve(probe),
-    });
+  const seedProbe = (probe: NodeProbe) => seedNodeProbe(configuredNode, probe);
 
   // Reaching the private method keeps these cases on the decision under test
   // instead of spawning a real worker process for each one.
@@ -438,13 +445,89 @@ describe('RstestApi with a configured nodeExecutable', () => {
 
   it('should refuse to spawn a worker after dispose', async () => {
     // The seed keeps the configured executable's probe off the real spawn
-    // path; the package dir (not `process.cwd()`) is a cwd where
-    // `@rstest/core` resolves, so the abort observed is the disposed check
-    // and not an earlier resolution failure.
+    // path, so the abort observed is the disposed check and not an earlier
+    // resolution failure.
     await seedProbe({ kind: 'ok', version: '24.0.0' });
-    const api = createApi(path.resolve(__dirname, '../../..'));
+    const api = createApi(packageDir);
     const spawning = api.createChildProcess();
     api.dispose();
     await expect(spawning).rejects.toThrow('disposed');
   });
 });
+
+// Worker spawn failures: only the wrong-executable case is the user's to fix
+// (and keeps its notification); the rest is absorbed — the rationale lives on
+// the guard and the 'error' handler in `master.ts`.
+describe('RstestApi worker spawn failures', () => {
+  let api: RstestApi | undefined;
+  let reported: StackState[];
+
+  const crashes = () => reported.filter((state) => state.kind === 'crashed');
+
+  const seedConfiguredNode = (executable: string) => {
+    settings['rstack.nodeExecutable'] = executable;
+    return seedNodeProbe(executable, { kind: 'ok', version: '24.0.0' });
+  };
+
+  beforeEach(() => {
+    shownMessages.length = 0;
+    loggedWarnings.length = 0;
+    resetUserNodeCaches();
+    const recorder = createStatusRecorder();
+    reported = recorder.reported;
+    status.bind(recorder.reporter);
+  });
+
+  afterEach(() => {
+    api?.dispose();
+    api = undefined;
+    status.unbind();
+    resetUserNodeCaches();
+  });
+
+  it('should log, not notify, when the spawn cwd no longer exists', async () => {
+    await seedConfiguredNode(process.execPath);
+    const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'rstest-gone-'));
+    api = createApi(cwd, packageDir);
+    fs.rmSync(cwd, { recursive: true, force: true });
+
+    await expect(api.createChildProcess()).rejects.toThrow('no longer exists');
+
+    expect(shownMessages).toEqual([]);
+    expect(crashes()).toEqual([]);
+    expect(loggedWarnings.join('\n')).toContain(cwd);
+  });
+
+  it('should keep notifying when the executable itself fails to spawn', async () => {
+    await seedConfiguredNode(path.join(os.tmpdir(), 'no-such-node-xyz'));
+    api = createApi(packageDir);
+
+    await api.createChildProcess();
+    await expect
+      .poll(() => shownMessages[0] ?? '', { timeout: 5000 })
+      .toContain('Rstest worker process failed');
+
+    expect(crashes()).toHaveLength(1);
+  });
+
+  it('should absorb a post-spawn error instead of notifying', async () => {
+    await seedConfiguredNode(process.execPath);
+    // `--eval` wins over the worker script path, so the child is a plain
+    // long-lived node — the point is the handler, not the worker protocol.
+    settings.nodeExecArgs = ['--eval', 'setInterval(() => {}, 1000)'];
+    api = createApi(packageDir);
+
+    await api.createChildProcess();
+    const child = [...((api as any).childProcesses as Set<ChildProcess>)][0]!;
+    // The handler's spawned latch is set by the master's own 'spawn'
+    // listener, which registered first and therefore runs first.
+    await new Promise<void>((resolve) => {
+      child.once('spawn', () => resolve());
+    });
+
+    child.emit('error', new Error('write EPIPE'));
+
+    expect(shownMessages).toEqual([]);
+    expect(crashes()).toEqual([]);
+  });
+});