fix(vscode): retire in-flight worker spawns and runtime verdicts on dispose

A settings-triggered restart can dispose an RstestApi while an await is
still pending inside it; three sites could then act for a master that no
longer exists:

- createChildProcess spawned its worker after the Node preflight settled,
  producing the one process dispose() cannot kill (it was never added to
  childProcesses), running on the very resolution the restart exists to
  replace.
- The configured-node verdict and the preflight failure both latched
  their version-mismatch into what is by then the replacement
  registration's status, with nothing left to clear it. Both now report
  through one disposed-aware helper.
- resolveRstestPath re-latched a core version mismatch after the
  project's status was forgotten — sticky for a root that never comes
  back.

A disposed master also fast-fails createChildProcess at entry, sparing
it the package resolution and probe costs of a doomed spawn.

Author: fi3ework

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

@@ -41,6 +41,8 @@ export const runningWorkers = new Set<BirpcReturn<Worker, TestRunReporter>>();
  * The host-level inputs to the worker-node preflight. `notify` must not close
  * over `this`: the resolution is memoized for the extension host's lifetime, so
  * a callback capturing a `Project` would pin it and its whole test tree.
+ * (Settle-bounded reactions, like the configured-node verdict's `.then`, may
+ * capture `this` — the rule is about host-lifetime retention.)
  * `cwd` is the caller's standpoint for the shell probe — see
  * `probeShellNodePath`.
  */
@@ -109,6 +111,10 @@ export class RstestApi {
   // Processes killed on purpose outside the `$close` → `off` path (dispose,
   // failed debugger attach). Their `exit` events are not crashes.
   private readonly expectedExits = new WeakSet<ChildProcess>();
+  // Flipped by `dispose()` and checked wherever an await can outlive a
+  // restart — see `reportNodeRuntimeIssue` and the spawn abort in
+  // `createChildProcess`.
+  private disposed = false;
 
   constructor(
     private workspace: vscode.WorkspaceFolder,
@@ -191,6 +197,20 @@ export class RstestApi {
    * `vscode.env.shell` is read here so `nodeResolution.ts` needs no VS Code
    * import.
    */
+  /**
+   * A Node-runtime verdict, latched under the host key (see
+   * `NODE_RUNTIME_STATUS_SOURCE`) — dropped when this API was disposed while
+   * the verdict was in flight: the memo is reset and `status` rebound by
+   * then, so the report would land in the *replacement* registration with
+   * nothing left to clear it.
+   */
+  private reportNodeRuntimeIssue(message: string): void {
+    if (this.disposed) {
+      return;
+    }
+    status.versionMismatch(message, NODE_RUNTIME_STATUS_SOURCE);
+  }
+
   private async resolveWorkerNodeCommand(): Promise<{
     nodeExecutable: string;
     nodeExecArgs: string[];
@@ -204,7 +224,7 @@ export class RstestApi {
       // the preflight failure below; re-reporting on a later spawn is a no-op.
       void configuredNodeBelowFloor(nodeExecutable).then((message) => {
         if (message) {
-          status.versionMismatch(message, NODE_RUNTIME_STATUS_SOURCE);
+          this.reportNodeRuntimeIssue(message);
         }
       });
       return { nodeExecutable, nodeExecArgs };
@@ -218,9 +238,7 @@ export class RstestApi {
       if (error instanceof NodePreflightError) {
         // The status-aggregation adaptation: no usable runtime anywhere is the
         // same "fix your toolchain" state as an unsupported package version.
-        // Latched under the host key, not `this.statusSource` — see
-        // `NODE_RUNTIME_STATUS_SOURCE`.
-        status.versionMismatch(error.message, NODE_RUNTIME_STATUS_SOURCE);
+        this.reportNodeRuntimeIssue(error.message);
       }
       throw error;
     }
@@ -341,19 +359,24 @@ export class RstestApi {
       // The status-aggregation adaptation: the one-shot `showWarningMessage`
       // becomes the shared `version mismatch` status bar state with actual vs
       // required versions. The floor is the same `>= 0.6.0`.
-      if (
-        !reportVersionCheck(
-          status,
-          '@rstest/core',
-          coreVersion,
-          this.statusSource,
-        )
-      ) {
-        logger.error(
-          `Unsupported @rstest/core version ${coreVersion ?? 'unknown'} resolved from ${this.cwd}`,
-        );
-      } else {
-        status.versionOk(this.statusSource);
+      // Skipped after dispose: the project's status was already forgotten,
+      // and a mismatch re-latched now — for a root that may never come back —
+      // would have nothing left to clear it.
+      if (!this.disposed) {
+        if (
+          !reportVersionCheck(
+            status,
+            '@rstest/core',
+            coreVersion,
+            this.statusSource,
+          )
+        ) {
+          logger.error(
+            `Unsupported @rstest/core version ${coreVersion ?? 'unknown'} resolved from ${this.cwd}`,
+          );
+        } else {
+          status.versionOk(this.statusSource);
+        }
       }
 
       return nodeExport;
@@ -574,6 +597,12 @@ export class RstestApi {
     startDebugging?: boolean,
     testRun?: vscode.TestRun,
   ) {
+    // Cheap fast-fail; the load-bearing check is the one after the Node
+    // preflight below, which covers a dispose landing mid-await. This one
+    // just spares a retired master the package resolution and probe costs.
+    if (this.disposed) {
+      throw new Error('worker spawn aborted: this master is disposed');
+    }
     const rstestPath = this.resolveRstestPath();
     if (!rstestPath) {
       throw new Error('Failed to resolve rstest path');
@@ -601,6 +630,14 @@ export class RstestApi {
     const workerPath = path.resolve(__dirname, 'worker.js');
     const { nodeExecutable, nodeExecArgs } =
       await this.resolveWorkerNodeCommand();
+    // A restart may have retired this API while the preflight above was
+    // pending. Spawning now would produce the one worker `dispose()` cannot
+    // kill, running on the very resolution the restart exists to replace.
+    if (this.disposed) {
+      throw new Error(
+        'worker spawn aborted: this master was disposed while its Node runtime was being resolved',
+      );
+    }
     const nodeEnv = getConfigValue('nodeEnv', this.workspace);
     const debugNodeEnv = startDebugging
       ? getConfigValue('debugNodeEnv', this.workspace)
@@ -745,6 +782,7 @@ export class RstestApi {
   }
 
   public dispose() {
+    this.disposed = true;
     for (const child of this.childProcesses) {
       this.expectedExits.add(child);
       child.kill();

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

@@ -115,13 +115,22 @@ rs.mock('vscode', () => {
 // the workspace `node_modules` and `@rstest/core` is genuinely missing.
 const noCoreDir = os.tmpdir();
 
+// 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(() => {
+  for (const key of Object.keys(settings)) delete settings[key];
+});
+
 const createApi = (cwd = noCoreDir) => {
   const workspace = { uri: { fsPath: cwd } };
+  // `sourceUri` backs the per-project status latch key, which the version
+  // check on the spawn path reads before anything can fail.
+  const project = { sourceUri: { toString: () => `test://${cwd}` } };
   return new RstestApi(
     workspace as any,
     cwd,
     `${cwd}/rstest.config.ts`,
-    {} as any,
+    project as any,
   );
 };
 
@@ -267,7 +276,6 @@ describe('RstestApi with a configured nodeExecutable', () => {
   afterEach(() => {
     status.unbind();
     resetWorkerNodeCaches();
-    delete settings.nodeExecutable;
   });
 
   // The verdict is reported off the spawn path, so a spawn resolves before the
@@ -293,4 +301,28 @@ describe('RstestApi with a configured nodeExecutable', () => {
     expect(nodeExecutable).toBe(configuredNode);
     expect(mismatches).toHaveLength(1);
   });
+
+  // The two mid-flight races a settings-triggered restart makes reachable:
+  // the verdict and the spawn each cross an await that can outlive `dispose()`.
+  it('should drop a verdict that settles after dispose', async () => {
+    await seedProbe({ kind: 'ok', version: '20.19.4' });
+    const api = createApi();
+    const pending = resolveWorkerNodeCommand(api);
+    api.dispose();
+    await pending;
+    await settleVerdict();
+    expect(mismatches).toEqual([]);
+  });
+
+  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.
+    await seedProbe({ kind: 'ok', version: '24.0.0' });
+    const api = createApi(path.resolve(__dirname, '../../..'));
+    const spawning = api.createChildProcess();
+    api.dispose();
+    await expect(spawning).rejects.toThrow('disposed');
+  });
 });