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

[taras]

Why

The concurrent deno task verify battery intermittently left the browser bundler's hash-named output in a test-owned scratch directory after scope teardown, failing build-web-client … leaves nothing behind when a build fails (#417). The same interruption paths run in production teardown, so any interrupted build:web could leave the same litter in the system temp directory.

Closes #417.

What changes

Before:

An interrupted bundleClient had two cancellation windows, each only wide enough to hit under heavy scheduler load:

1. A halt during until(Deno.makeTempFile(...)) abandons the promise — Effection's until cancel is a no-op — so the OS creates the scratch file after teardown, and the ensure(rm) on the next line was never registered. The reported leftover (2361ba29aac95f1f.js) matches makeTempFile's 16-hex naming exactly.
2. exec from @effectionx/process suspends several times between spawning the child and registering the teardown that kills it. A halt landing there orphans deno bundle and its esbuild service, which later write the real output after the removal already ran.

After:

The output path is chosen without touching the filesystem, its force-removal is registered before anything exists, and the bundler runs through a contained spawn whose terminate-and-join teardown is armed before the process exists, in the same synchronous continuation. Teardown signals the bundler's detached process group and joins the child's close event before the scratch cleanup runs. No sleeps, polling, retries, deadlines, serialization, or exclusions.

How it works

bundleClient → sync output name → ensure(rm force) → containedRun(deno bundle)
                                                      ├─ ensure(SIGTERM group + join close) armed first
                                                      └─ spawn(detached) in the same synchronous continuation
teardown (LIFO) → terminate + join bundler tree → rm(output, force) → scratch empty

The join is the child's close event, which settles only when every holder of the child's piped stderr has exited — the esbuild service inherits the bundler's fd 2 (verified with lsof), so grandchildren are covered. A halt can land before the spawn (nothing acquired, force-rm tolerates the missing file) or after the cleanup is armed — never between, because there is no suspension point between the two.

Review guide

Start with: scripts/lib/contained-run.ts

Then review:

1. The ensure-before-spawn ordering in containedRun and the pid/exited guards (skip the signal after a natural exit so a recycled pid is never signalled).
2. bundleClient in scripts/build-web-client.ts: sync name choice, rm(..., { force: true }) registered first, so LIFO teardown reaches it only after run's join.
3. scripts/tests/contained-run.test.ts, especially "terminates and joins the whole tree before earlier-registered cleanup runs".
4. The runtime-exclusion entries (the new test drives Deno.execPath() fixtures; the existing build-web-client reason no longer mentions makeTempFile).

Look carefully at:

• The teardown's catch around process.kill(-pid, …): on macOS, killpg on an all-zombie group throws EPERM rather than ESRCH; either way the join on close still runs, unlike upstream's catch which skips its wait.

What must stay true

• An interrupted build leaves nothing in scratch — enforced by ensure-first acquisition plus terminate-and-join ordering, checked by the existing leaves nothing behind … tests and the new join test.
• Builds stay cache-pure and install nothing — unchanged; the bundler invocation and its flags are byte-identical, and the dependency-layout tests still pass.

How to verify it

• deno test --allow-all --frozen scripts/tests/contained-run.test.ts — the join test's grandchild inherits the child's piped stderr and would run forever, so the scope releasing at all proves group signal delivery, and the marker it writes while shutting down proves the join finished before earlier-registered cleanup ran. Mutation-checked: child-only kill makes it hang; skipping the join fails its assertion.
• Differential soak (60 interrupted builds under full CPU load, halt delays swept 0–29ms, alternating halt() and failing-scope paths, with immediate and late leftover sweeps): the previous code leaks a makeTempFile-named file within ~3 iterations; this branch stays clean across all 60.
• deno task verify — full concurrent battery, green (report below in comments/CI).

Scope

Included

• Containment of the browser bundler's process tree and scratch output lifecycle.

Intentionally unchanged

• The upstream @effectionx/process acquisition gap itself — escalated as a separate issue; every other exec call site keeps its current behavior.
• The denoland/deno#36417 wedge note and the battery's per-command deadline handling of it.
• The phantom test:node failure from the same battery run, recorded on Intermittent phantom single-failure in Node/Bun runtime suites #275.

New abstractions

• containedRun (scripts/lib/contained-run.ts) exists because exec cannot guarantee the child is owned across a halt during acquisition; the bundler needs that guarantee since cleanup registered before it deletes the file its process tree writes. Single production consumer today, contract pinned by its own test file.
• Each new abstraction has multiple concrete uses or a clear justification.
• No speculative functionality is included.

Scope confirmation

• Every changed file supports the purpose described above.
• Unrelated cleanup and formatting changes are excluded.
• Generated or mechanical changes are clearly identified.
• The description matches the final diff and test results.

[taras]

Full concurrent battery on this branch (commit b8f35a8, macOS arm64, Deno 2.9.5):

deno task verify
verifying 9 commands concurrently (site/ unchanged since origin/main), 20m deadline each

  ok      vendor     3s
  ok      lint       2.4s
  ok      check      1s
  ok      test       466.9s
  ok      check:jsr  1.1s
  ok      tsc        18s
  ok      test:node  214.3s
  ok      test:bun   302.5s
  ok      docs       15.2s

the battery passed, and the tracked tree is unchanged

Differential soak (60 interrupted builds under full CPU saturation, halt delays swept 0–29ms, alternating halt() and failing-scope interruption, immediate + late leftover sweeps): origin/main leaks a makeTempFile-named file on iteration 3 (1a6dd25ebb939b0c.js, 3ms delay — squarely inside the abandoned makeTempFile promise); this branch stays clean across all 60.

[github-actions]

PR #422: 🐛 Contain the browser bundler's process tree within its build scope

4 files, +203 / -6

Scope

✅ PR scope looks good.

Structural

✅ No structural bloat detected.

Slop

Oxlint slop signals:

• no-console ×1: scripts/build-web-client.ts

Static Analysis

Oxlint: 1 diagnostic across 1 file (1 rule)
Density: 0.005 violations/added-line

no-console (1): scripts/build-web-client.ts

Correctness

FILE, PATTERN, CONCERN, QUESTION for author
scripts/build-web-client.ts, "node:os", "node:path", "containedRun" single consumer, "containedRun" is a speculative abstraction with only one consumer (violates Rule of Three)
scripts/tests/contained-run.test.ts, "node:fs", "node:os", "node:path", uses Node.js modules in Deno-only context (extraneous if Deno equivalents exist)
scripts/lib/contained-run.ts, "node:child_process", "node:process", uses Node.js modules in Deno-only context (extraneous if Deno equivalents exist)