Skip to content

Commit d80df29

Browse files
committed
refactor(mcp): split resolved browser config into discriminated union
LocalBrowserConfig / CDPBrowserConfig / RemoteBrowserConfig / ExtensionBrowserConfig (each tagged with `mode`). Extension drops launchOptions/contextOptions/userDataDir, has `isolated: false` literal and `browserName: 'chromium'` literal so call sites can use plain `config.browser.isolated` / `config.browser.browserName` and TS narrows extension correctly. createBrowserWithInfo splits into createAttachedBrowser (no FullConfig needed) and createLocalBrowserWithInfo. Adds `acquireBrowserContext` helper to dedupe newContext-vs-contexts[0] across three call sites.
1 parent d560e30 commit d80df29

6 files changed

Lines changed: 226 additions & 106 deletions

File tree

packages/playwright-core/src/tools/cli-daemon/program.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { getAsBooleanFromENV, guessClientName } from '@utils/env';
2424
import { libPath } from '../../package';
2525
import { startCliDaemonServer } from './daemon';
2626
import { setupExitWatchdog } from '../mcp/watchdog';
27-
import { createBrowserWithInfo } from '../mcp/browserFactory';
27+
import { acquireBrowserContext, createBrowserWithInfo } from '../mcp/browserFactory';
2828
import * as configUtils from '../mcp/config';
2929
import { createClientInfo } from '../cli-client/registry';
3030
import { registry as browserRegistry } from '../../server/registry/index';
@@ -61,10 +61,10 @@ export function decorateProgram(program: Command) {
6161
const { browser, browserInfo, canBind, ownership } = await createBrowserWithInfo(mcpConfig, mcpClientInfo);
6262
if (canBind)
6363
await browser.bind(sessionName, { workspaceDir: clientInfo.workspaceDir });
64-
const browserContext = mcpConfig.browser.isolated ? await browser.newContext(mcpConfig.browser.contextOptions) : browser.contexts()[0];
64+
const browserContext = await acquireBrowserContext(browser, mcpConfig.browser);
6565
if (!browserContext)
6666
throw new Error('Error: unable to connect to a browser that does not have any contexts');
67-
const persistent = options.persistent || options.profile || mcpConfig.browser.userDataDir ? true : undefined;
67+
const persistent = options.persistent || options.profile || (mcpConfig.browser.mode === 'local' && mcpConfig.browser.userDataDir) ? true : undefined;
6868
const socketPath = await startCliDaemonServer(sessionName, browserContext, browserInfo, mcpConfig, clientInfo, mcpClientInfo, { persistent, exitOnClose: true, ownership });
6969
console.log(`### Success\nDaemon listening on ${socketPath}`);
7070
console.log('<EOF>');
@@ -120,7 +120,7 @@ async function ensureConfiguredBrowserInstalled() {
120120
const clientInfo = createClientInfo();
121121
const config = await configUtils.resolveCLIConfigForCLI(clientInfo.daemonProfilesDir, 'default', {});
122122
const browserName = config.browser.browserName;
123-
const channel = config.browser.launchOptions.channel;
123+
const channel = config.browser.mode === 'local' ? config.browser.launchOptions.channel : undefined;
124124
if (!channel || channel.startsWith('chromium'))
125125
await resolveAndInstall(channel ?? browserName);
126126
} else {

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

Lines changed: 79 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { serverRegistry } from '../../serverRegistry';
2828
// eslint-disable-next-line no-restricted-imports
2929
import { connectToBrowser } from '../../client/connect';
3030

31-
import type { FullConfig } from './config';
31+
import type { CDPBrowserConfig, ExtensionBrowserConfig, FullConfig, LocalBrowserConfig, RemoteBrowserConfig, ResolvedBrowser } from './config';
3232
import type { ClientInfo } from '../utils/mcp/server';
3333
// eslint-disable-next-line no-restricted-imports
3434
import type { Playwright } from '../../client/playwright';
@@ -48,77 +48,96 @@ export async function createBrowser(config: FullConfig, clientInfo: ClientInfo):
4848
}
4949

5050
export async function createBrowserWithInfo(config: FullConfig, clientInfo: ClientInfo): Promise<BrowserWithInfo> {
51-
if (config.browser.remoteEndpoint)
52-
return await createRemoteBrowser(config);
53-
54-
let browser: playwrightTypes.Browser;
55-
let canBind = false;
56-
let ownership: 'attached' | 'own' = 'own';
57-
if (config.browser.cdpEndpoint) {
58-
browser = await createCDPBrowser(config);
59-
canBind = true;
60-
ownership = 'attached';
61-
} else if (config.browser.isolated) {
62-
browser = await createIsolatedBrowser(config, clientInfo);
63-
canBind = true;
64-
ownership = 'own';
65-
} else if (config.extension) {
66-
const channel = config.browser.launchOptions.channel || 'chrome';
67-
browser = await createExtensionBrowser(channel, clientInfo.clientName);
68-
ownership = 'attached';
69-
} else {
70-
browser = await createPersistentBrowser(config, clientInfo);
71-
canBind = true;
72-
ownership = 'own';
51+
if (config.browser.mode === 'local')
52+
return await createLocalBrowserWithInfo(config, config.browser, clientInfo);
53+
return await createAttachedBrowser(config.browser, clientInfo);
54+
}
55+
56+
async function createAttachedBrowser(browserConfig: CDPBrowserConfig | RemoteBrowserConfig | ExtensionBrowserConfig, clientInfo: ClientInfo): Promise<BrowserWithInfo> {
57+
switch (browserConfig.mode) {
58+
case 'remote':
59+
return await createRemoteBrowser(browserConfig);
60+
case 'cdp': {
61+
const browser = await createCDPBrowser(browserConfig);
62+
return { browser, browserInfo: browserInfo(browser, browserConfig, undefined), canBind: true, ownership: 'attached' };
63+
}
64+
case 'extension': {
65+
const browser = await createExtensionBrowser(browserConfig.channel, clientInfo.clientName);
66+
return { browser, browserInfo: browserInfo(browser, browserConfig, undefined), canBind: false, ownership: 'attached' };
67+
}
7368
}
69+
}
7470

75-
return { browser, browserInfo: browserInfo(browser, config), canBind, ownership };
71+
async function createLocalBrowserWithInfo(config: FullConfig, browserConfig: LocalBrowserConfig, clientInfo: ClientInfo): Promise<BrowserWithInfo> {
72+
if (browserConfig.isolated) {
73+
const browser = await createIsolatedBrowser(config, browserConfig, clientInfo);
74+
return { browser, browserInfo: browserInfo(browser, browserConfig, undefined), canBind: true, ownership: 'own' };
75+
}
76+
const userDataDir = browserConfig.userDataDir ?? await createUserDataDir(browserConfig, clientInfo);
77+
const browser = await createPersistentBrowser(config, browserConfig, userDataDir, clientInfo);
78+
return { browser, browserInfo: browserInfo(browser, browserConfig, userDataDir), canBind: true, ownership: 'own' };
7679
}
7780

7881
export interface BrowserContextFactory {
7982
contexts(clientInfo: ClientInfo): Promise<playwrightTypes.BrowserContext[]>;
8083
createContext(clientInfo: ClientInfo): Promise<playwrightTypes.BrowserContext>;
8184
}
8285

83-
function browserInfo(browser: playwrightTypes.Browser, config: FullConfig): BrowserInfo {
84-
return {
85-
// eslint-disable-next-line no-restricted-syntax
86-
guid: (browser as any)._guid,
87-
browserName: config.browser.browserName,
88-
launchOptions: config.browser.launchOptions,
89-
userDataDir: config.browser.userDataDir
90-
};
86+
type BrowserLike = {
87+
contexts(): playwrightTypes.BrowserContext[];
88+
newContext(options?: playwrightTypes.BrowserContextOptions): Promise<playwrightTypes.BrowserContext>;
89+
};
90+
91+
export async function acquireBrowserContext(browser: BrowserLike, browserConfig: ResolvedBrowser): Promise<playwrightTypes.BrowserContext> {
92+
if (browserConfig.isolated)
93+
return await browser.newContext(browserConfig.contextOptions);
94+
return browser.contexts()[0];
9195
}
9296

93-
async function createIsolatedBrowser(config: FullConfig, clientInfo: ClientInfo): Promise<playwrightTypes.Browser> {
97+
function browserInfo(browser: playwrightTypes.Browser, browserConfig: ResolvedBrowser, userDataDir: string | undefined): BrowserInfo {
98+
// eslint-disable-next-line no-restricted-syntax
99+
const guid = (browser as any)._guid;
100+
const browserName = browserConfig.browserName;
101+
switch (browserConfig.mode) {
102+
case 'local':
103+
return { guid, browserName, launchOptions: browserConfig.launchOptions, userDataDir };
104+
case 'extension':
105+
return { guid, browserName, launchOptions: { channel: browserConfig.channel }, userDataDir };
106+
case 'cdp':
107+
case 'remote':
108+
return { guid, browserName, launchOptions: {}, userDataDir };
109+
}
110+
}
111+
112+
async function createIsolatedBrowser(config: FullConfig, browserConfig: LocalBrowserConfig, clientInfo: ClientInfo): Promise<playwrightTypes.Browser> {
94113
testDebug('create browser (isolated)');
95-
const browserType = playwright[config.browser.browserName];
114+
const browserType = playwright[browserConfig.browserName];
96115
const tracesDir = await computeTracesDir(config, clientInfo);
97116
const browser = await browserType.launch({
98117
tracesDir,
99-
...config.browser.launchOptions,
118+
...browserConfig.launchOptions,
100119
handleSIGINT: false,
101120
handleSIGTERM: false,
102121
}).catch(error => {
103122
if (error.message.includes('Executable doesn\'t exist'))
104-
throwBrowserIsNotInstalledError(config);
123+
throwBrowserIsNotInstalledError(config, browserConfig);
105124
throw error;
106125
});
107126
return browser;
108127
}
109128

110-
async function createCDPBrowser(config: FullConfig): Promise<playwrightTypes.Browser> {
129+
async function createCDPBrowser(browserConfig: CDPBrowserConfig): Promise<playwrightTypes.Browser> {
111130
testDebug('create browser (cdp)');
112-
const browser = await playwright.chromium.connectOverCDP(config.browser.cdpEndpoint!, {
113-
headers: config.browser.cdpHeaders,
114-
timeout: config.browser.cdpTimeout
131+
const browser = await playwright.chromium.connectOverCDP(browserConfig.cdpEndpoint, {
132+
headers: browserConfig.cdpHeaders,
133+
timeout: browserConfig.cdpTimeout
115134
});
116135
return browser;
117136
}
118137

119-
async function createRemoteBrowser(config: FullConfig): Promise<BrowserWithInfo> {
138+
async function createRemoteBrowser(browserConfig: RemoteBrowserConfig): Promise<BrowserWithInfo> {
120139
testDebug('create browser (remote)');
121-
const descriptor = await serverRegistry.find(config.browser.remoteEndpoint!);
140+
const descriptor = await serverRegistry.find(browserConfig.remoteEndpoint);
122141
if (descriptor) {
123142
const browser = await connectToBrowserAcrossVersions(descriptor);
124143
return {
@@ -134,28 +153,31 @@ async function createRemoteBrowser(config: FullConfig): Promise<BrowserWithInfo>
134153
};
135154
}
136155

137-
const endpoint = config.browser.remoteEndpoint!;
138156
const playwrightObject = playwright as Playwright;
139157
// Use connectToBrowser instead of playwright[browserName].connect because we don't have browserName.
140-
const browser = await connectToBrowser(playwrightObject, { endpoint });
158+
const browser = await connectToBrowser(playwrightObject, { endpoint: browserConfig.remoteEndpoint });
141159
browser._connectToBrowserType(playwrightObject[browser._browserName], {}, undefined);
142-
return { browser, browserInfo: browserInfo(browser, config), canBind: false, ownership: 'attached' };
160+
return {
161+
browser,
162+
browserInfo: browserInfo(browser, browserConfig, undefined),
163+
canBind: false,
164+
ownership: 'attached',
165+
};
143166
}
144167

145-
async function createPersistentBrowser(config: FullConfig, clientInfo: ClientInfo): Promise<playwrightTypes.Browser> {
168+
async function createPersistentBrowser(config: FullConfig, browserConfig: LocalBrowserConfig, userDataDir: string, clientInfo: ClientInfo): Promise<playwrightTypes.Browser> {
146169
testDebug('create browser (persistent)');
147-
const userDataDir = config.browser.userDataDir ?? await createUserDataDir(config, clientInfo);
148170
const tracesDir = await computeTracesDir(config, clientInfo);
149171

150172
if (await isProfileLocked5Times(userDataDir))
151173
throw new Error(`Browser is already in use for ${userDataDir}, use --isolated to run multiple instances of the same browser`);
152174

153-
const browserType = playwright[config.browser.browserName];
154-
const configIgnoreDefaultArgs = config.browser.launchOptions?.ignoreDefaultArgs;
175+
const browserType = playwright[browserConfig.browserName];
176+
const configIgnoreDefaultArgs = browserConfig.launchOptions?.ignoreDefaultArgs;
155177
const launchOptions: playwrightTypes.LaunchOptions & playwrightTypes.BrowserContextOptions = {
156178
tracesDir,
157-
...config.browser.launchOptions,
158-
...config.browser.contextOptions,
179+
...browserConfig.launchOptions,
180+
...browserConfig.contextOptions,
159181
handleSIGINT: false,
160182
handleSIGTERM: false,
161183
ignoreDefaultArgs: configIgnoreDefaultArgs === true
@@ -171,9 +193,9 @@ async function createPersistentBrowser(config: FullConfig, clientInfo: ClientInf
171193
return browser;
172194
} catch (error: any) {
173195
if (error.message.includes('Executable doesn\'t exist'))
174-
throwBrowserIsNotInstalledError(config);
196+
throwBrowserIsNotInstalledError(config, browserConfig);
175197
if (error.message.includes('cannot open shared object file: No such file or directory')) {
176-
const browserName = launchOptions.channel ?? config.browser.browserName;
198+
const browserName = launchOptions.channel ?? browserConfig.browserName;
177199
throw new Error(`Missing system dependencies required to run browser ${browserName}. Install them with: sudo npx playwright install-deps ${browserName}`);
178200
}
179201
if (error.message.includes('ProcessSingleton') || error.message.includes('exitCode=21'))
@@ -182,9 +204,9 @@ async function createPersistentBrowser(config: FullConfig, clientInfo: ClientInf
182204
}
183205
}
184206

185-
async function createUserDataDir(config: FullConfig, clientInfo: ClientInfo) {
207+
async function createUserDataDir(browserConfig: LocalBrowserConfig, clientInfo: ClientInfo) {
186208
const dir = process.env.PWMCP_PROFILES_DIR_FOR_TEST ?? registryDirectory;
187-
const browserToken = config.browser.launchOptions?.channel ?? config.browser?.browserName;
209+
const browserToken = browserConfig.launchOptions.channel ?? browserConfig.browserName;
188210
// Hesitant putting hundreds of files into the user's workspace, so using it for hashing instead.
189211
const rootPathToken = createHash(clientInfo.cwd);
190212
const result = path.join(dir, `mcp-${browserToken}-${rootPathToken}`);
@@ -235,8 +257,8 @@ export function isProfileLocked(userDataDir: string): boolean {
235257
}
236258
}
237259

238-
function throwBrowserIsNotInstalledError(config: FullConfig): never {
239-
const channel = config.browser.launchOptions?.channel ?? config.browser.browserName;
260+
function throwBrowserIsNotInstalledError(config: FullConfig, browserConfig: LocalBrowserConfig): never {
261+
const channel = browserConfig.launchOptions.channel ?? browserConfig.browserName;
240262
if (config.skillMode)
241263
throw new Error(`Browser "${channel}" is not installed. Run \`playwright-cli install-browser ${channel}\` to install`);
242264
else

0 commit comments

Comments
 (0)