From 98b398606246f6c3f8736ad28498a75dcaae68f7 Mon Sep 17 00:00:00 2001 From: "Tommy D. Rossi" Date: Sun, 28 Dec 2025 15:16:17 +0100 Subject: [PATCH 1/4] fix Runtime.enable race condition with event-based synchronization Replace arbitrary sleep(200ms) with proper wait for Runtime.executionContextCreated event in the relay server. This ensures pages are fully ready before Runtime.enable returns to Playwright, fixing intermittent 'page not found' errors. --- extension/src/background.ts | 12 +-- playwriter/src/cdp-timing.md | 128 ++++++++++++++++++++++++++ playwriter/src/extension/cdp-relay.ts | 31 +++++++ playwriter/src/mcp.test.ts | 8 +- 4 files changed, 165 insertions(+), 14 deletions(-) create mode 100644 playwriter/src/cdp-timing.md diff --git a/extension/src/background.ts b/extension/src/background.ts index bbddea1..242164c 100644 --- a/extension/src/background.ts +++ b/extension/src/background.ts @@ -232,7 +232,7 @@ async function handleCommand(msg: ExtensionCommandMessage): Promise { } try { await chrome.debugger.sendCommand(debuggee, 'Runtime.disable') - await sleep(400) + await sleep(200) } catch (e) { logger.debug('Error disabling Runtime (ignoring):', e) } @@ -507,15 +507,7 @@ function handleConnectionClose(reason: string, code: number): void { return } - store.setState((state) => { - const newTabs = new Map(state.tabs) - for (const [tabId, tab] of newTabs) { - if (tab.state === 'connected') { - newTabs.set(tabId, { ...tab, state: 'connecting' }) - } - } - return { tabs: newTabs, connectionState: 'disconnected', errorText: undefined } - }) + store.setState({ connectionState: 'disconnected', errorText: undefined }) if (tabs.size > 0) { logger.debug('Tabs still connected, triggering reconnection') diff --git a/playwriter/src/cdp-timing.md b/playwriter/src/cdp-timing.md new file mode 100644 index 0000000..d6e2cad --- /dev/null +++ b/playwriter/src/cdp-timing.md @@ -0,0 +1,128 @@ +# CDP Event Timing and Synchronization + +This document describes the timing issues we discovered and fixed in the CDP relay server, and outlines potential future improvements. + +## The Problem + +When Playwright connects to Chrome via our CDP relay, there's a race condition between: +1. **Target attachment** - Extension attaches to a tab and sends `Target.attachedToTarget` +2. **Runtime initialization** - Playwright calls `Runtime.enable` to set up JavaScript execution contexts +3. **Page visibility** - `context.pages()` returns pages that are fully ready + +### Symptom +Tests or MCP calls that run immediately after toggling the extension would fail with "page not found" because `context.pages()` didn't include the newly attached page yet. + +### Root Cause +The `Runtime.enable` CDP command triggers `Runtime.executionContextCreated` events that tell Playwright the page's JavaScript context is ready. Without these events, pages aren't fully visible to `context.pages()`. + +Previously, the extension had an arbitrary `sleep(200ms)` in the `Runtime.enable` handler to work around this. When this was increased to 400ms, tests started failing due to timing mismatches. + +## The Fix + +We replaced the arbitrary sleep with **event-based synchronization** in the relay server: + +```typescript +case 'Runtime.enable': { + // Set up listener for executionContextCreated + const contextCreatedPromise = new Promise((resolve) => { + const handler = ({ event }) => { + if (event.method === 'Runtime.executionContextCreated' && + event.sessionId === sessionId) { + clearTimeout(timeout) + emitter.off('cdp:event', handler) + resolve() + } + } + const timeout = setTimeout(() => { + emitter.off('cdp:event', handler) + logger?.log('IMPORTANT: Runtime.enable timed out...') + resolve() + }, 3000) + emitter.on('cdp:event', handler) + }) + + // Forward command to extension + const result = await sendToExtension(...) + + // Wait for the event before returning + await contextCreatedPromise + + return result +} +``` + +### Why This Works +- When Playwright calls `Runtime.enable`, we forward it to the extension +- The extension enables Runtime on the Chrome tab +- Chrome sends `Runtime.executionContextCreated` events +- We wait for at least one such event before returning +- By the time `Runtime.enable` returns, the page's context is ready + +## Event Flow + +``` +Test toggles extension + ↓ +Extension attaches to tab + ↓ +Extension sends Target.attachedToTarget to relay + ↓ +Relay broadcasts to Playwright clients + ↓ +Playwright calls Runtime.enable ──────────────────┐ + ↓ │ +Relay forwards to extension │ + ↓ │ +Extension enables Runtime on Chrome tab │ + ↓ │ +Chrome sends Runtime.executionContextCreated │ + ↓ │ +Relay receives event, resolves promise ───────────┘ + ↓ +Runtime.enable returns to Playwright + ↓ +Page is now visible in context.pages() +``` + +## Future Improvements + +### 1. Wait for Target Attachment + +Currently, tests still need small waits after `toggleExtensionForActiveTab()` because the MCP's Playwright browser needs time to process `Target.attachedToTarget`. + +A potential improvement: Track "pending" vs "ready" targets in the relay server: +- When `Target.attachedToTarget` arrives → mark as pending +- When `Runtime.executionContextCreated` arrives → mark as ready +- Expose an endpoint or mechanism for MCP to wait for all targets to be ready + +### 2. Extension-Level Confirmation + +The extension could wait for confirmation from the relay server before returning from `attachTab()`: +- Extension sends `Target.attachedToTarget` +- Relay waits for `Runtime.executionContextCreated` +- Relay sends acknowledgment back to extension +- Extension's `attachTab()` returns +- `toggleExtensionForActiveTab()` returns with page fully ready + +This would eliminate the need for any waits in test code. + +### 3. MCP-Level Page Readiness Check + +The MCP could check page readiness before executing code: +- Before running user code, verify each page in `context.pages()` has a ready execution context +- Use Playwright's `page.evaluate()` with a simple expression to confirm the page is responsive + +## Test Implications + +The current test waits (100ms after toggle) exist for multiple reasons: +1. **Target attachment** - Waiting for `Target.attachedToTarget` to be processed +2. **Navigation completion** - Waiting for page loads/navigations +3. **State cleanup** - Ensuring previous test state is cleared +4. **Debugger synchronization** - Waiting for breakpoints/pause states + +The `Runtime.enable` fix addresses reason #1 at the CDP level, but test waits are still needed for the other reasons. + +## Files Changed + +- `playwriter/src/extension/cdp-relay.ts` - Added event-based wait in `Runtime.enable` handler +- `extension/src/background.ts` - Removed arbitrary `sleep(200)` from `Runtime.enable` handler diff --git a/playwriter/src/extension/cdp-relay.ts b/playwriter/src/extension/cdp-relay.ts index e763e97..5de54ef 100644 --- a/playwriter/src/extension/cdp-relay.ts +++ b/playwriter/src/extension/cdp-relay.ts @@ -262,6 +262,37 @@ export async function startPlayWriterCDPRelayServer({ port = 19988, host = '127. params: { method, params } }) } + + case 'Runtime.enable': { + if (!sessionId) { + break + } + + const contextCreatedPromise = new Promise((resolve) => { + const handler = ({ event }: { event: CDPEventBase }) => { + if (event.method === 'Runtime.executionContextCreated' && event.sessionId === sessionId) { + clearTimeout(timeout) + emitter.off('cdp:event', handler) + resolve() + } + } + const timeout = setTimeout(() => { + emitter.off('cdp:event', handler) + logger?.log(chalk.yellow(`IMPORTANT: Runtime.enable timed out waiting for executionContextCreated (sessionId: ${sessionId}). This may cause pages to not be visible immediately.`)) + resolve() + }, 3000) + emitter.on('cdp:event', handler) + }) + + const result = await sendToExtension({ + method: 'forwardCDPCommand', + params: { sessionId, method, params } + }) + + await contextCreatedPromise + + return result + } } return await sendToExtension({ diff --git a/playwriter/src/mcp.test.ts b/playwriter/src/mcp.test.ts index 26a5691..1021fa9 100644 --- a/playwriter/src/mcp.test.ts +++ b/playwriter/src/mcp.test.ts @@ -1586,13 +1586,13 @@ describe('MCP Server Tests', () => { expect(normalized).toMatchInlineSnapshot(` { "cssLayoutViewport": { - "clientHeight": 528, + "clientHeight": 720, "clientWidth": 1280, "pageX": 0, "pageY": 0, }, "cssVisualViewport": { - "clientHeight": 528, + "clientHeight": 720, "clientWidth": 1280, "offsetX": 0, "offsetY": 0, @@ -1603,13 +1603,13 @@ describe('MCP Server Tests', () => { }, "devicePixelRatio": 1, "layoutViewport": { - "clientHeight": 528, + "clientHeight": 720, "clientWidth": 1280, "pageX": 0, "pageY": 0, }, "visualViewport": { - "clientHeight": 528, + "clientHeight": 720, "clientWidth": 1280, "offsetX": 0, "offsetY": 0, From eb62f6223aa685ae4b56da6c5d2f6f4a0ecce52f Mon Sep 17 00:00:00 2001 From: "Tommy D. Rossi" Date: Sun, 28 Dec 2025 19:01:08 +0100 Subject: [PATCH 2/4] allow changing port and host via options and env --- README.md | 14 +++++++++++++- playwriter/src/cli.ts | 24 ++++++++++++++++++------ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index bde38bb..d8cc51f 100644 --- a/README.md +++ b/README.md @@ -185,18 +185,30 @@ Run agents in isolated environments (devcontainers, VMs, SSH) while controlling ```bash npx playwriter serve --token +# or use environment variable: +PLAYWRITER_TOKEN= npx playwriter serve ``` **In container/VM (where agent runs):** +Using environment variables: + ```bash -export PLAYWRITER_URL="ws://host.docker.internal:19988?token=" +export PLAYWRITER_HOST="host.docker.internal" +export PLAYWRITER_TOKEN="" +``` + +Or using CLI options: + +```bash +npx playwriter --host host.docker.internal --token ``` Use `host.docker.internal` for devcontainers, or your host's IP for VMs/SSH. ## Known Issues +- **Do not use `bunx` to run the MCP.** Bun has incomplete support for the `ws` library's WebSocket events, which breaks playwright-core's CDP connection. Use `npx` instead. See bun issues: [#5951](https://github.com/oven-sh/bun/issues/5951), [#9911](https://github.com/oven-sh/bun/issues/9911) - If all pages urls return `about:blank` in every MCP session restart your Chrome browser. This seems to be a Chrome bug that sometimes happen. It is some hidden state in `chrome.debugger` Extensions API. Restarting the extension worker does not fix it. - When connecting the MCP to a page, the browser may switch to light mode. This happens because Playwright, via CDP, automatically sends an "emulate media" command on start. If you'd like to see this behavior changed, you can upvote the related issue [here](https://github.com/microsoft/playwright/issues/37627). diff --git a/playwriter/src/cli.ts b/playwriter/src/cli.ts index 6d6fe64..5fa5f58 100644 --- a/playwriter/src/cli.ts +++ b/playwriter/src/cli.ts @@ -11,16 +11,28 @@ const cli = cac('playwriter') cli .command('', 'Start the MCP server (default)') - .action(async () => { + .option('--host ', 'Remote relay server host to connect to (or use PLAYWRITER_HOST env var)') + .option('--token ', 'Authentication token (or use PLAYWRITER_TOKEN env var)') + .action(async (options: { host?: string; token?: string }) => { const { startMcp } = await import('./mcp.js') - await startMcp() + await startMcp({ + host: options.host, + token: options.token, + }) }) cli .command('serve', 'Start the CDP relay server for remote MCP connections') .option('--host ', 'Host to bind to', { default: '0.0.0.0' }) - .option('--token ', 'Authentication token for /cdp/* endpoints') + .option('--token ', 'Authentication token (or use PLAYWRITER_TOKEN env var)') .action(async (options: { host: string; token?: string }) => { + const token = options.token || process.env.PLAYWRITER_TOKEN + if (!token) { + console.error('Error: Authentication token is required.') + console.error('Provide --token or set PLAYWRITER_TOKEN environment variable.') + process.exit(1) + } + const logger = createFileLogger() process.title = 'playwriter-serve' @@ -38,19 +50,19 @@ cli const server = await startPlayWriterCDPRelayServer({ port: RELAY_PORT, host: options.host, - token: options.token, + token, logger, }) console.log('Playwriter CDP relay server started') console.log(` Host: ${options.host}`) console.log(` Port: ${RELAY_PORT}`) - console.log(` Token: ${options.token ? '(configured)' : '(none)'}`) + console.log(` Token: (configured)`) console.log(` Logs: ${logger.logFilePath}`) console.log('') console.log('Endpoints:') console.log(` Extension: ws://${options.host}:${RELAY_PORT}/extension`) - console.log(` CDP: ws://${options.host}:${RELAY_PORT}/cdp/${options.token ? '?token=' : ''}`) + console.log(` CDP: ws://${options.host}:${RELAY_PORT}/cdp/?token=`) console.log('') console.log('Press Ctrl+C to stop.') From 6cf85ff01dfe330cbc6d0d06a1547ae874df682a Mon Sep 17 00:00:00 2001 From: "Tommy D. Rossi" Date: Sun, 28 Dec 2025 19:04:06 +0100 Subject: [PATCH 3/4] token variable --- playwriter/src/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/playwriter/src/utils.ts b/playwriter/src/utils.ts index 1027df9..6869ea2 100644 --- a/playwriter/src/utils.ts +++ b/playwriter/src/utils.ts @@ -3,9 +3,9 @@ import os from 'node:os' import path from 'node:path' import { fileURLToPath } from 'node:url' -export function getCdpUrl({ port = 19988, host = '127.0.0.1', query }: { port?: number; host?: string; query?: string } = {}) { +export function getCdpUrl({ port = 19988, host = '127.0.0.1', token }: { port?: number; host?: string; token?: string } = {}) { const id = `${Math.random().toString(36).substring(2, 15)}_${Date.now()}` - const queryString = query ? `?${query}` : '' + const queryString = token ? `?token=${token}` : '' return `ws://${host}:${port}/cdp/${id}${queryString}` } From 55d0a3eaec8b6f7ed73ef68edfc172b80dcfb592 Mon Sep 17 00:00:00 2001 From: "Tommy D. Rossi" Date: Sun, 28 Dec 2025 19:07:20 +0100 Subject: [PATCH 4/4] process.env.PLAYWRITER_HOST --- playwriter/src/mcp.ts | 55 ++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/playwriter/src/mcp.ts b/playwriter/src/mcp.ts index 4b600af..f602ea2 100644 --- a/playwriter/src/mcp.ts +++ b/playwriter/src/mcp.ts @@ -2,6 +2,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' import { z } from 'zod' import { Page, Browser, BrowserContext, chromium } from 'playwright-core' +import crypto from 'node:crypto' import fs from 'node:fs' import path from 'node:path' import os from 'node:os' @@ -104,18 +105,23 @@ const lastSnapshots: WeakMap = new WeakMap() const cdpSessionCache: WeakMap = new WeakMap() const RELAY_PORT = Number(process.env.PLAYWRITER_PORT) || 19988 -const REMOTE_URL = process.env.PLAYWRITER_URL const NO_TABS_ERROR = `No browser tabs are connected. Please install and enable the Playwriter extension on at least one tab: https://chromewebstore.google.com/detail/playwriter-mcp/jfeammnjpkecdekppnclgkkffahnhfhe` -function parseRemoteUrl() { - if (!REMOTE_URL) { +interface RemoteConfig { + host: string + port: number + token?: string +} + +function getRemoteConfig(): RemoteConfig | null { + const host = process.env.PLAYWRITER_HOST + if (!host) { return null } - const url = new URL(REMOTE_URL) return { - host: url.hostname, - port: Number(url.port) || 19988, - query: url.search.slice(1), + host, + port: RELAY_PORT, + token: process.env.PLAYWRITER_TOKEN, } } @@ -158,9 +164,9 @@ function clearConnectionState() { } function getLogServerUrl(): string { - if (REMOTE_URL) { - const url = new URL(REMOTE_URL) - return `http://${url.host}/mcp-log` + const remote = getRemoteConfig() + if (remote) { + return `http://${remote.host}:${remote.port}/mcp-log` } return `http://127.0.0.1:${RELAY_PORT}/mcp-log` } @@ -241,7 +247,7 @@ async function ensureConnection(): Promise<{ browser: Browser; page: Page }> { return { browser: state.browser, page: state.page } } - const remote = parseRemoteUrl() + const remote = getRemoteConfig() if (!remote) { await ensureRelayServer() } @@ -374,7 +380,7 @@ async function resetConnection(): Promise<{ browser: Browser; page: Page; contex // DO NOT clear browser logs on reset - logs should persist across reconnections // browserLogs.clear() - const remote = parseRemoteUrl() + const remote = getRemoteConfig() if (!remote) { await ensureRelayServer() } @@ -681,7 +687,7 @@ server.tool( if (cached) { return cached } - const wsUrl = getCdpUrl(parseRemoteUrl() || { port: RELAY_PORT }) + const wsUrl = getCdpUrl(getRemoteConfig() || { port: RELAY_PORT }) const session = await getCDPSessionForPage({ page: options.page, wsUrl }) cdpSessionCache.set(options.page, session) return session @@ -846,9 +852,8 @@ server.tool( }, ) -async function checkRemoteServer(url: string): Promise { - const parsed = new URL(url) - const versionUrl = `http://${parsed.host}/version` +async function checkRemoteServer({ host, port }: { host: string; port: number }): Promise { + const versionUrl = `http://${host}:${port}/version` try { const response = await fetch(versionUrl, { signal: AbortSignal.timeout(3000) }) if (!response.ok) { @@ -858,7 +863,7 @@ async function checkRemoteServer(url: string): Promise { const isConnectionError = error.cause?.code === 'ECONNREFUSED' || error.name === 'TimeoutError' if (isConnectionError) { throw new Error( - `Cannot connect to remote relay server at ${parsed.host}. ` + + `Cannot connect to remote relay server at ${host}:${port}. ` + `Make sure 'npx playwriter serve' is running on the host machine.`, ) } @@ -866,12 +871,20 @@ async function checkRemoteServer(url: string): Promise { } } -export async function startMcp() { - if (!REMOTE_URL) { +export async function startMcp(options: { host?: string; token?: string } = {}) { + if (options.host) { + process.env.PLAYWRITER_HOST = options.host + } + if (options.token) { + process.env.PLAYWRITER_TOKEN = options.token + } + + const remote = getRemoteConfig() + if (!remote) { await ensureRelayServer() } else { - console.error(`Using remote CDP relay server: ${REMOTE_URL}`) - await checkRemoteServer(REMOTE_URL) + console.error(`Using remote CDP relay server: ${remote.host}:${remote.port}`) + await checkRemoteServer(remote) } const transport = new StdioServerTransport() await server.connect(transport)