From fe05a4c56d7294679f286cd0c87cfa4c6a294167 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 21:07:13 +0800 Subject: [PATCH] refactor(verify): release five verifier roots through VerifierFixture after their clients confirm (#11118) * refactor(verify): release five verifier roots through VerifierFixture after their clients confirm * fix(verify): keep the binary-layout and EVM roots until their auxiliary MCP clients confirm * fix(verify): carry the retained root on each migrated verifier's final failure --- scripts/lib/verifier-fixture.d.mts | 6 ++ scripts/lib/verifier-fixture.mjs | 20 +++++ scripts/verify-binary-layout.mjs | 81 ++++++++++--------- scripts/verify-deno-standalone.mjs | 25 +++--- scripts/verify-evm-interface.mjs | 51 ++++++------ scripts/verify-pcap.mjs | 48 +++++------ scripts/verify-web-network-captures.mjs | 31 +++---- .../verify/architecture-guard-baselines.json | 5 -- tests/process-global/verifierFixture.test.ts | 41 ++++++++++ 9 files changed, 183 insertions(+), 125 deletions(-) diff --git a/scripts/lib/verifier-fixture.d.mts b/scripts/lib/verifier-fixture.d.mts index d15fe46001..a547fa16d8 100644 --- a/scripts/lib/verifier-fixture.d.mts +++ b/scripts/lib/verifier-fixture.d.mts @@ -38,6 +38,12 @@ export class VerifierFixture { readonly released: boolean; /** Register a cleanup that must confirm before the root may be removed. */ releaseAfter unknown>(cleanup: T): T; + /** + * Run cleanups now, in order; one that fails stays registered, so `release` + * retries it and keeps the root until it confirms. Failures are thrown as + * an AggregateError. + */ + closeEarly(cleanups: readonly (() => unknown)[]): Promise; /** * Keep the root for good: something that may still use it cannot be * confirmed stopped. Every later release fails with `reason`. diff --git a/scripts/lib/verifier-fixture.mjs b/scripts/lib/verifier-fixture.mjs index da0d9d58bb..dde26866c8 100644 --- a/scripts/lib/verifier-fixture.mjs +++ b/scripts/lib/verifier-fixture.mjs @@ -92,6 +92,26 @@ export class VerifierFixture { return cleanup; } + /** + * Close resources the verifier is done with before the fixture is released, + * such as an auxiliary MCP client. Each cleanup runs now, in order, even + * after an earlier one fails. One that fails stays registered, so `release` + * retries it and keeps the root until it confirms; the failures are thrown. + */ + async closeEarly(cleanups) { + const failures = []; + for (const cleanup of cleanups) { + const once = this.releaseAfter(onceConfirmed(cleanup)); + try { + await once(); + } catch (cause) { + failures.push(cause); + } + } + if (failures.length > 0) + throw new AggregateError(failures, "Verifier cleanup failed"); + } + /** * Keep the root for good: something that may still use its files cannot be * confirmed stopped and has no cleanup this fixture could retry (an owned diff --git a/scripts/verify-binary-layout.mjs b/scripts/verify-binary-layout.mjs index 428daf53f5..52e2ceb2ff 100644 --- a/scripts/verify-binary-layout.mjs +++ b/scripts/verify-binary-layout.mjs @@ -14,7 +14,6 @@ import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; -import { PrivateRuntimeRoot } from "../dist/process/PrivateRuntimeRoot.js"; import { OwnedCommandFailure, runOwnedCommand, @@ -27,6 +26,7 @@ import { import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs"; import { sectionNameFixtures } from "./lib/elf-section-name-fixtures.mjs"; import { withLargeResultMcp } from "./lib/large-result-mcp.mjs"; +import { VerifierFixture } from "./lib/verifier-fixture.mjs"; const verifyLargeMcp = process.env.REA_VERIFY_LARGE_ELF_MCP === "1"; const python = process.env.REA_PWNTOOLS_PYTHON; @@ -74,7 +74,7 @@ for (const command of [ } const entrypoint = process.argv[2] ?? fileURLToPath(new URL("./rea.mjs", import.meta.url)); -const root = await PrivateRuntimeRoot.create({ +const root = await VerifierFixture.open({ prefix: "rea-layout-verifier-", }); const environment = Object.fromEntries( @@ -1120,11 +1120,11 @@ try { cases++; } } finally { - try { - await limitedClient.close(); - } finally { - await limitedTransport.close(); - } + // A close that fails keeps the root until release confirms it. + await root.closeEarly([ + () => limitedClient.close(), + () => limitedTransport.close(), + ]); } } const unsupported = Buffer.from(object); @@ -1224,11 +1224,11 @@ try { } assert.deepEqual(await readFile(memoryHeavyPath), memoryHeavy); } finally { - try { - await memoryClient.close(); - } finally { - await memoryTransport.close(); - } + // A close that fails keeps the root until release confirms it. + await root.closeEarly([ + () => memoryClient.close(), + () => memoryTransport.close(), + ]); } // Extended section numbering keeps this valid while providing enough work // to reach the one-second CPU limit even on faster CI runners. @@ -1269,11 +1269,11 @@ try { } assert.deepEqual(await readFile(cpuHeavyPath), cpuHeavy); } finally { - try { - await cpuClient.close(); - } finally { - await cpuTransport.close(); - } + // A close that fails keeps the root until release confirms it. + await root.closeEarly([ + () => cpuClient.close(), + () => cpuTransport.close(), + ]); } for (const limit of [1024, 1]) { const fileEnvironment = limitedEnvironment(`--fsize=${limit}:`); @@ -1309,11 +1309,11 @@ try { } assert.deepEqual(await readFile(sectionHeavyPath), sectionHeavy); } finally { - try { - await fileClient.close(); - } finally { - await fileTransport.close(); - } + // A close that fails keeps the root until release confirms it. + await root.closeEarly([ + () => fileClient.close(), + () => fileTransport.close(), + ]); } } // EM_RISCV: AArch64 (183) is now an admitted profile. @@ -1424,11 +1424,11 @@ try { cases++; } } finally { - try { - await selectedClient.close(); - } finally { - await selectedTransport.close(); - } + // A close that fails keeps the root until release confirms it. + await root.closeEarly([ + () => selectedClient.close(), + () => selectedTransport.close(), + ]); } } for (const mode of ["cli", "mcp"]) { @@ -1493,16 +1493,13 @@ try { } catch (cause) { failures.push(cause); } finally { - for (const close of [ - () => client.close(), - () => transport.close(), - () => root.close(), - ]) { - try { - await close(); - } catch (cause) { - failures.push(cause); - } + // The root is removed only after the client and transport confirm. + root.releaseAfter(() => client.close()); + root.releaseAfter(() => transport.close()); + try { + await root.release(); + } catch (cause) { + failures.push(cause); } } const verifier = await completeVerifierRun(run); @@ -1520,9 +1517,13 @@ if (failures.length > 0) { 2, ), ); - throw new AggregateError( - failures, - "Offline binary layout verification failed; cleanup failures are retained.", + throw Object.assign( + new AggregateError( + failures, + "Offline binary layout verification failed; cleanup failures are retained.", + ), + // The caller can retry the retained root from the final failure. + root.released ? {} : { cleanupLeases: [root] }, ); } console.log( diff --git a/scripts/verify-deno-standalone.mjs b/scripts/verify-deno-standalone.mjs index f7ad4da889..9fea16ca36 100644 --- a/scripts/verify-deno-standalone.mjs +++ b/scripts/verify-deno-standalone.mjs @@ -5,6 +5,10 @@ import { join } from "node:path"; import { importVerifierSource } from "./lib/verifier-source-loader.mjs"; import { withVerifierCleanup } from "./lib/verifier-cleanup.mjs"; import { createVerifierCommands } from "./lib/verify-commands.mjs"; +import { + VerifierFixture, + withVerifierFixture, +} from "./lib/verifier-fixture.mjs"; /** Layout reference in docs/deno-standalone.md. Not "whatever deno is on PATH". */ const REQUIRED_DENO_VERSION = "2.9.8"; @@ -36,10 +40,6 @@ try { ); } -const { PrivateRuntimeRoot } = await importVerifierSource( - new URL("../src/process/PrivateRuntimeRoot.ts", import.meta.url).href, - import.meta.url, -); const { parseEvidence } = await importVerifierSource( new URL("../src/domain/evidence.ts", import.meta.url).href, import.meta.url, @@ -52,8 +52,9 @@ const longModuleText = 'export const long: string = "long";\n'; const sourceText = `import text from "./asset.txt" with { type: "text" };\nimport { long } from "./${longModule}";\nconsole.log(text, long);\n`; const assetText = "hello-asset\n"; const HOST_CARRIERS = { darwin: "mach-o", linux: "elf", win32: "pe" }; -const root = await PrivateRuntimeRoot.create({ prefix: "rea-deno-verifier-" }); -let commandsClosed = false; +const root = await VerifierFixture.open({ prefix: "rea-deno-verifier-" }); +// The root is removed only after the commands that use it confirm. +root.releaseAfter(() => commands.close()); const cli = async (binary) => JSON.parse( @@ -99,7 +100,7 @@ const member = (graph, suffix) => row.format_metadata.source_path?.endsWith(`/${suffix}`)), ); -await withVerifierCleanup(async () => { +await withVerifierFixture(root, async () => { const app = join(root.path, "main.ts"); const asset = join(root.path, "asset.txt"); const envFile = join(root.path, "app.env"); @@ -276,12 +277,4 @@ await withVerifierCleanup(async () => { 2, ), ); -}, [ - async () => { - await commands.close(); - commandsClosed = true; - }, - async () => { - if (commandsClosed) await root.close(); - }, -]); +}); diff --git a/scripts/verify-evm-interface.mjs b/scripts/verify-evm-interface.mjs index cec0526227..046faa7a4d 100644 --- a/scripts/verify-evm-interface.mjs +++ b/scripts/verify-evm-interface.mjs @@ -9,13 +9,13 @@ import { fileURLToPath, pathToFileURL } from "node:url"; import { promisify } from "node:util"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; -import { PrivateRuntimeRoot } from "../dist/process/PrivateRuntimeRoot.js"; import { parseEvidence } from "../dist/domain/evidence.js"; import { requireMcpOutput, requireMcpToolError, } from "./lib/mcp-verifier-results.mjs"; import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs"; +import { VerifierFixture } from "./lib/verifier-fixture.mjs"; const solcModule = process.env.REA_VERIFY_SOLC_MODULE; const strace = process.env.REA_VERIFY_STRACE_COMMAND; @@ -54,7 +54,7 @@ const environment = Object.fromEntries( Object.entries(process.env).filter(([, value]) => typeof value === "string"), ); const run = createVerifierRun(); -const root = await PrivateRuntimeRoot.create({ prefix: "rea-evm-verifier-" }); +const root = await VerifierFixture.open({ prefix: "rea-evm-verifier-" }); const client = new Client({ name: "evm-interface-verifier", version: "1" }); const transport = new StdioClientTransport({ command: process.execPath, @@ -369,11 +369,11 @@ try { cases++; } } finally { - try { - await limitedClient.close(); - } finally { - await limitedTransport.close(); - } + // A close that fails keeps the root until release confirms it. + await root.closeEarly([ + () => limitedClient.close(), + () => limitedTransport.close(), + ]); } } const brokenLimiter = join(root.path, "broken-limiter"); @@ -463,11 +463,11 @@ try { cases++; } } finally { - try { - await selectedClient.close(); - } finally { - await selectedTransport.close(); - } + // A close that fails keeps the root until release confirms it. + await root.closeEarly([ + () => selectedClient.close(), + () => selectedTransport.close(), + ]); } } for (const [name, encoding] of [ @@ -543,16 +543,13 @@ try { } catch (cause) { failures.push(cause); } finally { - for (const close of [ - () => client.close(), - () => transport.close(), - () => root.close(), - ]) { - try { - await close(); - } catch (cause) { - failures.push(cause); - } + // The root is removed only after the client and transport confirm. + root.releaseAfter(() => client.close()); + root.releaseAfter(() => transport.close()); + try { + await root.release(); + } catch (cause) { + failures.push(cause); } } const verifier = await completeVerifierRun(run); @@ -586,9 +583,13 @@ console.log( ), ); if (failures.length !== 0) - throw new AggregateError( - failures, - "Offline EVM interface verification failed; cleanup failures retained.", + throw Object.assign( + new AggregateError( + failures, + "Offline EVM interface verification failed; cleanup failures retained.", + ), + // The caller can retry the retained root from the final failure. + root.released ? {} : { cleanupLeases: [root] }, ); function permitsStdioSocketObservation(line, localDescriptors) { diff --git a/scripts/verify-pcap.mjs b/scripts/verify-pcap.mjs index 721db7b237..ae0f108a4e 100644 --- a/scripts/verify-pcap.mjs +++ b/scripts/verify-pcap.mjs @@ -1,19 +1,18 @@ import assert from "node:assert/strict"; import { execFile } from "node:child_process"; -import { mkdtemp, readdir, rmdir } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join, resolve } from "node:path"; +import { readdir } from "node:fs/promises"; +import { resolve } from "node:path"; import { promisify } from "node:util"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; import { toolContract } from "../dist/contracts/toolContracts.js"; import { ArtifactResourceScope } from "../dist/artifacts/ArtifactResourceScope.js"; import { requireMcpOutput } from "./lib/mcp-verifier-results.mjs"; -import { - closeVerifierResources, - withVerifierCleanup, -} from "./lib/verifier-cleanup.mjs"; import { hashVerifierFile } from "./lib/verifier-file-resources.mjs"; +import { + VerifierFixture, + withVerifierFixture, +} from "./lib/verifier-fixture.mjs"; const args = process.argv.slice(2); if (args.includes("--help") || args.includes("-h")) { @@ -27,7 +26,11 @@ if (!(args.length === 0 || (args.length === 2 && args[0] === "--pcap"))) const path = args[1] === undefined ? null : resolve(args[1]); // This parent is shared by independent CLI/MCP owners. A leased Windows // runtime root would prevent their native admission of its directory handle. -const root = await mkdtemp(join(tmpdir(), "rea-pcap-verifier-")); +const fixture = await VerifierFixture.open({ + prefix: "rea-pcap-verifier-", + private: false, +}); +const root = fixture.path; const resources = new ArtifactResourceScope(); const environment = Object.fromEntries( Object.entries(process.env).filter(([, value]) => typeof value === "string"), @@ -47,7 +50,18 @@ const transport = new StdioClientTransport({ stderr: "pipe", }); transport.stderr?.resume(); -await withVerifierCleanup(async () => { +fixture.releaseAfter(() => client.close()); +fixture.releaseAfter(() => transport.close()); +fixture.releaseAfter(() => resources.close()); +// The product releases what it created under TMPDIR; anything left fails. +fixture.releaseAfter(async () => + assert.deepEqual( + await readdir(root), + [], + `Owned capture resources remain under ${root}`, + ), +); +await withVerifierFixture(fixture, async () => { await client.connect(transport); const readiness = await run("inspect_pcap_client", {}); assert(readiness.client.tshark_version && readiness.client.capinfos_version); @@ -91,21 +105,7 @@ await withVerifierCleanup(async () => { `[ok] ${result.packet_count} packets; ${result.endpoint_count} addresses; coverage=${result.coverage}`, ); } -}, [ - async () => { - await closeVerifierResources([ - () => client.close(), - () => transport.close(), - () => resources.close(), - ]); - assert.deepEqual( - await readdir(root), - [], - `Owned capture resources remain under ${root}`, - ); - await rmdir(root); - }, -]); +}); console.log("All asserted pcap CLI/MCP checks passed"); async function run(name, input) { diff --git a/scripts/verify-web-network-captures.mjs b/scripts/verify-web-network-captures.mjs index 5ae44e81c3..3000634422 100644 --- a/scripts/verify-web-network-captures.mjs +++ b/scripts/verify-web-network-captures.mjs @@ -15,7 +15,6 @@ import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; -import { PrivateRuntimeRoot } from "../dist/process/PrivateRuntimeRoot.js"; import { runOwnedCommand } from "../dist/process/OwnedCommand.js"; import { WEB_NETWORK_CAPTURE_LIMITS } from "../dist/domain/webNetworkCapture.js"; import { @@ -24,6 +23,7 @@ import { requireMcpToolError, } from "./lib/mcp-verifier-results.mjs"; import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs"; +import { VerifierFixture } from "./lib/verifier-fixture.mjs"; const executable = process.env.REA_MITMDUMP_COMMAND; if ( @@ -38,7 +38,7 @@ await access(executable); const entrypoint = process.argv[2] ?? fileURLToPath(new URL("./rea.mjs", import.meta.url)); const run = createVerifierRun(); -const runtime = await PrivateRuntimeRoot.create({ +const runtime = await VerifierFixture.open({ prefix: "rea-history-verifier-", }); const environment = Object.fromEntries( @@ -893,16 +893,13 @@ try { } catch (cause) { failures.push(cause); } finally { - for (const cleanup of [ - () => client.close(), - () => transport.close(), - () => runtime.close(), - ]) { - try { - await cleanup(); - } catch (cause) { - failures.push(cause); - } + // The root is removed only after the client and transport confirm. + runtime.releaseAfter(() => client.close()); + runtime.releaseAfter(() => transport.close()); + try { + await runtime.release(); + } catch (cause) { + failures.push(cause); } } const verifier = await completeVerifierRun(run); @@ -920,9 +917,13 @@ if (failures.length > 0) { 2, ), ); - throw new AggregateError( - failures, - "Historical capture verification failed; analysis and cleanup failures are retained.", + throw Object.assign( + new AggregateError( + failures, + "Historical capture verification failed; analysis and cleanup failures are retained.", + ), + // The caller can retry the retained root from the final failure. + runtime.released ? {} : { cleanupLeases: [runtime] }, ); } console.log( diff --git a/scripts/verify/architecture-guard-baselines.json b/scripts/verify/architecture-guard-baselines.json index 890be7a11f..7f5d636fba 100644 --- a/scripts/verify/architecture-guard-baselines.json +++ b/scripts/verify/architecture-guard-baselines.json @@ -394,12 +394,9 @@ "scripts/verify-adb.mjs": 3, "scripts/verify-agentenv-environment.mjs": 1, "scripts/verify-binary-hardening.mjs": 1, - "scripts/verify-binary-layout.mjs": 1, "scripts/verify-browser-dom-destinations.mjs": 1, "scripts/verify-bun-standalone.mjs": 1, - "scripts/verify-deno-standalone.mjs": 1, "scripts/verify-elf-unwind-ranges.mjs": 1, - "scripts/verify-evm-interface.mjs": 1, "scripts/verify-flutter.mjs": 1, "scripts/verify-ghidra-bridge-sources.mjs": 1, "scripts/verify-ghidra-loader-relocation.mjs": 1, @@ -414,7 +411,6 @@ "scripts/verify-packaged-ghidra-windows.mjs": 1, "scripts/verify-packaged-wasm-artifact.mjs": 1, "scripts/verify-packet-capture.mjs": 1, - "scripts/verify-pcap.mjs": 1, "scripts/verify-pe-load-config.mjs": 1, "scripts/verify-published-package.mjs": 1, "scripts/verify-real-cutter.mjs": 1, @@ -432,7 +428,6 @@ "scripts/verify-test-discovery.mjs": 1, "scripts/verify-test-temp-hygiene.mjs": 1, "scripts/verify-wasm-interface-comparison.mjs": 1, - "scripts/verify-web-network-captures.mjs": 1, "scripts/verify-windows-native-inspection.mjs": 1, "scripts/verify-windows-native.mjs": 2, "scripts/verify-windows-package.mjs": 1 diff --git a/tests/process-global/verifierFixture.test.ts b/tests/process-global/verifierFixture.test.ts index 943b1a6cc8..a063179869 100644 --- a/tests/process-global/verifierFixture.test.ts +++ b/tests/process-global/verifierFixture.test.ts @@ -289,3 +289,44 @@ it("keeps a root on request after its cleanups confirm, and names it", async () /already being released/u, ); }); + +it("closes early resources now and keeps a failed one for release to retry", async () => { + let closes = 0; + const fixture = new VerifierFixture({ + path: "/fixture/early", + close: async () => { + closes += 1; + }, + }); + const calls: string[] = []; + let clientFails = true; + const failure: unknown = await fixture + .closeEarly([ + () => { + calls.push("client"); + if (clientFails) throw new Error("client did not close"); + }, + () => { + calls.push("transport"); + }, + ]) + .catch((cause: unknown) => cause); + // Both ran now, in order, and the client's failure is reported. + expect(calls).toEqual(["client", "transport"]); + expect(failure).toBeInstanceOf(AggregateError); + expect( + ((failure as AggregateError).errors as Error[]).map( + ({ message }) => message, + ), + ).toEqual(["client did not close"]); + // Release retries only the client and removes the root once it confirms. + await expect(fixture.release()).rejects.toMatchObject({ + cleanupLeases: [fixture], + }); + expect(closes).toBe(0); + clientFails = false; + await fixture.release(); + expect(calls).toEqual(["client", "transport", "client", "client"]); + expect(closes).toBe(1); + expect(fixture.released).toBe(true); +});