Skip to content
Merged
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
13 changes: 12 additions & 1 deletion packages/extension/src/background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ type PageMessage = {
type: 'getConnectionStatus';
} | {
type: 'disconnect';
} | {
type: 'rejectConnection';
};

const PLAYWRIGHT_GROUP_TITLE = 'Playwright';
Expand Down Expand Up @@ -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<void> {
Expand Down
3 changes: 3 additions & 0 deletions packages/extension/src/ui/connect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
8 changes: 4 additions & 4 deletions packages/playwright-core/src/tools/cli-client/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>((resolve, reject) => {
child.stdout!.on('data', data => {
outLog += data.toString();
Expand All @@ -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<EOF>/);
Expand All @@ -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 : ''));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not look necessary?

}
});
});
Expand Down
4 changes: 4 additions & 0 deletions packages/playwright-core/src/tools/cli-daemon/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('<EOF>');
// 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);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
72 changes: 51 additions & 21 deletions tests/extension/cli.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,44 +15,74 @@
*/

import fs from 'fs/promises';
import { test, expect, extensionId } from './extension-fixtures';
import { test as base, expect, extensionId } from './extension-fixtures';

test('attach <url> --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<CliResult> }>,
}>({
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<CliResult>): Promise<void> {
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 <url> --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`);
}

{
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`);
Expand Down
2 changes: 2 additions & 0 deletions tests/extension/extension-fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ export const test = base.extend<TestFixtures, WorkerFixtures & ExtensionTestOpti
// Default is 1.
if (protocolVersion === 2)
process.env.PLAYWRIGHT_EXTENSION_PROTOCOL = '2';
else
delete process.env.PLAYWRIGHT_EXTENSION_PROTOCOL;
await use();
}, { auto: true, scope: 'worker' }],

Expand Down
Loading