Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions packages/playwright-core/src/tools/mcp/watchdog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { testDebug } from './log';

export function setupExitWatchdog() {
let isExiting = false;
const handleExit = async (signal: string) => {
const handleExit = async () => {
if (isExiting)
return;
isExiting = true;
Expand All @@ -31,7 +31,19 @@ export function setupExitWatchdog() {
process.exit(0);
};

process.stdin.on('close', () => handleExit('close'));
process.on('SIGINT', () => handleExit('SIGINT'));
process.on('SIGTERM', () => handleExit('SIGTERM'));
process.stdin.on('close', handleExit);
process.on('SIGINT', handleExit);
process.on('SIGTERM', handleExit);

// The host can die without delivering a signal or closing stdin (SIGKILL, OOM,
// IDE reload), in which case we get re-parented. Poll for that to avoid leaking
// the browser. No re-parenting on Windows.
if (process.platform !== 'win32') {
const parentPid = process.ppid;
const interval = setInterval(() => {
if (process.ppid !== parentPid)
void handleExit();
}, 1000);
interval.unref();
}
}
9 changes: 9 additions & 0 deletions tests/config/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,12 @@ export function inheritAndCleanEnv(env: NodeJS.ProcessEnv | undefined): NodeJS.P
...env,
};
}

export function isAlive(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}
10 changes: 1 addition & 9 deletions tests/extension/cli.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { test as base, expect, extensionId, clickAllowAndSelect } from './extension-fixtures';
import { isAlive } from '../config/utils';

import type { CliResult } from './extension-fixtures';
import type { Page } from 'playwright';
Expand All @@ -37,15 +38,6 @@ const test = base.extend<{
},
});

function isAlive(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}

async function expectDaemonExited(cliPromise: Promise<CliResult>): Promise<void> {
const { error } = await cliPromise;
const pidMatch = error.match(/Daemon pid=(\d+)/);
Expand Down
10 changes: 1 addition & 9 deletions tests/mcp/cli-killall.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,7 @@
*/

import { test, expect } from './cli-fixtures';

function isAlive(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}
import { isAlive } from '../config/utils';

test('kill-all kills only filtered pid', async ({ cli, server }) => {
const { daemonPid } = await cli('open', server.HELLO_WORLD);
Expand Down
10 changes: 1 addition & 9 deletions tests/mcp/dashboard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import os from 'os';
import path from 'path';

import { test, expect, installSaveFilePickerMock } from './cli-fixtures';
import { isAlive } from '../config/utils';

function displayPath(p: string): string {
const home = os.homedir();
Expand Down Expand Up @@ -111,15 +112,6 @@ test('should activate session when show is called with -s', async ({ cli, server
await expect(activeSession(dashboard)).toHaveAccessibleName('Session sessB');
});

function isAlive(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}

test('daemon show: closing page exits the process', async ({ cli, connectToDashboard }) => {
const bindTitle = `--playwright-internal--${crypto.randomUUID()}`;
const { exitCode, dashboardPid } = await cli('show', { bindTitle });
Expand Down
107 changes: 107 additions & 0 deletions tests/mcp/watchdog.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import fs from 'fs';
import { execSync, spawn } from 'child_process';

import { test, expect, mcpServerPath } from './fixtures';
import { inheritAndCleanEnv, isAlive } from '../config/utils';

test.slow();

// Regression test for https://github.com/microsoft/playwright/issues/41013:
// when the host process dies without delivering SIGINT/SIGTERM (e.g. SIGKILL,
// OOM kill, IDE hard-reload) the MCP server is re-parented and must still shut
// the browser down. We reproduce this by putting the MCP server's stdin behind
// a FIFO held open by the test process, spawning it from an intermediate parent
// and then killing that parent: neither a stdin 'close' nor a signal reaches
// the MCP server, so only parent-death (PPID) detection can clean it up.
test('should close the browser when the MCP host is killed without a signal', async ({ mcpBrowser, mcpHeadless }, testInfo) => {
test.skip(process.platform === 'win32', 'There is no process re-parenting on Windows');

const fifo = testInfo.outputPath('mcp-stdin.fifo');
execSync(`mkfifo ${fifo}`);
// Open read-write so the open never blocks and the write end stays open even
// after the intermediate parent dies.
const stdinFd = fs.openSync(fifo, 'r+');

const pidFile = testInfo.outputPath('mcp.pid');
const stdoutLog = testInfo.outputPath('mcp-stdout.log');

const mcpArgs = [...mcpServerPath, '--isolated'];
if (mcpHeadless)
mcpArgs.push('--headless');
if (mcpBrowser)
mcpArgs.push(`--browser=${mcpBrowser}`);

// The intermediate parent launches the MCP server reading from the FIFO and
// then stays alive, so the MCP server only dies once we kill this parent.
const parentScript = `
const fs = require('fs');
const { spawn } = require('child_process');
const readFd = fs.openSync(${JSON.stringify(fifo)}, 'r');
const out = fs.openSync(${JSON.stringify(stdoutLog)}, 'a');
const child = spawn(process.execPath, ${JSON.stringify(mcpArgs)}, {
stdio: [readFd, out, 'inherit'],
});
fs.writeFileSync(${JSON.stringify(pidFile)}, String(child.pid));
setInterval(() => {}, 1000);
`;

const parent = spawn(process.execPath, ['-e', parentScript], {
stdio: ['ignore', 'inherit', 'inherit'],
env: inheritAndCleanEnv({
PWMCP_PROFILES_DIR_FOR_TEST: testInfo.outputPath('ms-playwright'),
PW_TMPDIR_FOR_TEST: testInfo.outputPath('tmp'),
}),
});

// Wait for the MCP server pid to be reported.
await expect.poll(() => fs.existsSync(pidFile), { timeout: 15000 }).toBe(true);
const mcpPid = Number(fs.readFileSync(pidFile, 'utf8'));
expect(mcpPid).toBeGreaterThan(0);

// Drive the MCP server to launch the browser.
const send = (msg: any) => fs.writeSync(stdinFd, JSON.stringify(msg) + '\n');
send({ jsonrpc: '2.0', id: 1, method: 'initialize', params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 'repro', version: '1' } } });
send({ jsonrpc: '2.0', method: 'notifications/initialized' });
send({ jsonrpc: '2.0', id: 2, method: 'tools/call', params: { name: 'browser_navigate', arguments: { url: 'about:blank' } } });

// Wait for the navigate response, which means the browser is up.
await expect.poll(() => {
try {
return fs.readFileSync(stdoutLog, 'utf8').includes('"id":2');
} catch {
return false;
}
}, { timeout: 30000 }).toBe(true);

// The browser is a direct child of the MCP server process.
const browserPids = execSync(`pgrep -P ${mcpPid} || true`).toString().split('\n').map(Number).filter(Boolean);
expect(browserPids.length).toBeGreaterThan(0);
expect(isAlive(mcpPid)).toBe(true);

// Simulate the host dying without a signal: the MCP server is re-parented but
// keeps its (FIFO) stdin and never receives SIGINT/SIGTERM.
process.kill(parent.pid!, 'SIGKILL');

// The watchdog should notice the orphaning and shut everything down.
await expect.poll(() => isAlive(mcpPid), { timeout: 20000, message: 'MCP server should exit after being orphaned' }).toBe(false);
for (const pid of browserPids)
await expect.poll(() => isAlive(pid), { timeout: 20000, message: `Browser process ${pid} should exit` }).toBe(false);

fs.closeSync(stdinFd);
});
Loading