From b40a3bac1d657453c24ca528609ed2384ad6c379 Mon Sep 17 00:00:00 2001 From: Abhishek Sharma <78235887+abhisheksharma001@users.noreply.github.com> Date: Tue, 3 Nov 2026 02:34:38 +0530 Subject: [PATCH] fix(verifiers): keep the signal fixture root until its owners finish The signal recording verifier read /proc/self/limits after acquiring its private root but before owning its cleanup, and its cleanup closed the root even when the MCP client or transport refused to close. Read the limits before the root exists, construct the owners inside the cleanup scope, and release the root only after every owner closed and the verifier's process lineage verified; otherwise retain it and name it. Fixes #8561. Co-Authored-By: Claude Fable 5.1 --- scripts/verify-signal-recording.mjs | 77 ++++++++---- .../process/signalVerifierCleanup.test.ts | 116 ++++++++++++++++++ tests/fixtures/signalVerifierPreload.mjs | 86 +++++++++++++ 3 files changed, 255 insertions(+), 24 deletions(-) create mode 100644 tests/boundary/process/signalVerifierCleanup.test.ts create mode 100644 tests/fixtures/signalVerifierPreload.mjs diff --git a/scripts/verify-signal-recording.mjs b/scripts/verify-signal-recording.mjs index 61d3afd8311..3428f1743e7 100644 --- a/scripts/verify-signal-recording.mjs +++ b/scripts/verify-signal-recording.mjs @@ -17,8 +17,15 @@ 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 { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs"; +import { + closeVerifierResources, + withVerifierCleanup, +} from "./lib/verifier-cleanup.mjs"; +import { + assertVerifierCleanupComplete, + createVerifierRun, + completeVerifierRun, +} from "./lib/verifier-run.mjs"; if (process.platform !== "linux") throw new Error( @@ -31,6 +38,18 @@ if (!isAbsolute(process.env.REA_SIGROK_COMMAND ?? "")) const entrypoint = process.argv[2] ?? fileURLToPath(new URL("./rea.mjs", import.meta.url)); const run = createVerifierRun(); +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 root = await PrivateRuntimeRoot.create({ prefix: "rea-signal-verifier-", }); @@ -43,7 +62,10 @@ const environment = { TMP: root.path, TEMP: root.path, }; -const client = new Client({ name: "signal-recording-verifier", version: "1" }); +let client; +let transport; +let verifier; +let rootRemoved = false; // Real sigrok refusals of an unknown decoder ID must name the caller's field. const assertRejectedSelector = ( response, @@ -61,12 +83,6 @@ const assertRejectedSelector = ( assert.equal(error.details.captured_output.exit_code, expectedExitCode); return error; }; -const transport = new StdioClientTransport({ - command: process.execPath, - args: [entrypoint, "mcp"], - env: environment, - stderr: "pipe", -}); // Independently synthesize 9600-baud 8N1 samples at 1 MHz: bytes 52 45 41. const lines = [ "$timescale 1 us $end", @@ -93,20 +109,14 @@ 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 () => { + client = new Client({ name: "signal-recording-verifier", version: "1" }); + transport = new StdioClientTransport({ + command: process.execPath, + args: [entrypoint, "mcp"], + env: environment, + stderr: "pipe", + }); await writeFile(path, source); await client.connect(transport); const advertised = (await client.listTools()).tools.find( @@ -437,7 +447,26 @@ await withVerifierCleanup(async () => { "paged.vcd", "recording with spaces.vcd", ]); -}, [() => client.close(), () => transport.close(), () => root.close()]); +}, [ + async () => { + // Every owner this run acquired must finish before its root can go. A + // refused close or unverified lineage keeps the root and reports why. + await closeVerifierResources([ + () => client?.close(), + () => transport?.close(), + ]); + verifier = await completeVerifierRun(run); + assertVerifierCleanupComplete(verifier.process_lineage); + await root.close(); + rootRemoved = true; + }, +]).catch((cause) => { + if (rootRemoved) throw cause; + throw new Error( + `Signal recording verification failed; retained fixture root ${root.path} because its cleanup is incomplete.`, + { cause }, + ); +}); console.log( JSON.stringify( { @@ -449,7 +478,7 @@ console.log( host: `${process.platform}/${process.arch}`, scope: "real sigrok UART VCD through compiled CLI/MCP; no physical acquisition", - ...(await completeVerifierRun(run)), + ...verifier, }, null, 2, diff --git a/tests/boundary/process/signalVerifierCleanup.test.ts b/tests/boundary/process/signalVerifierCleanup.test.ts new file mode 100644 index 00000000000..8fdb353167e --- /dev/null +++ b/tests/boundary/process/signalVerifierCleanup.test.ts @@ -0,0 +1,116 @@ +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 { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { + processTest, + type TestProcesses, +} from "../../support/process/processFixture.js"; + +const preload = fileURLToPath( + new URL("../../fixtures/signalVerifierPreload.mjs", import.meta.url), +); +const script = fileURLToPath( + new URL("../../../scripts/verify-signal-recording.mjs", import.meta.url), +); + +/** Run the real verifier with authored owners; only its cleanup ownership is observed. */ +const runScenario = async (processes: TestProcesses, scenario: string) => { + const root = await createTestTempDirectory("rea-signal-verifier-cleanup-"); + const temporary = join(root, "temporary"); + await mkdir(temporary); + const log = join(root, "events.log"); + const result = await processes.run( + process.execPath, + ["--import", preload, script], + { + env: { + ...process.env, + TMPDIR: temporary, + TMP: temporary, + TEMP: temporary, + REA_SIGROK_COMMAND: join(root, "authored-absent-sigrok-cli"), + REA_SIGNAL_TEST_EVENTS: log, + REA_SIGNAL_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); + return { + ...result, + events: recorded.map((line) => line.split(" ")[0]), + rootRetained: fixtureRoot !== undefined && existsSync(fixtureRoot), + }; +}; + +processTest( + "acquires no fixture root before the inherited limits are readable", + async ({ processes }) => { + const outcome = await runScenario(processes, "limits-unreadable"); + expect(outcome.exitCode).toBe(1); + expect(outcome.stderr).toContain("authored limits read failure"); + expect(outcome.events).toEqual([]); + }, +); + +processTest( + "releases the root only after its owners closed and lineage was verified", + async ({ processes }) => { + const outcome = await runScenario(processes, "owners-released"); + expect(outcome.exitCode).toBe(1); + expect(outcome.stderr).toContain("authored connection failure"); + expect(outcome.stderr).not.toContain("retained fixture root"); + expect(outcome.events).toEqual([ + "root-create", + "client-close", + "transport-close", + "lineage-observe", + "root-close", + ]); + expect(outcome.rootRetained).toBe(false); + }, +); + +processTest.for(["client", "transport"])( + "retains the root and the other owner's close when the %s close is refused", + async (owner, { processes }) => { + const outcome = await runScenario(processes, `${owner}-close-refused`); + expect(outcome.exitCode).toBe(1); + expect(outcome.stderr).toContain("authored connection failure"); + expect(outcome.stderr).toContain(`authored ${owner} close failure`); + expect(outcome.stderr).toContain("retained fixture root"); + expect(outcome.events).toEqual([ + "root-create", + "client-close", + "transport-close", + ]); + 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.exitCode).toBe(1); + expect(outcome.stderr).toContain("authored lineage unavailability"); + expect(outcome.stderr).toContain("retained fixture root"); + expect(outcome.events).toEqual([ + "root-create", + "client-close", + "transport-close", + "lineage-observe", + ]); + expect(outcome.rootRetained).toBe(true); + }, +); diff --git a/tests/fixtures/signalVerifierPreload.mjs b/tests/fixtures/signalVerifierPreload.mjs new file mode 100644 index 00000000000..4a68cc7234b --- /dev/null +++ b/tests/fixtures/signalVerifierPreload.mjs @@ -0,0 +1,86 @@ +// Runs the real signal recording verifier on any host. Its Linux guard, the +// /proc limits read, the private fixture root, the MCP owners and the lineage +// observation are authored here, so the verification fails at connection and +// only the verifier's cleanup ownership is observed, in order, in an event log. +import { registerHooks } from "node:module"; + +Object.defineProperty(process, "platform", { value: "linux" }); + +const script = "/scripts/verify-signal-recording.mjs"; +const scenario = process.env.REA_SIGNAL_TEST_SCENARIO ?? ""; +const events = JSON.stringify(process.env.REA_SIGNAL_TEST_EVENTS); +const realVerifierRun = JSON.stringify( + new URL("../../scripts/lib/verifier-run.mjs", import.meta.url).href, +); +const record = (kind) => + `appendFileSync(${events}, ${JSON.stringify(kind)} + "\\n");`; +const refusal = (owner) => + scenario === `${owner}-close-refused` + ? `throw new Error("authored ${owner} close failure");` + : ""; +const limits = + scenario === "limits-unreadable" + ? 'Promise.reject(new Error("authored limits read failure"))' + : 'Promise.resolve("Limit Soft Limit Hard Limit Units\\nMax address space unlimited unlimited bytes\\n")'; +const lineage = + scenario === "lineage-unverified" + ? '{ status: "unavailable", reason: "authored lineage unavailability", descendants: [] }' + : '{ status: "verified", descendants: [] }'; + +const mocks = { + "node:fs/promises": `import * as fs from "node:fs/promises"; +export const { writeFile, readdir } = fs; +export const readFile = (path, ...rest) => + path === "/proc/self/limits" ? ${limits} : fs.readFile(path, ...rest);`, + "../dist/process/PrivateRuntimeRoot.js": `import { appendFileSync } from "node:fs"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +export class PrivateRuntimeRoot { + static async create({ prefix }) { + const path = await mkdtemp(join(tmpdir(), prefix)); + appendFileSync(${events}, "root-create " + path + "\\n"); + return new PrivateRuntimeRoot(path); + } + constructor(path) { this.path = path; } + async close() { + ${record("root-close")} + await rm(this.path, { recursive: true, force: true }); + } +}`, + "../dist/domain/evidence.js": + "export const parseEvidence = (value) => value;", + "./lib/mcp-verifier-results.mjs": `const unreachable = () => { + throw new Error("The authored connection never succeeds"); +}; +export const requireMcpEvidenceBundle = unreachable; +export const requireMcpOutput = unreachable; +export const requireMcpToolError = unreachable;`, + "./lib/verifier-run.mjs": `import { appendFileSync } from "node:fs"; +export { assertVerifierCleanupComplete } from ${realVerifierRun}; +export const createVerifierRun = () => ({ run_id: "authored-run" }); +export const completeVerifierRun = async (run) => { + ${record("lineage-observe")} + return { ...run, process_lineage: ${lineage} }; +};`, + "@modelcontextprotocol/client": `import { appendFileSync } from "node:fs"; +export class Client { + async connect() { throw new Error("authored connection failure"); } + async close() { ${record("client-close")} ${refusal("client")} } +}`, + "@modelcontextprotocol/client/stdio": `import { appendFileSync } from "node:fs"; +export class StdioClientTransport { + async close() { ${record("transport-close")} ${refusal("transport")} } +}`, +}; + +registerHooks({ + resolve(specifier, context, next) { + if (context.parentURL?.endsWith(script) && Object.hasOwn(mocks, specifier)) + return { + shortCircuit: true, + url: "data:text/javascript," + encodeURIComponent(mocks[specifier]), + }; + return next(specifier, context); + }, +});