Skip to content

Commit 0c04e83

Browse files
authored
fix(mcp): propagate --browser channel on --extension path (#40567)
1 parent c5ac83c commit 0c04e83

5 files changed

Lines changed: 33 additions & 31 deletions

File tree

packages/playwright-core/src/tools/mcp/browserFactory.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,6 @@ type BrowserWithInfo = {
4343
ownership: 'attached' | 'own',
4444
};
4545

46-
export async function createBrowser(config: FullConfig, clientInfo: ClientInfo): Promise<playwrightTypes.Browser> {
47-
const { browser } = await createBrowserWithInfo(config, clientInfo, {});
48-
return browser;
49-
}
50-
5146
export async function createBrowserWithInfo(config: FullConfig, clientInfo: ClientInfo, cliOptions: CLIOptions): Promise<BrowserWithInfo> {
5247
if (config.browser.remoteEndpoint)
5348
return await createRemoteBrowser(config);

packages/playwright-core/src/tools/mcp/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
import { resolveConfig } from './config';
1818
import { filteredTools } from '../backend/tools';
19-
import { createBrowser } from './browserFactory';
19+
import { createBrowserWithInfo } from './browserFactory';
2020
import { BrowserBackend } from '../backend/browserBackend';
2121
import { createServer } from '../utils/mcp/server';
2222
import { packageJSON } from '../../package';
@@ -35,7 +35,9 @@ export async function createConnection(userConfig: Config = {}, contextGetter?:
3535
version: packageJSON.version,
3636
toolSchemas: tools.map(tool => tool.schema),
3737
create: async (clientInfo: ClientInfo) => {
38-
const browser = contextGetter ? new SimpleBrowser(await contextGetter()) : await createBrowser(config, clientInfo);
38+
const browser = contextGetter
39+
? new SimpleBrowser(await contextGetter())
40+
: (await createBrowserWithInfo(config, clientInfo, {})).browser;
3941
const context = config.browser.isolated ? await browser.newContext(config.browser.contextOptions) : browser.contexts()[0];
4042
return new BrowserBackend(config, context, tools);
4143
},

packages/playwright-core/src/tools/mcp/program.ts

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { Option as ProgramOption } from 'commander';
1818
import * as mcpServer from '../utils/mcp/server';
1919
import { commaSeparatedList, dotenvFileLoader, enumParser, headerParser, numberParser, resolutionParser, resolveCLIConfigForMCP, semicolonSeparatedList } from './config';
2020
import { setupExitWatchdog } from './watchdog';
21-
import { createBrowser, createBrowserWithInfo } from './browserFactory';
21+
import { createBrowserWithInfo } from './browserFactory';
2222
import { BrowserBackend } from '../backend/browserBackend';
2323
import { filteredTools } from '../backend/tools';
2424
import { testDebug } from './log';
@@ -94,23 +94,6 @@ export function decorateMCPCommand(command: Command) {
9494

9595
const config = await resolveCLIConfigForMCP(options);
9696
const tools = filteredTools(config);
97-
if (config.extension) {
98-
const serverBackendFactory: mcpServer.ServerBackendFactory = {
99-
name: 'Playwright w/ extension',
100-
nameInConfig: 'playwright-extension',
101-
version,
102-
toolSchemas: tools.map(tool => tool.schema),
103-
create: async (clientInfo: ClientInfo) => {
104-
const browser = await createBrowser(config, clientInfo);
105-
const browserContext = browser.contexts()[0];
106-
return new BrowserBackend(config, browserContext, tools);
107-
},
108-
disposed: async () => { }
109-
};
110-
await mcpServer.start(serverBackendFactory, config.server);
111-
return;
112-
}
113-
11497
const useSharedBrowser = config.sharedBrowserContext || config.browser.isolated;
11598
let sharedBrowser: playwright.Browser | undefined;
11699
let clientCount = 0;

tests/extension/extension.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
*/
1616

1717
import { test, testWithOldExtensionVersion, expect, extensionId, clickAllowAndSelect, startWithExtensionFlag } from './extension-fixtures';
18+
import { utils } from '../../packages/playwright-core/lib/coreBundle';
19+
20+
const { defaultUserDataDirForChannel } = utils;
1821

1922
test(`navigate with extension`, async ({ startExtensionClient, server }) => {
2023
const { browserContext, client } = await startExtensionClient();
@@ -245,6 +248,25 @@ test(`fails when extension is missing in custom userDataDir`, async ({ startClie
245248
});
246249
});
247250

251+
test(`--browser <channel> selects channel-specific userDataDir`, {
252+
annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright-mcp/issues/1589' },
253+
}, async ({ startClient, server }) => {
254+
const expectedUserDataDir = defaultUserDataDirForChannel('chrome-dev')!;
255+
256+
const { client } = await startClient({
257+
args: [`--extension`, `--browser=chrome-dev`],
258+
omitArgs: [`--browser=chromium`],
259+
});
260+
261+
expect(await client.callTool({
262+
name: 'browser_navigate',
263+
arguments: { url: server.HELLO_WORLD },
264+
})).toHaveResponse({
265+
error: expect.stringContaining(`Playwright Extension not found in "${expectedUserDataDir}"`),
266+
isError: true,
267+
});
268+
});
269+
248270
test(`bypass connection dialog with token`, async ({ browserWithExtension, startClient, server }) => {
249271
const browserContext = await browserWithExtension.launch();
250272

tests/mcp/cli-cdp.spec.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,28 +22,28 @@ test.describe.configure({
2222
});
2323

2424
test('connect by channel name error', async ({ cli }) => {
25-
const { error } = await cli('attach', '--cdp=chrome-canary');
26-
expect(error).toContain('Could not connect to chrome-canary');
25+
const { error } = await cli('attach', '--cdp=chrome-dev');
26+
expect(error).toContain('Could not connect to chrome-dev');
2727
expect(error).toContain('chrome://inspect/#remote-debugging');
2828
});
2929

3030
test('attach --cdp=<channel> defaults session name to the channel', async ({ cli }) => {
3131
// Connection fails, but the daemon writes its err log under the resolved session name.
32-
await cli('attach', '--cdp=chrome-canary');
32+
await cli('attach', '--cdp=chrome-dev');
3333
const folder = await daemonFolder();
3434
expect(folder).toBeTruthy();
3535
const files = await fs.promises.readdir(folder!);
36-
expect(files).toContain('chrome-canary.err');
36+
expect(files).toContain('chrome-dev.err');
3737
expect(files).not.toContain('default.err');
3838
});
3939

4040
test('explicit --session wins over --cdp=<channel>', async ({ cli }) => {
41-
await cli('attach', '--cdp=chrome-canary', '-s=explicit');
41+
await cli('attach', '--cdp=chrome-dev', '-s=explicit');
4242
const folder = await daemonFolder();
4343
expect(folder).toBeTruthy();
4444
const files = await fs.promises.readdir(folder!);
4545
expect(files).toContain('explicit.err');
46-
expect(files).not.toContain('chrome-canary.err');
46+
expect(files).not.toContain('chrome-dev.err');
4747
});
4848

4949
test('attach via cdp URL keeps the default session', async ({ cdpServer, cli, server }) => {

0 commit comments

Comments
 (0)