From bf2901a1918ea76f2302fac633303f6c3e834769 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 1 Sep 2026 13:56:13 -0700 Subject: [PATCH 1/3] fix(mcp): do not reuse the cached backend after it is disposed browser_close disposes the backend, but with a shared browser no 'disconnected' event fires, so the server kept handing out the disposed backend. Let the server ask the backend whether it disposed itself after each tool call and drop the cached one if so. Also detach the backend listeners from the browser context on dispose so they do not accumulate on a long-lived shared context. This replaces the approach from #42365 (reverted in #42492), which made dispose() emit the 'disconnected' event on explicit disposal. Fixes: https://github.com/microsoft/playwright/issues/42363 --- .../src/tools/backend/browserBackend.ts | 14 ++++- .../src/tools/utils/mcp/server.ts | 7 ++- tests/mcp/http.spec.ts | 56 +++++++++++++++++++ tests/mcp/library.spec.ts | 52 +++++++++++++++++ 4 files changed, 125 insertions(+), 4 deletions(-) diff --git a/packages/playwright-core/src/tools/backend/browserBackend.ts b/packages/playwright-core/src/tools/backend/browserBackend.ts index ac23adda870e3..46a9bb6e6b0b5 100644 --- a/packages/playwright-core/src/tools/backend/browserBackend.ts +++ b/packages/playwright-core/src/tools/backend/browserBackend.ts @@ -38,6 +38,7 @@ export class BrowserBackend extends EventEmitter<{ disconnected: [] }> implement private _disposed = false; private _browserContext: playwright.BrowserContext; private _disposeCallback: (() => Promise) | undefined; + private _markDisconnected: () => void; constructor(config: ContextConfig, browserContext: playwright.BrowserContext, tools: Tool[], disposeCallback?: () => Promise) { super(); @@ -45,15 +46,15 @@ export class BrowserBackend extends EventEmitter<{ disconnected: [] }> implement this._tools = tools; this._browserContext = browserContext; this._disposeCallback = disposeCallback; - const markDisconnected = () => { + this._markDisconnected = () => { if (this._disconnected) return; backendDebug('browser disconnected'); this._disconnected = true; this.emit('disconnected'); }; - this._browserContext.once('close', markDisconnected); - this._browserContext.browser()?.once('disconnected', markDisconnected); + this._browserContext.once('close', this._markDisconnected); + this._browserContext.browser()?.once('disconnected', this._markDisconnected); } async initialize(clientInfo: ClientInfo): Promise { @@ -69,10 +70,17 @@ export class BrowserBackend extends EventEmitter<{ disconnected: [] }> implement if (this._disposed) return; this._disposed = true; + // Detach, so that disposed backends do not accumulate listeners on a long-lived shared context. + this._browserContext.off('close', this._markDisconnected); + this._browserContext.browser()?.off('disconnected', this._markDisconnected); await this._context?.dispose().catch(e => debug('pw:tools:error')(e)); await this._disposeCallback?.().catch(e => debug('pw:tools:error')(e)); } + isDisposed(): boolean { + return this._disposed; + } + async callTool(name: string, rawArguments: mcpServer.CallToolRequest['params']['arguments'] & { _meta?: Record } = {}, signal?: AbortSignal): Promise { const json = !!rawArguments._meta?.json; const formatError = (message: string): mcpServer.CallToolResult => ({ diff --git a/packages/playwright-core/src/tools/utils/mcp/server.ts b/packages/playwright-core/src/tools/utils/mcp/server.ts index 4799818189a9c..2d6002949187b 100644 --- a/packages/playwright-core/src/tools/utils/mcp/server.ts +++ b/packages/playwright-core/src/tools/utils/mcp/server.ts @@ -42,6 +42,7 @@ export interface ServerBackend { initialize?(clientInfo: ClientInfo): Promise; callTool(name: string, args: CallToolRequest['params']['arguments'], signal: AbortSignal): Promise; dispose?(): Promise; + isDisposed?(): boolean; once(event: 'disconnected', listener: () => void): void; } @@ -100,8 +101,12 @@ export function createServer(name: string, version: string, factory: ServerBacke backendPromise = promise; } - const backend = await backendPromise; + const currentPromise = backendPromise; + const backend = await currentPromise; const toolResult = await backend.callTool(request.params.name, request.params.arguments || {}, extra.signal); + // The backend may have disposed itself while handling the call; do not hand it out again. + if (backend.isDisposed?.() && backendPromise === currentPromise) + backendPromise = undefined; const mergedResult = mergeTextParts(toolResult); serverDebugResponse('callResult', mergedResult); return mergedResult; diff --git a/tests/mcp/http.spec.ts b/tests/mcp/http.spec.ts index f24b65e28f13c..6952eef2e8f03 100644 --- a/tests/mcp/http.spec.ts +++ b/tests/mcp/http.spec.ts @@ -438,6 +438,62 @@ test('http transport shared context', async ({ serverEndpoint, server }) => { }); }); +test('http transport shared context survives browser_close', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42363' } }, async ({ serverEndpoint, server }) => { + const { url, stderr } = await serverEndpoint({ args: ['--shared-browser-context'] }); + + const transport1 = new StreamableHTTPClientTransport(new URL('/mcp', url)); + const client1 = new Client({ name: 'test1', version: '1.0.0' }); + await client1.connect(transport1); + await client1.callTool({ + name: 'browser_navigate', + arguments: { url: server.HELLO_WORLD }, + }); + + const transport2 = new StreamableHTTPClientTransport(new URL('/mcp', url)); + const client2 = new Client({ name: 'test2', version: '1.0.0' }); + await client2.connect(transport2); + await client2.callTool({ + name: 'browser_navigate', + arguments: { url: server.HELLO_WORLD }, + }); + + // The second client keeps the shared browser alive, so closing only + // disposes the first client's backend. + await client1.callTool({ + name: 'browser_close', + arguments: {}, + }); + + // The next call from the first client must get a fresh backend. + expect(await client1.callTool({ + name: 'browser_tabs', + arguments: { action: 'new', url: server.HELLO_WORLD }, + })).toHaveResponse({ + snapshot: expect.stringContaining(`Hello, world!`), + }); + + // The second client is unaffected. + expect(await client2.callTool({ + name: 'browser_snapshot', + arguments: {}, + })).toHaveResponse({ + inlineSnapshot: expect.stringContaining(`Hello, world!`), + }); + + await transport1.terminateSession(); + await client1.close(); + await transport2.terminateSession(); + await client2.close(); + + await expect.poll(() => formatLog(stderr())).toEqual({ + 'create browser (persistent)': 1, + 'create http session': 2, + 'delete http session': 2, + 'create context': 3, + 'close browser': 1, + }); +}); + test('http transport (default)', async ({ serverEndpoint }) => { const { url } = await serverEndpoint(); const transport = new StreamableHTTPClientTransport(url); diff --git a/tests/mcp/library.spec.ts b/tests/mcp/library.spec.ts index c853b530978e5..793bf6650392f 100644 --- a/tests/mcp/library.spec.ts +++ b/tests/mcp/library.spec.ts @@ -15,7 +15,16 @@ */ import child_process from 'child_process'; import fs from 'fs/promises'; + +import * as playwright from 'playwright'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; import { test, expect } from './fixtures'; +import { tools } from '../../packages/playwright-core/lib/coreBundle'; + +import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; + +const { createConnection } = tools; test('library can be used from CommonJS', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright-mcp/issues/456' } }, async ({}, testInfo) => { const file = testInfo.outputPath('main.cjs'); @@ -26,3 +35,46 @@ test('library can be used from CommonJS', { annotation: { type: 'issue', descrip `); expect(child_process.execSync(`node ${file}`, { encoding: 'utf-8' })).toContain('OK'); }); + +test('createConnection detaches backend listeners from a caller owned context', async ({ mcpBrowser, mcpHeadless, server }, testInfo) => { + const channel = mcpBrowser === 'chrome' || mcpBrowser === 'msedge' ? mcpBrowser : undefined; + const browserName = (channel ? 'chromium' : mcpBrowser) as 'chromium' | 'firefox' | 'webkit'; + const browser = await playwright[browserName].launch({ channel, headless: mcpHeadless }); + const browserContext = await browser.newContext(); + + const client = await connectClient(await createConnection({ + outputDir: testInfo.outputPath('output'), + }, async () => browserContext)); + + const contextListeners = listenerCount(browserContext, 'close'); + const browserListeners = listenerCount(browser, 'disconnected'); + + // browser_close disposes the backend while the caller keeps the context, so + // the next tool call builds another backend over the same two objects. + for (let i = 0; i < 3; i++) { + await client.callTool({ name: 'browser_navigate', arguments: { url: server.HELLO_WORLD } }); + // A live backend listens on both objects. + expect(listenerCount(browserContext, 'close')).toBe(contextListeners + 1); + expect(listenerCount(browser, 'disconnected')).toBe(browserListeners + 1); + + await client.callTool({ name: 'browser_close', arguments: {} }); + // A disposed one gives both back, so they do not pile up. + expect(listenerCount(browserContext, 'close')).toBe(contextListeners); + expect(listenerCount(browser, 'disconnected')).toBe(browserListeners); + } + + await client.close(); + await browser.close(); +}); + +function listenerCount(emitter: object, event: string): number { + return (emitter as unknown as { listenerCount(event: string): number }).listenerCount(event); +} + +async function connectClient(server: Server): Promise { + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + const client = new Client({ name: 'test', version: '1.0.0' }); + await client.connect(clientTransport); + return client; +} From 0c206b5f1c2957e6a75a8e109b9e8bb592b35f31 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 1 Sep 2026 14:11:08 -0700 Subject: [PATCH 2/3] fix(mcp): refuse browser_close when the browser context is shared With --shared-browser-context the context belongs to all connected clients, so one client closing it only pretend-closed: the backend was disposed while every page stayed open and the next call silently rebuilt it. Return an error to browser_close instead. --- .../playwright-core/src/tools/backend/common.ts | 2 ++ .../playwright-core/src/tools/backend/context.ts | 1 + tests/mcp/http.spec.ts | 14 ++++++++------ 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/playwright-core/src/tools/backend/common.ts b/packages/playwright-core/src/tools/backend/common.ts index fb0375535f118..3e6c3635895e6 100644 --- a/packages/playwright-core/src/tools/backend/common.ts +++ b/packages/playwright-core/src/tools/backend/common.ts @@ -30,6 +30,8 @@ const close = defineTool({ }, handle: async (context, params, response) => { + if (context.config.sharedBrowserContext) + throw new Error('The browser context is shared between clients and cannot be closed.'); const result = renderTabsMarkdown([]); response.addTextResult(result.join('\n')); response.addCode(`await page.close()`); diff --git a/packages/playwright-core/src/tools/backend/context.ts b/packages/playwright-core/src/tools/backend/context.ts index fcee3695fbdb3..068e8e8339dd5 100644 --- a/packages/playwright-core/src/tools/backend/context.ts +++ b/packages/playwright-core/src/tools/backend/context.ts @@ -51,6 +51,7 @@ export type ContextConfig = { outputMaxSize?: number; saveSession?: boolean; secrets?: Record; + sharedBrowserContext?: boolean; snapshot?: { mode?: 'full' | 'none'; boxes?: boolean; diff --git a/tests/mcp/http.spec.ts b/tests/mcp/http.spec.ts index 6952eef2e8f03..f47e3d3b31aa9 100644 --- a/tests/mcp/http.spec.ts +++ b/tests/mcp/http.spec.ts @@ -438,7 +438,7 @@ test('http transport shared context', async ({ serverEndpoint, server }) => { }); }); -test('http transport shared context survives browser_close', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42363' } }, async ({ serverEndpoint, server }) => { +test('http transport shared context refuses browser_close', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42363' } }, async ({ serverEndpoint, server }) => { const { url, stderr } = await serverEndpoint({ args: ['--shared-browser-context'] }); const transport1 = new StreamableHTTPClientTransport(new URL('/mcp', url)); @@ -457,14 +457,16 @@ test('http transport shared context survives browser_close', { annotation: { typ arguments: { url: server.HELLO_WORLD }, }); - // The second client keeps the shared browser alive, so closing only - // disposes the first client's backend. - await client1.callTool({ + // The context is shared with the second client, so closing it is refused. + expect(await client1.callTool({ name: 'browser_close', arguments: {}, + })).toHaveResponse({ + error: 'Error: The browser context is shared between clients and cannot be closed.', + isError: true, }); - // The next call from the first client must get a fresh backend. + // The first client keeps working. expect(await client1.callTool({ name: 'browser_tabs', arguments: { action: 'new', url: server.HELLO_WORLD }, @@ -489,7 +491,7 @@ test('http transport shared context survives browser_close', { annotation: { typ 'create browser (persistent)': 1, 'create http session': 2, 'delete http session': 2, - 'create context': 3, + 'create context': 2, 'close browser': 1, }); }); From 1f174aa596d1f2d019b2fdf97ecf8f235d5914ce Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 1 Sep 2026 14:16:28 -0700 Subject: [PATCH 3/3] chore(mcp): drop the backend cache invalidation With browser_close refused on a shared context, every remaining close path closes the context or the browser, so the existing 'disconnected' event already clears the cached backend. --- .../src/tools/backend/browserBackend.ts | 14 ++--- .../src/tools/utils/mcp/server.ts | 7 +-- tests/mcp/library.spec.ts | 52 ------------------- 3 files changed, 4 insertions(+), 69 deletions(-) diff --git a/packages/playwright-core/src/tools/backend/browserBackend.ts b/packages/playwright-core/src/tools/backend/browserBackend.ts index 46a9bb6e6b0b5..ac23adda870e3 100644 --- a/packages/playwright-core/src/tools/backend/browserBackend.ts +++ b/packages/playwright-core/src/tools/backend/browserBackend.ts @@ -38,7 +38,6 @@ export class BrowserBackend extends EventEmitter<{ disconnected: [] }> implement private _disposed = false; private _browserContext: playwright.BrowserContext; private _disposeCallback: (() => Promise) | undefined; - private _markDisconnected: () => void; constructor(config: ContextConfig, browserContext: playwright.BrowserContext, tools: Tool[], disposeCallback?: () => Promise) { super(); @@ -46,15 +45,15 @@ export class BrowserBackend extends EventEmitter<{ disconnected: [] }> implement this._tools = tools; this._browserContext = browserContext; this._disposeCallback = disposeCallback; - this._markDisconnected = () => { + const markDisconnected = () => { if (this._disconnected) return; backendDebug('browser disconnected'); this._disconnected = true; this.emit('disconnected'); }; - this._browserContext.once('close', this._markDisconnected); - this._browserContext.browser()?.once('disconnected', this._markDisconnected); + this._browserContext.once('close', markDisconnected); + this._browserContext.browser()?.once('disconnected', markDisconnected); } async initialize(clientInfo: ClientInfo): Promise { @@ -70,17 +69,10 @@ export class BrowserBackend extends EventEmitter<{ disconnected: [] }> implement if (this._disposed) return; this._disposed = true; - // Detach, so that disposed backends do not accumulate listeners on a long-lived shared context. - this._browserContext.off('close', this._markDisconnected); - this._browserContext.browser()?.off('disconnected', this._markDisconnected); await this._context?.dispose().catch(e => debug('pw:tools:error')(e)); await this._disposeCallback?.().catch(e => debug('pw:tools:error')(e)); } - isDisposed(): boolean { - return this._disposed; - } - async callTool(name: string, rawArguments: mcpServer.CallToolRequest['params']['arguments'] & { _meta?: Record } = {}, signal?: AbortSignal): Promise { const json = !!rawArguments._meta?.json; const formatError = (message: string): mcpServer.CallToolResult => ({ diff --git a/packages/playwright-core/src/tools/utils/mcp/server.ts b/packages/playwright-core/src/tools/utils/mcp/server.ts index 2d6002949187b..4799818189a9c 100644 --- a/packages/playwright-core/src/tools/utils/mcp/server.ts +++ b/packages/playwright-core/src/tools/utils/mcp/server.ts @@ -42,7 +42,6 @@ export interface ServerBackend { initialize?(clientInfo: ClientInfo): Promise; callTool(name: string, args: CallToolRequest['params']['arguments'], signal: AbortSignal): Promise; dispose?(): Promise; - isDisposed?(): boolean; once(event: 'disconnected', listener: () => void): void; } @@ -101,12 +100,8 @@ export function createServer(name: string, version: string, factory: ServerBacke backendPromise = promise; } - const currentPromise = backendPromise; - const backend = await currentPromise; + const backend = await backendPromise; const toolResult = await backend.callTool(request.params.name, request.params.arguments || {}, extra.signal); - // The backend may have disposed itself while handling the call; do not hand it out again. - if (backend.isDisposed?.() && backendPromise === currentPromise) - backendPromise = undefined; const mergedResult = mergeTextParts(toolResult); serverDebugResponse('callResult', mergedResult); return mergedResult; diff --git a/tests/mcp/library.spec.ts b/tests/mcp/library.spec.ts index 793bf6650392f..c853b530978e5 100644 --- a/tests/mcp/library.spec.ts +++ b/tests/mcp/library.spec.ts @@ -15,16 +15,7 @@ */ import child_process from 'child_process'; import fs from 'fs/promises'; - -import * as playwright from 'playwright'; -import { Client } from '@modelcontextprotocol/sdk/client/index.js'; -import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; import { test, expect } from './fixtures'; -import { tools } from '../../packages/playwright-core/lib/coreBundle'; - -import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; - -const { createConnection } = tools; test('library can be used from CommonJS', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright-mcp/issues/456' } }, async ({}, testInfo) => { const file = testInfo.outputPath('main.cjs'); @@ -35,46 +26,3 @@ test('library can be used from CommonJS', { annotation: { type: 'issue', descrip `); expect(child_process.execSync(`node ${file}`, { encoding: 'utf-8' })).toContain('OK'); }); - -test('createConnection detaches backend listeners from a caller owned context', async ({ mcpBrowser, mcpHeadless, server }, testInfo) => { - const channel = mcpBrowser === 'chrome' || mcpBrowser === 'msedge' ? mcpBrowser : undefined; - const browserName = (channel ? 'chromium' : mcpBrowser) as 'chromium' | 'firefox' | 'webkit'; - const browser = await playwright[browserName].launch({ channel, headless: mcpHeadless }); - const browserContext = await browser.newContext(); - - const client = await connectClient(await createConnection({ - outputDir: testInfo.outputPath('output'), - }, async () => browserContext)); - - const contextListeners = listenerCount(browserContext, 'close'); - const browserListeners = listenerCount(browser, 'disconnected'); - - // browser_close disposes the backend while the caller keeps the context, so - // the next tool call builds another backend over the same two objects. - for (let i = 0; i < 3; i++) { - await client.callTool({ name: 'browser_navigate', arguments: { url: server.HELLO_WORLD } }); - // A live backend listens on both objects. - expect(listenerCount(browserContext, 'close')).toBe(contextListeners + 1); - expect(listenerCount(browser, 'disconnected')).toBe(browserListeners + 1); - - await client.callTool({ name: 'browser_close', arguments: {} }); - // A disposed one gives both back, so they do not pile up. - expect(listenerCount(browserContext, 'close')).toBe(contextListeners); - expect(listenerCount(browser, 'disconnected')).toBe(browserListeners); - } - - await client.close(); - await browser.close(); -}); - -function listenerCount(emitter: object, event: string): number { - return (emitter as unknown as { listenerCount(event: string): number }).listenerCount(event); -} - -async function connectClient(server: Server): Promise { - const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); - await server.connect(serverTransport); - const client = new Client({ name: 'test', version: '1.0.0' }); - await client.connect(clientTransport); - return client; -}