fix(cdp): isolate extension builds and keep oopif auto-attach

This commit is contained in:
Tommy D. Rossi
2026-01-27 16:05:15 +01:00
parent a8412343e1
commit 67d017237e
8 changed files with 224 additions and 17 deletions
+1
View File
@@ -1,6 +1,7 @@
node_modules node_modules
.vercel .vercel
dist dist
extension/dist*
esm esm
.DS_Store .DS_Store
*.tsbuildinfo *.tsbuildinfo
+1 -1
View File
@@ -130,7 +130,7 @@ each test should reset the extension connection. NEVER call `browser.close()` in
remember: toggling extension on a tab adds it to available pages. if you toggle then call `context.newPage()`, you'll have 2 pages. remember: toggling extension on a tab adds it to available pages. if you toggle then call `context.newPage()`, you'll have 2 pages.
IMPORTANT: set bash timeout to at least 90000ms when running `pnpm test` IMPORTANT: set bash timeout to at least 300000ms (5 minutes) when running `pnpm test`
to debug test failures, inspect the relay server log file. during tests, logs are written to `./relay-server.log` in the playwriter folder (not the system temp directory). contains extension, MCP and WS server logs with all CDP events. to debug test failures, inspect the relay server log file. during tests, logs are written to `./relay-server.log` in the playwriter folder (not the system temp directory). contains extension, MCP and WS server logs with all CDP events.
+1 -1
View File
@@ -128,7 +128,7 @@ each test should reset the extension connection. NEVER call `browser.close()` in
remember: toggling extension on a tab adds it to available pages. if you toggle then call `context.newPage()`, you'll have 2 pages. remember: toggling extension on a tab adds it to available pages. if you toggle then call `context.newPage()`, you'll have 2 pages.
IMPORTANT: set bash timeout to at least 90000ms when running `pnpm test` IMPORTANT: set bash timeout to at least 300000ms (5 minutes) when running `pnpm test`
to debug test failures, inspect the relay server log file. during tests, logs are written to `./relay-server.log` in the playwriter folder (not the system temp directory). contains extension, MCP and WS server logs with all CDP events. to debug test failures, inspect the relay server log file. during tests, logs are written to `./relay-server.log` in the playwriter folder (not the system temp directory). contains extension, MCP and WS server logs with all CDP events.
+36
View File
@@ -23,6 +23,9 @@ function sleep(ms: number): Promise<void> {
let childSessions: Map<string, { tabId: number; targetId?: string }> = new Map() let childSessions: Map<string, { tabId: number; targetId?: string }> = new Map()
let nextSessionId = 1 let nextSessionId = 1
let tabGroupQueue: Promise<void> = Promise.resolve() let tabGroupQueue: Promise<void> = Promise.resolve()
// Cache Target.setAutoAttach params so existing and future tabs enable OOPIF target events.
// This ensures Playwright can build the iframe frame tree when connecting over CDP.
let autoAttachParams: Protocol.Target.SetAutoAttachRequest | null = null
class ConnectionManager { class ConnectionManager {
ws: WebSocket | null = null ws: WebSocket | null = null
@@ -673,6 +676,30 @@ async function handleCommand(msg: ExtensionCommandMessage): Promise<any> {
const debuggee = targetTabId ? { tabId: targetTabId } : undefined const debuggee = targetTabId ? { tabId: targetTabId } : undefined
// Root-level Target.setAutoAttach must apply to all connected tabs since
// CDP auto-attach is per-debugger-session. Without this, OOPIF targets never attach.
if (msg.params.method === 'Target.setAutoAttach' && !msg.params.sessionId) {
const params = msg.params.params as Protocol.Target.SetAutoAttachRequest | undefined
if (!params) {
return {}
}
autoAttachParams = params
const connectedTabIds = Array.from(store.getState().tabs.entries())
.filter(([_, info]) => info.state === 'connected')
.map(([tabId]) => tabId)
await Promise.all(connectedTabIds.map(async (tabId) => {
try {
await chrome.debugger.sendCommand({ tabId }, 'Target.setAutoAttach', params)
} catch (error) {
logger.debug('Failed to set auto-attach for tab:', tabId, error)
}
}))
return {}
}
// TODO disable network things? // TODO disable network things?
// if (msg.params.method === 'Network.enable' && msg.params.source !== 'playwriter') { // if (msg.params.method === 'Network.enable' && msg.params.source !== 'playwriter') {
// logger.debug('Skipping Network.enable from non-playwriter CDP client:', msg.params.sessionId) // logger.debug('Skipping Network.enable from non-playwriter CDP client:', msg.params.sessionId)
@@ -831,6 +858,15 @@ async function attachTab(tabId: number, { skipAttachedEvent = false }: { skipAtt
await chrome.debugger.sendCommand(debuggee, 'Page.enable') await chrome.debugger.sendCommand(debuggee, 'Page.enable')
// Reapply cached auto-attach for new tabs so OOPIF targets are reported immediately.
if (autoAttachParams) {
try {
await chrome.debugger.sendCommand(debuggee, 'Target.setAutoAttach', autoAttachParams)
} catch (error) {
logger.debug('Failed to apply auto-attach for tab:', tabId, error)
}
}
const contextMenuScript = ` const contextMenuScript = `
document.addEventListener('contextmenu', (e) => { document.addEventListener('contextmenu', (e) => {
window.__playwriter_lastRightClicked = e.target; window.__playwriter_lastRightClicked = e.target;
+6 -3
View File
@@ -1,5 +1,5 @@
import { fileURLToPath } from 'url'; import { fileURLToPath } from 'node:url';
import { dirname, resolve } from 'path'; import { dirname, resolve } from 'node:path';
import { defineConfig } from 'vite'; import { defineConfig } from 'vite';
import { viteStaticCopy } from 'vite-plugin-static-copy'; import { viteStaticCopy } from 'vite-plugin-static-copy';
@@ -13,6 +13,9 @@ if (process.env.TESTING) {
defineEnv['import.meta.env.TESTING'] = 'true'; defineEnv['import.meta.env.TESTING'] = 'true';
} }
// Allow tests to build per-port extension outputs to avoid parallel run conflicts.
const outDir = process.env.PLAYWRITER_EXTENSION_DIST || 'dist';
export default defineConfig({ export default defineConfig({
plugins: [ plugins: [
viteStaticCopy({ viteStaticCopy({
@@ -48,7 +51,7 @@ export default defineConfig({
], ],
build: { build: {
outDir: 'dist', outDir,
emptyOutDir: false, emptyOutDir: false,
minify: false, minify: false,
rollupOptions: { rollupOptions: {
+6
View File
@@ -347,6 +347,12 @@ export async function startPlayWriterCDPRelayServer({
break break
} }
await maybeAutoCreateInitialTab() await maybeAutoCreateInitialTab()
// Forward auto-attach so Chrome emits iframe Target.attachedToTarget events.
// Playwright relies on these (with parentFrameId) when reconnecting over CDP.
await sendToExtension({
method: 'forwardCDPCommand',
params: { method, params, source }
})
return {} return {}
} }
+152 -1
View File
@@ -4,6 +4,8 @@ import { chromium } from 'playwright-core'
import path from 'node:path' import path from 'node:path'
import fs from 'node:fs' import fs from 'node:fs'
import os from 'node:os' import os from 'node:os'
import http from 'node:http'
import net from 'node:net'
import { getCdpUrl } from './utils.js' import { getCdpUrl } from './utils.js'
import type { ExtensionState } from 'mcp-extension/src/types.js' import type { ExtensionState } from 'mcp-extension/src/types.js'
import type { Protocol } from 'devtools-protocol' import type { Protocol } from 'devtools-protocol'
@@ -28,6 +30,72 @@ function js(strings: TemplateStringsArray, ...values: any[]): string {
) )
} }
type SimpleServer = {
baseUrl: string
close: () => Promise<void>
}
// Minimal local servers to create a cross-origin iframe without external dependencies.
async function createSimpleServer({ routes }: { routes: Record<string, string> }): Promise<SimpleServer> {
const openSockets: Set<net.Socket> = new Set()
const server = http.createServer((req, res) => {
const url = req.url || '/'
const body = routes[url]
if (!body) {
res.writeHead(404, { 'Content-Type': 'text/plain' })
res.end('not found')
return
}
res.writeHead(200, { 'Content-Type': 'text/html' })
res.end(body)
})
server.on('connection', (socket) => {
openSockets.add(socket)
socket.on('close', () => {
openSockets.delete(socket)
})
})
await new Promise<void>((resolve) => {
server.listen(0, '127.0.0.1', () => {
resolve()
})
})
const address = server.address()
if (!address || typeof address === 'string') {
await new Promise<void>((resolve, reject) => {
server.close((error) => {
if (error) {
reject(error)
return
}
resolve()
})
})
throw new Error('Failed to start test server')
}
return {
baseUrl: `http://127.0.0.1:${address.port}`,
close: async () => {
for (const socket of openSockets) {
socket.destroy()
}
await new Promise<void>((resolve, reject) => {
server.close((error) => {
if (error) {
reject(error)
return
}
resolve()
})
})
},
}
}
declare global { declare global {
var toggleExtensionForActiveTab: () => Promise<{ isConnected: boolean; state: ExtensionState }>; var toggleExtensionForActiveTab: () => Promise<{ isConnected: boolean; state: ExtensionState }>;
@@ -61,7 +129,11 @@ describe('MCP Server Tests', () => {
it('should inject script via addScriptTag through CDP relay', async () => { it('should inject script via addScriptTag through CDP relay', async () => {
const browserContext = getBrowserContext() const browserContext = getBrowserContext()
const serviceWorker = await getExtensionServiceWorker(browserContext) const serviceWorker = await withTimeout({
promise: getExtensionServiceWorker(browserContext),
timeoutMs: 5000,
errorMessage: 'Timed out waiting for extension service worker for iframe test',
})
const page = await browserContext.newPage() const page = await browserContext.newPage()
await page.setContent('<html><body><button id="btn">Click</button></body></html>') await page.setContent('<html><body><button id="btn">Click</button></body></html>')
@@ -1277,6 +1349,85 @@ describe('MCP Server Tests', () => {
await page.close() await page.close()
}, 60000) }, 60000)
// Reproduces the CDP reconnect issue: without auto-attach, Playwright only sees the main frame.
it('should expose iframe frames when connecting to an existing page over CDP', async () => {
const browserContext = getBrowserContext()
const serviceWorker = await getExtensionServiceWorker(browserContext)
const childServer = await createSimpleServer({
routes: {
'/child.html': '<!doctype html><html><body>child</body></html>',
},
})
const childUrl = `${childServer.baseUrl}/child.html`
const parentServer = await createSimpleServer({
routes: {
'/': `<!doctype html><html><body><iframe src="${childUrl}"></iframe></body></html>`,
},
})
const page = await browserContext.newPage()
try {
await withTimeout({
promise: page.goto(parentServer.baseUrl, { waitUntil: 'domcontentloaded', timeout: 5000 }),
timeoutMs: 6000,
errorMessage: 'Timed out loading parent page for iframe test',
})
await withTimeout({
promise: page.frameLocator('iframe').locator('body').waitFor({ timeout: 5000 }),
timeoutMs: 6000,
errorMessage: 'Timed out waiting for iframe to attach in parent page',
})
expect(page.frames().map((frame) => frame.url())).toContain(childUrl)
await page.bringToFront()
await withTimeout({
promise: serviceWorker.evaluate(async () => {
await globalThis.toggleExtensionForActiveTab()
}),
timeoutMs: 5000,
errorMessage: 'Timed out toggling extension for iframe test',
})
await new Promise((r) => { setTimeout(r, 400) })
const browser = await withTimeout({
promise: chromium.connectOverCDP(getCdpUrl({ port: TEST_PORT })),
timeoutMs: 5000,
errorMessage: 'Timed out connecting over CDP for iframe test',
})
const context = browser.contexts()[0]
const cdpPage = context.pages().find((candidate) => {
return candidate.url().startsWith(parentServer.baseUrl)
})
expect(cdpPage).toBeDefined()
const frames = cdpPage!.frames()
const childFrame = frames.find((frame) => {
return frame.url() === childUrl
})
expect(frames.length).toBe(2)
expect(childFrame).toBeDefined()
await withTimeout({
promise: browser.close(),
timeoutMs: 5000,
errorMessage: 'Timed out closing CDP browser for iframe test',
})
} finally {
await withTimeout({
promise: page.close(),
timeoutMs: 5000,
errorMessage: 'Timed out closing page for iframe test',
})
await Promise.all([
parentServer.close(),
childServer.close(),
])
}
}, 60000)
it('should have non-empty URLs when connecting to already-loaded pages', async () => { it('should have non-empty URLs when connecting to already-loaded pages', async () => {
// This test validates that when we connect to a browser with already-loaded pages, // This test validates that when we connect to a browser with already-loaded pages,
// all pages have non-empty URLs. Empty URLs break Playwright permanently. // all pages have non-empty URLs. Empty URLs break Playwright permanently.
+21 -11
View File
@@ -11,7 +11,22 @@ import { createFileLogger } from './create-logger.js'
import { killPortProcess } from 'kill-port-process' import { killPortProcess } from 'kill-port-process'
const execAsync = promisify(exec) const execAsync = promisify(exec)
let extensionBuildQueue: Promise<void> = Promise.resolve() const extensionBuildQueues: Map<string, Promise<void>> = new Map()
async function buildExtension({ port, distDir }: { port: number; distDir: string }): Promise<void> {
const previous = extensionBuildQueues.get(distDir) || Promise.resolve()
const buildPromise = previous
.catch((error) => {
console.error('Previous extension build failed:', error)
})
.then(async () => {
// Build into a per-port dist to avoid parallel test runs overwriting each other.
await execAsync(`TESTING=1 PLAYWRITER_PORT=${port} PLAYWRITER_EXTENSION_DIST=${distDir} pnpm build`, { cwd: '../extension' })
})
extensionBuildQueues.set(distDir, buildPromise.finally(() => {}))
await buildPromise
}
export async function getExtensionServiceWorker(context: BrowserContext) { export async function getExtensionServiceWorker(context: BrowserContext) {
let serviceWorkers = context.serviceWorkers().filter((sw) => sw.url().startsWith('chrome-extension://')) let serviceWorkers = context.serviceWorkers().filter((sw) => sw.url().startsWith('chrome-extension://'))
@@ -54,16 +69,11 @@ export async function setupTestContext({
}): Promise<TestContext> { }): Promise<TestContext> {
await killPortProcess(port).catch(() => {}) await killPortProcess(port).catch(() => {})
// Use a port-scoped dist folder so parallel tests don't replace each other's extension builds.
const distDir = `dist-${port}`
console.log('Building extension...') console.log('Building extension...')
const buildPromise = extensionBuildQueue await buildExtension({ port, distDir })
.catch((error) => {
console.error('Previous extension build failed:', error)
})
.then(async () => {
await execAsync(`TESTING=1 PLAYWRITER_PORT=${port} pnpm build`, { cwd: '../extension' })
})
extensionBuildQueue = buildPromise.finally(() => {})
await buildPromise
console.log('Extension built') console.log('Extension built')
const localLogPath = path.join(process.cwd(), 'relay-server.log') const localLogPath = path.join(process.cwd(), 'relay-server.log')
@@ -71,7 +81,7 @@ export async function setupTestContext({
const relayServer = await startPlayWriterCDPRelayServer({ port, logger }) const relayServer = await startPlayWriterCDPRelayServer({ port, logger })
const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), tempDirPrefix)) const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), tempDirPrefix))
const extensionPath = path.resolve('../extension/dist') const extensionPath = path.resolve('../extension', distDir)
const browserContext = await chromium.launchPersistentContext(userDataDir, { const browserContext = await chromium.launchPersistentContext(userDataDir, {
channel: 'chromium', channel: 'chromium',