cleanups

Author: mxschmitt

package.json

@@ -24,6 +24,7 @@
     "ctest": "playwright test --project=chrome",
     "ftest": "playwright test --project=firefox",
     "wtest": "playwright test --project=webkit",
+    "etest": "playwright test --project=chromium-extension",
     "run-server": "node lib/browserServer.js",
     "clean": "rm -rf lib",
     "npm-publish": "npm run clean && npm run build && npm run test && npm publish"

src/browserContextFactory.ts

@@ -28,10 +28,10 @@ import type { BrowserInfo, LaunchBrowserRequest } from './browserServer.js';
 
 const testDebug = debug('pw:mcp:test');
 
-export function contextFactory(browserConfig: FullConfig['browser']): BrowserContextFactory {
+export function contextFactory(browserConfig: FullConfig['browser'], { forceCdp }: { forceCdp?: boolean } = {}): BrowserContextFactory {
   if (browserConfig.remoteEndpoint)
     return new RemoteContextFactory(browserConfig);
-  if (browserConfig.cdpEndpoint)
+  if (browserConfig.cdpEndpoint || forceCdp)
     return new CdpContextFactory(browserConfig);
   if (browserConfig.isolated)
     return new IsolatedContextFactory(browserConfig);

src/cdp-relay.ts

@@ -38,8 +38,8 @@ export class CDPBridgeServer extends EventEmitter {
     sessionId: string;
   } | undefined;
 
-  public readonly CDP_PATH = '/cdp';
-  public readonly EXTENSION_PATH = '/extension';
+  public static readonly CDP_PATH = '/cdp';
+  public static readonly EXTENSION_PATH = '/extension';
 
   constructor(server: http.Server) {
     super();
@@ -57,9 +57,9 @@ export class CDPBridgeServer extends EventEmitter {
 
     debugLogger(`New connection to ${url.pathname}`);
 
-    if (url.pathname === this.CDP_PATH) {
+    if (url.pathname === CDPBridgeServer.CDP_PATH) {
       this._handlePlaywrightConnection(ws);
-    } else if (url.pathname === this.EXTENSION_PATH) {
+    } else if (url.pathname === CDPBridgeServer.EXTENSION_PATH) {
       this._handleExtensionConnection(ws);
     } else {
       debugLogger(`Invalid path: ${url.pathname}`);
@@ -293,8 +293,8 @@ if (import.meta.url === `file://${process.argv[1]}`) {
   const server = new CDPBridgeServer(httpServer);
 
   console.error(`CDP Bridge Server listening on ws://localhost:${port}`);
-  console.error(`- Playwright MCP: ws://localhost:${port}${server.CDP_PATH}`);
-  console.error(`- Extension: ws://localhost:${port}${server.EXTENSION_PATH}`);
+  console.error(`- Playwright MCP: ws://localhost:${port}${CDPBridgeServer.CDP_PATH}`);
+  console.error(`- Extension: ws://localhost:${port}${CDPBridgeServer.EXTENSION_PATH}`);
 
   process.on('SIGINT', () => {
     debugLogger('\nShutting down bridge server...');

src/program.ts

@@ -19,12 +19,11 @@ import { program } from 'commander';
 // @ts-ignore
 import { startTraceViewerServer } from 'playwright-core/lib/server';
 
-import { startHttpTransport, startStdioTransport } from './transport.js';
+import { httpAddressToString, startHttpTransport, startStdioTransport } from './transport.js';
 import { resolveCLIConfig } from './config.js';
 import { Server } from './server.js';
 import { packageJSON } from './package.js';
 import { CDPBridgeServer } from './cdp-relay.js';
-import { AddressInfo } from 'net';
 
 program
     .version('Version ' + packageJSON.version)
@@ -58,11 +57,7 @@ program
     .option('--extension', 'Allow connecting to a running browser instance (Edge/Chrome only). Requires the \'Playwright MCP\' browser extension to be installed.')
     .action(async options => {
       const config = await resolveCLIConfig(options);
-      if (config.extension)
-        // TODO: The issue is that inside 'new Server' we create the ContextFactory
-        // instances based on if there is a CDP address given.
-        config.browser.cdpEndpoint = '1';
-      const server = new Server(config);
+      const server = new Server(config, { forceCdp: !!config.extension });
       server.setupExitWatchdog();
 
       let httpServer: http.Server | undefined = undefined;
@@ -79,12 +74,12 @@ program
         console.error('\nTrace viewer listening on ' + url);
       }
       if (config.extension && httpServer) {
-        config.browser.cdpEndpoint = `ws://127.0.0.1:${(httpServer.address() as AddressInfo).port}/cdp`;
+        const wsAddress = httpAddressToString(httpServer.address()).replace('http', 'ws');
+        config.browser.cdpEndpoint = `${wsAddress}${CDPBridgeServer.CDP_PATH}`;
         const cdpRelayServer = new CDPBridgeServer(httpServer);
-        // TODO: use watchdog to stop the server on exit
         process.on('exit', () => cdpRelayServer.stop());
         // eslint-disable-next-line no-console
-        console.error(`CDP relay server started on ws://127.0.0.1:${(httpServer.address() as AddressInfo).port}/extension - Connect to it using the browser extension.`);
+        console.error(`CDP relay server started on ${wsAddress}${CDPBridgeServer.EXTENSION_PATH} - Connect to it using the browser extension.`);
       }
     });
 

src/server.ts

@@ -28,10 +28,10 @@ export class Server {
   private _browserConfig: FullConfig['browser'];
   private _contextFactory: BrowserContextFactory;
 
-  constructor(config: FullConfig) {
+  constructor(config: FullConfig, { forceCdp }: { forceCdp: boolean }) {
     this.config = config;
     this._browserConfig = config.browser;
-    this._contextFactory = contextFactory(this._browserConfig);
+    this._contextFactory = contextFactory(this._browserConfig, { forceCdp });
   }
 
   async createConnection(transport: Transport): Promise<Connection> {

src/transport.ts

@@ -15,6 +15,7 @@
  */
 
 import http from 'node:http';
+import type { AddressInfo } from 'node:net';
 import assert from 'node:assert';
 import crypto from 'node:crypto';
 
@@ -108,18 +109,7 @@ export async function startHttpTransport(server: Server): Promise<http.Server> {
   });
   const { host, port } = server.config.server;
   await new Promise<void>(resolve => httpServer.listen(port, host, resolve));
-  const address = httpServer.address();
-  assert(address, 'Could not bind server socket');
-  let url: string;
-  if (typeof address === 'string') {
-    url = address;
-  } else {
-    const resolvedPort = address.port;
-    let resolvedHost = address.family === 'IPv4' ? address.address : `[${address.address}]`;
-    if (resolvedHost === '0.0.0.0' || resolvedHost === '[::]')
-      resolvedHost = 'localhost';
-    url = `http://${resolvedHost}:${resolvedPort}`;
-  }
+  const url = httpAddressToString(httpServer.address());
   const message = [
     `Listening on ${url}`,
     'Put this in your client config:',
@@ -136,3 +126,14 @@ export async function startHttpTransport(server: Server): Promise<http.Server> {
   console.error(message);
   return httpServer;
 }
+
+export function httpAddressToString(address: string | AddressInfo | null): string {
+  assert(address, 'Could not bind server socket');
+  if (typeof address === 'string')
+    return address;
+  const resolvedPort = address.port;
+  let resolvedHost = address.family === 'IPv4' ? address.address : `[${address.address}]`;
+  if (resolvedHost === '0.0.0.0' || resolvedHost === '[::]')
+    resolvedHost = 'localhost';
+  return `http://${resolvedHost}:${resolvedPort}`;
+}

tests/fixtures.ts

@@ -78,8 +78,6 @@ export const test = baseTest.extend<TestFixtures & TestOptions, WorkerFixtures>(
     let dispose: (() => void) | undefined;
 
     await use(async options => {
-      if (client)
-        throw new Error('Client already started');
       const args: string[] = [];
       if (userDataDir)
         args.push('--user-data-dir', userDataDir);
@@ -222,9 +220,6 @@ async function createTransport(args: string[], mcpMode: TestOptions['mcpMode']):
       command: 'docker',
       args: [...dockerArgs, 'playwright-mcp-dev:latest', ...args],
     });
-    transport.stderr?.on('data', data => {
-      stderrBuffer += data.toString();
-    });
     return {
       transport,
       stderr,
@@ -260,19 +255,23 @@ async function createTransport(args: string[], mcpMode: TestOptions['mcpMode']):
     };
   }
 
+  const transport = new StdioClientTransport({
+    command: 'node',
+    args: [path.join(path.dirname(__filename), '../cli.js'), ...args],
+    cwd: path.join(path.dirname(__filename), '..'),
+    stderr: 'pipe',
+    env: {
+      ...process.env,
+      DEBUG: 'pw:mcp:test',
+      DEBUG_COLORS: '0',
+      DEBUG_HIDE_DATE: '1',
+    },
+  });
+  transport.stderr?.on('data', data => {
+    stderrBuffer += data.toString();
+  });
   return {
-    transport: new StdioClientTransport({
-      command: 'node',
-      args: [path.join(path.dirname(__filename), '../cli.js'), ...args],
-      cwd: path.join(path.dirname(__filename), '..'),
-      stderr: 'pipe',
-      env: {
-        ...process.env,
-        DEBUG: 'pw:mcp:test',
-        DEBUG_COLORS: '0',
-        DEBUG_HIDE_DATE: '1',
-      },
-    }),
+    transport,
     stderr,
   };
 }

tests/pdf.spec.ts

@@ -30,7 +30,7 @@ test('save as pdf unavailable', async ({ startClient, server }) => {
   })).toHaveTextContent(/Tool \"browser_pdf_save\" not found/);
 });
 
-test('save as pdf', async ({ startClient, mcpBrowser, server, mcpExtensionPage }, testInfo) => {
+test('save as pdf', async ({ startClient, mcpBrowser, server }, testInfo) => {
   const { client } = await startClient({
     config: { outputDir: testInfo.outputPath('output') },
   });
@@ -41,6 +41,7 @@ test('save as pdf', async ({ startClient, mcpBrowser, server, mcpExtensionPage }
     name: 'browser_navigate',
     arguments: { url: server.HELLO_WORLD },
   })).toContainTextContent(`- generic [ref=e1]: Hello, world!`);
+
   const response = await client.callTool({
     name: 'browser_pdf_save',
   });