test: isolate plugin tools mcp handlers

src/mcp/plugin-tools-handlers.ts

@@ -0,0 +1,66 @@
+import {
+  isToolWrappedWithBeforeToolCallHook,
+  wrapToolWithBeforeToolCallHook,
+} from "../agents/pi-tools.before-tool-call.js";
+import type { AnyAgentTool } from "../agents/tools/common.js";
+import { formatErrorMessage } from "../infra/errors.js";
+
+type CallPluginToolParams = {
+  name: string;
+  arguments?: unknown;
+};
+
+function resolveJsonSchemaForTool(tool: AnyAgentTool): Record<string, unknown> {
+  const params = tool.parameters;
+  if (params && typeof params === "object" && "type" in params) {
+    return params as Record<string, unknown>;
+  }
+  return { type: "object", properties: {} };
+}
+
+export function createPluginToolsMcpHandlers(tools: AnyAgentTool[]) {
+  const wrappedTools = tools.map((tool) => {
+    if (isToolWrappedWithBeforeToolCallHook(tool)) {
+      return tool;
+    }
+    // The ACPX MCP bridge should enforce the same pre-execution hook boundary
+    // as the agent and HTTP tool execution paths.
+    return wrapToolWithBeforeToolCallHook(tool);
+  });
+  const toolMap = new Map<string, AnyAgentTool>();
+  for (const tool of wrappedTools) {
+    toolMap.set(tool.name, tool);
+  }
+
+  return {
+    listTools: async () => ({
+      tools: wrappedTools.map((tool) => ({
+        name: tool.name,
+        description: tool.description ?? "",
+        inputSchema: resolveJsonSchemaForTool(tool),
+      })),
+    }),
+    callTool: async (params: CallPluginToolParams) => {
+      const tool = toolMap.get(params.name);
+      if (!tool) {
+        return {
+          content: [{ type: "text", text: `Unknown tool: ${params.name}` }],
+          isError: true,
+        };
+      }
+      try {
+        const result = await tool.execute(`mcp-${Date.now()}`, params.arguments ?? {});
+        return {
+          content: Array.isArray(result.content)
+            ? result.content
+            : [{ type: "text", text: String(result.content) }],
+        };
+      } catch (err) {
+        return {
+          content: [{ type: "text", text: `Tool error: ${formatErrorMessage(err)}` }],
+          isError: true,
+        };
+      }
+    },
+  };
+}

src/mcp/plugin-tools-serve.test.ts

@@ -1,5 +1,3 @@
-import { Client } from "@modelcontextprotocol/sdk/client/index.js";
-import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import type { AnyAgentTool } from "../agents/tools/common.js";
 import {
@@ -7,25 +5,17 @@ import {
   resetGlobalHookRunner,
 } from "../plugins/hook-runner-global.js";
 import { createMockPluginRegistry } from "../plugins/hooks.test-helpers.js";
-import { createPluginToolsMcpServer } from "./plugin-tools-serve.js";
+import { createPluginToolsMcpHandlers } from "./plugin-tools-handlers.js";
 
-async function connectPluginToolsServer(tools: AnyAgentTool[]) {
-  const server = createPluginToolsMcpServer({ tools });
-  const client = new Client({ name: "plugin-tools-test-client", version: "1.0.0" });
-  const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
-  await server.connect(serverTransport);
-  await client.connect(clientTransport);
-  return {
-    client,
-    close: async () => {
-      await client.close();
-      await server.close();
-    },
-  };
-}
+const callGatewayTool = vi.hoisted(() => vi.fn());
+
+vi.mock("../agents/tools/gateway.js", () => ({
+  callGatewayTool,
+}));
 
 afterEach(() => {
   vi.restoreAllMocks();
+  callGatewayTool.mockReset();
   resetGlobalHookRunner();
 });
 
@@ -47,36 +37,32 @@ describe("plugin tools MCP server", () => {
       execute,
     } as unknown as AnyAgentTool;
 
-    const session = await connectPluginToolsServer([tool]);
-    try {
-      const listed = await session.client.listTools();
-      expect(listed.tools).toEqual([
-        expect.objectContaining({
-          name: "memory_recall",
-          description: "Recall stored memory",
-          inputSchema: expect.objectContaining({
-            type: "object",
-            required: ["query"],
-          }),
-        }),
-      ]);
-
-      const result = await session.client.callTool({
+    const handlers = createPluginToolsMcpHandlers([tool]);
+    const listed = await handlers.listTools();
+    expect(listed.tools).toEqual([
+      expect.objectContaining({
         name: "memory_recall",
-        arguments: { query: "remember this" },
-      });
-      expect(execute).toHaveBeenCalledWith(
-        expect.stringMatching(/^mcp-\d+$/),
-        {
-          query: "remember this",
-        },
-        undefined,
-        undefined,
-      );
-      expect(result.content).toEqual([{ type: "text", text: "Stored." }]);
-    } finally {
-      await session.close();
-    }
+        description: "Recall stored memory",
+        inputSchema: expect.objectContaining({
+          type: "object",
+          required: ["query"],
+        }),
+      }),
+    ]);
+
+    const result = await handlers.callTool({
+      name: "memory_recall",
+      arguments: { query: "remember this" },
+    });
+    expect(execute).toHaveBeenCalledWith(
+      expect.stringMatching(/^mcp-\d+$/),
+      {
+        query: "remember this",
+      },
+      undefined,
+      undefined,
+    );
+    expect(result.content).toEqual([{ type: "text", text: "Stored." }]);
   });
 
   it("returns MCP errors for unknown tools and thrown tool errors", async () => {
@@ -87,24 +73,20 @@ describe("plugin tools MCP server", () => {
       execute: vi.fn().mockRejectedValue(new Error("boom")),
     } as unknown as AnyAgentTool;
 
-    const session = await connectPluginToolsServer([failingTool]);
-    try {
-      const unknown = await session.client.callTool({
-        name: "missing_tool",
-        arguments: {},
-      });
-      expect(unknown.isError).toBe(true);
-      expect(unknown.content).toEqual([{ type: "text", text: "Unknown tool: missing_tool" }]);
+    const handlers = createPluginToolsMcpHandlers([failingTool]);
+    const unknown = await handlers.callTool({
+      name: "missing_tool",
+      arguments: {},
+    });
+    expect(unknown.isError).toBe(true);
+    expect(unknown.content).toEqual([{ type: "text", text: "Unknown tool: missing_tool" }]);
 
-      const failed = await session.client.callTool({
-        name: "memory_forget",
-        arguments: {},
-      });
-      expect(failed.isError).toBe(true);
-      expect(failed.content).toEqual([{ type: "text", text: "Tool error: boom" }]);
-    } finally {
-      await session.close();
-    }
+    const failed = await handlers.callTool({
+      name: "memory_forget",
+      arguments: {},
+    });
+    expect(failed.isError).toBe(true);
+    expect(failed.content).toEqual([{ type: "text", text: "Tool error: boom" }]);
   });
 
   it("blocks tool execution when before_tool_call requires approval on the MCP bridge", async () => {
@@ -129,6 +111,7 @@ describe("plugin tools MCP server", () => {
         },
       ]),
     );
+    callGatewayTool.mockRejectedValueOnce(new Error("gateway unavailable"));
     const tool = {
       name: "memory_store",
       description: "Store memory",
@@ -136,20 +119,16 @@ describe("plugin tools MCP server", () => {
       execute,
     } as unknown as AnyAgentTool;
 
-    const session = await connectPluginToolsServer([tool]);
-    try {
-      const result = await session.client.callTool({
-        name: "memory_store",
-        arguments: { text: "remember this" },
-      });
-      expect(hookCalls).toBe(1);
-      expect(execute).not.toHaveBeenCalled();
-      expect(result.isError).toBe(true);
-      expect(result.content).toEqual([
-        { type: "text", text: "Tool error: Plugin approval required (gateway unavailable)" },
-      ]);
-    } finally {
-      await session.close();
-    }
+    const handlers = createPluginToolsMcpHandlers([tool]);
+    const result = await handlers.callTool({
+      name: "memory_store",
+      arguments: { text: "remember this" },
+    });
+    expect(hookCalls).toBe(1);
+    expect(execute).not.toHaveBeenCalled();
+    expect(result.isError).toBe(true);
+    expect(result.content).toEqual([
+      { type: "text", text: "Tool error: Plugin approval required (gateway unavailable)" },
+    ]);
   });
 });

src/mcp/plugin-tools-serve.ts

@@ -10,10 +10,6 @@ import { pathToFileURL } from "node:url";
 import { Server } from "@modelcontextprotocol/sdk/server/index.js";
 import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
 import { CallToolRequestSchema, ListToolsRequestSchema } from "@modelcontextprotocol/sdk/types.js";
-import {
-  isToolWrappedWithBeforeToolCallHook,
-  wrapToolWithBeforeToolCallHook,
-} from "../agents/pi-tools.before-tool-call.js";
 import type { AnyAgentTool } from "../agents/tools/common.js";
 import { loadConfig } from "../config/config.js";
 import type { OpenClawConfig } from "../config/types.openclaw.js";
@@ -21,15 +17,7 @@ import { formatErrorMessage } from "../infra/errors.js";
 import { routeLogsToStderr } from "../logging/console.js";
 import { resolvePluginTools } from "../plugins/tools.js";
 import { VERSION } from "../version.js";
-
-function resolveJsonSchemaForTool(tool: AnyAgentTool): Record<string, unknown> {
-  const params = tool.parameters;
-  if (params && typeof params === "object" && "type" in params) {
-    return params as Record<string, unknown>;
-  }
-  // Fallback: accept any object
-  return { type: "object", properties: {} };
-}
+import { createPluginToolsMcpHandlers } from "./plugin-tools-handlers.js";
 
 function resolveTools(config: OpenClawConfig): AnyAgentTool[] {
   return resolvePluginTools({
@@ -45,54 +33,18 @@ export function createPluginToolsMcpServer(
   } = {},
 ): Server {
   const cfg = params.config ?? loadConfig();
-  const tools = (params.tools ?? resolveTools(cfg)).map((tool) => {
-    if (isToolWrappedWithBeforeToolCallHook(tool)) {
-      return tool;
-    }
-    // The ACPX MCP bridge should enforce the same pre-execution hook boundary
-    // as the agent and HTTP tool execution paths.
-    return wrapToolWithBeforeToolCallHook(tool);
-  });
-
-  const toolMap = new Map<string, AnyAgentTool>();
-  for (const tool of tools) {
-    toolMap.set(tool.name, tool);
-  }
+  const tools = params.tools ?? resolveTools(cfg);
+  const handlers = createPluginToolsMcpHandlers(tools);
 
   const server = new Server(
     { name: "openclaw-plugin-tools", version: VERSION },
     { capabilities: { tools: {} } },
   );
 
-  server.setRequestHandler(ListToolsRequestSchema, async () => ({
-    tools: tools.map((tool) => ({
-      name: tool.name,
-      description: tool.description ?? "",
-      inputSchema: resolveJsonSchemaForTool(tool),
-    })),
-  }));
+  server.setRequestHandler(ListToolsRequestSchema, handlers.listTools);
 
   server.setRequestHandler(CallToolRequestSchema, async (request) => {
-    const tool = toolMap.get(request.params.name);
-    if (!tool) {
-      return {
-        content: [{ type: "text", text: `Unknown tool: ${request.params.name}` }],
-        isError: true,
-      };
-    }
-    try {
-      const result = await tool.execute(`mcp-${Date.now()}`, request.params.arguments ?? {});
-      return {
-        content: Array.isArray(result.content)
-          ? result.content
-          : [{ type: "text", text: String(result.content) }],
-      };
-    } catch (err) {
-      return {
-        content: [{ type: "text", text: `Tool error: ${formatErrorMessage(err)}` }],
-        isError: true,
-      };
-    }
+    return await handlers.callTool(request.params);
   });
 
   return server;