fix(verifiers): release fixture roots only after their owners finish (#10491)

* fix(verifiers): release fixture roots only after their owners finish

Several real-tool verifiers released a fixture root even when a cleanup
that still used it had failed, so a surviving MCP server, session or
descendant kept running against a deleted directory and the failure
looked like ordinary cleanup.

- scripts/lib/verifier-cleanup.mjs: new releaseAfter(dependents,
  release, retained) task. It attempts every dependent cleanup and
  releases the resource only if all succeed; otherwise it keeps the
  resource and reports it with the dependent failures.
- verify-signal-recording: the root is released after the MCP client
  and transport close, and the host-limits read moves before the root
  is acquired so its failure cannot leak the root.
- verify-recorded-crash: sessions, the sentinel and the process-lineage
  check must all succeed before the root is released. A transport close
  failure after a failed connect keeps the connect failure.
- verify-wasm-artifact: both roots are released after the MCP client
  and transport close; the residue check and the two directories stay
  independent after that.
- verify-native-ui: `open -W` exits only after the app exits, so the
  root is removed only once that exit is observed; otherwise it is
  retained and reported.

Fixes #8561
Fixes #8562
Fixes #8564
Refs #8559

Co-Authored-By: gensokyo-dev <gensokyo-dev@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(verifiers): treat a signaled open as unproven fixture exit

A terminated `open` launcher does not prove the LaunchServices app it
started has exited, so only an `open -W` that exits on its own counts.
Also clear the grace timer so a prompt exit does not hold the process
for the rest of the grace period.

Refs #8559

Co-Authored-By: gensokyo-dev <gensokyo-dev@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(verifiers): model open -W exiting after the fixture is signaled

The native UI verifier now removes its root only after `open -W` exits
on its own. The authored recording's `open` never exited when the
fixture was signaled and had no signalCode, so the complete case waited
out the grace period and kept the root. The double now behaves like
`open -W`: it exits once the fixture process is signaled.

Refs #8559

Co-Authored-By: gensokyo-dev <gensokyo-dev@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: gensokyo-dev <gensokyo-dev@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

scripts/lib/verifier-cleanup.d.mts

@@ -8,3 +8,13 @@ export function withVerifierCleanup<T>(
   operation: () => T | Promise<T>,
   cleanupTasks: readonly (() => void | Promise<void>)[],
 ): Promise<T>;
+
+/**
+ * A cleanup task that releases one resource only after every cleanup that may
+ * still use it has succeeded; otherwise the resource is retained and reported.
+ */
+export function releaseAfter(
+  dependents: readonly (() => void | Promise<void>)[],
+  release: () => void | Promise<void>,
+  retained: string,
+): () => Promise<void>;

scripts/lib/verifier-cleanup.mjs

@@ -11,6 +11,26 @@ async function collectCleanupFailures(tasks) {
   return failures;
 }
 
+/**
+ * A cleanup task that releases one resource only after every cleanup that may
+ * still use it has succeeded. When a dependent fails, the resource is retained
+ * and reported with that failure instead of being released underneath it.
+ */
+export function releaseAfter(dependents, release, retained) {
+  return async () => {
+    const failures = await collectCleanupFailures(dependents);
+    if (failures.length > 0)
+      throw new AggregateError(
+        [
+          ...failures,
+          new Error(`Retained ${retained} because a dependent cleanup failed.`),
+        ],
+        "Verifier cleanup failed",
+      );
+    await release();
+  };
+}
+
 /** Attempt every independent cleanup and preserve each failure. */
 export async function closeVerifierResources(tasks) {
   const failures = await collectCleanupFailures(tasks);

scripts/verify-native-ui.mjs

@@ -21,6 +21,10 @@ let childExit;
 let fixturePid;
 let fixtureIdentity;
 let report;
+// `open -W` exits only after the app exits, so its own exit is the proof that
+// the fixture no longer runs from the root. Without it the root is retained.
+let retainedRoot;
+const FIXTURE_EXIT_GRACE_MS = 10_000;
 try {
   const childRetrievalTest = join(root, "child-retrieval-test");
   await promisify(execFile)("/usr/bin/xcrun", [
@@ -288,10 +292,41 @@ try {
       }
     }
   }
-  if (child !== undefined && child.exitCode === null) {
-    child.kill("SIGTERM");
-    await childExit;
+  const fixtureExited =
+    child === undefined || (await openExitedWithFixture(child, childExit));
+  if (fixtureExited) await rm(root, { recursive: true, force: true });
+  else {
+    // The app may have been reparented away from `open`; stopping `open`
+    // does not prove the fixture exited, so its root stays in place.
+    if (child.exitCode === null && child.signalCode === null) {
+      child.kill("SIGTERM");
+      await childExit;
+    }
+    retainedRoot = root;
+    process.stderr.write(
+      `Source-owned UI fixture exit was not observed; retained ${root}\n`,
+    );
   }
-  await rm(root, { recursive: true, force: true });
 }
+if (retainedRoot !== undefined)
+  throw new Error(
+    `Source-owned UI fixture exit was not observed; retained ${retainedRoot}`,
+  );
 process.stdout.write(`${JSON.stringify(report)}\n`);
+
+// `open -W` exits on its own only after the app exits (or never launched it).
+// A signaled `open`, or none within the grace period, leaves the app unproven.
+async function openExitedWithFixture(launcher, exit) {
+  if (launcher.exitCode === null && launcher.signalCode === null) {
+    let timer;
+    const exited = await Promise.race([
+      exit.then(() => true),
+      new Promise((resolve) => {
+        timer = setTimeout(() => resolve(false), FIXTURE_EXIT_GRACE_MS);
+      }),
+    ]);
+    clearTimeout(timer);
+    if (!exited) return false;
+  }
+  return launcher.exitCode !== null && launcher.signalCode === null;
+}

scripts/verify-recorded-crash.mjs

@@ -4,7 +4,12 @@ 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 { releaseAfter } 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 {
@@ -41,6 +46,7 @@ const entrypoint =
 const failures = [];
 const sessions = [];
 let sentinel;
+let verifier;
 let cases = 0;
 let trace;
 let catalogTools;
@@ -340,25 +346,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);
-    }
+  // Sessions, the sentinel and any surviving descendant may still use files
+  // under the root, so it is released only after all of them are proven gone.
+  try {
+    await releaseAfter(
+      [
+        ...sessions.map((session) => () => session.close()),
+        ...(sentinel ? [() => sentinel.close()] : []),
+        async () => {
+          verifier = await completeVerifierRun(run);
+          assertVerifierCleanupComplete(verifier.process_lineage);
+        },
+      ],
+      () => root.close(),
+      root.path,
+    )();
+  } catch (cause) {
+    failures.push(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);
-}
 const result = {
   status: failures.length === 0 ? "passed" : "failed",
   public_cases: cases,

scripts/verify-signal-recording.mjs

@@ -17,7 +17,7 @@ import { StdioClientTransport } from "@modelcontextprotocol/client/stdio";
 import { Ajv2020 } from "ajv/dist/2020.js";
 import { PrivateRuntimeRoot } from "../dist/process/PrivateRuntimeRoot.js";
 import { parseEvidence } from "../dist/domain/evidence.js";
-import { withVerifierCleanup } from "./lib/verifier-cleanup.mjs";
+import { releaseAfter, withVerifierCleanup } from "./lib/verifier-cleanup.mjs";
 import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs";
 
 if (process.platform !== "linux")
@@ -30,6 +30,19 @@ if (!isAbsolute(process.env.REA_SIGROK_COMMAND ?? ""))
   );
 const entrypoint =
   process.argv[2] ?? fileURLToPath(new URL("./rea.mjs", import.meta.url));
+// Read host limits before acquiring the root so a failure cannot leak it.
+const inheritedAddressSpace = (await readFile("/proc/self/limits", "utf8"))
+  .split("\n")
+  .find((line) => line.startsWith("Max address space "))
+  ?.trim()
+  .match(/^Max address space\s+(\S+)\s+(\S+)\s+bytes$/u);
+assert.ok(inheritedAddressSpace);
+const expectedAddressSpace = Math.min(
+  1073741824,
+  ...inheritedAddressSpace
+    .slice(1)
+    .map((value) => (value === "unlimited" ? Infinity : Number(value))),
+);
 const run = createVerifierRun();
 const root = await PrivateRuntimeRoot.create({
   prefix: "rea-signal-verifier-",
@@ -93,18 +106,6 @@ lines.push(`#${tick + 1000}`, "1!");
 const source = lines.join("\n") + "\n";
 let cases = 0;
 const discoveryMeasurements = [];
-const inheritedAddressSpace = (await readFile("/proc/self/limits", "utf8"))
-  .split("\n")
-  .find((line) => line.startsWith("Max address space "))
-  ?.trim()
-  .match(/^Max address space\s+(\S+)\s+(\S+)\s+bytes$/u);
-assert.ok(inheritedAddressSpace);
-const expectedAddressSpace = Math.min(
-  1073741824,
-  ...inheritedAddressSpace
-    .slice(1)
-    .map((value) => (value === "unlimited" ? Infinity : Number(value))),
-);
 
 await withVerifierCleanup(async () => {
   await writeFile(path, source);
@@ -437,7 +438,14 @@ await withVerifierCleanup(async () => {
     "paged.vcd",
     "recording with spaces.vcd",
   ]);
-}, [() => client.close(), () => transport.close(), () => root.close()]);
+}, [
+  // The MCP server child reads files under the root until it has exited.
+  releaseAfter(
+    [() => client.close(), () => transport.close()],
+    () => root.close(),
+    root.path,
+  ),
+]);
 console.log(
   JSON.stringify(
     {

scripts/verify-wasm-artifact.mjs

@@ -25,6 +25,10 @@ import {
   mcpTextValue,
   requireMcpToolError,
 } from "./lib/mcp-verifier-results.mjs";
+import {
+  closeVerifierResources,
+  releaseAfter,
+} from "./lib/verifier-cleanup.mjs";
 import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs";
 import { WABT_OPERATION_TIMEOUT_MS } from "./lib/wabt-verifier-timeouts.mjs";
 
@@ -623,35 +627,26 @@ try {
 } catch (cause) {
   failures.push(cause);
 }
+// The MCP server child uses both roots until it exits, so they are released
+// only after the client and transport close. After that the residue check and
+// the two directories are independent owners; attempt each and keep failures.
 try {
-  await client.close();
-} catch (cause) {
-  failures.push(cause);
-}
-try {
-  await transport.close();
-} catch (cause) {
-  failures.push(cause);
-}
-try {
-  assert.ok(
-    !(await readdir(childTempRoot)).some((name) =>
-      name.startsWith("rea-wabt-"),
-    ),
-    "Provider workspace leaked",
-  );
-} catch (cause) {
-  failures.push(cause);
-}
-// Each directory is an independent owner. Attempt every cleanup even if a
-// residue assertion or another owner's close fails, retaining every error.
-try {
-  await root.close();
-} catch (cause) {
-  failures.push(cause);
-}
-try {
-  await rm(childTempRoot, { recursive: true, force: true });
+  await releaseAfter(
+    [() => client.close(), () => transport.close()],
+    () =>
+      closeVerifierResources([
+        async () =>
+          assert.ok(
+            !(await readdir(childTempRoot)).some((name) =>
+              name.startsWith("rea-wabt-"),
+            ),
+            "Provider workspace leaked",
+          ),
+        () => root.close(),
+        () => rm(childTempRoot, { recursive: true, force: true }),
+      ]),
+    `${root.path} and ${childTempRoot}`,
+  )();
 } catch (cause) {
   failures.push(cause);
 }

scripts/verify/recorded-crash/public.mjs

@@ -23,7 +23,16 @@ export async function connectRecordedCrash({ entrypoint, environment }) {
   try {
     await client.connect(transport);
   } catch (cause) {
-    await transport.close();
+    // Keep the connect failure when closing the transport also fails.
+    try {
+      await transport.close();
+    } catch (closeCause) {
+      throw new AggregateError(
+        [cause, closeCause],
+        "Recorded-crash MCP connection and transport cleanup failed",
+        { cause },
+      );
+    }
     throw cause;
   }
   return {

tests/boundary/filesystem/verifierCleanup.test.ts

@@ -0,0 +1,57 @@
+import { expect, it } from "vitest";
+import {
+  closeVerifierResources,
+  releaseAfter,
+} from "../../../scripts/lib/verifier-cleanup.mjs";
+
+it("releases a root only after every dependent cleanup succeeds", async () => {
+  const order: string[] = [];
+  await closeVerifierResources([
+    releaseAfter(
+      [
+        async () => {
+          order.push("client");
+        },
+        async () => {
+          order.push("transport");
+        },
+      ],
+      async () => {
+        order.push("root");
+      },
+      "/fixture/root",
+    ),
+  ]);
+  expect(order).toEqual(["client", "transport", "root"]);
+});
+
+it("retains the root and reports every dependent failure", async () => {
+  const order: string[] = [];
+  const clientFailure = new Error("client close failed");
+  const failure = await closeVerifierResources([
+    releaseAfter(
+      [
+        async () => {
+          order.push("client");
+          throw clientFailure;
+        },
+        async () => {
+          order.push("transport");
+        },
+      ],
+      async () => {
+        order.push("root");
+      },
+      "/fixture/root",
+    ),
+  ]).catch((cause: unknown) => cause);
+  // The later dependent is still attempted, but the root is not released.
+  expect(order).toEqual(["client", "transport"]);
+  expect(failure).toBeInstanceOf(AggregateError);
+  const [retention] = (failure as AggregateError).errors;
+  expect(retention).toBeInstanceOf(AggregateError);
+  expect((retention as AggregateError).errors).toEqual([
+    clientFailure,
+    new Error("Retained /fixture/root because a dependent cleanup failed."),
+  ]);
+});

tests/boundary/process/nativeUiVerifierActions.test.ts

@@ -26,7 +26,7 @@ export const execFile=Object.assign(()=>{}, {[promisify.custom]:async (file,args
  if(args?.includes('capture-native-ui-scenario')){if(process.env.MODE!=='drop CLI')count++;return {stdout:JSON.stringify({normalized_result:captured()})};}
  return {stdout:'{"v":"slash/producer"}\\n{"v":1.0}\\n'};
 }});
-export const spawn=(_command,args)=>{const child=new EventEmitter();child.exitCode=null;child.kill=()=>{child.exitCode=0;child.emit('exit',0);return true;};void writeFile(args[args.indexOf('--stdout')+1],JSON.stringify({pid:424242,window_id:7})+'\\n');return child;};
+export const spawn=(_command,args)=>{const child=new EventEmitter();child.exitCode=null;child.signalCode=null;globalThis.reaOpenChild=child;child.kill=()=>{child.exitCode=0;child.emit('exit',0);return true;};void writeFile(args[args.indexOf('--stdout')+1],JSON.stringify({pid:424242,window_id:7})+'\\n');return child;};
 export const parseBinaryTarget=async path=>({ok:true,value:{path,sha256:'a'.repeat(64)}});
 export const observeProcessStartIdentity=async()=>({state:'readable',identity:'authored-process-identity'});
 export const observeNativeUi=async(target,operation)=>{
@@ -42,7 +42,7 @@ export const artifactMcpResult=async(_client,name)=>{if(name==='observe_native_u
         join(directory, "hook.mjs"),
         `
 import {registerHooks} from 'node:module';import {pathToFileURL} from 'node:url';
-Object.defineProperty(process,'platform',{value:'darwin'});process.kill=()=>true;
+Object.defineProperty(process,'platform',{value:'darwin'});process.kill=()=>{const open=globalThis.reaOpenChild;if(open?.exitCode===null){open.exitCode=0;queueMicrotask(()=>open.emit('exit',0,null));}return true;};
 registerHooks({resolve(specifier,context,next){if(context.parentURL?.endsWith('/scripts/verify-native-ui.mjs')&&['node:child_process','../dist/application/BinaryTargetResolver.js','../dist/native/NativeUiObservation.js','./lib/artifact-e2e.mjs','../dist/process/ProcessOwnershipObservation.js'].includes(specifier))return {shortCircuit:true,url:pathToFileURL(process.env.RECORDING).href};return next(specifier,context);}});
 `,
       );