diff --git a/src/main/transport.ts b/src/main/transport.ts index 8735fce606..847f7b42d9 100644 --- a/src/main/transport.ts +++ b/src/main/transport.ts @@ -87,8 +87,12 @@ export const startMcpTransport = async ( androidProviders.delete(provider); }), ); - for (const result of results) - if (result.status === "rejected") throw result.reason; + const failures = results.flatMap((result) => + result.status === "rejected" ? [result.reason] : [], + ); + if (failures.length === 1) throw failures[0]; + if (failures.length > 1) + throw new AggregateError(failures, "Android provider cleanup failed"); }; // Optional providers are shared by every server instance the SDK creates, // including a discarded discovery probe, so only process shutdown closes them. diff --git a/tests/boundary/mcp/transportCleanup.test.ts b/tests/boundary/mcp/transportCleanup.test.ts new file mode 100644 index 0000000000..0b7247b2a2 --- /dev/null +++ b/tests/boundary/mcp/transportCleanup.test.ts @@ -0,0 +1,74 @@ +import { expect, it, vi } from "vitest"; +import { createManagedBinarySession } from "../../../src/composition/binary.js"; +import { silentLogger } from "../../../src/logger.js"; +import { startMcpTransport } from "../../../src/main/transport.js"; +import { createServer } from "../../../src/server/createServer.js"; +import { createToolResultDelivery } from "../../../src/server/toolResult.js"; + +it("preserves Android sibling cleanup failures and retries only unfinished owners", async () => { + const session = createManagedBinarySession(); + const failures = [ + new Error("first Android cleanup"), + new Error("second Android cleanup"), + ]; + const closes: ReturnType[] = []; + let fail = true; + const transport = await startMcpTransport( + { + env: {}, + serve: async (factory) => { + // The SDK can create a discovery probe followed by replacement servers. + await factory({ era: "modern" }); + await factory({ era: "legacy" }); + await factory({ era: "modern" }); + return { close: async () => undefined }; + }, + createServer: (source, options) => { + const provider = options?.androidAnalysis; + if (provider === undefined) + throw new Error("Missing owned Android provider"); + const failure = failures[closes.length]; + closes.push( + vi.spyOn(provider, "close").mockImplementation(async () => { + if (fail && failure !== undefined) throw failure; + }), + ); + return createServer(source, options); + }, + writeStderr: () => undefined, + setExitCode: () => undefined, + registerShutdown: () => () => undefined, + }, + session, + { + logger: silentLogger, + serverLogger: silentLogger, + environment: {}, + delivery: createToolResultDelivery(undefined), + loadOptionalProviders: async () => ({}), + }, + ); + try { + expect(transport.ok).toBe(true); + await expect(transport.closeAndroid()).rejects.toMatchObject({ + name: "AggregateError", + errors: failures, + }); + expect(closes.map((close) => close.mock.calls.length)).toEqual([1, 1, 1]); + // One recovered owner leaves the exact remaining failure available. + closes[0]?.mockResolvedValue(undefined); + await expect(transport.closeAndroid()).rejects.toBe(failures[1]); + expect(closes.map((close) => close.mock.calls.length)).toEqual([2, 2, 1]); + fail = false; + await transport.closeAndroid(); + await transport.closeAndroid(); + expect(closes.map((close) => close.mock.calls.length)).toEqual([2, 3, 1]); + } finally { + fail = false; + for (const close of closes) close.mockRestore(); + await transport.handle.close(); + await transport.closeAndroid(); + await transport.closeOptionalProviders(); + await session.close(); + } +});