cleanups

Author: mxschmitt

README.md

@@ -188,9 +188,6 @@ Playwright MCP server supports following arguments. They can be provided in the
                                example "1280, 720"
   --vision                     Run server that uses screenshots (Aria snapshots
                                are used by default)
-  --extension                  Allow connecting to a running browser instance
-                               (Edge/Chrome only). Requires the 'Playwright MCP'
-                               browser extension to be installed.
 ```
 
 <!--- End of options generated section -->

config.d.ts

@@ -104,11 +104,6 @@ export type Config = {
    */
   saveTrace?: boolean;
 
-  /**
-   * Run server that is able to connect to the 'Playwright MCP' Chrome extension.
-   */
-  extension?: boolean;
-
   /**
    * The directory to save output files.
    */

extension/background.js

@@ -5,7 +5,7 @@
  */
 
 function debugLog(...args) {
-  const enabled = true;
+  const enabled = false;
   if (enabled) {
     console.log('[Extension]', ...args);
   }

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/config.ts

@@ -19,10 +19,18 @@ import os from 'os';
 import path from 'path';
 import { devices } from 'playwright';
 
-import type { Config, ToolCapability } from '../config.js';
+import type { Config as PublicConfig, ToolCapability } from '../config.js';
 import type { BrowserContextOptions, LaunchOptions } from 'playwright';
 import { sanitizeForFilePath } from './tools/utils.js';
 
+type Config = PublicConfig & {
+  /**
+   * TODO: Move to PublicConfig once we are ready to release this feature.
+   * Run server that is able to connect to the 'Playwright MCP' Chrome extension.
+   */
+  extension?: boolean;
+};
+
 export type CLIOptions = {
   allowedOrigins?: string[];
   blockedOrigins?: string[];

src/program.ts

@@ -15,16 +15,15 @@
  */
 
 import type http from 'http';
-import { program } from 'commander';
+import { Option, 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)
@@ -55,14 +54,10 @@ program
     .option('--user-data-dir <path>', 'path to the user data directory. If not specified, a temporary directory will be created.')
     .option('--viewport-size <size>', 'specify browser viewport size in pixels, for example "1280, 720"')
     .option('--vision', 'Run server that uses screenshots (Aria snapshots are used by default)')
-    .option('--extension', 'Allow connecting to a running browser instance (Edge/Chrome only). Requires the \'Playwright MCP\' browser extension to be installed.')
+    .addOption(new Option('--extension', 'Allow connecting to a running browser instance (Edge/Chrome only). Requires the \'Playwright MCP\' browser extension to be installed.').hideHelp())
     .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}`;
+}