diff --git a/tests/boundary/mcp/applicationMcpHarnessLifecycle.test.ts b/tests/boundary/mcp/applicationMcpHarnessLifecycle.test.ts new file mode 100644 index 0000000000..0003844932 --- /dev/null +++ b/tests/boundary/mcp/applicationMcpHarnessLifecycle.test.ts @@ -0,0 +1,188 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect, it, vi } from "vitest"; + +import type { AnalysisClient } from "../../../src/application/AnalysisProvider.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; +import { err, ok } from "../../../src/domain/result.js"; +import { + createApplicationMcpHarness, + type ApplicationMcpHarness, +} from "../../fixtures/applicationMcpHarness.js"; +import { + createCacheProvider, + createDeferred, + createTestBinarySession, +} from "../../fixtures/binarySession.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +const failureLeaves = (reason: unknown): unknown[] => + reason instanceof AggregateError + ? reason.errors.flatMap(failureLeaves) + : [reason]; + +const acquireProvider = async ( + close: AnalysisClient["close"], + serverOptions?: NonNullable< + Parameters[0] + >["serverOptions"], +) => { + const directory = await createTestTempDirectory("rea-mcp-harness-close-"); + const path = join(directory, "target.hop"); + await writeFile(path, "fixture database bytes"); + const operations: string[] = []; + const provider = createCacheProvider(operations); + const createClient = vi.fn(provider.createClient); + provider.createClient = (...args) => ({ ...createClient(...args), close }); + const harness = await createApplicationMcpHarness({ + serverOptions, + session: createTestBinarySession(provider), + }); + expect( + ( + await harness.client.callTool({ + name: "open_binary", + arguments: { path }, + }) + ).isError, + ).not.toBe(true); + expect( + ( + await harness.client.callTool({ + name: "address_name", + arguments: { address: "0x1000", document: "fixture" }, + }) + ).isError, + ).not.toBe(true); + expect(operations).toEqual(["address_name"]); + expect(createClient).toHaveBeenCalledTimes(1); + return { harness, createClient }; +}; + +it("reports a failed session close, joins concurrent attempts and retries the retained provider", async () => { + const failure = new ProviderCleanupError("fixture", ["binary-owner"], { + reason: "controlled binary close denial", + }); + const entered = createDeferred(); + const release = createDeferred(); + let allowClose = false; + const closeProvider = vi.fn(async () => { + entered.resolve(); + await release.promise; + if (!allowClose) return err(failure); + failure.resolveCleanup(); + return ok(null); + }); + let harness: ApplicationMcpHarness | undefined; + try { + const acquired = await acquireProvider(closeProvider); + harness = acquired.harness; + const closing = harness.close(); + const outcome = closing.catch((reason: unknown) => reason); + await entered.promise; + const concurrent = harness.close(); + const concurrentOutcome = concurrent.catch((reason: unknown) => reason); + expect(closeProvider).toHaveBeenCalledTimes(1); + release.resolve(); + const reason: unknown = await outcome; + expect(reason).toBeInstanceOf(AggregateError); + if (!(reason instanceof AggregateError)) + throw new Error("Harness ignored the session close failure"); + expect(reason.errors).toContain(failure); + expect(concurrent).toBe(closing); + expect(await concurrentOutcome).toBe(reason); + expect(failure.cleanupIncomplete).toBe(true); + expect(await harness.session.status()).toMatchObject({ + cleanup_pending: true, + }); + + allowClose = true; + await harness.close(); + expect(acquired.createClient).toHaveBeenCalledTimes(1); + expect(closeProvider).toHaveBeenCalledTimes(2); + expect(failure.cleanupIncomplete).toBe(false); + expect(await harness.session.status()).toMatchObject({ + cleanup_pending: false, + }); + await harness.close(); + expect(closeProvider).toHaveBeenCalledTimes(2); + } finally { + allowClose = true; + release.resolve(); + await harness?.close(); + } +}); + +it("preserves independent MCP provider and session close failures and releases each owner once", async () => { + const binaryFailure = new ProviderCleanupError("fixture", ["binary-owner"], { + reason: "controlled binary close denial", + }); + const adbFailure = new Error("controlled ADB close denial"); + let allowBinaryClose = false; + let allowAdbClose = false; + const closeBinary = vi.fn(async () => { + if (!allowBinaryClose) return err(binaryFailure); + binaryFailure.resolveCleanup(); + return ok(null); + }); + const closeAdb = vi.fn(async () => { + if (!allowAdbClose) throw adbFailure; + }); + let harness: ApplicationMcpHarness | undefined; + try { + const acquired = await acquireProvider(closeBinary, { + adbDeviceAnalysis: { + inspectAvailability: async () => ({ + status: "available", + code: null, + reason: null, + diagnostics: {}, + }), + execute: async () => { + throw new Error("This fixture must not execute ADB operations"); + }, + close: closeAdb, + }, + }); + harness = acquired.harness; + const reason: unknown = await harness + .close() + .catch((cause: unknown) => cause); + expect(reason).toBeInstanceOf(AggregateError); + if (!(reason instanceof AggregateError)) + throw new Error("Harness lost independent cleanup failures"); + expect(failureLeaves(reason)).toEqual( + expect.arrayContaining([binaryFailure, adbFailure]), + ); + expect(failureLeaves(reason)).toHaveLength(2); + expect(closeBinary).toHaveBeenCalledTimes(1); + expect(closeAdb).toHaveBeenCalled(); + expect(await harness.session.status()).toMatchObject({ + cleanup_pending: true, + }); + + allowAdbClose = true; + await expect(harness.close()).rejects.toMatchObject({ + errors: [binaryFailure], + }); + const completedAdbCloses = closeAdb.mock.calls.length; + expect(closeBinary).toHaveBeenCalledTimes(2); + allowBinaryClose = true; + await harness.close(); + expect(acquired.createClient).toHaveBeenCalledTimes(1); + expect(closeBinary).toHaveBeenCalledTimes(3); + expect(closeAdb).toHaveBeenCalledTimes(completedAdbCloses); + expect(binaryFailure.cleanupIncomplete).toBe(false); + expect(await harness.session.status()).toMatchObject({ + cleanup_pending: false, + }); + await harness.close(); + expect(closeBinary).toHaveBeenCalledTimes(3); + expect(closeAdb).toHaveBeenCalledTimes(completedAdbCloses); + } finally { + allowBinaryClose = true; + allowAdbClose = true; + await harness?.close(); + } +}); diff --git a/tests/boundary/mcp/mcpFixtureLifecycle.test.ts b/tests/boundary/mcp/mcpFixtureLifecycle.test.ts new file mode 100644 index 0000000000..4f8becd064 --- /dev/null +++ b/tests/boundary/mcp/mcpFixtureLifecycle.test.ts @@ -0,0 +1,367 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { McpServer } from "@modelcontextprotocol/server"; +import { expect, it, vi } from "vitest"; + +import { createDeferred } from "../../fixtures/binarySession.js"; +import { createTestMcp } from "../../support/mcp/mcpFixture.js"; + +/** Keep real SDK objects recoverable even when a red ownership assertion fails. */ +const createSdkFixture = () => { + const mcp = createTestMcp(); + const clientClose = Client.prototype.close; + const clientConnects = vi.spyOn(Client.prototype, "connect"); + const clientCloses = vi.spyOn(Client.prototype, "close"); + const serverCloses: Array<() => Promise> = []; + return { + mcp, + clientCloses, + server(name: string) { + const server = new McpServer({ name, version: "1" }); + serverCloses.push(server.close.bind(server)); + return server; + }, + async dispose() { + const clients = new Set([ + ...clientConnects.mock.contexts, + ...clientCloses.mock.contexts, + ]); + clientConnects.mockRestore(); + clientCloses.mockRestore(); + const outcomes = await Promise.allSettled([ + ...[...clients].map((client) => clientClose.call(client)), + ...serverCloses.map((close) => close()), + ]); + const failures = outcomes.flatMap((outcome) => + outcome.status === "rejected" ? [outcome.reason as unknown] : [], + ); + if (failures.length === 1) throw failures[0]; + if (failures.length > 1) + throw new AggregateError(failures, "SDK fixture cleanup failed"); + }, + }; +}; + +it("joins an accepted late handshake and refuses to publish it after shutdown", async () => { + const fixture = createSdkFixture(); + const server = fixture.server("late-handshake"); + const connected = createDeferred(); + const release = createDeferred(); + const connect = server.connect.bind(server); + server.connect = async (transport) => { + await connect(transport); + connected.resolve(); + await release.promise; + }; + const serverClose = vi.spyOn(server, "close"); + const opening = fixture.mcp.connect(server).then( + (client) => ({ ok: true as const, client }), + (reason: unknown) => ({ ok: false as const, reason }), + ); + let closeSettled = false; + let closing: Promise | undefined; + try { + await connected.promise; + closing = fixture.mcp.close().then(() => { + closeSettled = true; + }); + // The producer is held, so a shutdown with no pending acquisition would + // already settle through these promise continuations. + await Promise.resolve(); + await Promise.resolve(); + expect(closeSettled).toBe(false); + release.resolve(); + expect(await opening).toMatchObject({ ok: false }); + await closing; + expect(serverClose).toHaveBeenCalledTimes(1); + expect(fixture.clientCloses).toHaveBeenCalledTimes(1); + } finally { + release.resolve(); + await opening; + await closing?.catch(() => undefined); + await fixture.dispose(); + } +}); + +it("registers acquisition before a producer can reenter shutdown and attach its transport late", async () => { + const fixture = createSdkFixture(); + const server = fixture.server("reentrant-shutdown"); + const entered = createDeferred(); + const release = createDeferred(); + const connect = server.connect.bind(server); + const close = server.close.bind(server); + const lifecycle: string[] = []; + let closing: Promise | undefined; + let closeSettled = false; + server.connect = async (transport) => { + closing = fixture.mcp.close().then(() => { + closeSettled = true; + }); + entered.resolve(); + await release.promise; + await connect(transport); + lifecycle.push("attached"); + }; + const serverClose = vi.spyOn(server, "close").mockImplementation(async () => { + lifecycle.push("closed"); + await close(); + }); + const opening = fixture.mcp.connect(server).then( + (client) => ({ ok: true as const, client }), + (reason: unknown) => ({ ok: false as const, reason }), + ); + try { + await entered.promise; + await Promise.resolve(); + await Promise.resolve(); + expect(closeSettled).toBe(false); + expect(serverClose).not.toHaveBeenCalled(); + expect(fixture.clientCloses).not.toHaveBeenCalled(); + release.resolve(); + const outcome = await opening; + expect(outcome.ok).toBe(false); + if (outcome.ok) throw new Error("Shutdown published a late MCP client"); + expect(outcome.reason).toMatchObject({ + message: expect.stringMatching(/closed/iu), + }); + await closing; + expect(closeSettled).toBe(true); + expect(lifecycle).toEqual(["attached", "closed"]); + expect(serverClose).toHaveBeenCalledTimes(1); + expect(fixture.clientCloses).toHaveBeenCalledTimes(1); + } finally { + release.resolve(); + await opening; + await closing?.catch(() => undefined); + await fixture.dispose(); + } +}); + +it("shares one concurrent close attempt and does not reclose released SDK objects", async () => { + const fixture = createSdkFixture(); + const server = fixture.server("concurrent-close"); + const entered = createDeferred(); + const release = createDeferred(); + let first: Promise | undefined; + let second: Promise | undefined; + try { + const client = await fixture.mcp.connect(server); + await expect(client.ping()).resolves.toEqual({}); + const close = server.close.bind(server); + const serverClose = vi + .spyOn(server, "close") + .mockImplementation(async () => { + entered.resolve(); + await release.promise; + await close(); + }); + first = fixture.mcp.close(); + await entered.promise; + second = fixture.mcp.close(); + await Promise.resolve(); + await Promise.resolve(); + expect(serverClose).toHaveBeenCalledTimes(1); + expect(fixture.clientCloses).toHaveBeenCalledTimes(1); + release.resolve(); + await Promise.all([first, second]); + await fixture.mcp.close(); + expect(serverClose).toHaveBeenCalledTimes(1); + expect(fixture.clientCloses).toHaveBeenCalledTimes(1); + } finally { + release.resolve(); + await Promise.allSettled([first, second]); + await fixture.dispose(); + } +}); + +it("cancels a real initialize handshake before awaiting its held server handler", async () => { + const fixture = createSdkFixture(); + const server = fixture.server("stalled-initialize"); + const entered = createDeferred(); + const release = createDeferred(); + server.server.setRequestHandler("initialize", async () => { + entered.resolve(); + await release.promise; + throw new Error("Held initialize handler released during test cleanup"); + }); + let openingSettled = false; + const opening = fixture.mcp.connect(server).then( + (client) => { + openingSettled = true; + return { ok: true as const, client }; + }, + (reason: unknown) => { + openingSettled = true; + return { ok: false as const, reason }; + }, + ); + const serverClose = vi.spyOn(server, "close"); + try { + await entered.promise; + await fixture.mcp.close(); + expect(openingSettled).toBe(true); + expect(await opening).toMatchObject({ ok: false }); + expect(serverClose).toHaveBeenCalledTimes(1); + } finally { + release.resolve(); + await opening; + await fixture.dispose(); + } +}); + +it("keeps a failed handshake as the exact error when its SDK cleanup succeeds", async () => { + const fixture = createSdkFixture(); + const server = fixture.server("failed-handshake"); + const startup = new Error("controlled server handshake failure"); + const connect = server.connect.bind(server); + server.connect = async (transport) => { + await connect(transport); + throw startup; + }; + const serverClose = vi.spyOn(server, "close"); + try { + await expect(fixture.mcp.connect(server)).rejects.toBe(startup); + expect(serverClose).toHaveBeenCalledTimes(1); + expect(fixture.clientCloses).toHaveBeenCalledTimes(1); + await fixture.mcp.close(); + expect(serverClose).toHaveBeenCalledTimes(1); + expect(fixture.clientCloses).toHaveBeenCalledTimes(1); + } finally { + await fixture.dispose(); + } +}); + +it.each([new Error("controlled server close failure"), undefined])( + "retains failed handshake cleanup and retries its exact owner: %s", + async (cleanup) => { + const fixture = createSdkFixture(); + const server = fixture.server("failed-handshake-cleanup"); + const startup = new Error("controlled server handshake failure"); + const connect = server.connect.bind(server); + const close = server.close.bind(server); + let denied = true; + server.connect = async (transport) => { + await connect(transport); + throw startup; + }; + const serverClose = vi + .spyOn(server, "close") + .mockImplementation(async () => { + if (denied) throw cleanup; + await close(); + }); + try { + const error: unknown = await fixture.mcp + .connect(server) + .catch((reason: unknown) => reason); + expect(error).toBeInstanceOf(AggregateError); + if (!(error instanceof AggregateError)) + throw new Error("Expected startup and retained cleanup failures"); + expect(error.cause).toBe(startup); + expect(error.errors).toEqual(expect.arrayContaining([startup, cleanup])); + expect(error.errors).toHaveLength(2); + expect(serverClose).toHaveBeenCalledTimes(1); + expect(fixture.clientCloses).toHaveBeenCalledTimes(1); + await expect(fixture.mcp.close()).rejects.toBe(cleanup); + expect(serverClose).toHaveBeenCalledTimes(2); + denied = false; + await fixture.mcp.close(); + expect(serverClose).toHaveBeenCalledTimes(3); + expect(fixture.clientCloses).toHaveBeenCalledTimes(1); + await fixture.mcp.close(); + expect(serverClose).toHaveBeenCalledTimes(3); + } finally { + denied = false; + await fixture.dispose(); + } + }, +); + +it("attempts every SDK owner, reports all close failures and retries only unresolved resources", async () => { + const fixture = createSdkFixture(); + const firstServer = fixture.server("mixed-close-first"); + const secondServer = fixture.server("mixed-close-second"); + const clientFailure = new Error("controlled client close failure"); + const serverFailure = new Error("controlled server close failure"); + let denyClient = true; + let denyServer = true; + try { + const firstClient = await fixture.mcp.connect(firstServer); + const secondClient = await fixture.mcp.connect(secondServer); + await expect(firstClient.ping()).resolves.toEqual({}); + await expect(secondClient.ping()).resolves.toEqual({}); + const closeClient = firstClient.close.bind(firstClient); + const closeServer = firstServer.close.bind(firstServer); + const firstClientClose = vi.fn(async () => { + if (denyClient) throw clientFailure; + await closeClient(); + }); + firstClient.close = firstClientClose; + const firstServerClose = vi + .spyOn(firstServer, "close") + .mockImplementation(async () => { + if (denyServer) throw serverFailure; + await closeServer(); + }); + const closeSecondClient = secondClient.close.bind(secondClient); + const secondClientClose = vi.fn(closeSecondClient); + secondClient.close = secondClientClose; + const secondServerClose = vi.spyOn(secondServer, "close"); + const error: unknown = await fixture.mcp + .close() + .catch((reason: unknown) => reason); + expect(error).toBeInstanceOf(AggregateError); + if (!(error instanceof AggregateError)) + throw new Error("Expected both independent SDK close failures"); + expect(error.errors).toEqual( + expect.arrayContaining([clientFailure, serverFailure]), + ); + expect(error.errors).toHaveLength(2); + for (const closed of [ + firstClientClose, + firstServerClose, + secondClientClose, + secondServerClose, + ]) + expect(closed).toHaveBeenCalledTimes(1); + + denyClient = false; + await expect(fixture.mcp.close()).rejects.toBe(serverFailure); + expect(firstClientClose).toHaveBeenCalledTimes(2); + expect(firstServerClose).toHaveBeenCalledTimes(2); + expect(secondClientClose).toHaveBeenCalledTimes(1); + expect(secondServerClose).toHaveBeenCalledTimes(1); + + denyServer = false; + await fixture.mcp.close(); + await fixture.mcp.close(); + expect(firstClientClose).toHaveBeenCalledTimes(2); + expect(firstServerClose).toHaveBeenCalledTimes(3); + expect(secondClientClose).toHaveBeenCalledTimes(1); + expect(secondServerClose).toHaveBeenCalledTimes(1); + } finally { + denyClient = false; + denyServer = false; + await fixture.dispose(); + } +}); + +it("rejects new connections after shutdown before acquiring a pair and leaves the unadmitted server caller-owned", async () => { + const fixture = createSdkFixture(); + const server = fixture.server("unadmitted-server"); + const pair = vi.spyOn(InMemoryTransport, "createLinkedPair"); + const serverConnect = vi.spyOn(server, "connect"); + const serverClose = vi.spyOn(server, "close"); + try { + await fixture.mcp.close(); + await expect(fixture.mcp.connect(server)).rejects.toThrow(/closed/iu); + expect(pair).not.toHaveBeenCalled(); + expect(serverConnect).not.toHaveBeenCalled(); + expect(serverClose).not.toHaveBeenCalled(); + expect(fixture.clientCloses).not.toHaveBeenCalled(); + await server.close(); + expect(serverClose).toHaveBeenCalledTimes(1); + } finally { + pair.mockRestore(); + await fixture.dispose(); + } +}); diff --git a/tests/boundary/mcp/reverseDebuggerMcpHarnessLifecycle.test.ts b/tests/boundary/mcp/reverseDebuggerMcpHarnessLifecycle.test.ts new file mode 100644 index 0000000000..d2e27bb976 --- /dev/null +++ b/tests/boundary/mcp/reverseDebuggerMcpHarnessLifecycle.test.ts @@ -0,0 +1,131 @@ +import { access, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { McpServer } from "@modelcontextprotocol/server"; +import { expect, it, onTestFinished, vi } from "vitest"; + +import { createProviderEnvironmentMcpHarness } from "../../support/reverseDebuggerMcpTestHelpers.js"; + +const providerSource = `#!${process.execPath}\nprocess.stdout.write("selected provider\\n");\n`; +const inputSource = "selected input bytes\n"; + +const selectedProviderFiles = async () => { + const directory = await mkdtemp(join(tmpdir(), "rea-provider-owner-")); + const provider = join(directory, "selected-rizin"); + const input = join(directory, "fixture.bin"); + await Promise.all([ + writeFile(provider, providerSource, { mode: 0o700 }), + writeFile(input, inputSource), + ]); + return { directory, provider, input }; +}; + +const providerOptions = (provider: string, close: () => Promise) => ({ + environment: { REA_RIZIN_COMMAND: provider }, + adbDeviceAnalysis: { + inspectAvailability: async () => ({ + status: "available" as const, + code: null, + reason: null, + diagnostics: {}, + }), + execute: async () => { + throw new Error("This fixture must not execute ADB operations"); + }, + close, + }, +}); + +const failureLeaves = (reason: unknown): unknown[] => + reason instanceof AggregateError + ? reason.errors.flatMap(failureLeaves) + : [reason]; + +it("retains selected provider files after failed close and removes them after owner recovery", async () => { + const files = await selectedProviderFiles(); + const closeFailure = new Error("controlled provider close denial"); + let allowClose = false; + const closeProvider = vi.fn(async () => { + if (!allowClose) throw closeFailure; + }); + let harness: + | Awaited> + | undefined; + try { + harness = await createProviderEnvironmentMcpHarness({ + directory: files.directory, + clientName: "provider-owner-close", + serverOptions: providerOptions(files.provider, closeProvider), + }); + await expect(harness.client.ping()).resolves.toEqual({}); + const reason: unknown = await harness + .close() + .catch((cause: unknown) => cause); + expect(failureLeaves(reason)).toContain(closeFailure); + expect(await readFile(files.provider, "utf8")).toBe(providerSource); + expect(await readFile(files.input, "utf8")).toBe(inputSource); + expect(closeProvider).toHaveBeenCalled(); + + allowClose = true; + await harness.close(); + await expect(access(files.directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + const completedCloses = closeProvider.mock.calls.length; + await harness.close(); + expect(closeProvider).toHaveBeenCalledTimes(completedCloses); + } finally { + allowClose = true; + await harness?.close(); + } +}); + +it("retains selected files when attached MCP startup and provider cleanup both fail before publication", async () => { + const files = await selectedProviderFiles(); + const startupFailure = new Error("controlled attached MCP startup failure"); + const closeFailure = new Error("controlled startup cleanup denial"); + let allowClose = false; + const closeProvider = vi.fn(async () => { + if (!allowClose) throw closeFailure; + }); + const connect = McpServer.prototype.connect; + const startup = vi + .spyOn(McpServer.prototype, "connect") + .mockImplementationOnce(async function (this: McpServer, ...args) { + await connect.apply(this, args); + throw startupFailure; + }); + let harness: + | Awaited> + | undefined; + onTestFinished(async () => { + await expect(access(files.directory)).rejects.toMatchObject({ + code: "ENOENT", + }); + }); + try { + let reason: unknown; + try { + harness = await createProviderEnvironmentMcpHarness({ + directory: files.directory, + clientName: "provider-owner-startup", + serverOptions: providerOptions(files.provider, closeProvider), + }); + } catch (cause) { + reason = cause; + } + expect(harness).toBeUndefined(); + expect(startup).toHaveBeenCalledTimes(1); + expect(failureLeaves(reason)).toEqual( + expect.arrayContaining([startupFailure, closeFailure]), + ); + expect(closeProvider).toHaveBeenCalled(); + expect(await readFile(files.provider, "utf8")).toBe(providerSource); + expect(await readFile(files.input, "utf8")).toBe(inputSource); + } finally { + allowClose = true; + startup.mockRestore(); + await harness?.close(); + } +}); diff --git a/tests/boundary/mcp/sessionFixtureCleanup.test.ts b/tests/boundary/mcp/sessionFixtureCleanup.test.ts new file mode 100644 index 0000000000..6315ef6715 --- /dev/null +++ b/tests/boundary/mcp/sessionFixtureCleanup.test.ts @@ -0,0 +1,120 @@ +import { stat, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect, vi, type OnTestFinishedHandler } from "vitest"; + +import { closeVerifierResources } from "../../../scripts/lib/verifier-cleanup.mjs"; +import { AnalysisError } from "../../../src/domain/analysisErrorBase.js"; +import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { openArtifactTarget } from "../../support/mcp/artifactSession.js"; +import { mcpTest } from "../../support/mcp/mcpFixture.js"; +import { openProductionTarget } from "../../support/mcp/productionSession.js"; + +mcpTest.for([ + { name: "production", open: openProductionTarget }, + { name: "artifact", open: openArtifactTarget }, +])( + "$name session fixture reports failed teardown and retries its exact descriptor owner", + async ({ open }, { mcp, task }) => { + const directory = await createTestTempDirectory( + "rea-session-fixture-cleanup-", + ); + // Descriptor ownership needs real target bytes, without selecting an + // installed deep-analysis engine whose availability varies by host. + const path = join(directory, "target.plist"); + await writeFile(path, ''); + const identity = await stat(path); + let cleanup: OnTestFinishedHandler | undefined; + let retained: OwnedFileHandle | undefined; + let denied = true; + let deniedAttempts = 0; + let restoreClose = () => {}; + try { + const client = await open( + mcp, + (handler) => { + cleanup = handler; + }, + path, + ); + if (cleanup === undefined) + throw new Error("Session fixture did not register teardown"); + const nativeClose = OwnedFileHandle.prototype.close; + const closeFailure = new Error("controlled fixture target close denial"); + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(async function (this: OwnedFileHandle) { + if (retained === undefined && this.handle.fd >= 0) { + const observed = await this.handle.stat(); + if (observed.dev === identity.dev && observed.ino === identity.ino) + retained = this; + } + if (this === retained && denied) { + deniedAttempts += 1; + throw closeFailure; + } + await nativeClose.call(this); + }); + restoreClose = () => close.mockRestore(); + + // A successful admission has already closed its read descriptor. Reopen + // the same target to leave a real failed close owned by this session. + const failedOpen = parseMcpToolError( + await client.callTool({ name: "open_binary", arguments: { path } }), + ); + expect(failedOpen.error).toMatchObject({ + code: "cleanup_incomplete", + details: { + resources: [path], + cleanup_reason: expect.stringContaining(closeFailure.message), + }, + }); + if (retained === undefined) + throw new Error( + "Target close denial did not retain a descriptor owner", + ); + const owner = retained; + const descriptor = owner.handle.fd; + expect(descriptor).toBeGreaterThanOrEqual(0); + expect(await owner.handle.stat()).toMatchObject({ + dev: identity.dev, + ino: identity.ino, + }); + + const beforeFirstCleanup = deniedAttempts; + await expect(cleanup(task.context)).rejects.toBeInstanceOf(AnalysisError); + expect(deniedAttempts).toBeGreaterThan(beforeFirstCleanup); + expect(retained).toBe(owner); + expect(owner.handle.fd).toBe(descriptor); + + const beforeRetry = deniedAttempts; + await expect(cleanup(task.context)).rejects.toMatchObject({ + cleanupIncomplete: true, + cleanupResources: [path], + diagnostics: { reason: expect.stringContaining(closeFailure.message) }, + }); + expect(deniedAttempts).toBeGreaterThan(beforeRetry); + expect(retained).toBe(owner); + expect(owner.handle.fd).toBe(descriptor); + + denied = false; + restoreClose(); + await cleanup(task.context); + expect(owner.handle.fd).toBe(-1); + await expect(owner.handle.stat()).rejects.toMatchObject({ + code: "EBADF", + }); + } finally { + denied = false; + restoreClose(); + await closeVerifierResources([ + async () => { + await cleanup?.(task.context); + }, + () => mcp.close(), + ]); + } + }, +); diff --git a/tests/fixtures/applicationMcpHarness.ts b/tests/fixtures/applicationMcpHarness.ts index 95196e6218..03ef8320df 100644 --- a/tests/fixtures/applicationMcpHarness.ts +++ b/tests/fixtures/applicationMcpHarness.ts @@ -1,10 +1,16 @@ -import { ok as resultOk } from "../../src/domain/result.js"; -import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import type { Client } from "@modelcontextprotocol/client"; +import { onTestFinished } from "vitest"; +import { closeVerifierResources } from "../../scripts/lib/verifier-cleanup.mjs"; +import { ok as resultOk } from "../../src/domain/result.js"; import { createTestBinarySession } from "./binarySession.js"; import type { BinarySession } from "../../src/application/binary/BinarySession.js"; import { observed } from "./analysisExecution.js"; import { createServer } from "../../src/server/createServer.js"; +import { + createTestMcp, + type McpClientIdentity, +} from "../support/mcp/mcpFixture.js"; /** Owned MCP SDK session shared by application workflow scenarios. */ export interface ApplicationMcpHarness { @@ -18,6 +24,8 @@ export async function createApplicationMcpHarness( options: { readonly session?: BinarySession; readonly serverOptions?: Parameters[1]; + readonly clientIdentity?: McpClientIdentity; + readonly registerCleanup?: (close: ApplicationMcpHarness["close"]) => void; } = {}, ): Promise { const session = @@ -26,25 +34,52 @@ export async function createApplicationMcpHarness( execute: () => Promise.resolve(observed(null)), close: () => Promise.resolve(resultOk(null)), })); - const server = createServer( - { kind: "session", session }, - options.serverOptions, - ); - const client = new Client({ - name: "application-workflow-test", - version: "1", - }); - const [clientTransport, serverTransport] = - InMemoryTransport.createLinkedPair(); - - await server.connect(serverTransport); - await client.connect(clientTransport); - - return { - client, - session, - close: async () => { - await Promise.all([client.close(), server.close(), session.close()]); - }, + const mcp = createTestMcp(); + let sessionClosed = false; + let closing: Promise | undefined; + const close = (): Promise => { + if (closing !== undefined) return closing; + const attempt = closeVerifierResources([ + () => mcp.close(), + async () => { + if (sessionClosed) return; + const result = await session.close(); + if (!result.ok) throw result.error; + sessionClosed = true; + }, + ]); + closing = attempt; + void attempt.catch(() => { + if (closing === attempt) closing = undefined; + }); + return attempt; }; + onTestFinished(close); + + try { + options.registerCleanup?.(close); + const server = createServer( + { kind: "session", session }, + options.serverOptions, + ); + const client = await mcp.connect( + server, + options.clientIdentity ?? { + name: "application-workflow-test", + version: "1", + }, + ); + return { client, session, close }; + } catch (cause) { + try { + await close(); + } catch (cleanup) { + throw new AggregateError( + [cause, cleanup], + "Application MCP harness setup and cleanup failed", + { cause }, + ); + } + throw cause; + } } diff --git a/tests/support/mcp/artifactSession.ts b/tests/support/mcp/artifactSession.ts index 53ca015602..ddd8f46118 100644 --- a/tests/support/mcp/artifactSession.ts +++ b/tests/support/mcp/artifactSession.ts @@ -13,7 +13,8 @@ const openSessionTarget = async ( ): Promise => { const session = await createArtifactBinarySession(process.env); onTestFinished(async () => { - await session.close(); + const result = await session.close(); + if (!result.ok) throw result.error; }); const client = await mcp.connect(createServer({ kind: "session", session })); const opened = await client.callTool({ diff --git a/tests/support/mcp/mcpFixture.ts b/tests/support/mcp/mcpFixture.ts index 2eef34a963..340918eec0 100644 --- a/tests/support/mcp/mcpFixture.ts +++ b/tests/support/mcp/mcpFixture.ts @@ -10,6 +10,7 @@ export interface McpClientIdentity { /** Registry for production MCP servers and SDK clients owned by one test. */ export interface TestMcp { + /** Transfer server ownership on admission; a closed registry leaves it with the caller. */ connect(server: McpServer, identity?: McpClientIdentity): Promise; close(): Promise; } @@ -19,41 +20,117 @@ const DEFAULT_CLIENT_IDENTITY: McpClientIdentity = { version: "1.0.0", }; -/** Create an MCP connection registry for a fixture composition root. */ +interface McpConnectionOwner { + readonly client: Client; + readonly server: McpServer; + readonly handshake: AbortController; + clientClosed: boolean; + serverClosed: boolean; +} + +/** Each SDK object stays owned until its own close resolves successfully. */ +const closeConnection = async ( + connection: McpConnectionOwner, +): Promise => { + const results = await Promise.allSettled([ + (async () => { + if (connection.clientClosed) return; + await connection.client.close(); + connection.clientClosed = true; + })(), + (async () => { + if (connection.serverClosed) return; + await connection.server.close(); + connection.serverClosed = true; + })(), + ]); + return results.flatMap((result) => + result.status === "rejected" ? [result.reason as unknown] : [], + ); +}; + +const registryClosed = (): Error => new Error("Test MCP registry is closed"); + +/** Own accepted acquisitions, SDK resources, and failed cleanup through shutdown. */ export const createTestMcp = (): TestMcp => { - const connections: { readonly client: Client; readonly server: McpServer }[] = - []; + const connections = new Set(); + const pending = new Set>(); + let closed = false; + let closing: Promise | undefined; + const release = async ( + connection: McpConnectionOwner, + ): Promise => { + const failures = await closeConnection(connection); + if (connection.clientClosed && connection.serverClosed) + connections.delete(connection); + return failures; + }; return { - connect: async (server, identity = DEFAULT_CLIENT_IDENTITY) => { + connect: (server, identity = DEFAULT_CLIENT_IDENTITY) => { + if (closed) return Promise.reject(registryClosed()); const client = new Client(identity); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); - try { - await server.connect(serverTransport); - await client.connect(clientTransport); - } catch (cause) { - await Promise.allSettled([client.close(), server.close()]); - throw cause; - } - connections.push({ client, server }); - return client; - }, - close: async () => { - const results = await Promise.allSettled( - [...connections].reverse().map(async ({ client, server }) => { - try { - await client.close(); - } finally { - await server.close(); + const connection: McpConnectionOwner = { + client, + server, + handshake: new AbortController(), + clientClosed: false, + serverClosed: false, + }; + connections.add(connection); + const opening = Promise.resolve().then(async () => { + try { + if (closed) throw connection.handshake.signal.reason; + await server.connect(serverTransport); + if (closed) throw connection.handshake.signal.reason; + await client.connect(clientTransport, { + signal: connection.handshake.signal, + }); + if (closed) throw connection.handshake.signal.reason; + return client; + } catch (cause) { + // Shutdown joins this acquisition, then owns its cleanup. Otherwise + // failed startup releases immediately without losing failed owners. + if (!closed) { + const failures = await release(connection); + if (failures.length > 0) + throw new AggregateError( + [cause, ...failures], + "MCP startup and cleanup failed", + { cause }, + ); } - }), + throw cause; + } + }); + pending.add(opening); + void opening.then( + () => pending.delete(opening), + () => pending.delete(opening), ); - const failure = results.find( - (result): result is PromiseRejectedResult => - result.status === "rejected", - ); - if (failure !== undefined) throw failure.reason; + return opening; + }, + close: () => { + closed = true; + if (closing !== undefined) return closing; + for (const connection of connections) + connection.handshake.abort(registryClosed()); + const attempt = (async () => { + await Promise.allSettled(pending); + const failures = ( + await Promise.all([...connections].reverse().map(release)) + ).flat(); + if (failures.length === 1) throw failures[0]; + if (failures.length > 1) + throw new AggregateError(failures, "Test MCP cleanup failed"); + })(); + closing = attempt; + void attempt.catch(() => { + if (closing === attempt) closing = undefined; + }); + return attempt; }, }; }; diff --git a/tests/support/mcp/productionSession.ts b/tests/support/mcp/productionSession.ts index 38ce87e093..ea09130b76 100644 --- a/tests/support/mcp/productionSession.ts +++ b/tests/support/mcp/productionSession.ts @@ -21,7 +21,8 @@ export const openProductionTarget = async ( process.env, ); onTestFinished(async () => { - await session.close(); + const result = await session.close(); + if (!result.ok) throw result.error; }); const client = await mcp.connect(createServer({ kind: "session", session })); const opened = await client.callTool({ diff --git a/tests/support/reverseDebuggerMcpTestHelpers.ts b/tests/support/reverseDebuggerMcpTestHelpers.ts index 5b7762325d..5ba79c869f 100644 --- a/tests/support/reverseDebuggerMcpTestHelpers.ts +++ b/tests/support/reverseDebuggerMcpTestHelpers.ts @@ -1,12 +1,16 @@ import { readFile, rm } from "node:fs/promises"; -import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import type { Client } from "@modelcontextprotocol/client"; import { Ajv2020 } from "ajv/dist/2020.js"; import { expect, onTestFinished } from "vitest"; import { z } from "zod"; import { toolContract } from "../../src/contracts/toolContracts.js"; -import { createServer } from "../../src/server/createServer.js"; +import type { createServer } from "../../src/server/createServer.js"; +import { + createApplicationMcpHarness, + type ApplicationMcpHarness, +} from "../fixtures/applicationMcpHarness.js"; import { createTestBinarySession } from "../fixtures/binarySession.js"; export const waitForStartedFile = async (path: string): Promise => { @@ -47,24 +51,47 @@ export const createProviderEnvironmentMcpHarness = async (options: { readonly clientName: string; readonly serverOptions: NonNullable[1]>; }) => { - const session = createTestBinarySession(() => { - throw new Error("No binary analysis provider should start"); - }); - const server = createServer( - { kind: "session", session }, - options.serverOptions, - ); - const client = new Client({ name: options.clientName, version: "1" }); - onTestFinished(async () => { - await client.close(); - await server.close(); - await session.close(); - await rm(options.directory, { recursive: true, force: true }); - }); + let closeHarness: ApplicationMcpHarness["close"] | undefined; + let directoryOwned = true; + let closing: Promise | undefined; + const close = (): Promise => { + if (closing !== undefined) return closing; + const attempt = (async () => { + await closeHarness?.(); + if (!directoryOwned) return; + await rm(options.directory, { recursive: true, force: true }); + directoryOwned = false; + })(); + closing = attempt; + void attempt.catch(() => { + if (closing === attempt) closing = undefined; + }); + return attempt; + }; + onTestFinished(close); - const [clientTransport, serverTransport] = - InMemoryTransport.createLinkedPair(); - await server.connect(serverTransport); - await client.connect(clientTransport); - return { client }; + try { + const harness = await createApplicationMcpHarness({ + session: createTestBinarySession(() => { + throw new Error("No binary analysis provider should start"); + }), + serverOptions: options.serverOptions, + clientIdentity: { name: options.clientName, version: "1" }, + registerCleanup: (close) => { + closeHarness = close; + }, + }); + return { client: harness.client, close }; + } catch (cause) { + try { + await close(); + } catch (cleanup) { + throw new AggregateError( + [cause, cleanup], + "Provider environment MCP harness setup and cleanup failed", + { cause }, + ); + } + throw cause; + } };