From 3d4ffde3c93c38a96dc51dafdcaff853d6416f67 Mon Sep 17 00:00:00 2001 From: EugeneTes Date: Wed, 23 Sep 2026 19:29:10 +0000 Subject: [PATCH] Build Playwright MCP bridge on the generic bridge --- src/agent/playwright-mcp.test.ts | 105 ++++--------------------------- src/agent/playwright-mcp.ts | 99 ++++++----------------------- 2 files changed, 30 insertions(+), 174 deletions(-) diff --git a/src/agent/playwright-mcp.test.ts b/src/agent/playwright-mcp.test.ts index bc9f508..60d0b52 100644 --- a/src/agent/playwright-mcp.test.ts +++ b/src/agent/playwright-mcp.test.ts @@ -1,105 +1,22 @@ -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { createLogger } from "../log.js"; -import { createPlaywrightMcpBridge, type McpClientLike } from "./playwright-mcp.js"; +import type { McpClientLike } from "./mcp-bridge.js"; +import { _internals, createPlaywrightMcpBridge } from "./playwright-mcp.js"; const silentLogger = createLogger("error", () => {}); -function fakeClient(overrides: Partial = {}): McpClientLike & { closed: number; calls: Array<{ name: string; arguments: Record }> } { - const state = { - closed: 0, - calls: [] as Array<{ name: string; arguments: Record }>, - }; - const client: McpClientLike = { - listTools: async () => ({ - tools: [ - { - name: "browser_navigate", - description: "Navigate to a URL", - inputSchema: { type: "object", properties: { url: { type: "string" } }, required: ["url"] }, - }, - { - name: "browser_snapshot", - description: "Capture accessibility snapshot", - inputSchema: { type: "object", properties: {} }, - }, - ], - }), - callTool: async (req) => { - state.calls.push(req); - return { content: [{ type: "text", text: `called ${req.name}` }] }; - }, - close: async () => { - state.closed += 1; - }, - ...overrides, - }; - // Expose state as live getters so mutations inside close/callTool are visible on the returned client. - return Object.defineProperties(client, { - closed: { get: () => state.closed, enumerable: true }, - calls: { get: () => state.calls, enumerable: true }, - }) as never; -} - describe("createPlaywrightMcpBridge", () => { - it("wraps every MCP tool as a Pi custom tool with matching name and description", async () => { - const client = fakeClient(); - const bridge = await createPlaywrightMcpBridge({ - headless: true, - logger: silentLogger, - clientFactory: async () => client, - }); + it("exposes MCP tools under their original browser_* names", async () => { + const client: McpClientLike = { + listTools: async () => ({ tools: [{ name: "browser_navigate" }, { name: "browser_snapshot" }] }), + callTool: async () => ({ content: [] }), + close: async () => {}, + }; + const bridge = await createPlaywrightMcpBridge({ headless: true, logger: silentLogger, clientFactory: async () => client }); expect(bridge.tools.map((t) => t.name)).toEqual(["browser_navigate", "browser_snapshot"]); - expect(bridge.tools[0].description).toBe("Navigate to a URL"); }); - it("execute forwards params to callTool and returns its content", async () => { - const client = fakeClient(); - const bridge = await createPlaywrightMcpBridge({ - headless: true, - logger: silentLogger, - clientFactory: async () => client, - }); - const navigate = bridge.tools.find((t) => t.name === "browser_navigate")!; - // Pi's execute signature: (toolCallId, params, signal, onUpdate, ctx). - // Our wrapper only reads the first two args; the rest are passed as undefined / stub. - const result = await navigate.execute( - "call-1", - { url: "https://example.com" }, - undefined, - undefined, - {} as never, - ); - expect((client as unknown as { calls: unknown[] }).calls).toEqual([ - { name: "browser_navigate", arguments: { url: "https://example.com" } }, - ]); - expect(result.content).toEqual([{ type: "text", text: "called browser_navigate" }]); - }); - - it("dispose closes the underlying client", async () => { - const client = fakeClient(); - const bridge = await createPlaywrightMcpBridge({ - headless: true, - logger: silentLogger, - clientFactory: async () => client, - }); - await bridge.dispose(); - expect((client as unknown as { closed: number }).closed).toBe(1); - }); - - it("dispose swallows client.close errors and logs a warning", async () => { - const client = fakeClient({ close: async () => { throw new Error("boom"); } }); - const warn = vi.fn(); - const bridge = await createPlaywrightMcpBridge({ - headless: true, - logger: { ...silentLogger, warn }, - clientFactory: async () => client, - }); - await expect(bridge.dispose()).resolves.toBeUndefined(); - expect(warn).toHaveBeenCalledWith(expect.stringContaining("playwright mcp close failed"), expect.any(Error)); - }); - - it("buildArgs assembles CLI flags: headless, browser, extraArgs", async () => { - const { _internals } = await import("./playwright-mcp.js"); + it("buildArgs assembles CLI flags: headless, browser, extraArgs", () => { expect(_internals.buildArgs({ headless: true, logger: silentLogger })).toEqual(["--headless"]); expect(_internals.buildArgs({ headless: false, browser: "firefox", logger: silentLogger })).toEqual(["--browser", "firefox"]); expect(_internals.buildArgs({ diff --git a/src/agent/playwright-mcp.ts b/src/agent/playwright-mcp.ts index 126c936..985df45 100644 --- a/src/agent/playwright-mcp.ts +++ b/src/agent/playwright-mcp.ts @@ -1,77 +1,30 @@ import { createRequire } from "node:module"; -import { Client } from "@modelcontextprotocol/sdk/client/index.js"; -import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; -import { defineTool } from "@earendil-works/pi-coding-agent"; -import { Type } from "typebox"; +import path from "node:path"; import type { Logger } from "../log.js"; -import type { CustomTools } from "./pi-backend.js"; +import { createMcpBridge, type McpBridge, type McpBridgeOptions } from "./mcp-bridge.js"; + +export type { McpClientLike } from "./mcp-bridge.js"; export interface PlaywrightMcpOptions { headless: boolean; browser?: string; extraArgs?: string[]; logger: Logger; - // Test seam: inject an already-connected MCP client. When omitted, the - // bridge spawns @playwright/mcp as a stdio subprocess. - clientFactory?: (argv: string[]) => Promise; + // Test seam, forwarded to createMcpBridge. + clientFactory?: McpBridgeOptions["clientFactory"]; } -export interface PlaywrightMcpBridge { - tools: CustomTools; - dispose: () => Promise; -} - -// Minimal shape of the MCP Client we depend on. Kept narrow so tests can -// implement it in a few lines. -export interface McpClientLike { - listTools: () => Promise<{ tools: McpToolDescriptor[] }>; - callTool: (req: { name: string; arguments: Record }) => Promise; - close: () => Promise; -} - -export interface McpToolDescriptor { - name: string; - description?: string; - inputSchema?: Record; - annotations?: { title?: string }; -} - -export interface McpCallResult { - content: Array>; - isError?: boolean; -} +export type PlaywrightMcpBridge = McpBridge; export async function createPlaywrightMcpBridge(opts: PlaywrightMcpOptions): Promise { - const argv = buildArgs(opts); - const factory = opts.clientFactory ?? ((a) => defaultClientFactory(a, opts.logger)); - const client = await factory(argv); - const { tools: mcpTools } = await client.listTools(); - const tools: CustomTools = mcpTools.map((t) => wrapTool(t, client)); - return { - tools, - dispose: async () => { - try { - await client.close(); - } catch (err) { - opts.logger.warn("playwright mcp close failed", err); - } - }, - }; -} - -function wrapTool(t: McpToolDescriptor, client: McpClientLike) { - return defineTool({ - name: t.name, - label: t.annotations?.title ?? t.name, - description: t.description ?? "", - parameters: Type.Unsafe>(t.inputSchema ?? { type: "object" }), - execute: async (_toolCallId, params) => { - const result = await client.callTool({ name: t.name, arguments: params as Record }); - // MCP callTool returns { content, isError? }; Pi tools return { content, details }. - // MCP's content shape (type: "text" | "image" | "resource"; text/data/uri) is compatible - // with what Pi passes through to the model. - return { content: result.content as never, details: { isError: result.isError ?? false } }; - }, + // Only resolve the bin when we'll actually spawn it, so tests don't depend on node_modules layout. + const binAbs = opts.clientFactory ? "@playwright/mcp" : resolvePlaywrightMcpBin(); + return createMcpBridge({ + label: "playwright", + command: process.execPath, // the current node binary + args: [binAbs, ...buildArgs(opts)], + logger: opts.logger, + clientFactory: opts.clientFactory, }); } @@ -83,29 +36,15 @@ function buildArgs(opts: PlaywrightMcpOptions): string[] { return args; } -async function defaultClientFactory(argv: string[], logger: Logger): Promise { - // Resolve the bin from our own node_modules so we never depend on `npx` at - // runtime and always get the pinned @playwright/mcp version. +// Resolve the bin from our own node_modules so we never depend on `npx` at +// runtime and always get the pinned @playwright/mcp version. +function resolvePlaywrightMcpBin(): string { const require = createRequire(import.meta.url); const pkgPath = require.resolve("@playwright/mcp/package.json"); const pkg = require("@playwright/mcp/package.json") as { bin?: string | Record }; const binField = typeof pkg.bin === "string" ? pkg.bin : Object.values(pkg.bin ?? {})[0]; if (!binField) throw new Error("@playwright/mcp package.json has no bin entry"); - const path = await import("node:path"); - const binAbs = path.resolve(path.dirname(pkgPath), binField); - - const transport = new StdioClientTransport({ - command: process.execPath, // the current node binary - args: [binAbs, ...argv], - stderr: "pipe", - }); - const client = new Client({ name: "volcanic-secretary", version: "0.1.0" }); - await client.connect(transport); - transport.stderr?.on("data", (buf: Buffer | string) => { - const line = (typeof buf === "string" ? buf : buf.toString("utf8")).trimEnd(); - if (line) logger.warn(`playwright mcp: ${line}`); - }); - return client as unknown as McpClientLike; + return path.resolve(path.dirname(pkgPath), binField); } // exported for tests