From 03ccdb9fbc2d6a556ccf2ea63d1bd6b0d1a82ef7 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 3 Jun 2026 13:59:25 +0200 Subject: [PATCH] test(e2e): assert mcp reconnect temp state --- CHANGELOG.md | 1 + scripts/e2e/mcp-channels-docker-client.ts | 29 ++++----- scripts/e2e/mcp-client-temp-state.ts | 31 +++++++++ test/scripts/mcp-channels-harness.test.ts | 76 ++++++++++++++++++++--- 4 files changed, 114 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ab045f282ea..3bbdffcf1f3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ Docs: https://docs.openclaw.ai - Release/CI/E2E: reject oversized OpenAI image-auth mock request bodies before Docker proof runs can accumulate unbounded payloads. - Release/CI/E2E: require the Kitchen Sink RPC walk to prove every expected plugin tool is cataloged and effective before invoking tool fixtures. - Release/CI/E2E: stop tracked Docker build commands when centralized build wrappers receive shutdown signals. +- Release/CI/E2E: cover MCP channel pairing reconnects by asserting the same temporary client state is reused across reconnects. - Release/CI/E2E: fail secret-provider proof runs when temporary state cleanup still fails after retries instead of hiding the cleanup error. - Release/CI/E2E: fail package-candidate ref proofs when temporary source worktree cleanup fails instead of leaving stale worktrees behind. - Release/CI/E2E: remove package tarball extract directories when tar extraction fails before validation can continue. diff --git a/scripts/e2e/mcp-channels-docker-client.ts b/scripts/e2e/mcp-channels-docker-client.ts index 50da58a703e1..ec124bdbb7f5 100644 --- a/scripts/e2e/mcp-channels-docker-client.ts +++ b/scripts/e2e/mcp-channels-docker-client.ts @@ -11,7 +11,10 @@ import { maybeApprovePendingBridgePairing, waitFor, } from "./mcp-channels-harness.ts"; -import { createMcpClientTempState } from "./mcp-client-temp-state.ts"; +import { + connectMcpClientWithPairingReconnect, + createMcpClientTempState, +} from "./mcp-client-temp-state.ts"; function summarizeSessionRows(rows: Array> | undefined) { return (rows ?? []).map((entry) => ({ @@ -107,23 +110,17 @@ async function main() { "expected seeded gateway deliveryContext target", ); - mcpHandle = await connectMcpClient({ - gatewayUrl, - gatewayToken, + mcpHandle = await connectMcpClientWithPairingReconnect({ tempState: mcpTempState, + connect: (tempState) => + connectMcpClient({ + gatewayUrl, + gatewayToken, + tempState, + }), + maybeApprovePairing: () => maybeApprovePendingBridgePairing(gateway), }); - let mcp = mcpHandle.client; - - if (await maybeApprovePendingBridgePairing(gateway)) { - await Promise.allSettled([mcp.close(), mcpHandle.transport.close()]); - mcpHandle.cleanup(); - mcpHandle = await connectMcpClient({ - gatewayUrl, - gatewayToken, - tempState: mcpTempState, - }); - mcp = mcpHandle.client; - } + const mcp = mcpHandle.client; const callTool = (params: Parameters[0]) => mcp.callTool(params, undefined, { timeout: 240_000 }) as Promise; diff --git a/scripts/e2e/mcp-client-temp-state.ts b/scripts/e2e/mcp-client-temp-state.ts index 99654cf440c7..5e6d9310eebf 100644 --- a/scripts/e2e/mcp-client-temp-state.ts +++ b/scripts/e2e/mcp-client-temp-state.ts @@ -9,6 +9,12 @@ export type McpClientTempState = { tokenFile: string; }; +export type ReconnectableMcpClientHandle = { + cleanup: () => void; + client: { close: () => Promise }; + transport: { close: () => Promise }; +}; + export function createMcpClientTempState(params: { gatewayToken: string; tempRoot?: string; @@ -27,3 +33,28 @@ export function createMcpClientTempState(params: { tokenFile, }; } + +export async function connectMcpClientWithPairingReconnect< + T extends ReconnectableMcpClientHandle, +>(params: { + connect: (tempState: McpClientTempState) => Promise; + maybeApprovePairing: () => Promise; + tempState: McpClientTempState; +}): Promise { + let handle = await params.connect(params.tempState); + let shouldReconnect: boolean; + try { + shouldReconnect = await params.maybeApprovePairing(); + } catch (error) { + await Promise.allSettled([handle.client.close(), handle.transport.close()]); + handle.cleanup(); + throw error; + } + if (!shouldReconnect) { + return handle; + } + await Promise.allSettled([handle.client.close(), handle.transport.close()]); + handle.cleanup(); + handle = await params.connect(params.tempState); + return handle; +} diff --git a/test/scripts/mcp-channels-harness.test.ts b/test/scripts/mcp-channels-harness.test.ts index 0b17ba78e762..dc35d4502910 100644 --- a/test/scripts/mcp-channels-harness.test.ts +++ b/test/scripts/mcp-channels-harness.test.ts @@ -1,8 +1,12 @@ import { existsSync, mkdtempSync, readFileSync, rmSync, statSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; -import { createMcpClientTempState } from "../../scripts/e2e/mcp-client-temp-state.js"; +import { describe, expect, it, vi } from "vitest"; +import { + connectMcpClientWithPairingReconnect, + createMcpClientTempState, + type McpClientTempState, +} from "../../scripts/e2e/mcp-client-temp-state.js"; describe("mcp-channels harness", () => { it("creates unique client temp state and removes token files on cleanup", () => { @@ -27,11 +31,69 @@ describe("mcp-channels harness", () => { } }); - it("reuses one MCP temp state across the pairing reconnect path", () => { - const source = readFileSync("scripts/e2e/mcp-channels-docker-client.ts", "utf8"); + it("reuses one MCP temp state across the pairing reconnect path", async () => { + const tempState = createMcpClientTempState({ gatewayToken: "pairing-token" }); + const firstHandle = { + cleanup: vi.fn(), + client: { close: vi.fn(async () => undefined) }, + transport: { close: vi.fn(async () => undefined) }, + }; + const secondHandle = { + cleanup: vi.fn(), + client: { close: vi.fn(async () => undefined) }, + transport: { close: vi.fn(async () => undefined) }, + }; + const connectCalls: McpClientTempState[] = []; + const connect = vi.fn(async (state: McpClientTempState) => { + connectCalls.push(state); + return connectCalls.length === 1 ? firstHandle : secondHandle; + }); - expect(source).toContain("const mcpTempState = createMcpClientTempState({ gatewayToken });"); - expect(source.match(/tempState: mcpTempState/gu)).toHaveLength(2); - expect(source).toContain("mcpTempState.cleanup();"); + try { + await expect( + connectMcpClientWithPairingReconnect({ + connect, + maybeApprovePairing: async () => true, + tempState, + }), + ).resolves.toBe(secondHandle); + + expect(connect).toHaveBeenCalledTimes(2); + expect(connectCalls).toEqual([tempState, tempState]); + expect(firstHandle.client.close).toHaveBeenCalledOnce(); + expect(firstHandle.transport.close).toHaveBeenCalledOnce(); + expect(firstHandle.cleanup).toHaveBeenCalledOnce(); + expect(secondHandle.cleanup).not.toHaveBeenCalled(); + } finally { + tempState.cleanup(); + } + }); + + it("cleans up the first MCP client when pairing approval fails", async () => { + const tempState = createMcpClientTempState({ gatewayToken: "pairing-token" }); + const handle = { + cleanup: vi.fn(), + client: { close: vi.fn(async () => undefined) }, + transport: { close: vi.fn(async () => undefined) }, + }; + const failure = new Error("pairing approval failed"); + + try { + await expect( + connectMcpClientWithPairingReconnect({ + connect: async () => handle, + maybeApprovePairing: async () => { + throw failure; + }, + tempState, + }), + ).rejects.toBe(failure); + + expect(handle.client.close).toHaveBeenCalledOnce(); + expect(handle.transport.close).toHaveBeenCalledOnce(); + expect(handle.cleanup).toHaveBeenCalledOnce(); + } finally { + tempState.cleanup(); + } }); });