From 724211b4682cfcf3e7b97a760661fc0dfb9e0bf4 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:46:42 -0500 Subject: [PATCH] test: finish independent session cleanup and completion gates --- .../cli/commandEvidenceDelivery.test.ts | 8 +- .../directAnalysisSnapshotReplay.test.ts | 258 +++++++++--------- .../boundary/mcp/pcapParentIssuePath.test.ts | 16 +- .../mcp/pcapWorkspaceAllocation.test.ts | 27 +- .../mcp/processCleanupRetryMcp.test.ts | 71 +++-- tests/boundary/mcp/promptMcp.test.ts | 71 +++-- .../mcp/pseudocodeDeliveryMcp.test.ts | 8 +- 7 files changed, 269 insertions(+), 190 deletions(-) diff --git a/tests/boundary/cli/commandEvidenceDelivery.test.ts b/tests/boundary/cli/commandEvidenceDelivery.test.ts index f6bcf935a4..bd886e925d 100644 --- a/tests/boundary/cli/commandEvidenceDelivery.test.ts +++ b/tests/boundary/cli/commandEvidenceDelivery.test.ts @@ -1,3 +1,4 @@ +import { closeVerifierResources } from "../../../scripts/lib/verifier-cleanup.mjs"; import { Cli, z } from "incur"; import pino from "pino"; import { readFile, readdir, writeFile } from "node:fs/promises"; @@ -76,10 +77,9 @@ const fixture = async (kind: (typeof kinds)[number]) => { createManager, () => service, ); - onTestFinished(async () => { - await service.close(); - await manager.closeAll(); - }); + onTestFinished(() => + closeVerifierResources([() => service.close(), () => manager.closeAll()]), + ); const commandName = kind === "gdb" ? CLI_COMMANDS.debugWithGdb diff --git a/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts b/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts index 1ad3db81fb..b9457274a3 100644 --- a/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts +++ b/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts @@ -1,3 +1,4 @@ +import { withVerifierCleanup } from "../../../scripts/lib/verifier-cleanup.mjs"; import { parseEvidenceBundle } from "../../../src/domain/evidenceBundle.js"; import { EvidenceIntegrityError } from "../../../src/domain/evidenceErrors.js"; import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; @@ -1670,132 +1671,141 @@ const expectProfilelessWorkflowSnapshotRejections = async ( snapshot: AnalysisSnapshot, ): Promise => { const session = createTestBinarySession(provider); - const opened = await session.open(path); - if (!opened.ok) throw opened.error; - const readerIdentity = APPLE_DISPATCH_METADATA_ANALYSIS_PROFILE.provider; - const wrongReader = structuredClone(snapshot); - // Reader v2 could discard owners when their adopted-protocol list was unreadable. - const otherReaderProfile = createAnalysisProfile( - { ...readerIdentity, version: "apple-metadata-reader-2" }, - { - operation: "inspect_native_dispatch_metadata", - reader: "apple-dispatch-metadata", - revision: 2, - }, - ); - wrongReader.binding = { - provider: otherReaderProfile.provider, - analysis_profile: otherReaderProfile, - }; - const rejectedReader = session.importAnalysisSnapshot(wrongReader); - expect(rejectedReader.ok).toBe(false); - if (!rejectedReader.ok) { - expectReaderBindingMismatch( - rejectedReader.error, - readerIdentity.version, - otherReaderProfile.digest, - ); - } - - expectProfilelessOperationMismatch(session, snapshot); - - const mixedOperation = "inspect_native_api"; - const mixedProfile = workflowAnalysisProfile( - APPLE_DISPATCH_METADATA_ANALYSIS_PROFILE, - mixedOperation, - ); - const mixedSnapshot = { - ...snapshot, - workflow_entries: [ - ...snapshot.workflow_entries, - createAnalysisSnapshotWorkflowEntry({ - target: snapshot.target, - binding: snapshot.binding, - operation: mixedOperation, - parameters: {}, - execution: { - result: null, - rawResult: null, - provider: mixedProfile.provider, - analysisProfile: mixedProfile, - limitations: [], - locations: [], - subject: null, - }, - }), - ], - }; - const rejectedMixed = session.importAnalysisSnapshot(mixedSnapshot); - expect(rejectedMixed.ok).toBe(false); - if (!rejectedMixed.ok) - expect(projectAnalysisError(rejectedMixed.error).details).toMatchObject({ - binding_mismatch: { workflow_scope: "incompatible" }, - }); - - const deepProfile = createAnalysisProfile( - { id: "ghidra", name: "Ghidra", version: "fixture-version" }, - { fixture: "deep-provider-only" }, - ); - const deepWorkflowProfile = workflowAnalysisProfile( - deepProfile, - "inspect_native_dispatch_metadata", - ); - const originalWorkflow = snapshot.workflow_entries[0]!; - const deepSnapshot = { - ...snapshot, - binding: { - provider: deepProfile.provider, - analysis_profile: deepProfile, - }, - workflow_entries: [ - createAnalysisSnapshotWorkflowEntry({ - target: snapshot.target, - binding: { - provider: deepProfile.provider, - analysis_profile: deepProfile, - }, + await withVerifierCleanup(async () => { + const opened = await session.open(path); + if (!opened.ok) throw opened.error; + const readerIdentity = APPLE_DISPATCH_METADATA_ANALYSIS_PROFILE.provider; + const wrongReader = structuredClone(snapshot); + // Reader v2 could discard owners when their adopted-protocol list was unreadable. + const otherReaderProfile = createAnalysisProfile( + { ...readerIdentity, version: "apple-metadata-reader-2" }, + { operation: "inspect_native_dispatch_metadata", - parameters: originalWorkflow.parameters, - execution: { - result: originalWorkflow.execution.result, - rawResult: originalWorkflow.execution.raw_result, - provider: deepWorkflowProfile.provider, - analysisProfile: deepWorkflowProfile, - limitations: originalWorkflow.execution.limitations, - ...(originalWorkflow.execution.relationship_coverage === null - ? {} - : { - relationshipCoverage: - originalWorkflow.execution.relationship_coverage, - }), - locations: originalWorkflow.execution.locations, - subject: - originalWorkflow.execution.subject === null - ? null - : { - path: originalWorkflow.execution.subject.path, - sha256: originalWorkflow.execution.subject.sha256, - format: originalWorkflow.execution.subject.format, - ...(originalWorkflow.execution.subject.architecture === null - ? {} - : { - architecture: - originalWorkflow.execution.subject.architecture, - }), - }, - }, - }), - ], - } as AnalysisSnapshot; - const rejectedDeepOnly = session.importAnalysisSnapshot(deepSnapshot); - expect(rejectedDeepOnly.ok).toBe(false); - if (!rejectedDeepOnly.ok) - expect(projectAnalysisError(rejectedDeepOnly.error).details).toMatchObject({ - binding_mismatch: { - provider: { snapshot: { id: "ghidra" } }, - profile_digest: { snapshot: deepProfile.digest }, + reader: "apple-dispatch-metadata", + revision: 2, }, - }); + ); + wrongReader.binding = { + provider: otherReaderProfile.provider, + analysis_profile: otherReaderProfile, + }; + const rejectedReader = session.importAnalysisSnapshot(wrongReader); + expect(rejectedReader.ok).toBe(false); + if (!rejectedReader.ok) { + expectReaderBindingMismatch( + rejectedReader.error, + readerIdentity.version, + otherReaderProfile.digest, + ); + } + + expectProfilelessOperationMismatch(session, snapshot); + + const mixedOperation = "inspect_native_api"; + const mixedProfile = workflowAnalysisProfile( + APPLE_DISPATCH_METADATA_ANALYSIS_PROFILE, + mixedOperation, + ); + const mixedSnapshot = { + ...snapshot, + workflow_entries: [ + ...snapshot.workflow_entries, + createAnalysisSnapshotWorkflowEntry({ + target: snapshot.target, + binding: snapshot.binding, + operation: mixedOperation, + parameters: {}, + execution: { + result: null, + rawResult: null, + provider: mixedProfile.provider, + analysisProfile: mixedProfile, + limitations: [], + locations: [], + subject: null, + }, + }), + ], + }; + const rejectedMixed = session.importAnalysisSnapshot(mixedSnapshot); + expect(rejectedMixed.ok).toBe(false); + if (!rejectedMixed.ok) + expect(projectAnalysisError(rejectedMixed.error).details).toMatchObject({ + binding_mismatch: { workflow_scope: "incompatible" }, + }); + + const deepProfile = createAnalysisProfile( + { id: "ghidra", name: "Ghidra", version: "fixture-version" }, + { fixture: "deep-provider-only" }, + ); + const deepWorkflowProfile = workflowAnalysisProfile( + deepProfile, + "inspect_native_dispatch_metadata", + ); + const originalWorkflow = snapshot.workflow_entries[0]!; + const deepSnapshot = { + ...snapshot, + binding: { + provider: deepProfile.provider, + analysis_profile: deepProfile, + }, + workflow_entries: [ + createAnalysisSnapshotWorkflowEntry({ + target: snapshot.target, + binding: { + provider: deepProfile.provider, + analysis_profile: deepProfile, + }, + operation: "inspect_native_dispatch_metadata", + parameters: originalWorkflow.parameters, + execution: { + result: originalWorkflow.execution.result, + rawResult: originalWorkflow.execution.raw_result, + provider: deepWorkflowProfile.provider, + analysisProfile: deepWorkflowProfile, + limitations: originalWorkflow.execution.limitations, + ...(originalWorkflow.execution.relationship_coverage === null + ? {} + : { + relationshipCoverage: + originalWorkflow.execution.relationship_coverage, + }), + locations: originalWorkflow.execution.locations, + subject: + originalWorkflow.execution.subject === null + ? null + : { + path: originalWorkflow.execution.subject.path, + sha256: originalWorkflow.execution.subject.sha256, + format: originalWorkflow.execution.subject.format, + ...(originalWorkflow.execution.subject.architecture === null + ? {} + : { + architecture: + originalWorkflow.execution.subject.architecture, + }), + }, + }, + }), + ], + } as AnalysisSnapshot; + const rejectedDeepOnly = session.importAnalysisSnapshot(deepSnapshot); + expect(rejectedDeepOnly.ok).toBe(false); + if (!rejectedDeepOnly.ok) + expect( + projectAnalysisError(rejectedDeepOnly.error).details, + ).toMatchObject({ + binding_mismatch: { + provider: { snapshot: { id: "ghidra" } }, + profile_digest: { snapshot: deepProfile.digest }, + }, + }); + }, [ + async () => { + const closed = await session.close(); + if (!closed.ok) throw closed.error; + }, + ]); }; it("retains a native dispatch workflow snapshot without a deep provider profile", async () => { diff --git a/tests/boundary/mcp/pcapParentIssuePath.test.ts b/tests/boundary/mcp/pcapParentIssuePath.test.ts index be1ed6d3e6..bef8b25916 100644 --- a/tests/boundary/mcp/pcapParentIssuePath.test.ts +++ b/tests/boundary/mcp/pcapParentIssuePath.test.ts @@ -1,3 +1,4 @@ +import { withVerifierCleanup } from "../../../scripts/lib/verifier-cleanup.mjs"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { expect, it } from "vitest"; import type { JsonValue } from "../../../src/domain/jsonValue.js"; @@ -63,7 +64,7 @@ it("locates malformed pcap parent fields beneath the analysis-view source", asyn }); const server = createServer({ kind: "session", session }); const client = new Client({ name: "pcap-parent-path-repro", version: "1" }); - try { + await withVerifierCleanup(async () => { const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); await server.connect(serverTransport); @@ -111,9 +112,12 @@ it("locates malformed pcap parent fields beneath the analysis-view source", asyn }); } await client.ping(); - } finally { - await client.close(); - await server.close(); - await session.close(); - } + }, [ + () => client.close(), + () => server.close(), + async () => { + const closed = await session.close(); + if (!closed.ok) throw closed.error; + }, + ]); }); diff --git a/tests/boundary/mcp/pcapWorkspaceAllocation.test.ts b/tests/boundary/mcp/pcapWorkspaceAllocation.test.ts index d235e36b8b..5e91cfc255 100644 --- a/tests/boundary/mcp/pcapWorkspaceAllocation.test.ts +++ b/tests/boundary/mcp/pcapWorkspaceAllocation.test.ts @@ -1,3 +1,4 @@ +import { closeVerifierResources } from "../../../scripts/lib/verifier-cleanup.mjs"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { chmod, mkdir, open, readdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -46,23 +47,27 @@ const inspectWith = async (allocate: Allocate, path: string) => { REA_CAPINFOS_COMMAND: join(stubs, "capinfos-stub.sh"), }, }); + const session = createTestBinarySession(() => { + throw new Error("No native binary provider"); + }); const server = createServer( - { - kind: "session", - session: createTestBinarySession(() => { - throw new Error("No native binary provider"); - }), - }, + { kind: "session", session }, { pcapAnalysis: provider }, ); const client = new Client({ name: "pcap-workspace-test", version: "1" }); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); - onTestFinished(async () => { - await client.close(); - await server.close(); - await provider.close(); - }); + onTestFinished(() => + closeVerifierResources([ + () => client.close(), + () => server.close(), + () => provider.close(), + async () => { + const closed = await session.close(); + if (!closed.ok) throw closed.error; + }, + ]), + ); await server.connect(serverTransport); await client.connect(clientTransport); return parseMcpToolError( diff --git a/tests/boundary/mcp/processCleanupRetryMcp.test.ts b/tests/boundary/mcp/processCleanupRetryMcp.test.ts index 62fb119c0f..235ad5ba24 100644 --- a/tests/boundary/mcp/processCleanupRetryMcp.test.ts +++ b/tests/boundary/mcp/processCleanupRetryMcp.test.ts @@ -1,9 +1,13 @@ +import { + closeVerifierResources, + withVerifierCleanup, +} from "../../../scripts/lib/verifier-cleanup.mjs"; import { access, mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; -import { expect, it, vi } from "vitest"; +import { expect, it, onTestFinished, vi } from "vitest"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; @@ -53,6 +57,7 @@ const connectSession = async () => { const client = new Client({ name: "process-cleanup-retry", version: "1" }); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + onTestFinished(() => closeSession({ client, server, session })); await server.connect(serverTransport); await client.connect(clientTransport); return { client, server, session }; @@ -61,10 +66,14 @@ const connectSession = async () => { const closeSession = async ( resources: Awaited>, ): Promise => { - await resources.client.close(); - await resources.server.close(); - const closed = await resources.session.close(); - if (!closed.ok) throw closed.error; + await closeVerifierResources([ + () => resources.client.close(), + () => resources.server.close(), + async () => { + const closed = await resources.session.close(); + if (!closed.ok) throw closed.error; + }, + ]); }; const captureCleanupRetry = (root: string, isDenied: () => boolean) => { @@ -116,7 +125,7 @@ it("projects the retained process root through close_binary and resolves its ret const callClose = () => resources.client.callTool({ name: "close_binary", arguments: {} }); - try { + await withVerifierCleanup(async () => { const failed = parseMcpToolError(await callClose()); const firstWire = JSON.stringify(failed); expect(failed.error).toMatchObject({ @@ -143,12 +152,20 @@ it("projects the retained process root through close_binary and resolves its ret ); expect(JSON.stringify(failed)).toBe(firstWire); await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" }); - } finally { - denyRemoval = false; - fault.retry.mockRestore(); - await closeSession(resources); - await rm(root, { recursive: true, force: true }); - } + }, [ + async () => { + denyRemoval = false; + }, + async () => { + fault.retry.mockRestore(); + }, + async () => { + await closeSession(resources); + }, + async () => { + await rm(root, { recursive: true, force: true }); + }, + ]); }); it.each(["returned", "thrown", "synchronous"] as const)( @@ -176,7 +193,7 @@ it.each(["returned", "thrown", "synchronous"] as const)( resources.client.callTool({ name: "close_binary", arguments: {} }); const delivery = vi.spyOn(ToolResultDelivery.prototype, "toCallToolResult"); - try { + await withVerifierCleanup(async () => { const failed = parseMcpToolError(await callClose()); const firstWire = JSON.stringify(failed); const delivered = delivery.mock.calls.at(-1)?.[0]; @@ -230,13 +247,25 @@ it.each(["returned", "thrown", "synchronous"] as const)( expect(JSON.stringify(failed)).toBe(firstWire); expect(primary.cleanupIncomplete).toBe(false); await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" }); - } finally { - denyRemoval = false; - closeSpy.mockRestore(); - delivery.mockRestore(); - fault.retry.mockRestore(); - await closeSession(resources); - await rm(root, { recursive: true, force: true }); - } + }, [ + async () => { + denyRemoval = false; + }, + async () => { + closeSpy.mockRestore(); + }, + async () => { + delivery.mockRestore(); + }, + async () => { + fault.retry.mockRestore(); + }, + async () => { + await closeSession(resources); + }, + async () => { + await rm(root, { recursive: true, force: true }); + }, + ]); }, ); diff --git a/tests/boundary/mcp/promptMcp.test.ts b/tests/boundary/mcp/promptMcp.test.ts index 652db8cfbf..707da164d6 100644 --- a/tests/boundary/mcp/promptMcp.test.ts +++ b/tests/boundary/mcp/promptMcp.test.ts @@ -1,3 +1,4 @@ +import { withVerifierCleanup } from "../../../scripts/lib/verifier-cleanup.mjs"; import { ok as resultOk, err, ok } from "../../../src/domain/result.js"; import { readFile, writeFile } from "node:fs/promises"; import { basename, join } from "node:path"; @@ -561,16 +562,25 @@ it("guided prompt completion avoids cold startup and cancels warm lookup on clos const startup = fixture.holdStartup(); const warming = client.callTool({ name: "list_documents", arguments: {} }); - await startup.started; - expect(await complete(client, "document", "")).toEqual([]); - expect(await complete(client, "procedure", "")).toEqual([]); - expect(fixture.childrenCreated()).toBe(1); - startup.finish(); - const warmed = await warming; - expect(warmed.isError, JSON.stringify(warmed)).not.toBe(true); - expect(fixture.childrenCreated()).toBe(1); - expect(await complete(client, "document", "fixture")).toEqual([ - "fixture-document", + await withVerifierCleanup(async () => { + await startup.started; + expect(await complete(client, "document", "")).toEqual([]); + expect(await complete(client, "procedure", "")).toEqual([]); + expect(fixture.childrenCreated()).toBe(1); + startup.finish(); + const warmed = await warming; + expect(warmed.isError, JSON.stringify(warmed)).not.toBe(true); + expect(fixture.childrenCreated()).toBe(1); + expect(await complete(client, "document", "fixture")).toEqual([ + "fixture-document", + ]); + }, [ + async () => { + startup.finish(); + }, + async () => { + await warming; + }, ]); const cancellation = fixture.holdCompletion(); @@ -704,6 +714,10 @@ describe("guided prompt completion lifecycle", () => { const gate = new Promise((resolve) => { release = resolve; }); + let producerDone!: () => void; + const producerFinished = new Promise((resolve) => { + producerDone = resolve; + }); const delivery = createToolResultDelivery(STDIO_DEFAULT_MAX_BUFFER_SIZE); const server = new EvidenceMcpServer( { name: "prompt-cancellation-test", version: "1" }, @@ -716,8 +730,12 @@ describe("guided prompt completion lifecycle", () => { calls += 1; if (calls === 1) { markStarted(); - await gate; - return observed(["late-document"]); + try { + await gate; + return observed(["late-document"]); + } finally { + producerDone(); + } } return observed(["fresh-document"]); }, @@ -732,16 +750,29 @@ describe("guided prompt completion lifecycle", () => { }, { signal: controller.signal }, ); - await started; - controller.abort(); - await expect(pending).rejects.toThrow(); - release(); - await new Promise((resolve) => setTimeout(resolve, 0)); + await withVerifierCleanup(async () => { + await started; + controller.abort(); + await expect(pending).rejects.toThrow(); + release(); + await new Promise((resolve) => setTimeout(resolve, 0)); - expect(await complete(client, "document", "fresh")).toEqual([ - "fresh-document", + expect(await complete(client, "document", "fresh")).toEqual([ + "fresh-document", + ]); + expect(calls).toBe(2); + }, [ + async () => { + controller.abort(); + release(); + }, + async () => { + await pending.catch(() => undefined); + }, + async () => { + await producerFinished; + }, ]); - expect(calls).toBe(2); }); it("refreshes wire completions across open, switch, concurrency, and close", async () => { diff --git a/tests/boundary/mcp/pseudocodeDeliveryMcp.test.ts b/tests/boundary/mcp/pseudocodeDeliveryMcp.test.ts index ef8d43e77b..a7a1ab4963 100644 --- a/tests/boundary/mcp/pseudocodeDeliveryMcp.test.ts +++ b/tests/boundary/mcp/pseudocodeDeliveryMcp.test.ts @@ -1,3 +1,4 @@ +import { closeVerifierResources } from "../../../scripts/lib/verifier-cleanup.mjs"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { expect, it, onTestFinished } from "vitest"; @@ -271,10 +272,9 @@ it("refuses summary delivery before calling a fixed provider without retention", { environment: {} }, ); const client = new Client({ name: "pseudocode-no-retention", version: "1" }); - onTestFinished(async () => { - await client.close(); - await server.close(); - }); + onTestFinished(() => + closeVerifierResources([() => client.close(), () => server.close()]), + ); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); await server.connect(serverTransport);