fix(verifiers): keep the recorded-crash root until its owners finish

scripts/verify-recorded-crash.mjs

@@ -4,9 +4,14 @@ import { writeFile, readFile, copyFile } from "node:fs/promises";
 import { isAbsolute, join } from "node:path";
 import { fileURLToPath } from "node:url";
 import { PrivateRuntimeRoot } from "../dist/process/PrivateRuntimeRoot.js";
-import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs";
+import { closeVerifierResources } from "./lib/verifier-cleanup.mjs";
+import {
+  assertVerifierCleanupComplete,
+  createVerifierRun,
+  completeVerifierRun,
+} from "./lib/verifier-run.mjs";
 import { verifyCompleteToolCatalog } from "./lib/verify-package-core.mjs";
-import { connectRecordedCrash } from "./verify/recorded-crash/public.mjs";
+import { openRecordedCrashSession } from "./verify/recorded-crash/public.mjs";
 import {
   createRecordedCore,
   fixtureCommand,
@@ -44,6 +49,8 @@ let sentinel;
 let cases = 0;
 let trace;
 let catalogTools;
+let verifier;
+let rootRemoved = false;
 try {
   for (const [command, args] of [
     ["gcc", ["--version"]],
@@ -84,8 +91,11 @@ try {
   );
   const spaced = join(root.path, "recording with spaces.core");
   await copyFile(core, spaced);
-  const basic = await connectRecordedCrash({ entrypoint, environment });
+  // Each session is owned before it connects, so a failed connection still
+  // closes through the cleanup below.
+  const basic = openRecordedCrashSession({ entrypoint, environment });
   sessions.push(basic);
+  await basic.connect();
   const advertised = (await basic.client.listTools()).tools;
   assert.equal(
     advertised.filter((tool) => tool.name === "inspect_recorded_crash").length,
@@ -290,11 +300,12 @@ try {
         `from pathlib import Path\nentry = ${JSON.stringify(environment.REA_PWNDBG_GDBINIT)}\nexec(compile(Path(entry).read_bytes(), entry, 'exec'), {'__name__': '__main__', '__file__': entry})\nimport pwndbg.lib.version\npwndbg.lib.version.__version__ = '0.0.0-source-owned'\n`,
       );
     }
-    const session = await connectRecordedCrash({
+    const session = openRecordedCrashSession({
       entrypoint,
       environment: { ...environment, ...changes },
     });
     sessions.push(session);
+    await session.connect();
     for (const mode of ["cli", "mcp"]) {
       const error = await session.inspect(mode, core, {
         debuggerContext: true,
@@ -339,25 +350,25 @@ try {
 } catch (cause) {
   failures.push(cause);
 } finally {
-  for (const close of [
-    ...sessions.map((session) => () => session.close()),
-    ...(sentinel ? [() => sentinel.close()] : []),
-    () => root.close(),
-  ]) {
-    try {
-      await close();
-    } catch (cause) {
-      failures.push(cause);
-    }
+  // Every owner this run acquired must finish, and its lineage must verify,
+  // before the root can go. Otherwise the root stays and the report names it.
+  try {
+    await closeVerifierResources([
+      ...sessions.map((session) => () => session.close()),
+      ...(sentinel ? [() => sentinel.close()] : []),
+    ]);
+    verifier = await completeVerifierRun(run);
+    assertVerifierCleanupComplete(verifier.process_lineage);
+    await root.close();
+    rootRemoved = true;
+  } catch (cause) {
+    failures.push(
+      ...(cause instanceof AggregateError ? cause.errors : [cause]),
+    );
   }
 }
-const verifier = await completeVerifierRun(run);
-try {
-  assert.equal(verifier.process_lineage.status, "verified");
-  assert.deepEqual(verifier.process_lineage.descendants, []);
-} catch (cause) {
-  failures.push(cause);
-}
+// A release that stopped early never observed lineage; report it regardless.
+verifier ??= await completeVerifierRun(run);
 const result = {
   status: failures.length === 0 ? "passed" : "failed",
   public_cases: cases,
@@ -367,11 +378,14 @@ const result = {
   fixture_generation: "owned test program executed",
   inspection_trace: trace,
   historical_pid_access_coverage: historicalPidTraceContract,
+  retained_root: rootRemoved ? null : root.path,
   verifier,
 };
 console.log(JSON.stringify(result, null, 2));
 if (failures.length > 0)
   throw new AggregateError(
     failures,
-    "Recorded crash verification failed; cleanup failures are retained.",
+    rootRemoved
+      ? "Recorded crash verification failed; cleanup failures are retained."
+      : `Recorded crash verification failed; retained fixture root ${root.path} because its cleanup is incomplete.`,
   );

scripts/verify/recorded-crash/public.mjs

@@ -11,23 +11,26 @@ import { mcpTextValue } from "../../lib/mcp-verifier-results.mjs";
 
 const execute = promisify(execFile);
 
-/** Exercise the public installed CLI and SDK transport with the same selected environment. */
-export async function connectRecordedCrash({ entrypoint, environment }) {
+/**
+ * Own the public installed CLI and SDK transport for one selected environment.
+ * The transport is acquired here and released only by `close`, so a caller
+ * owns the session before `connect` starts the SDK stdio child: a failed
+ * connection, or a refused close after one, leaves the owner in the caller's
+ * cleanup instead of losing the only handle to a possibly live transport.
+ */
+export function openRecordedCrashSession({ entrypoint, environment }) {
   const client = new Client({ name: "recorded-crash-verifier", version: "1" });
   const transport = new StdioClientTransport({
     command: process.execPath,
     args: [entrypoint, "mcp"],
     env: environment,
     stderr: "pipe",
   });
-  try {
-    await client.connect(transport);
-  } catch (cause) {
-    await transport.close();
-    throw cause;
-  }
   return {
     client,
+    async connect() {
+      await client.connect(transport);
+    },
     async inspect(mode, path, { debuggerContext = false, category } = {}) {
       const original = await readFile(path);
       let evidence;

scripts/verify/recorded-crash/trace-client.mjs

@@ -1,11 +1,12 @@
-import { connectRecordedCrash } from "./public.mjs";
+import { openRecordedCrashSession } from "./public.mjs";
 import assert from "node:assert/strict";
 
-const session = await connectRecordedCrash({
+const session = openRecordedCrashSession({
   entrypoint: process.argv[2],
   environment: process.env,
 });
 try {
+  await session.connect();
   const report = await session.inspect("mcp", process.argv[3], {
     debuggerContext: true,
   });

tests/boundary/process/recordedCrashVerifierCleanup.test.ts

@@ -0,0 +1,196 @@
+import { existsSync } from "node:fs";
+import { mkdir, readFile } from "node:fs/promises";
+import { join } from "node:path";
+import { fileURLToPath } from "node:url";
+import { expect } from "vitest";
+import { z } from "zod";
+
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+import {
+  processTest,
+  type TestProcesses,
+} from "../../support/process/processFixture.js";
+
+const preload = fileURLToPath(
+  new URL("../../fixtures/recordedCrashVerifierPreload.mjs", import.meta.url),
+);
+const script = fileURLToPath(
+  new URL("../../../scripts/verify-recorded-crash.mjs", import.meta.url),
+);
+
+const reportSchema = z.object({
+  status: z.enum(["passed", "failed"]),
+  retained_root: z.string().nullable(),
+  verifier: z.object({
+    process_lineage: z.object({ status: z.string() }),
+  }),
+});
+
+/** Run the real verifier with authored owners; only its ownership handling is observed. */
+const runScenario = async (processes: TestProcesses, scenario: string) => {
+  const root = await createTestTempDirectory("rea-core-verifier-cleanup-");
+  const temporary = join(root, "temporary");
+  await mkdir(temporary);
+  const log = join(root, "events.log");
+  const authored = join(root, "authored-absent-tool");
+  const result = await processes.run(
+    process.execPath,
+    ["--import", preload, script],
+    {
+      env: {
+        ...process.env,
+        TMPDIR: temporary,
+        TMP: temporary,
+        TEMP: temporary,
+        REA_PWNTOOLS_PYTHON: authored,
+        REA_PWNDBG_GDB: authored,
+        REA_PWNDBG_GDBINIT: authored,
+        REA_PWNDBG_VENV_PATH: authored,
+        REA_VERIFY_STRACE_COMMAND: authored,
+        REA_CORE_TEST_EVENTS: log,
+        REA_CORE_TEST_SCENARIO: scenario,
+      },
+      timeoutMs: 20_000,
+    },
+  );
+  const recorded = await readFile(log, "utf8").then(
+    (text) => text.split("\n").filter((line) => line !== ""),
+    () => [],
+  );
+  const fixtureRoot = recorded
+    .find((line) => line.startsWith("root-create "))
+    ?.slice("root-create ".length);
+  expect(result.exitCode, result.stderr).toBe(1);
+  return {
+    ...result,
+    report: reportSchema.parse(JSON.parse(result.stdout)),
+    events: recorded.map((line) => line.split(" ")[0]),
+    fixtureRoot,
+    rootRetained: fixtureRoot !== undefined && existsSync(fixtureRoot),
+  };
+};
+
+const ownersClosed = ["client-close", "transport-close", "sentinel-close"];
+
+processTest(
+  "releases the root after a prerequisite failure acquired no owner",
+  async ({ processes }) => {
+    const outcome = await runScenario(processes, "prerequisite-unavailable");
+    expect(outcome.stderr).toContain("prerequisite unavailable: gcc");
+    expect(outcome.stderr).toContain("authored gcc absence");
+    expect(outcome.events).toEqual([
+      "root-create",
+      "lineage-observe",
+      "root-close",
+    ]);
+    expect(outcome.report).toMatchObject({
+      status: "failed",
+      retained_root: null,
+    });
+    expect(outcome.rootRetained).toBe(false);
+  },
+);
+
+processTest(
+  "releases the root only after its owners closed and lineage was verified",
+  async ({ processes }) => {
+    const outcome = await runScenario(processes, "owners-released");
+    expect(outcome.stderr).toContain("authored CLI failure");
+    expect(outcome.stderr).not.toContain("retained fixture root");
+    expect(outcome.events).toEqual([
+      "root-create",
+      ...ownersClosed,
+      "lineage-observe",
+      "root-close",
+    ]);
+    expect(outcome.report).toMatchObject({
+      status: "failed",
+      retained_root: null,
+      verifier: { process_lineage: { status: "verified" } },
+    });
+    expect(outcome.rootRetained).toBe(false);
+  },
+);
+
+processTest.for(["client", "transport", "sentinel"])(
+  "retains the root and the other owners' closes when the %s close is refused",
+  async (owner, { processes }) => {
+    const outcome = await runScenario(processes, `${owner}-close-refused`);
+    expect(outcome.stderr).toContain("authored CLI failure");
+    expect(outcome.stderr).toContain(`authored ${owner} close failure`);
+    expect(outcome.stderr).toContain("retained fixture root");
+    // Lineage is still observed for the report, after every owner and
+    // without releasing the root.
+    expect(outcome.events).toEqual([
+      "root-create",
+      ...ownersClosed,
+      "lineage-observe",
+    ]);
+    expect(outcome.report).toMatchObject({
+      status: "failed",
+      retained_root: outcome.fixtureRoot,
+    });
+    expect(outcome.rootRetained).toBe(true);
+  },
+);
+
+processTest(
+  "retains the root when owned process lineage cannot be verified",
+  async ({ processes }) => {
+    const outcome = await runScenario(processes, "lineage-unverified");
+    expect(outcome.stderr).toContain("authored lineage unavailability");
+    expect(outcome.stderr).toContain("retained fixture root");
+    expect(outcome.events).toEqual([
+      "root-create",
+      ...ownersClosed,
+      "lineage-observe",
+    ]);
+    expect(outcome.report).toMatchObject({
+      status: "failed",
+      retained_root: outcome.fixtureRoot,
+      verifier: { process_lineage: { status: "unavailable" } },
+    });
+    expect(outcome.rootRetained).toBe(true);
+  },
+);
+
+processTest(
+  "closes a session whose connection failed before releasing the root",
+  async ({ processes }) => {
+    const outcome = await runScenario(processes, "connect-refused");
+    expect(outcome.stderr).toContain("authored connection failure");
+    expect(outcome.stderr).not.toContain("retained fixture root");
+    // The session is owned before it connects, so its owners still close.
+    expect(outcome.events).toEqual([
+      "root-create",
+      ...ownersClosed,
+      "lineage-observe",
+      "root-close",
+    ]);
+    expect(outcome.report).toMatchObject({
+      status: "failed",
+      retained_root: null,
+    });
+    expect(outcome.rootRetained).toBe(false);
+  },
+);
+
+processTest(
+  "retains the root when a failed connection's transport close is refused",
+  async ({ processes }) => {
+    const outcome = await runScenario(processes, "connect-and-close-refused");
+    expect(outcome.stderr).toContain("authored connection failure");
+    expect(outcome.stderr).toContain("authored transport close failure");
+    expect(outcome.stderr).toContain("retained fixture root");
+    expect(outcome.events).toEqual([
+      "root-create",
+      ...ownersClosed,
+      "lineage-observe",
+    ]);
+    expect(outcome.report).toMatchObject({
+      status: "failed",
+      retained_root: outcome.fixtureRoot,
+    });
+    expect(outcome.rootRetained).toBe(true);
+  },
+);