From 131b2d85f344cbaac0160480244bea5f1bf8fd80 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:40:53 +0800 Subject: [PATCH] fix(verify): report every owned cleanup failure before success Use one cleanup owner across browser, Frida and JEB verifiers. Attempt all independent cleanup tasks, retain primary failures, and emit successful reports only after cleanup. Remove the superseded browser helper and duplicate tests. Validation: cleanup aggregation and runtime fixture tests, build and static checks passed. No real Frida or JEB run. --- scripts/lib/browser-module-e2e.mjs | 4 +- scripts/lib/browser-nested-source-map-e2e.mjs | 8 +-- scripts/lib/browser-network-e2e.mjs | 4 +- scripts/lib/browser-runtime-e2e.mjs | 8 +-- .../lib/browser-runtime-fixture-lifecycle.mjs | 17 ------ .../lib/browser-runtime-termination-e2e.mjs | 4 +- scripts/lib/browser-script-export-e2e.mjs | 4 +- scripts/lib/browser-source-map-e2e.mjs | 4 +- scripts/lib/verifier-cleanup.d.mts | 5 ++ scripts/lib/verifier-cleanup.mjs | 16 ++++++ scripts/verify-browser-dom-destinations.mjs | 4 +- scripts/verify-frida.mjs | 28 ++++++++-- scripts/verify-jeb.mjs | 38 ++++++++++--- scripts/verify-real-browser.mjs | 4 +- .../browser/runtimeFixtureCleanup.test.ts | 32 ----------- .../boundary/process/verifierCleanup.test.ts | 56 +++++++++++++++++++ 16 files changed, 148 insertions(+), 88 deletions(-) create mode 100644 scripts/lib/verifier-cleanup.d.mts create mode 100644 scripts/lib/verifier-cleanup.mjs create mode 100644 tests/boundary/process/verifierCleanup.test.ts diff --git a/scripts/lib/browser-module-e2e.mjs b/scripts/lib/browser-module-e2e.mjs index 5c6a154f31..dddeb5e817 100644 --- a/scripts/lib/browser-module-e2e.mjs +++ b/scripts/lib/browser-module-e2e.mjs @@ -14,7 +14,7 @@ import { webModuleTraceResultSchema } from "../../dist/domain/webModuleTrace.js" import { NativeModuleResolver } from "../../dist/browser/modules/NativeModuleResolver.js"; import { startBrowserModuleSite } from "../fixtures/browser-module-site.mjs"; import { mcpTextValue } from "./mcp-verifier-results.mjs"; -import { closeRuntimeFixtureResources } from "./browser-runtime-fixture-lifecycle.mjs"; +import { closeVerifierResources } from "./verifier-cleanup.mjs"; /** Check native URL resolution against an independently loaded module graph through CLI/MCP. */ export async function verifyBrowserModules( @@ -314,7 +314,7 @@ export async function verifyBrowserModules( if (site !== undefined) cleanup.push(() => site.close()); if (root !== undefined) cleanup.push(() => rm(root, { recursive: true, force: true })); - await closeRuntimeFixtureResources(cleanup, primaryError); + await closeVerifierResources(cleanup, primaryError); } } diff --git a/scripts/lib/browser-nested-source-map-e2e.mjs b/scripts/lib/browser-nested-source-map-e2e.mjs index d7bac5ee22..20ef4effe1 100644 --- a/scripts/lib/browser-nested-source-map-e2e.mjs +++ b/scripts/lib/browser-nested-source-map-e2e.mjs @@ -10,10 +10,8 @@ import { CdpConnection } from "../../dist/browser/CdpConnection.js"; import { parseEvidence } from "../../dist/domain/evidence.js"; import { traceSourceMap } from "../../dist/javascript/sourceMaps/TraceSourceMap.js"; import { startBrowserSourceMapSite } from "../fixtures/browser-source-map-site.mjs"; -import { - startRuntimeFixtureBrowser, - closeRuntimeFixtureResources, -} from "./browser-runtime-fixture-lifecycle.mjs"; +import { startRuntimeFixtureBrowser } from "./browser-runtime-fixture-lifecycle.mjs"; +import { closeVerifierResources } from "./verifier-cleanup.mjs"; const cases = [ { name: "same-line nested column control", prefixes: ["\n\n ", " "] }, @@ -184,7 +182,7 @@ async function verifyCase(executable, entrypoint, scenario) { primaryError = failure; throw failure; } finally { - await closeRuntimeFixtureResources( + await closeVerifierResources( [() => connection?.close(), () => fixture?.close(), () => site.close()], primaryError, ); diff --git a/scripts/lib/browser-network-e2e.mjs b/scripts/lib/browser-network-e2e.mjs index 33aad95c77..2e8ba8411b 100644 --- a/scripts/lib/browser-network-e2e.mjs +++ b/scripts/lib/browser-network-e2e.mjs @@ -14,7 +14,7 @@ import { } from "../fixtures/browser-network-site.mjs"; import { assertBrowserNetworkEvidence } from "./browser-network-assertions.mjs"; import { scenarioProfiles } from "./browser-scenario-verifier.mjs"; -import { closeRuntimeFixtureResources } from "./browser-runtime-fixture-lifecycle.mjs"; +import { closeVerifierResources } from "./verifier-cleanup.mjs"; /** Exercise selected network content through real CLI and stdio MCP, one browser at a time. */ export async function verifyBrowserNetworkEvidence( @@ -186,6 +186,6 @@ export async function verifyBrowserNetworkEvidence( if (client !== undefined) cleanup.push(() => client.close()); if (transport !== undefined) cleanup.push(() => transport.close()); if (site !== undefined) cleanup.push(() => site.close()); - await closeRuntimeFixtureResources(cleanup, primaryError); + await closeVerifierResources(cleanup, primaryError); } } diff --git a/scripts/lib/browser-runtime-e2e.mjs b/scripts/lib/browser-runtime-e2e.mjs index 78d9d46158..f0c69843a9 100644 --- a/scripts/lib/browser-runtime-e2e.mjs +++ b/scripts/lib/browser-runtime-e2e.mjs @@ -16,10 +16,8 @@ import { webEventListenersSchema } from "../../dist/domain/webEventListeners.js" import { startBrowserRuntimeSite } from "../fixtures/browser-runtime-site.mjs"; import { requireOrdinaryWindowsVerifierCaller } from "./windows-ordinary-user-verifier.mjs"; import { mcpTextValue } from "./mcp-verifier-results.mjs"; -import { - startRuntimeFixtureBrowser, - closeRuntimeFixtureResources, -} from "./browser-runtime-fixture-lifecycle.mjs"; +import { startRuntimeFixtureBrowser } from "./browser-runtime-fixture-lifecycle.mjs"; +import { closeVerifierResources } from "./verifier-cleanup.mjs"; import { verifyRuntimeTargetTermination } from "./browser-runtime-termination-e2e.mjs"; import { verifyRuntimeInvalidSelectors } from "./browser-runtime-selector-e2e.mjs"; import { @@ -182,7 +180,7 @@ export async function verifyBrowserRuntime( primaryError = cause; throw cause; } finally { - await closeRuntimeFixtureResources( + await closeVerifierResources( [ () => client?.close(), () => action?.close(), diff --git a/scripts/lib/browser-runtime-fixture-lifecycle.mjs b/scripts/lib/browser-runtime-fixture-lifecycle.mjs index 78709646d4..3205e34112 100644 --- a/scripts/lib/browser-runtime-fixture-lifecycle.mjs +++ b/scripts/lib/browser-runtime-fixture-lifecycle.mjs @@ -153,20 +153,3 @@ async function stopOwnedBrowser(owned, profile) { await new Promise((resolve) => setTimeout(resolve, 25)); } } - -/** Attempt every owned cleanup and retain the original verification failure if cleanup also fails. */ -export async function closeRuntimeFixtureResources(tasks, primaryError) { - const failures = []; - for (const task of tasks) { - try { - await task(); - } catch (cause) { - failures.push(cause); - } - } - if (failures.length > 0) - throw new AggregateError( - primaryError === undefined ? failures : [primaryError, ...failures], - "Runtime fixture cleanup failed", - ); -} diff --git a/scripts/lib/browser-runtime-termination-e2e.mjs b/scripts/lib/browser-runtime-termination-e2e.mjs index a4ad9097e2..52dd90d5ac 100644 --- a/scripts/lib/browser-runtime-termination-e2e.mjs +++ b/scripts/lib/browser-runtime-termination-e2e.mjs @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { CdpConnection } from "../../dist/browser/CdpConnection.js"; import { mcpTextValue } from "./mcp-verifier-results.mjs"; -import { closeRuntimeFixtureResources } from "./browser-runtime-fixture-lifecycle.mjs"; +import { closeVerifierResources } from "./verifier-cleanup.mjs"; /** Close a separately owned fixture page after actual MCP arming and inspect native cleanup diagnostics. */ export async function verifyRuntimeTargetTermination(client, input, origin) { @@ -76,7 +76,7 @@ export async function verifyRuntimeTargetTermination(client, input, origin) { primaryError = cause; throw cause; } finally { - await closeRuntimeFixtureResources( + await closeVerifierResources( [ () => targetId === undefined || closed diff --git a/scripts/lib/browser-script-export-e2e.mjs b/scripts/lib/browser-script-export-e2e.mjs index ce1599e179..818d660de2 100644 --- a/scripts/lib/browser-script-export-e2e.mjs +++ b/scripts/lib/browser-script-export-e2e.mjs @@ -15,7 +15,7 @@ import { assertExportedScriptAnalysis, } from "./browser-script-export-assertions.mjs"; import { mcpTextValue } from "./mcp-verifier-results.mjs"; -import { closeRuntimeFixtureResources } from "./browser-runtime-fixture-lifecycle.mjs"; +import { closeVerifierResources } from "./verifier-cleanup.mjs"; /** Verify capture → export → existing analysis through real CLI and stdio MCP. */ export async function verifyBrowserScriptExport( @@ -119,7 +119,7 @@ export async function verifyBrowserScriptExport( if (site !== undefined) cleanup.push(() => site.close()); if (root !== undefined) cleanup.push(() => rm(root, { recursive: true, force: true })); - await closeRuntimeFixtureResources(cleanup, primaryError); + await closeVerifierResources(cleanup, primaryError); } } diff --git a/scripts/lib/browser-source-map-e2e.mjs b/scripts/lib/browser-source-map-e2e.mjs index 635dcb5e6e..74cc595ce6 100644 --- a/scripts/lib/browser-source-map-e2e.mjs +++ b/scripts/lib/browser-source-map-e2e.mjs @@ -12,7 +12,7 @@ import { parseEvidence } from "../../dist/domain/evidence.js"; import { webSourceLocationResultSchema } from "../../dist/domain/webSourceLocation.js"; import { startBrowserSourceMapSite } from "../fixtures/browser-source-map-site.mjs"; import { mcpTextValue } from "./mcp-verifier-results.mjs"; -import { closeRuntimeFixtureResources } from "./browser-runtime-fixture-lifecycle.mjs"; +import { closeVerifierResources } from "./verifier-cleanup.mjs"; /** Verify real Chrome capture → export → original compiler point through packaged CLI/MCP. */ export async function verifyBrowserSourceMap( @@ -142,7 +142,7 @@ export async function verifyBrowserSourceMap( if (site !== undefined) cleanup.push(() => site.close()); if (root !== undefined) cleanup.push(() => rm(root, { recursive: true, force: true })); - await closeRuntimeFixtureResources(cleanup, primaryError); + await closeVerifierResources(cleanup, primaryError); } } function assertPoint(evidence, { site, input, capture, identity }) { diff --git a/scripts/lib/verifier-cleanup.d.mts b/scripts/lib/verifier-cleanup.d.mts new file mode 100644 index 0000000000..732b4ca687 --- /dev/null +++ b/scripts/lib/verifier-cleanup.d.mts @@ -0,0 +1,5 @@ +/** Attempt every owned cleanup and retain the primary verification failure. */ +export function closeVerifierResources( + tasks: readonly (() => void | Promise)[], + primaryError?: unknown, +): Promise; diff --git a/scripts/lib/verifier-cleanup.mjs b/scripts/lib/verifier-cleanup.mjs new file mode 100644 index 0000000000..1a257f9ce7 --- /dev/null +++ b/scripts/lib/verifier-cleanup.mjs @@ -0,0 +1,16 @@ +/** Attempt every owned cleanup and retain the primary verification failure. */ +export async function closeVerifierResources(tasks, primaryError) { + const failures = []; + for (const task of tasks) { + try { + await task(); + } catch (cause) { + failures.push(cause); + } + } + if (failures.length > 0) + throw new AggregateError( + primaryError === undefined ? failures : [primaryError, ...failures], + "Verifier cleanup failed", + ); +} diff --git a/scripts/verify-browser-dom-destinations.mjs b/scripts/verify-browser-dom-destinations.mjs index 78ee670cbe..e753eee0c4 100644 --- a/scripts/verify-browser-dom-destinations.mjs +++ b/scripts/verify-browser-dom-destinations.mjs @@ -6,7 +6,7 @@ import { join } from "node:path"; import { chromium } from "playwright-core"; import { startBrowserVerifierSite } from "./fixtures/browser-verifier-site.mjs"; import { verifyBrowserDomDestinations } from "./lib/browser-dom-destinations-e2e.mjs"; -import { closeRuntimeFixtureResources } from "./lib/browser-runtime-fixture-lifecycle.mjs"; +import { closeVerifierResources } from "./lib/verifier-cleanup.mjs"; import { requireOrdinaryWindowsVerifierCaller } from "./lib/windows-ordinary-user-verifier.mjs"; const executable = process.env.REA_BROWSER_EXECUTABLE; @@ -65,6 +65,6 @@ try { if (site !== undefined) cleanup.push(() => site.close()); if (profile !== undefined) cleanup.push(() => rm(profile, { recursive: true, force: true })); - await closeRuntimeFixtureResources(cleanup, primaryError); + await closeVerifierResources(cleanup, primaryError); } process.stdout.write(`${JSON.stringify(report)}\n`); diff --git a/scripts/verify-frida.mjs b/scripts/verify-frida.mjs index ece121a14a..36832c4117 100644 --- a/scripts/verify-frida.mjs +++ b/scripts/verify-frida.mjs @@ -3,6 +3,7 @@ import { basename } from "node:path"; import { FridaInstrumentationService } from "../dist/application/frida/FridaInstrumentationService.js"; import { FridaInstrumentationManager } from "../dist/frida/FridaInstrumentationManager.js"; import { isValidFridaRemoteAddress } from "../dist/contracts/fridaRemoteAddress.js"; +import { closeVerifierResources } from "./lib/verifier-cleanup.mjs"; const options = parseArguments(process.argv.slice(2)); if (options.help) { @@ -30,6 +31,8 @@ const remote = : { keepaliveInterval: options.keepaliveInterval }), }; +let primaryFailure; +let report; try { let targetProcess; let input; @@ -92,7 +95,7 @@ try { ); if (!messageObserved) throw new Error("Selected target did not return the Frida marker"); - const report = { + report = { ok: result.value.cleanupError === null, provider: "frida", host_platform: process.platform, @@ -103,15 +106,28 @@ try { script_message_observed: true, cleanup_error: result.value.cleanupError, }; - process.stdout.write(`${JSON.stringify(report)}\n`); - if (!report.ok) process.exitCode = 1; } catch (cause) { + primaryFailure = cause; +} + +let cleanupFailure; +try { + await closeVerifierResources([() => manager.closeAll()], primaryFailure); +} catch (cause) { + cleanupFailure = cause; +} +if (primaryFailure !== undefined || cleanupFailure !== undefined) { + const causes = + cleanupFailure instanceof AggregateError + ? cleanupFailure.errors + : [cleanupFailure ?? primaryFailure]; process.stderr.write( - `Frida real-provider verification failed: ${cause instanceof Error ? cause.message : String(cause)}\n`, + `Frida real-provider verification failed: ${causes.map((cause) => (cause instanceof Error ? cause.message : String(cause))).join("; ")}\n`, ); process.exitCode = 1; -} finally { - await manager.closeAll().catch(() => undefined); +} else { + process.stdout.write(`${JSON.stringify(report)}\n`); + if (!report.ok) process.exitCode = 1; } function parseArguments(args) { diff --git a/scripts/verify-jeb.mjs b/scripts/verify-jeb.mjs index addba3f33a..87f676a5c5 100644 --- a/scripts/verify-jeb.mjs +++ b/scripts/verify-jeb.mjs @@ -6,6 +6,7 @@ import { promisify } from "node:util"; import { JebAnalysisService } from "../dist/application/jeb/JebAnalysisService.js"; import { createJebAnalysisProvider } from "../dist/composition/jeb.js"; +import { closeVerifierResources } from "./lib/verifier-cleanup.mjs"; const run = promisify(execFile); @@ -25,8 +26,11 @@ if (options.help) { const provider = createJebAnalysisProvider(process.env); const service = new JebAnalysisService(provider); -const fixtureDirectory = await mkdtemp(join(tmpdir(), "rea-jeb-")); +let fixtureDirectory; +let primaryFailure; +let report; try { + fixtureDirectory = await mkdtemp(join(tmpdir(), "rea-jeb-")); const client = await service.execute("inspect_jeb_client", {}); if (!client.ok) throw new Error(client.error.message); const engine = client.value.normalized_result.engine; @@ -70,7 +74,7 @@ try { "Decompiled pseudo-code did not contain the fixture marker", ); - const report = { + report = { ok: true, provider: "jeb", host_platform: process.platform, @@ -82,17 +86,33 @@ try { unit_count: units.value.normalized_result.units.length, decompiled_marker_observed: true, }; - process.stdout.write(`${JSON.stringify(report)}\n`); } catch (cause) { + primaryFailure = cause; +} + +const cleanupTasks = [ + () => provider.close(), + ...(fixtureDirectory === undefined + ? [] + : [() => rm(fixtureDirectory, { recursive: true, force: true })]), +]; +let cleanupFailure; +try { + await closeVerifierResources(cleanupTasks, primaryFailure); +} catch (cause) { + cleanupFailure = cause; +} +if (primaryFailure !== undefined || cleanupFailure !== undefined) { + const causes = + cleanupFailure instanceof AggregateError + ? cleanupFailure.errors + : [cleanupFailure ?? primaryFailure]; process.stderr.write( - `JEB real-provider verification failed: ${cause instanceof Error ? cause.message : String(cause)}\n`, + `JEB real-provider verification failed: ${causes.map((cause) => (cause instanceof Error ? cause.message : String(cause))).join("; ")}\n`, ); process.exitCode = 1; -} finally { - await provider.close().catch(() => undefined); - await rm(fixtureDirectory, { recursive: true, force: true }).catch( - () => undefined, - ); +} else { + process.stdout.write(`${JSON.stringify(report)}\n`); } async function resolveTarget(selected, directory) { diff --git a/scripts/verify-real-browser.mjs b/scripts/verify-real-browser.mjs index 28c53067ad..f412aafddc 100644 --- a/scripts/verify-real-browser.mjs +++ b/scripts/verify-real-browser.mjs @@ -39,7 +39,7 @@ import { verifyScenarioFailureEvidence, } from "./lib/browser-scenario-verifier.mjs"; import { completeVerifierRun, createVerifierRun } from "./lib/verifier-run.mjs"; -import { closeRuntimeFixtureResources } from "./lib/browser-runtime-fixture-lifecycle.mjs"; +import { closeVerifierResources } from "./lib/verifier-cleanup.mjs"; import { requireOrdinaryWindowsVerifierCaller } from "./lib/windows-ordinary-user-verifier.mjs"; import { verifyLargeScreenshotE2e } from "./lib/browser-screenshot-e2e.mjs"; import { verifyPopupEventCoverage } from "./lib/browser-popup-e2e.mjs"; @@ -447,7 +447,7 @@ try { retryDelay: 100, }), ); - await closeRuntimeFixtureResources(cleanup, verificationError); + await closeVerifierResources(cleanup, verificationError); } process.stdout.write( `${JSON.stringify({ diff --git a/tests/boundary/browser/runtimeFixtureCleanup.test.ts b/tests/boundary/browser/runtimeFixtureCleanup.test.ts index 4aafa317f9..49dafe3567 100644 --- a/tests/boundary/browser/runtimeFixtureCleanup.test.ts +++ b/tests/boundary/browser/runtimeFixtureCleanup.test.ts @@ -44,35 +44,3 @@ it.skipIf(process.platform === "win32")( expect(await readdir(root)).toEqual([]); }, ); - -it("attempts every fixture cleanup and retains the primary capture failure", async () => { - const helper = new URL( - "../../../scripts/lib/browser-runtime-fixture-lifecycle.mjs", - import.meta.url, - ); - const script = ` - const { closeRuntimeFixtureResources } = await import(${JSON.stringify(helper.href)}); - const seen = []; - try { - await closeRuntimeFixtureResources([ - async () => { seen.push('first'); throw new Error('first cleanup failed'); }, - async () => { seen.push('second'); throw new Error('second cleanup failed'); }, - async () => { seen.push('third'); }, - ], new Error('capture failed')); - throw new Error('Expected aggregate cleanup failure'); - } catch (error) { - if (!(error instanceof AggregateError)) throw error; - console.log(JSON.stringify({ seen, errors: error.errors.map(item => item.message) })); - } - `; - const { stdout } = await promisify(execFile)( - process.execPath, - ["--input-type=module", "-e", script], - { timeout: 10_000 }, - ); - const result: unknown = JSON.parse(stdout); - expect(result).toEqual({ - seen: ["first", "second", "third"], - errors: ["capture failed", "first cleanup failed", "second cleanup failed"], - }); -}); diff --git a/tests/boundary/process/verifierCleanup.test.ts b/tests/boundary/process/verifierCleanup.test.ts new file mode 100644 index 0000000000..d0bb9bda8f --- /dev/null +++ b/tests/boundary/process/verifierCleanup.test.ts @@ -0,0 +1,56 @@ +import { expect, it } from "vitest"; +import { closeVerifierResources } from "../../../scripts/lib/verifier-cleanup.mjs"; + +it("attempts every cleanup and reports failures when there is no primary error", async () => { + const attempted: string[] = []; + const tasks = [ + async () => { + attempted.push("provider"); + throw new Error("provider close failed"); + }, + async () => { + attempted.push("fixture"); + }, + ]; + + let failure: unknown; + try { + await closeVerifierResources(tasks); + } catch (cause) { + failure = cause; + } + expect(failure).toBeInstanceOf(AggregateError); + expect( + (failure as AggregateError).errors.map((error) => error.message), + ).toEqual(["provider close failed"]); + expect(attempted).toEqual(["provider", "fixture"]); +}); + +it("retains primary verification and independent cleanup failures", async () => { + const primary = new Error("analysis assertion failed"); + const attempted: string[] = []; + const tasks = [ + async () => { + attempted.push("provider"); + throw new Error("provider close failed"); + }, + async () => { + attempted.push("fixture"); + throw new Error("fixture removal failed"); + }, + ]; + + let failure: unknown; + try { + await closeVerifierResources(tasks, primary); + } catch (cause) { + failure = cause; + } + expect(failure).toBeInstanceOf(AggregateError); + expect((failure as AggregateError).errors).toEqual([ + primary, + expect.objectContaining({ message: "provider close failed" }), + expect.objectContaining({ message: "fixture removal failed" }), + ]); + expect(attempted).toEqual(["provider", "fixture"]); +});