From fea50e68401ad765ea85b4f74c858a81394bff8e Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 14 May 2025 16:01:08 -0700 Subject: [PATCH] chore: introduce resolved config (#425) --- src/config.ts | 47 ++++++++++++++++++++++++++++++---------- src/connection.ts | 4 ++-- src/context.ts | 26 +++++++++++----------- src/index.ts | 4 +++- src/program.ts | 4 ++-- src/transport.ts | 10 ++++----- tests/pdf.spec.ts | 5 +++-- tests/screenshot.spec.ts | 10 +++++---- 8 files changed, 69 insertions(+), 41 deletions(-) diff --git a/src/config.ts b/src/config.ts index 977c6b5..92deec4 100644 --- a/src/config.ts +++ b/src/config.ts @@ -51,7 +51,7 @@ export type CLIOptions = { vision?: boolean; }; -const defaultConfig: Config = { +const defaultConfig: FullConfig = { browser: { browserName: 'chromium', launchOptions: { @@ -67,12 +67,32 @@ const defaultConfig: Config = { allowedOrigins: undefined, blockedOrigins: undefined, }, + outputDir: path.join(os.tmpdir(), 'playwright-mcp-output'), }; -export async function resolveConfig(cliOptions: CLIOptions): Promise { - const config = await loadConfig(cliOptions.config); +type BrowserUserConfig = NonNullable; + +export type FullConfig = Config & { + browser: BrowserUserConfig & { + browserName: NonNullable; + launchOptions: NonNullable; + contextOptions: NonNullable; + }, + network: NonNullable, + outputDir: string; +}; + +export async function resolveConfig(config: Config): Promise { + return mergeConfig(defaultConfig, config); +} + +export async function resolveCLIConfig(cliOptions: CLIOptions): Promise { + const configInFile = await loadConfig(cliOptions.config); const cliOverrides = await configFromCLIOptions(cliOptions); - return mergeConfig(defaultConfig, mergeConfig(config, cliOverrides)); + const result = mergeConfig(mergeConfig(defaultConfig, configInFile), cliOverrides); + // Derive artifact output directory from config.outputDir + result.browser.launchOptions.tracesDir = path.join(result.outputDir, 'traces'); + return result; } export async function configFromCLIOptions(cliOptions: CLIOptions): Promise { @@ -202,11 +222,10 @@ async function loadConfig(configFile: string | undefined): Promise { } } -export async function outputFile(config: Config, name: string): Promise { - const result = config.outputDir ?? os.tmpdir(); - await fs.promises.mkdir(result, { recursive: true }); +export async function outputFile(config: FullConfig, name: string): Promise { + await fs.promises.mkdir(config.outputDir, { recursive: true }); const fileName = sanitizeForFilePath(name); - return path.join(result, fileName); + return path.join(config.outputDir, fileName); } function pickDefined(obj: T | undefined): Partial { @@ -215,10 +234,10 @@ function pickDefined(obj: T | undefined): Partial { ) as Partial; } -function mergeConfig(base: Config, overrides: Config): Config { - const browser: Config['browser'] = { - ...pickDefined(base.browser), - ...pickDefined(overrides.browser), +function mergeConfig(base: FullConfig, overrides: Config): FullConfig { + const browser: FullConfig['browser'] = { + browserName: overrides.browser?.browserName ?? base.browser?.browserName ?? 'chromium', + isolated: overrides.browser?.isolated ?? base.browser?.isolated ?? false, launchOptions: { ...pickDefined(base.browser?.launchOptions), ...pickDefined(overrides.browser?.launchOptions), @@ -228,6 +247,9 @@ function mergeConfig(base: Config, overrides: Config): Config { ...pickDefined(base.browser?.contextOptions), ...pickDefined(overrides.browser?.contextOptions), }, + userDataDir: overrides.browser?.userDataDir ?? base.browser?.userDataDir, + cdpEndpoint: overrides.browser?.cdpEndpoint ?? base.browser?.cdpEndpoint, + remoteEndpoint: overrides.browser?.remoteEndpoint ?? base.browser?.remoteEndpoint, }; if (browser.browserName !== 'chromium' && browser.launchOptions) @@ -241,5 +263,6 @@ function mergeConfig(base: Config, overrides: Config): Config { ...pickDefined(base.network), ...pickDefined(overrides.network), }, + outputDir: overrides.outputDir ?? base.outputDir ?? defaultConfig.outputDir, }; } diff --git a/src/connection.ts b/src/connection.ts index e29e4ac..7083382 100644 --- a/src/connection.ts +++ b/src/connection.ts @@ -21,10 +21,10 @@ import { zodToJsonSchema } from 'zod-to-json-schema'; import { Context, packageJSON } from './context.js'; import { snapshotTools, visionTools } from './tools.js'; -import type { Config } from '../config.js'; import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; +import { FullConfig } from './config.js'; -export async function createConnection(config: Config): Promise { +export async function createConnection(config: FullConfig): Promise { const allTools = config.vision ? visionTools : snapshotTools; const tools = allTools.filter(tool => !config.capabilities || tool.capability === 'core' || config.capabilities.includes(tool.capability)); diff --git a/src/context.ts b/src/context.ts index 7c571e6..1bcd4b1 100644 --- a/src/context.ts +++ b/src/context.ts @@ -24,11 +24,11 @@ import * as playwright from 'playwright'; import { waitForCompletion } from './tools/utils.js'; import { ManualPromise } from './manualPromise.js'; import { Tab } from './tab.js'; +import { outputFile } from './config.js'; import type { ImageContent, TextContent } from '@modelcontextprotocol/sdk/types.js'; import type { ModalState, Tool, ToolActionResult } from './tools/tool.js'; -import type { Config } from '../config.js'; -import { outputFile } from './config.js'; +import type { FullConfig } from './config.js'; type PendingAction = { dialogShown: ManualPromise; @@ -41,7 +41,7 @@ type BrowserContextAndBrowser = { export class Context { readonly tools: Tool[]; - readonly config: Config; + readonly config: FullConfig; private _browserContextPromise: Promise | undefined; private _tabs: Tab[] = []; private _currentTab: Tab | undefined; @@ -49,7 +49,7 @@ export class Context { private _pendingAction: PendingAction | undefined; private _downloads: { download: playwright.Download, finished: boolean, outputFile: string }[] = []; - constructor(tools: Tool[], config: Config) { + constructor(tools: Tool[], config: FullConfig) { this.tools = tools; this.config = config; } @@ -351,12 +351,12 @@ ${code.join('\n')} } } -async function createIsolatedContext(browserConfig: Config['browser']): Promise { +async function createIsolatedContext(browserConfig: FullConfig['browser']): Promise { try { const browserName = browserConfig?.browserName ?? 'chromium'; const browserType = playwright[browserName]; - const browser = await browserType.launch(browserConfig?.launchOptions); - const browserContext = await browser.newContext(browserConfig?.contextOptions); + const browser = await browserType.launch(browserConfig.launchOptions); + const browserContext = await browser.newContext(browserConfig.contextOptions); return { browser, browserContext }; } catch (error: any) { if (error.message.includes('Executable doesn\'t exist')) @@ -365,12 +365,12 @@ async function createIsolatedContext(browserConfig: Config['browser']): Promise< } } -async function launchPersistentContext(browserConfig: Config['browser']): Promise { +async function launchPersistentContext(browserConfig: FullConfig['browser']): Promise { try { - const browserName = browserConfig?.browserName ?? 'chromium'; - const userDataDir = browserConfig?.userDataDir ?? await createUserDataDir({ ...browserConfig, browserName }); + const browserName = browserConfig.browserName ?? 'chromium'; + const userDataDir = browserConfig.userDataDir ?? await createUserDataDir({ ...browserConfig, browserName }); const browserType = playwright[browserName]; - const browserContext = await browserType.launchPersistentContext(userDataDir, { ...browserConfig?.launchOptions, ...browserConfig?.contextOptions }); + const browserContext = await browserType.launchPersistentContext(userDataDir, { ...browserConfig.launchOptions, ...browserConfig.contextOptions }); return { browserContext }; } catch (error: any) { if (error.message.includes('Executable doesn\'t exist')) @@ -379,7 +379,7 @@ async function launchPersistentContext(browserConfig: Config['browser']): Promis } } -async function createUserDataDir(browserConfig: Config['browser']) { +async function createUserDataDir(browserConfig: FullConfig['browser']) { let cacheDirectory: string; if (process.platform === 'linux') cacheDirectory = process.env.XDG_CACHE_HOME || path.join(os.homedir(), '.cache'); @@ -389,7 +389,7 @@ async function createUserDataDir(browserConfig: Config['browser']) { cacheDirectory = process.env.LOCALAPPDATA || path.join(os.homedir(), 'AppData', 'Local'); else throw new Error('Unsupported platform: ' + process.platform); - const result = path.join(cacheDirectory, 'ms-playwright', `mcp-${browserConfig?.launchOptions?.channel ?? browserConfig?.browserName}-profile`); + const result = path.join(cacheDirectory, 'ms-playwright', `mcp-${browserConfig.launchOptions?.channel ?? browserConfig?.browserName}-profile`); await fs.promises.mkdir(result, { recursive: true }); return result; } diff --git a/src/index.ts b/src/index.ts index a7169e3..2542616 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,9 +15,11 @@ */ import { Connection, createConnection as createConnectionImpl } from './connection.js'; +import { resolveConfig } from './config.js'; import type { Config } from '../config.js'; -export async function createConnection(config: Config = {}): Promise { +export async function createConnection(userConfig: Config = {}): Promise { + const config = await resolveConfig(userConfig); return createConnectionImpl(config); } diff --git a/src/program.ts b/src/program.ts index 6df68ad..f8e18f5 100644 --- a/src/program.ts +++ b/src/program.ts @@ -17,7 +17,7 @@ import { program } from 'commander'; import { startHttpTransport, startStdioTransport } from './transport.js'; -import { resolveConfig } from './config.js'; +import { resolveCLIConfig } from './config.js'; import type { Connection } from './connection.js'; import { packageJSON } from './context.js'; @@ -50,7 +50,7 @@ program .option('--viewport-size ', 'specify browser viewport size in pixels, for example "1280, 720"') .option('--vision', 'Run server that uses screenshots (Aria snapshots are used by default)') .action(async options => { - const config = await resolveConfig(options); + const config = await resolveCLIConfig(options); const connectionList: Connection[] = []; setupExitWatchdog(connectionList); diff --git a/src/transport.ts b/src/transport.ts index 8d38fc3..2ab7f03 100644 --- a/src/transport.ts +++ b/src/transport.ts @@ -24,16 +24,16 @@ import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' import { createConnection } from './connection.js'; -import type { Config } from '../config.js'; import type { Connection } from './connection.js'; +import type { FullConfig } from './config.js'; -export async function startStdioTransport(config: Config, connectionList: Connection[]) { +export async function startStdioTransport(config: FullConfig, connectionList: Connection[]) { const connection = await createConnection(config); await connection.connect(new StdioServerTransport()); connectionList.push(connection); } -async function handleSSE(config: Config, req: http.IncomingMessage, res: http.ServerResponse, url: URL, sessions: Map, connectionList: Connection[]) { +async function handleSSE(config: FullConfig, req: http.IncomingMessage, res: http.ServerResponse, url: URL, sessions: Map, connectionList: Connection[]) { if (req.method === 'POST') { const sessionId = url.searchParams.get('sessionId'); if (!sessionId) { @@ -68,7 +68,7 @@ async function handleSSE(config: Config, req: http.IncomingMessage, res: http.Se res.end('Method not allowed'); } -async function handleStreamable(config: Config, req: http.IncomingMessage, res: http.ServerResponse, sessions: Map, connectionList: Connection[]) { +async function handleStreamable(config: FullConfig, req: http.IncomingMessage, res: http.ServerResponse, sessions: Map, connectionList: Connection[]) { const sessionId = req.headers['mcp-session-id'] as string | undefined; if (sessionId) { const transport = sessions.get(sessionId); @@ -104,7 +104,7 @@ async function handleStreamable(config: Config, req: http.IncomingMessage, res: res.end('Invalid request'); } -export function startHttpTransport(config: Config, port: number, hostname: string | undefined, connectionList: Connection[]) { +export function startHttpTransport(config: FullConfig, port: number, hostname: string | undefined, connectionList: Connection[]) { const sseSessions = new Map(); const streamableSessions = new Map(); const httpServer = http.createServer(async (req, res) => { diff --git a/tests/pdf.spec.ts b/tests/pdf.spec.ts index 1e870e6..84d26c2 100644 --- a/tests/pdf.spec.ts +++ b/tests/pdf.spec.ts @@ -73,6 +73,7 @@ test('save as pdf (filename: output.pdf)', async ({ startClient, mcpBrowser, ser const files = [...fs.readdirSync(outputDir)]; expect(fs.existsSync(outputDir)).toBeTruthy(); - expect(files).toHaveLength(1); - expect(files[0]).toMatch(/^output.pdf$/); + const pdfFiles = files.filter(f => f.endsWith('.pdf')); + expect(pdfFiles).toHaveLength(1); + expect(pdfFiles[0]).toMatch(/^output.pdf$/); }); diff --git a/tests/screenshot.spec.ts b/tests/screenshot.spec.ts index 1909a8b..3f8c851 100644 --- a/tests/screenshot.spec.ts +++ b/tests/screenshot.spec.ts @@ -83,7 +83,9 @@ test('--output-dir should work', async ({ startClient, localOutputPath, server } }); expect(fs.existsSync(outputDir)).toBeTruthy(); - expect([...fs.readdirSync(outputDir)]).toHaveLength(1); + const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith('.jpeg')); + expect(files).toHaveLength(1); + expect(files[0]).toMatch(/^page-\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}Z\.jpeg$/); }); for (const raw of [undefined, true]) { @@ -117,7 +119,7 @@ for (const raw of [undefined, true]) { ], }); - const files = [...fs.readdirSync(outputDir)]; + const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith(`.${ext}`)); expect(fs.existsSync(outputDir)).toBeTruthy(); expect(files).toHaveLength(1); @@ -157,11 +159,11 @@ test('browser_take_screenshot (filename: "output.jpeg")', async ({ startClient, ], }); - const files = [...fs.readdirSync(outputDir)]; + const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith('.jpeg')); expect(fs.existsSync(outputDir)).toBeTruthy(); expect(files).toHaveLength(1); - expect(files[0]).toMatch(/^output.jpeg$/); + expect(files[0]).toMatch(/^output\.jpeg$/); }); test('browser_take_screenshot (noImageResponses)', async ({ startClient, server }) => {