test(scratch): make the lifecycle oracle pass on Ubuntu 26.04 runners and fail instead of hang

.github/workflows/test-scratch-lifecycle.yml

@@ -76,7 +76,20 @@ jobs:
       - name: Scoped strict TypeScript
         run: pnpm exec tsc --project scripts/test-scratch/tsconfig.json --pretty false
 
+      - name: Require bubblewrap
+        # The /tmp visibility probe runs with TEST_SANDBOX_REQUIRE=1. Without a working bubblewrap it must fail,
+        # never fall back to the host /tmp, so install it, lift Ubuntu's unprivileged-userns restriction and
+        # prove it here with the flags test-sandbox uses.
+        run: |
+          sudo apt-get update -qq
+          sudo apt-get install -y --no-install-recommends bubblewrap
+          sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 || true
+          bwrap --dev-bind / / --tmpfs /tmp --unshare-net true
+          echo "bubblewrap verified: $(bwrap --version)"
+
       - name: Lifecycle owner oracle
+        # The whole file runs in well under a minute; a hung process must fail the step, not the job.
+        timeout-minutes: 5
         run: pnpm exec tsx --test scripts/test-scratch/run-command.test.ts
 
       - name: Canonical-entrypoint ratchet

scripts/test-sandbox/README.md

@@ -10,7 +10,7 @@ names the enforcement that remains. `TEST_SANDBOX_REQUIRE=1` makes missing
 enforcement an error. `TEST_SANDBOX_DISABLE=1` runs the command directly while
 keeping scratch ownership and cleanup.
 
-The executable is copied byte-for-byte from `ai/skills/local/test-architecture/scripts/test-sandbox` in Tim Nunamaker's dotfiles at commit `ce0d966`. Change and test that source first, then update this copy and digest:
+The executable is copied from `ai/skills/local/test-architecture/scripts/test-sandbox` in Tim Nunamaker's dotfiles at commit `ce0d966`, plus one later upstream fix applied as-is: commit `e4b320a` makes `TEST_SANDBOX_REQUIRE=1` exit 3 on bash older than 5 instead of running the command without isolation. Change and test that source first, then update this copy and digest:
 
 ```sh
 cp <dotfiles>/ai/skills/local/test-architecture/scripts/test-sandbox scripts/test-sandbox/test-sandbox

scripts/test-sandbox/test-sandbox

@@ -20,6 +20,9 @@
 # unless --receipt is given.
 # Old bash (macOS ships 3.2) cannot parse what follows: run the command directly.
 if [ -z "${BASH_VERSINFO:-}" ] || [ "${BASH_VERSINFO[0]}" -lt 5 ]; then
+  if [ "${TEST_SANDBOX_REQUIRE:-0}" = 1 ] && [ "${TEST_SANDBOX_DISABLE:-0}" != 1 ]; then
+    echo "test-sandbox: bash 5 not found; required enforcement layer unavailable" >&2; exit 3
+  fi
   echo "test-sandbox: bash 5 not found; running without enforcement" >&2
   # Drop our options (bash 3.2 syntax only): everything after "--" is the command.
   # The scratch contract still holds: TMPDIR is --scratch (created 0700 if

scripts/test-sandbox/test-sandbox.sha256

@@ -1 +1 @@
-b51043419454d9836b9a90014fe7a077129711913751bf9c59cba8c747203941  test-sandbox
+f4ede419008088fdaa751087b202098adfe5b65075edb9f057b7b26eb2c4284d  test-sandbox

scripts/test-scratch/fixtures/leaked-group-after-failure.ts

@@ -0,0 +1,69 @@
+// Copyright The PDP-Connect Contributors
+// SPDX-License-Identifier: Apache-2.0
+
+// Run only by run-command.test.ts, as a child `node --test`: kill a wrapper's owner while
+// its command and a background child are alive, then fail an assertion. The parent checks
+// the run still exits and both processes are gone.
+
+import assert from "node:assert/strict";
+import { readFile, writeFile } from "node:fs/promises";
+import test from "node:test";
+import { setTimeout as sleep } from "node:timers/promises";
+import { lifecycleFixture } from "./lifecycle-fixture.ts";
+
+const wrapperPath = new URL("../run-command.ts", import.meta.url).pathname;
+const PID_FILE = /^\d+\n$/;
+
+// Shorter than the parent's 60 s timeout, so a stuck setup fails here and runs this file's after-hooks.
+const READY_DEADLINE_MS = 20_000;
+
+// Each program publishes its own pid from inside the running process, atomically, once it is up.
+const PUBLISH =
+  'const f=require("node:fs");const w=(p)=>{f.writeFileSync(p+".tmp",process.pid+"\\n");f.renameSync(p+".tmp",p)};';
+// The background keep-alive. It inherits the wrapper's pipes, as the background child did in the CI hang.
+const KEEP_ALIVE = `${PUBLISH}w(process.argv[1]);setInterval(()=>{},1e6)`;
+// The command (owner). It publishes only after its keep-alive child reports ready, and exits at once if that child exits.
+const COMMAND = `${PUBLISH}const c=require("node:child_process").spawn(process.execPath,["-e",${JSON.stringify(KEEP_ALIVE)},process.argv[2]],{stdio:"inherit"});c.on("exit",()=>process.exit(1));c.on("error",()=>process.exit(1));const t=setInterval(()=>{if(f.existsSync(process.argv[2])){clearInterval(t);w(process.argv[1]);setInterval(()=>{},1e6)}},10)`;
+
+/** Wait for both self-reported pids: the command's and its background child's. */
+async function readyPids(ownerFile: string, childFile: string): Promise<[number, number]> {
+  const deadline = Date.now() + READY_DEADLINE_MS;
+  while (Date.now() < deadline) {
+    // biome-ignore lint/performance/noAwaitInLoops: bounded by READY_DEADLINE_MS.
+    const owner = await readFile(ownerFile, "utf8").catch(() => "");
+    const child = await readFile(childFile, "utf8").catch(() => "");
+    if (PID_FILE.test(owner) && PID_FILE.test(child)) {
+      return [Number(owner), Number(child)];
+    }
+    await sleep(25);
+  }
+  throw new Error("setup failed: the command never reported its pids");
+}
+
+function alive(pid: number): boolean {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+test("owner killed while its command and a background child run, then an assertion fails", async (t) => {
+  const fx = await lifecycleFixture(t);
+  const ownerFile = fx.path("owner-pid");
+  const childFile = fx.path("child-pid");
+  const wrapper = fx.spawn(
+    process.execPath,
+    ["--import", "tsx", wrapperPath, "--", process.execPath, "-e", COMMAND, ownerFile, childFile],
+    { helpers: [ownerFile, childFile] }
+  );
+  const pids = await readyPids(ownerFile, childFile);
+  const gone = pids.filter((pid) => !alive(pid));
+  if (gone.length > 0 || wrapper.exitCode !== null || wrapper.signalCode !== null) {
+    throw new Error(`setup failed: the command or its background child is not running (gone: ${gone.join(",")})`);
+  }
+  await writeFile(process.env.PIDS_OUT as string, `${pids.join(" ")}\n`);
+  wrapper.kill("SIGKILL");
+  assert.fail("deliberate failure while an orphaned command holds the wrapper's pipes");
+});

scripts/test-scratch/fixtures/lifecycle-fixture.ts

@@ -0,0 +1,170 @@
+// Copyright The PDP-Connect Contributors
+// SPDX-License-Identifier: Apache-2.0
+
+// The one place the lifecycle oracle creates directories for, launches, and cleans up
+// after a command that runs through the scratch wrapper. It guards against two mistakes that
+// hung CI on a runner image with bubblewrap, so no test has to remember them:
+//
+// 1. A helper the command cannot see. `scripts/test-sandbox/test-sandbox` gives the command
+//    a private /tmp and binds back only the top directory of its scratch root. Fixture-created
+//    paths stay inside `runnerTemp`; declared outside helpers are rejected. A path a test
+//    builds by hand and does not declare is not checked.
+// 2. A process the test leaves behind. A leftover command group keeps the wrapper's pipes
+//    open, so the test process never exits. Teardown closes the pipes and kills what the
+//    fixture launched.
+//
+// Ownership is the launch environment: `spawn` requires `RUNNER_TEMP` to be this fixture's
+// unique `runnerTemp`, and every descendant, including one in the sandbox, inherits it.
+// Teardown kills exactly the processes carrying that value. It never signals a pid or group
+// id recorded earlier: each process is matched and signalled in one synchronous step.
+
+import { type ChildProcess, spawn as nodeSpawn, type SpawnOptions } from "node:child_process";
+import { readdirSync, readFileSync } from "node:fs";
+import { mkdir, mkdtemp, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { isAbsolute, join, relative, resolve } from "node:path";
+import type { TestContext } from "node:test";
+
+export interface LifecycleFixture {
+  /** Create `name` inside `runnerTemp` and return its path. */
+  dir: (name: string) => Promise<string>;
+  /** `process.env` with `RUNNER_TEMP` pointing at `runnerTemp`, plus `overrides`. */
+  env: (overrides?: NodeJS.ProcessEnv) => NodeJS.ProcessEnv;
+  /** A path inside `runnerTemp`; throws for anything that would resolve outside it. */
+  path: (...segments: string[]) => string;
+  /** The only directory tree a helper handed to a launched command may live in. */
+  readonly runnerTemp: string;
+  /** Where the wrapper allocates its per-run scratch roots. */
+  readonly scratchParent: string;
+  /** Start a process, killed with its pipes closed when the test ends. Each of `helpers` (files the command uses) must be inside `runnerTemp`. */
+  spawn: (
+    file: string,
+    args: readonly string[],
+    options?: SpawnOptions & { helpers?: readonly string[] }
+  ) => ChildProcess;
+  /** Point this process's `RUNNER_TEMP` at `runnerTemp` until the test ends; repeated calls change nothing. */
+  useRunnerTempForThisTest: () => void;
+}
+
+const DIGITS = /^\d+$/;
+
+function isInside(root: string, candidate: string): boolean {
+  const rel = relative(root, candidate);
+  return rel === "" || !(rel.startsWith("..") || isAbsolute(rel));
+}
+
+/** Kill every other process whose environment carries `entry`, rescanning while a parent forks; false when `/proc` is unreadable. */
+function killCarriers(entry: string): boolean {
+  let killed = -1;
+  for (let scan = 0; scan < 6 && killed !== 0; scan += 1) {
+    killed = 0;
+    let names: string[];
+    try {
+      names = readdirSync("/proc");
+    } catch {
+      return false;
+    }
+    for (const name of names.filter((n) => DIGITS.test(n) && Number(n) !== process.pid)) {
+      try {
+        // Nothing is awaited between this read and the kill; that gap is the window Jenkins' ProcessTreeKiller accepts.
+        if (readFileSync(`/proc/${name}/environ`, "utf8").split("\0").includes(entry)) {
+          process.kill(Number(name), "SIGKILL");
+          killed += 1;
+        }
+      } catch {
+        // The environment is unreadable or the process already exited.
+      }
+    }
+  }
+  return true;
+}
+
+function killQuietly(pid: number): void {
+  try {
+    process.kill(pid, "SIGKILL");
+  } catch {
+    // The process already exited.
+  }
+}
+
+/** Create a fixture for one test; everything it handed out is cleaned up in the test's `after` hook. */
+export async function lifecycleFixture(
+  t: TestContext,
+  options: { /** Parent of `runnerTemp`; defaults to the host temporary directory. */ under?: string } = {}
+): Promise<LifecycleFixture> {
+  const runnerTemp = await mkdtemp(join(options.under ?? tmpdir(), "pdpp-test-scratch-test-"));
+  const children: ChildProcess[] = [];
+  let restoreRunnerTemp: (() => void) | undefined;
+
+  const inside = (candidate: string): string => {
+    if (!isInside(runnerTemp, candidate)) {
+      throw new Error(
+        `${candidate} is outside the bound runner temp ${runnerTemp}; the sandbox's private /tmp hides it from the command`
+      );
+    }
+    return candidate;
+  };
+  const path = (...segments: string[]): string => inside(resolve(runnerTemp, ...segments));
+  const env = (overrides: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv => ({
+    ...process.env,
+    ...overrides,
+    RUNNER_TEMP: runnerTemp,
+  });
+
+  t.after(async () => {
+    const mayHaveLaunched = children.length > 0 || restoreRunnerTemp !== undefined;
+    restoreRunnerTemp?.();
+    for (const proc of children) {
+      proc.stdin?.destroy();
+      proc.stdout?.destroy();
+      proc.stderr?.destroy();
+      // An unreaped child still owns its pid, so signalling it by handle cannot hit another process.
+      if (proc.pid !== undefined && proc.exitCode === null && proc.signalCode === null) {
+        killQuietly(proc.pid);
+      }
+    }
+    // A test that launched nothing and never adopted RUNNER_TEMP has nothing to reap.
+    if (mayHaveLaunched && !killCarriers(`RUNNER_TEMP=${runnerTemp}`)) {
+      t.diagnostic(`/proc is unreadable, so descendants of ${runnerTemp} cannot be proven ours; none were killed`);
+    }
+    await rm(runnerTemp, { force: true, recursive: true });
+  });
+
+  return {
+    runnerTemp,
+    scratchParent: join(runnerTemp, "pdpp-test-scratch"),
+    path,
+    env,
+    async dir(name) {
+      const target = path(name);
+      await mkdir(target, { recursive: true });
+      return target;
+    },
+    spawn(file, args, { helpers = [], ...spawnOptions } = {}) {
+      const launchEnv = spawnOptions.env ?? env();
+      if (launchEnv.RUNNER_TEMP !== runnerTemp) {
+        throw new Error(`RUNNER_TEMP must be the fixture's runner temp ${runnerTemp}; got ${launchEnv.RUNNER_TEMP}`);
+      }
+      for (const helper of helpers) {
+        inside(resolve(helper));
+      }
+      const proc = nodeSpawn(file, args, { ...spawnOptions, env: launchEnv });
+      children.push(proc);
+      return proc;
+    },
+    useRunnerTempForThisTest() {
+      if (restoreRunnerTemp) {
+        return;
+      }
+      const previous = process.env.RUNNER_TEMP;
+      process.env.RUNNER_TEMP = runnerTemp;
+      restoreRunnerTemp = () => {
+        if (previous === undefined) {
+          delete process.env.RUNNER_TEMP;
+        } else {
+          process.env.RUNNER_TEMP = previous;
+        }
+      };
+    },
+  };
+}