diff --git a/packages/extension/src/background.ts b/packages/extension/src/background.ts index 5a33eaa87e70e..881e69c8d80dd 100644 --- a/packages/extension/src/background.ts +++ b/packages/extension/src/background.ts @@ -31,6 +31,8 @@ type PageMessage = { type: 'getConnectionStatus'; } | { type: 'disconnect'; +} | { + type: 'rejectConnection'; }; const PLAYWRIGHT_GROUP_TITLE = 'Playwright'; @@ -110,8 +112,17 @@ class TabShareExtension { sendResponse({ success: false, error: error.message }); } return true; + case 'rejectConnection': { + const selectorTabId = sender.tab?.id; + const pending = selectorTabId !== undefined ? this._pendingTabSelection.get(selectorTabId) : undefined; + if (pending) { + this._pendingTabSelection.delete(selectorTabId!); + pending.close('Rejected by user'); + } + sendResponse({ success: true }); + return true; + } } - return false; } private async _connectToRelay(selectorTabId: number, mcpRelayUrl: string, protocolVersion: number): Promise { diff --git a/packages/extension/src/ui/connect.tsx b/packages/extension/src/ui/connect.tsx index 8c248328a8326..3094dab138fa1 100644 --- a/packages/extension/src/ui/connect.tsx +++ b/packages/extension/src/ui/connect.tsx @@ -115,6 +115,9 @@ const ConnectApp: React.FC = () => { setShowButtons(false); setShowTabList(false); setStatus({ type: 'error', message }); + // Ask the background to close the pending MCP relay connection so the + // client daemon sees a disconnect and exits. + chrome.runtime.sendMessage({ type: 'rejectConnection' }).catch(() => {}); }, []); const connectToMCPRelay = useCallback(async (mcpRelayUrl: string, protocolVersion: number) => { diff --git a/packages/playwright-core/src/tools/cli-client/session.ts b/packages/playwright-core/src/tools/cli-client/session.ts index 3aed7eee2e7e1..ce533cbe420cc 100644 --- a/packages/playwright-core/src/tools/cli-client/session.ts +++ b/packages/playwright-core/src/tools/cli-client/session.ts @@ -161,6 +161,8 @@ export class Session { process.on('SIGTERM', sigtermHandler); let outLog = ''; + const rejectWithPid = (reject: (e: Error) => void, message: string) => + reject(Object.assign(new Error(`Daemon pid=${child.pid}: ${message}`), { daemonPid: child.pid })); await new Promise((resolve, reject) => { child.stdout!.on('data', data => { outLog += data.toString(); @@ -170,8 +172,7 @@ export class Session { const error = errorMatch ? errorMatch[1].trim() : undefined; if (error) { const errLogContent = fs.readFileSync(errLog, 'utf-8'); - const message = error + (errLogContent ? '\n' + errLogContent : ''); - reject(new Error(message)); + rejectWithPid(reject, error + (errLogContent ? '\n' + errLogContent : '')); } const successMatch = outLog.match(/### Success\nDaemon listening on (.*)\n/); @@ -181,8 +182,7 @@ export class Session { child.on('close', code => { if (!signalled) { const errLogContent = fs.readFileSync(errLog, 'utf-8'); - const message = `Daemon process exited with code ${code}` + (errLogContent ? '\n' + errLogContent : ''); - reject(new Error(message)); + rejectWithPid(reject, `Daemon process exited with code ${code}` + (errLogContent ? '\n' + errLogContent : '')); } }); }); diff --git a/packages/playwright-core/src/tools/cli-daemon/program.ts b/packages/playwright-core/src/tools/cli-daemon/program.ts index 4d06a661622b2..39c3e54d70caf 100644 --- a/packages/playwright-core/src/tools/cli-daemon/program.ts +++ b/packages/playwright-core/src/tools/cli-daemon/program.ts @@ -72,6 +72,10 @@ export function decorateProgram(program: Command) { const message = process.env.PWDEBUGIMPL ? (error as Error).stack || (error as Error).message : (error as Error).message; console.log(`### Error\n${message}`); console.log(''); + // The cli-client never destroys our stdout pipe on the error path, + // so the libuv handle would keep the daemon alive forever. + // eslint-disable-next-line no-restricted-properties + process.exit(1); } }); } diff --git a/packages/playwright-core/src/tools/mcp/extensionContextFactory.ts b/packages/playwright-core/src/tools/mcp/extensionContextFactory.ts index a5d9e5696aac9..fb7d95b7a2f2a 100644 --- a/packages/playwright-core/src/tools/mcp/extensionContextFactory.ts +++ b/packages/playwright-core/src/tools/mcp/extensionContextFactory.ts @@ -34,6 +34,12 @@ export async function createExtensionBrowser(config: FullConfig, clientName: str config.browser.launchOptions.executablePath); debugLogger(`CDP relay server started, extension endpoint: ${relay.extensionEndpoint()}.`); - await relay.ensureExtensionConnectionForMCPContext(clientName); - return await playwright.chromium.connectOverCDP(relay.cdpEndpoint(), { isLocal: true, timeout: 0 }); + try { + await relay.ensureExtensionConnectionForMCPContext(clientName); + return await playwright.chromium.connectOverCDP(relay.cdpEndpoint(), { isLocal: true, timeout: 0 }); + } catch (error) { + relay.stop(); + httpServer.close(); + throw error; + } } diff --git a/tests/extension/cli.spec.ts b/tests/extension/cli.spec.ts index a90a54081274e..9bf4ba7b19056 100644 --- a/tests/extension/cli.spec.ts +++ b/tests/extension/cli.spec.ts @@ -15,36 +15,67 @@ */ import fs from 'fs/promises'; -import { test, expect, extensionId } from './extension-fixtures'; +import { test as base, expect, extensionId } from './extension-fixtures'; -test('attach --extension', async ({ browserWithExtension, cli, server }, testInfo) => { - const browserContext = await browserWithExtension.launch(); +import type { CliResult } from './extension-fixtures'; +import type { Page } from 'playwright'; - // Write config file with userDataDir - const configPath = testInfo.outputPath('cli-config.json'); - await fs.writeFile(configPath, JSON.stringify({ - browser: { - userDataDir: browserWithExtension.userDataDir, - } - }, null, 2)); +const test = base.extend<{ + startAttach: () => Promise<{ confirmationPage: Page, cliPromise: Promise }>, +}>({ + startAttach: async ({ browserWithExtension, cli }, use, testInfo) => { + await use(async () => { + await fs.writeFile(testInfo.outputPath('cli-config.json'), JSON.stringify({ + browser: { + userDataDir: browserWithExtension.userDataDir, + } + }, null, 2)); + const browserContext = await browserWithExtension.launch(); + const confirmationPagePromise = browserContext.waitForEvent('page', page => + page.url().startsWith(`chrome-extension://${extensionId}/connect.html`) + ); + const cliPromise = cli('attach', '--extension', `--config=cli-config.json`); + const confirmationPage = await confirmationPagePromise; + return { confirmationPage, cliPromise }; + }); + }, +}); - const confirmationPagePromise = browserContext.waitForEvent('page', page => { - return page.url().startsWith(`chrome-extension://${extensionId}/connect.html`); - }); +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} - // Start the CLI command in the background - const cliPromise = cli('attach', '--extension', `--config=cli-config.json`); +async function expectDaemonExited(cliPromise: Promise): Promise { + const { error } = await cliPromise; + const pidMatch = error.match(/Daemon pid=(\d+)/); + expect(pidMatch, `expected daemon pid in cli error:\n${error}`).toBeTruthy(); + const pid = parseInt(pidMatch![1], 10); + await expect.poll(() => isAlive(pid)).toBe(false); +} - // Wait for the confirmation page to appear - const confirmationPage = await confirmationPagePromise; +test('daemon exits when user rejects the extension connection', async ({ startAttach }) => { + const { confirmationPage, cliPromise } = await startAttach(); + await confirmationPage.getByRole('button', { name: 'Reject' }).click(); + await expectDaemonExited(cliPromise); +}); + +test('daemon exits when user closes the connect tab', async ({ startAttach }) => { + const { confirmationPage, cliPromise } = await startAttach(); + await confirmationPage.close(); + await expectDaemonExited(cliPromise); +}); - // Click the Connect button +test('attach --extension', async ({ startAttach, cli, server }) => { + const { confirmationPage, cliPromise } = await startAttach(); await confirmationPage.locator('.tab-item', { hasText: 'Welcome' }).getByRole('button', { name: 'Allow & select' }).click(); { - // Wait for the CLI command to complete const { output } = await cliPromise; - // Verify the output expect(output).toContain(`### Page`); expect(output).toContain(`- Page URL: chrome-extension://${extensionId}/connect.html?`); expect(output).toContain(`- Page Title: Welcome`); @@ -52,7 +83,6 @@ test('attach --extension', async ({ browserWithExtension, cli, server }, t { const { output } = await cli('goto', server.HELLO_WORLD); - // Verify the output expect(output).toContain(`### Page`); expect(output).toContain(`- Page URL: ${server.HELLO_WORLD}`); expect(output).toContain(`- Page Title: Title`); diff --git a/tests/extension/extension-fixtures.ts b/tests/extension/extension-fixtures.ts index 666c83cd1e40c..8231648895293 100644 --- a/tests/extension/extension-fixtures.ts +++ b/tests/extension/extension-fixtures.ts @@ -59,6 +59,8 @@ export const test = base.extend