From cd7ddebf38e5bc6355fc5a4597268aea7bf97671 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 6 Oct 2026 13:37:33 -0700 Subject: [PATCH] fix(mcp): do not close the browser twice when the client disconnects On stdin end, the MCP server closed its transport, which closed the browser, and the exit watchdog closed the browser at the same time. The second close force-killed the browser. Remove the 'end' handler, so the watchdog is the only shutdown path. The 'end' handler also killed a pending browser_annotate client. That client now notices the exit of its parent by itself: it reads stdin, so the 'close' event fires. Fixes: https://github.com/microsoft/playwright/issues/43098 --- .../src/tools/dashboard/dashboardApp.ts | 3 +++ .../src/tools/utils/mcp/server.ts | 5 ++--- tests/mcp/launch.spec.ts | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/playwright-core/src/tools/dashboard/dashboardApp.ts b/packages/playwright-core/src/tools/dashboard/dashboardApp.ts index 5faa603223775..08bb90c4c6782 100644 --- a/packages/playwright-core/src/tools/dashboard/dashboardApp.ts +++ b/packages/playwright-core/src/tools/dashboard/dashboardApp.ts @@ -397,6 +397,9 @@ async function runKillClient(): Promise { async function runAnnotateClient(options: DashboardOptions): Promise { selfDestructOnParentGone(); + // stdin is a pipe from the process that started this client. It emits 'close' only after it + // has been read to the end, so read it. Unref it, so that it does not keep the process alive. + process.stdin.resume().unref(); const socketPath = dashboardSocketPath(); const tryConnect = () => new Promise(resolve => { diff --git a/packages/playwright-core/src/tools/utils/mcp/server.ts b/packages/playwright-core/src/tools/utils/mcp/server.ts index 8be86c12065e3..28317f27d4739 100644 --- a/packages/playwright-core/src/tools/utils/mcp/server.ts +++ b/packages/playwright-core/src/tools/utils/mcp/server.ts @@ -188,9 +188,8 @@ function addServerListener(server: ServerType, event: 'close' | 'initialized', l export async function start(serverBackendFactory: ServerBackendFactory, options: { host?: string; port?: number, allowedHosts?: string[], socketPath?: string } = {}) { if (options.port === undefined) { const transport = new StdioServerTransport(); - // The SDK's StdioServerTransport doesn't detect peer disconnect — it never listens for stdin - // end-of-stream. Wire it up so callTool requests can be cancelled when the client goes away. - process.stdin.on('end', () => void transport.close()); + // Do not close the transport on stdin end. The exit watchdog closes the browsers on stdin + // close, and closing a browser that is already closing force-kills it. await connect(serverBackendFactory, transport, Promise.resolve(), false); return; } diff --git a/tests/mcp/launch.spec.ts b/tests/mcp/launch.spec.ts index 0b4e9599dbeeb..1678f43cb556e 100644 --- a/tests/mcp/launch.spec.ts +++ b/tests/mcp/launch.spec.ts @@ -317,3 +317,23 @@ exit 1 error: expect.not.stringContaining(`Browser is already in use`), }); }); + +test('should gracefully close the browser once on stdin end', { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/43098' } +}, async ({ startClient, server }, testInfo) => { + const { client, stderr } = await startClient({ + args: [`--user-data-dir=${testInfo.outputPath('user-data-dir')}`], + env: { DEBUG: 'pw:browser' }, + }); + await client.callTool({ + name: 'browser_navigate', + arguments: { url: server.HELLO_WORLD }, + }); + + await client.close(); + + await expect.poll(() => stderr()).toContain(''); + expect(stderr()).toContain(''); + expect(stderr()).not.toContain(''); + expect(stderr()).not.toContain(''); +});