🐛 Contain the browser bundler's process tree within its build scope

scripts/build-web-client.ts

@@ -62,12 +62,13 @@
 
 import { ensure, main, scoped, until } from "effection";
 import type { Operation } from "effection";
-import { exec } from "@effectionx/process";
 import { ensureDir, readTextFile, rm, writeTextFile } from "@effectionx/fs";
 import { encodeBase64 } from "@std/encoding/base64";
-import { resolve } from "node:path";
+import { tmpdir } from "node:os";
+import { join, resolve } from "node:path";
 import { pathToFileURL } from "node:url";
 
+import { containedRun } from "./lib/contained-run.ts";
 import { assertSideEffectFree, sideEffectFreeManifests } from "./lib/side-effect-free.ts";
 import { byteLength, generatedModule } from "./lib/web-client-module.ts";
 
@@ -113,7 +114,12 @@ const FONT_FACES: FontFace[] = [
 export const SIDE_EFFECT_FREE_MANIFESTS: URL[] = sideEffectFreeManifests(repoRoot);
 
 function* run(command: string, args: string[]): Operation<void> {
-  yield* exec(command, { arguments: args, cwd: new URL(repoRoot).pathname }).expect();
+  const exit = yield* containedRun(command, args, { cwd: new URL(repoRoot).pathname });
+  if (exit.code !== 0) {
+    throw new Error(
+      `${command} ${args.join(" ")} exited with ${exit.code ?? exit.signal}\n${exit.stderr}`,
+    );
+  }
 }
 
 /**
@@ -122,11 +128,20 @@ function* run(command: string, args: string[]): Operation<void> {
  * the battery's own per-command deadline settles a wedged command once and
  * reports it, so a worsening upstream defect stays visible instead of being
  * absorbed by an attempt count.
+ *
+ * The output path is chosen without touching the filesystem, and its removal
+ * is registered before the bundler starts. An interrupted build therefore has
+ * nothing on disk to leak before the bundler exists, and once it does, the
+ * removal runs strictly after `run`'s teardown has terminated and joined the
+ * bundler's process tree — nothing survives that could recreate the file.
+ * Creating the file up front instead would open a window where an abandoned
+ * in-flight creation lands after teardown, with no removal registered to
+ * cover it.
  */
 function* bundleClient(scratch?: string): Operation<string> {
   return yield* scoped(function* () {
-    const output = yield* until(Deno.makeTempFile({ dir: scratch, suffix: ".js" }));
-    yield* ensure(() => rm(output));
+    const output = join(scratch ?? tmpdir(), `${crypto.randomUUID()}.js`);
+    yield* ensure(() => rm(output, { force: true }));
     yield* run(Deno.execPath(), [
       "bundle",
       "--platform=browser",

scripts/lib/contained-run.ts

@@ -0,0 +1,82 @@
+/**
+ * Run a command so its process tree cannot outlive the calling scope.
+ *
+ * `exec` from @effectionx/process suspends between creating the child process
+ * and registering the teardown that terminates it, so a halt arriving in that
+ * window leaves the process running with nothing owning it. Here the
+ * terminate-and-join teardown is registered before the process exists and the
+ * process is created in the same synchronous continuation, so a halt lands
+ * either before there is anything to clean up or after the cleanup is armed —
+ * never between.
+ *
+ * The child is detached into its own process group and teardown signals the
+ * whole group, then joins the child's `close` event, which settles only when
+ * every holder of the child's piped stderr has exited — grandchildren
+ * included. Cleanup registered before this call therefore runs strictly after
+ * the tree is gone.
+ */
+
+import { spawn } from "node:child_process";
+import { Buffer } from "node:buffer";
+import process from "node:process";
+import { ensure, withResolvers } from "effection";
+import type { Operation } from "effection";
+
+export interface ContainedExit {
+  code: number | null;
+  signal: string | null;
+  stderr: string;
+}
+
+interface Exit {
+  code: number | null;
+  signal: string | null;
+}
+
+export function* containedRun(
+  command: string,
+  args: string[],
+  options: { cwd: string },
+): Operation<ContainedExit> {
+  const closed = withResolvers<Exit>();
+  let pid: number | undefined;
+  let exited = false;
+  let failed: Error | undefined;
+  yield* ensure(function* () {
+    if (pid === undefined) {
+      return;
+    }
+    if (!exited) {
+      try {
+        process.kill(-pid, "SIGTERM");
+      } catch {
+        // the group ended between the exit observation and the signal
+      }
+    }
+    yield* closed.operation;
+  });
+  const child = spawn(command, args, {
+    cwd: options.cwd,
+    detached: true,
+    stdio: ["ignore", "ignore", "pipe"],
+  });
+  const stderr: Uint8Array[] = [];
+  if (child.stderr) {
+    child.stderr.on("data", (chunk: Uint8Array) => stderr.push(chunk));
+  }
+  child.once("error", (error: Error) => {
+    exited = true;
+    failed = error;
+    closed.resolve({ code: null, signal: null });
+  });
+  child.once("close", (code: number | null, signal: string | null) => {
+    exited = true;
+    closed.resolve({ code, signal });
+  });
+  pid = child.pid;
+  const exit = yield* closed.operation;
+  if (failed) {
+    throw failed;
+  }
+  return { ...exit, stderr: Buffer.concat(stderr).toString("utf8") };
+}

scripts/runtime-test-exclusions.ts

@@ -45,7 +45,13 @@ const DENO_ONLY_TOOLING: RuntimeExclusion[] = [
   {
     path: "scripts/tests/build-web-client.test.ts",
     reason:
-      "subject is scripts/build-web-client.ts, which runs `deno bundle` and calls Deno.execPath()/makeTempFile — Deno-only",
+      "subject is scripts/build-web-client.ts, which runs `deno bundle` and calls Deno.execPath() — Deno-only",
+    issue: DERIVED_SCOPE,
+  },
+  {
+    path: "scripts/tests/contained-run.test.ts",
+    reason:
+      "subject is scripts/lib/contained-run.ts, the process containment behind the Deno-only browser bundle build; the fixtures drive real process trees under Deno.execPath()",
     issue: DERIVED_SCOPE,
   },
   {

scripts/tests/contained-run.test.ts

@@ -0,0 +1,94 @@
+import { describe, it } from "@executablemd/test-support/bdd";
+import { expect } from "@executablemd/test-support/expect";
+import { ensure, scoped, spawn } from "effection";
+import type { Operation } from "effection";
+import { rm } from "@effectionx/fs";
+import { when } from "@effectionx/converge";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+import { containedRun } from "../lib/contained-run.ts";
+
+const CWD = new URL("../../", import.meta.url).pathname;
+
+/** A directory of the calling operation's own, gone when that operation shuts down. */
+function* scratchDirectory(prefix: string): Operation<string> {
+  // @effectionx/fs has no mkdtemp.
+  const base = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
+  yield* ensure(() => rm(base, { recursive: true, force: true }));
+  return base;
+}
+
+describe("contained-run", () => {
+  it("returns the exit status and captured stderr of a failed command", function* () {
+    const exit = yield* containedRun(
+      Deno.execPath(),
+      ["eval", "console.error('boom'); Deno.exit(3);"],
+      { cwd: CWD },
+    );
+
+    expect(exit.code).toBe(3);
+    expect(exit.stderr).toContain("boom");
+  });
+
+  it("completes a successful command", function* () {
+    const exit = yield* containedRun(Deno.execPath(), ["eval", "Deno.exit(0);"], { cwd: CWD });
+
+    expect(exit).toEqual({ code: 0, signal: null, stderr: "" });
+  });
+
+  /**
+   * The property `bundleClient` stands on: cleanup registered before the
+   * command starts must observe the command's whole process tree already
+   * gone — a grandchild included, because the bundler's own grandchild is
+   * the esbuild service that writes the output file.
+   *
+   * The grandchild inherits the child's piped stderr and would run forever,
+   * so the scope releasing at all proves the terminate reached the whole
+   * group, and the marker it writes while shutting down proves the join
+   * finished before the earlier-registered cleanup ran.
+   */
+  it("terminates and joins the whole tree before earlier-registered cleanup runs", function* () {
+    const base = yield* scratchDirectory("contained-run-");
+    const ready = path.join(base, "ready");
+    const terminated = path.join(base, "terminated");
+    const grandchild = `
+      Deno.writeTextFileSync(${JSON.stringify(ready)}, "ready");
+      Deno.addSignalListener("SIGTERM", () => {
+        Deno.writeTextFileSync(${JSON.stringify(terminated)}, "terminated");
+        Deno.exit(0);
+      });
+      setInterval(() => {}, 1000);
+    `;
+    const child = `
+      const grandchild = new Deno.Command(Deno.execPath(), {
+        args: ["eval", ${JSON.stringify(grandchild)}],
+        stdin: "null",
+        stdout: "null",
+        stderr: "inherit",
+      }).spawn();
+      await grandchild.status;
+    `;
+    const observed: string[] = [];
+
+    yield* scoped(function* () {
+      yield* ensure(() => {
+        observed.push(fs.existsSync(terminated) ? "tree gone" : "tree still alive");
+      });
+      yield* spawn(function* () {
+        yield* containedRun(Deno.execPath(), ["eval", child], { cwd: CWD });
+      });
+      yield* when(
+        function* () {
+          if (!fs.existsSync(ready)) {
+            throw new Error("the fixture tree has not started");
+          }
+        },
+        { timeout: 30_000 },
+      );
+    });
+
+    expect(observed).toEqual(["tree gone"]);
+  });
+});