From a4522d4f36dc753718a84a6e2b10a0e4fba51f6d Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 12 May 2026 16:51:05 -0700 Subject: [PATCH 1/2] fix(cli): persist traces when CLI attaches to existing CDP browser The MCP browser_start_tracing tool advertised trace files under `.playwright-cli/traces/`, but for CDP-attached browsers the underlying server used an auto-created temp `artifactsDir`, so trace files never appeared at the advertised paths. Add an optional `artifactsDir` parameter to `BrowserType.connectOverCDP` and have the MCP browser factory pass the desired directory through. The server uses the supplied directory directly (creating it if missing) and skips cleanup on disconnect when the directory was provided by the caller. Fixes: https://github.com/microsoft/playwright-cli/issues/378 --- docs/src/api/class-browsertype.md | 6 ++++ packages/playwright-client/types/types.d.ts | 5 +++ .../playwright-core/src/client/browserType.ts | 1 + .../playwright-core/src/protocol/validator.ts | 1 + .../playwright-core/src/server/browserType.ts | 2 +- .../src/server/chromium/chromium.ts | 17 ++++++--- .../src/tools/mcp/browserFactory.ts | 8 +++-- packages/playwright-core/types/types.d.ts | 5 +++ packages/protocol/spec/browserType.yml | 1 + packages/protocol/src/channels.d.ts | 2 ++ .../library/chromium/connect-over-cdp.spec.ts | 36 +++++++++++++++++++ tests/mcp/cli-cdp.spec.ts | 20 +++++++++++ 12 files changed, 95 insertions(+), 9 deletions(-) diff --git a/docs/src/api/class-browsertype.md b/docs/src/api/class-browsertype.md index 05a999dd869db..9f94ff4beb68c 100644 --- a/docs/src/api/class-browsertype.md +++ b/docs/src/api/class-browsertype.md @@ -236,6 +236,12 @@ emulation is not enabled, and media emulation options (such as [`option: Browser browser where these overrides would interfere with existing browser state. New contexts created via [`method: Browser.newContext`] are not affected. Defaults to `false`. +### option: BrowserType.connectOverCDP.artifactsDir +* since: v1.61 +- `artifactsDir` <[path]> + +If specified, browser artifacts (such as traces and downloads) are saved into this directory. + ## method: BrowserType.executablePath * since: v1.8 diff --git a/packages/playwright-client/types/types.d.ts b/packages/playwright-client/types/types.d.ts index f3488dd71ad80..8ac56c53a3ead 100644 --- a/packages/playwright-client/types/types.d.ts +++ b/packages/playwright-client/types/types.d.ts @@ -23531,6 +23531,11 @@ export interface LaunchOptions { } export interface ConnectOverCDPOptions { + /** + * If specified, browser artifacts (such as traces and downloads) are saved into this directory. + */ + artifactsDir?: string; + /** * @deprecated Use the first argument instead. */ diff --git a/packages/playwright-core/src/client/browserType.ts b/packages/playwright-core/src/client/browserType.ts index 2b069c05d36be..3a98bb8120099 100644 --- a/packages/playwright-core/src/client/browserType.ts +++ b/packages/playwright-core/src/client/browserType.ts @@ -157,6 +157,7 @@ export class BrowserType extends ChannelOwner imple timeout: new TimeoutSettings(this._platform).timeout(params), isLocal: params.isLocal, noDefaults: params.noDefaults, + artifactsDir: params.artifactsDir, }); const browser = Browser.from(result.browser); browser._connectToBrowserType(this, {}, undefined); diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 71328727aa22c..8a5a79ffb239f 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -1059,6 +1059,7 @@ scheme.BrowserTypeConnectOverCDPParams = tObject({ timeout: tFloat, isLocal: tOptional(tBoolean), noDefaults: tOptional(tBoolean), + artifactsDir: tOptional(tString), }); scheme.BrowserTypeConnectOverCDPResult = tObject({ browser: tChannel(['Browser']), diff --git a/packages/playwright-core/src/server/browserType.ts b/packages/playwright-core/src/server/browserType.ts index 3515c8b934925..8096c9eedab47 100644 --- a/packages/playwright-core/src/server/browserType.ts +++ b/packages/playwright-core/src/server/browserType.ts @@ -286,7 +286,7 @@ export abstract class BrowserType extends SdkObject { } } - async connectOverCDP(progress: Progress, endpointURL: string, options: { slowMo?: number, timeout?: number, headers?: types.HeadersArray, isLocal?: boolean, noDefaults?: boolean }): Promise { + async connectOverCDP(progress: Progress, endpointURL: string, options: { slowMo?: number, timeout?: number, headers?: types.HeadersArray, isLocal?: boolean, noDefaults?: boolean, artifactsDir?: string }): Promise { throw new Error('CDP connections are only supported by Chromium'); } diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index 27841eea9c93b..e0b4d362590d8 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -81,11 +81,11 @@ export class Chromium extends BrowserType { return super.launchPersistentContext(progress, userDataDir, options); } - override async connectOverCDP(progress: Progress, endpointURL: string, options: { slowMo?: number, headers?: types.HeadersArray, isLocal?: boolean, noDefaults?: boolean }) { + override async connectOverCDP(progress: Progress, endpointURL: string, options: { slowMo?: number, headers?: types.HeadersArray, isLocal?: boolean, noDefaults?: boolean, artifactsDir?: string }) { return await this._connectOverCDPInternal(progress, endpointURL, options); } - async _connectOverCDPInternal(progress: Progress, endpointURL: string, options: types.LaunchOptions & { headers?: types.HeadersArray, isLocal?: boolean, noDefaults?: boolean }, onClose?: () => Promise) { + async _connectOverCDPInternal(progress: Progress, endpointURL: string, options: types.LaunchOptions & { headers?: types.HeadersArray, isLocal?: boolean, noDefaults?: boolean, artifactsDir?: string }, onClose?: () => Promise) { let headersMap: { [key: string]: string; } | undefined; if (options.headers) headersMap = headersArrayToObject(options.headers, false); @@ -113,10 +113,17 @@ export class Chromium extends BrowserType { return this._connectOverCDPImpl(progress, chromeTransport, closeAndWait, options, onClose); } - private async _connectOverCDPImpl(progress: Progress, transport: ConnectionTransport, closeAndWait: () => Promise, options: types.LaunchOptions & { isLocal?: boolean, noDefaults?: boolean }, onClose?: () => Promise) { - const artifactsDir = await progress.race(fs.promises.mkdtemp(ARTIFACTS_FOLDER)); + private async _connectOverCDPImpl(progress: Progress, transport: ConnectionTransport, closeAndWait: () => Promise, options: types.LaunchOptions & { isLocal?: boolean, noDefaults?: boolean, artifactsDir?: string }, onClose?: () => Promise) { + let artifactsDir: string; + const tempDirectories: string[] = []; + if (options.artifactsDir) { + artifactsDir = options.artifactsDir; + } else { + artifactsDir = await progress.race(fs.promises.mkdtemp(ARTIFACTS_FOLDER)); + tempDirectories.push(artifactsDir); + } const doCleanup = async () => { - await removeFolders([artifactsDir]); + await removeFolders(tempDirectories); const cb = onClose; onClose = undefined; // Make sure to only call onClose once. await cb?.(); diff --git a/packages/playwright-core/src/tools/mcp/browserFactory.ts b/packages/playwright-core/src/tools/mcp/browserFactory.ts index 80c9d0b8d3277..62d1cf7795b17 100644 --- a/packages/playwright-core/src/tools/mcp/browserFactory.ts +++ b/packages/playwright-core/src/tools/mcp/browserFactory.ts @@ -51,7 +51,7 @@ export async function createBrowserWithInfo(config: FullConfig, clientInfo: Clie let canBind = false; let ownership: 'attached' | 'own' = 'own'; if (config.browser.cdpEndpoint) { - browser = await createCDPBrowser(config); + browser = await createCDPBrowser(config, clientInfo); canBind = true; ownership = 'attached'; } else if (config.browser.isolated) { @@ -103,11 +103,13 @@ async function createIsolatedBrowser(config: FullConfig, clientInfo: ClientInfo) return browser; } -async function createCDPBrowser(config: FullConfig): Promise { +async function createCDPBrowser(config: FullConfig, clientInfo: ClientInfo): Promise { testDebug('create browser (cdp)'); + const artifactsDir = await computeTracesDir(config, clientInfo); const browser = await playwright.chromium.connectOverCDP(config.browser.cdpEndpoint!, { headers: config.browser.cdpHeaders, - timeout: config.browser.cdpTimeout + timeout: config.browser.cdpTimeout, + artifactsDir, }); return browser; } diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index f3488dd71ad80..8ac56c53a3ead 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -23531,6 +23531,11 @@ export interface LaunchOptions { } export interface ConnectOverCDPOptions { + /** + * If specified, browser artifacts (such as traces and downloads) are saved into this directory. + */ + artifactsDir?: string; + /** * @deprecated Use the first argument instead. */ diff --git a/packages/protocol/spec/browserType.yml b/packages/protocol/spec/browserType.yml index 0ff5f4dc85c09..b86f06b7cfde9 100644 --- a/packages/protocol/spec/browserType.yml +++ b/packages/protocol/spec/browserType.yml @@ -51,6 +51,7 @@ BrowserType: timeout: float isLocal: boolean? noDefaults: boolean? + artifactsDir: string? returns: browser: Browser defaultContext: BrowserContext? diff --git a/packages/protocol/src/channels.d.ts b/packages/protocol/src/channels.d.ts index 5529a4e3e1953..9cfb1c11c73cf 100644 --- a/packages/protocol/src/channels.d.ts +++ b/packages/protocol/src/channels.d.ts @@ -2005,12 +2005,14 @@ export type BrowserTypeConnectOverCDPParams = { timeout: number, isLocal?: boolean, noDefaults?: boolean, + artifactsDir?: string, }; export type BrowserTypeConnectOverCDPOptions = { headers?: NameValue[], slowMo?: number, isLocal?: boolean, noDefaults?: boolean, + artifactsDir?: string, }; export type BrowserTypeConnectOverCDPResult = { browser: BrowserChannel, diff --git a/tests/library/chromium/connect-over-cdp.spec.ts b/tests/library/chromium/connect-over-cdp.spec.ts index f68634fa55e0c..1705e7118d2bc 100644 --- a/tests/library/chromium/connect-over-cdp.spec.ts +++ b/tests/library/chromium/connect-over-cdp.spec.ts @@ -18,6 +18,7 @@ import { playwrightTest as test, expect } from '../../config/browserTest'; import http from 'http'; import fs from 'fs'; +import path from 'path'; import { getUserAgent, server as coreServer } from '../../../packages/playwright-core/lib/coreBundle'; import { suppressCertificateWarning } from '../../config/utils'; @@ -60,6 +61,41 @@ test('should cleanup artifacts dir after connectOverCDP disconnects due to ws cl expect(exists2).toBe(false); }); +test('should write traces to provided artifactsDir on connectOverCDP', async ({ browserType, toImpl }, testInfo) => { + const port = 9339 + testInfo.workerIndex; + const browserServer = await browserType.launch({ + args: ['--remote-debugging-port=' + port] + }); + const artifactsDir = testInfo.outputPath('custom-artifacts'); + try { + const cdpBrowser = await browserType.connectOverCDP({ + endpointURL: `http://127.0.0.1:${port}/`, + artifactsDir, + }); + expect(toImpl(cdpBrowser).options.artifactsDir).toBe(artifactsDir); + expect(toImpl(cdpBrowser).options.tracesDir).toBe(artifactsDir); + + const context = cdpBrowser.contexts()[0]; + await context.tracing.start({ name: 'cdp-trace', snapshots: true, screenshots: true }); + const page = await context.newPage(); + await page.setContent(''); + await context.tracing.stopChunk(); + + expect(fs.existsSync(path.join(artifactsDir, 'cdp-trace.trace'))).toBe(true); + expect(fs.existsSync(path.join(artifactsDir, 'cdp-trace.network'))).toBe(true); + expect(fs.existsSync(path.join(artifactsDir, 'resources'))).toBe(true); + + await Promise.all([ + new Promise(f => cdpBrowser.on('disconnected', f)), + browserServer.close() + ]); + + expect(fs.existsSync(artifactsDir)).toBe(true); + } finally { + await browserServer.close().catch(() => {}); + } +}); + test('should connectOverCDP and manage downloads in default context', async ({ browserType, mode, server }, testInfo) => { server.setRoute('/downloadWithFilename', (req, res) => { res.setHeader('Content-Type', 'application/octet-stream'); diff --git a/tests/mcp/cli-cdp.spec.ts b/tests/mcp/cli-cdp.spec.ts index c0637592a8b41..880e358c6e05e 100644 --- a/tests/mcp/cli-cdp.spec.ts +++ b/tests/mcp/cli-cdp.spec.ts @@ -109,3 +109,23 @@ test('attach via cdp', async ({ cdpServer, cli, server }) => { const { inlineSnapshot } = await cli('snapshot'); expect(inlineSnapshot).toContain(`- generic [active] [ref=e1]: Hello, world!`); }); + +test('tracing-start-stop over cdp', async ({ cdpServer, cli, server }, testInfo) => { + const browserContext = await cdpServer.start(); + const [page] = browserContext.pages(); + await page.goto(server.HELLO_WORLD); + + await cli('attach', `--cdp=${cdpServer.endpoint}`); + + const { output } = await cli('tracing-start'); + expect(output).toContain('Trace recording started'); + await cli('eval', '() => fetch("/hello-world")'); + + const { output: tracingStopOutput } = await cli('tracing-stop'); + expect(tracingStopOutput).toContain('Trace recording stopped'); + const [, timestamp] = tracingStopOutput.match(/trace-(\d+)\.trace/); + + expect(fs.existsSync(testInfo.outputPath('.playwright-cli', 'traces', 'resources'))).toBeTruthy(); + expect(fs.existsSync(testInfo.outputPath('.playwright-cli', 'traces', `trace-${timestamp}.trace`))).toBeTruthy(); + expect(fs.existsSync(testInfo.outputPath('.playwright-cli', 'traces', `trace-${timestamp}.network`))).toBeTruthy(); +}); From df5ed395dcea7e7ea02c98a30550aa2d734a975f Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 12 May 2026 17:36:09 -0700 Subject: [PATCH 2/2] test: relax error substring in CDP reconnect test The client's _wrapApiCall prepends the API frame name to error messages, so the actual error is "Error: async initializeServer: connect ECONNREFUSED ..." rather than "Error: connect ECONNREFUSED ...". Match on the underlying socket error substring instead. --- tests/mcp/cdp.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mcp/cdp.spec.ts b/tests/mcp/cdp.spec.ts index fbfe60f09134a..a316baff86286 100644 --- a/tests/mcp/cdp.spec.ts +++ b/tests/mcp/cdp.spec.ts @@ -71,7 +71,7 @@ test('should throw connection error and allow re-connecting', async ({ cdpServer name: 'browser_navigate', arguments: { url: server.PREFIX }, })).toHaveResponse({ - error: expect.stringContaining(`Error: connect ECONNREFUSED`), + error: expect.stringContaining(`connect ECONNREFUSED`), isError: true, }); await cdpServer.start();