diff --git a/src/config/browser-options.ts b/src/config/browser-options.ts index 2b9af492d..9a3bb7483 100644 --- a/src/config/browser-options.ts +++ b/src/config/browser-options.ts @@ -59,7 +59,6 @@ export const browserOptions = { type: 'string', description: 'Custom headers for WebSocket connection in JSON format (e.g., \'{"Authorization":"Bearer token"}\'). Only works with --wsEndpoint.', - implies: 'wsEndpoint', coerce: (val: string | undefined) => { if (!val) { return; @@ -91,7 +90,7 @@ export const browserOptions = { type: 'boolean', description: 'If specified, creates a temporary user-data-dir that is automatically cleaned up after the browser is closed. Defaults to false.', - defaultDescription: 'false', + default: false, }, userDataDir: { type: 'string', @@ -103,7 +102,7 @@ export const browserOptions = { description: 'Specify a different Chrome channel that should be used. The default is the stable channel version.', choices: ['canary', 'dev', 'beta', 'stable'] as const, - defaultDescription: 'stable', + default: 'stable' as const, }, proxyServer: { type: 'string', diff --git a/src/config/mcp-options.ts b/src/config/mcp-options.ts index 292bc7ad0..671cbebac 100644 --- a/src/config/mcp-options.ts +++ b/src/config/mcp-options.ts @@ -119,13 +119,11 @@ export const mcpOptions = { experimentalFfmpegPath: { type: 'string', describe: 'Path to ffmpeg executable for screencast recording.', - implies: 'experimentalScreencast', }, experimentalScreencastFps: { type: 'number', describe: 'Frames per second to use for screencast recording. Lower values can reduce memory pressure on pages that produce frames faster than ffmpeg can encode them.', - implies: 'experimentalScreencast', coerce: (value: number | undefined) => { if (value === undefined) { return; @@ -144,7 +142,7 @@ export const mcpOptions = { describe: "Restricts browser's network access by blocking specified URL patterns (uses https://urlpattern.spec.whatwg.org/). Silently detaches from targets with blocked URLs upon connection, and blocks runtime requests (including navigations and subresources). Accepts an array of patterns. A pattern that uses a regexp group in any component (for example `(127\\.\\d+\\.\\d+\\.\\d+)` in the hostname) is rejected, because it is not enforced on redirects or subresources; use an exact value or a `*`/`:name` wildcard instead.", coerce: (arg: string[] | undefined) => { - if (arg === undefined) { + if (arg === undefined || arg.length === 0) { return undefined; } const pattern = findUnenforceablePattern(arg); @@ -165,6 +163,11 @@ export const mcpOptions = { if (arg === undefined) { return undefined; } + if (arg.length === 0) { + throw new Error( + 'Invalid --allowedUrlPattern: at least one pattern is required.', + ); + } const pattern = findUnenforceablePattern(arg); if (pattern) { throw new Error( @@ -319,9 +322,20 @@ export const mcpOptions = { }, } satisfies Record; -export type ParsedArguments = ReturnType; +type RawParsedArguments = ReturnType< + ReturnType>['parseSync'] +>; + +export type ParsedArguments = { + [ + K in keyof RawParsedArguments as K extends '_' | '$0' ? never : K + ]: RawParsedArguments[K]; +}; -export function getMcpOptionsForViaCli(): typeof mcpOptions { +export function getMcpOptionsForViaCli(): Record< + keyof typeof mcpOptions, + YargsOptions +> { if (!('default' in mcpOptions.headless)) { throw new Error('headless cli option unexpectedly does not have a default'); } @@ -363,9 +377,8 @@ export function getMcpOptionsForViaCli(): typeof mcpOptions { export function getCliOptions(): Partial< Record > { - const options: Partial> = { - ...getMcpOptionsForViaCli(), - }; + const options: Partial> = + withoutDefaults(getMcpOptionsForViaCli()); // Missing CLI serialization. delete options.viewport; @@ -374,19 +387,6 @@ export function getCliOptions(): Partial< delete options.experimentalStructuredContent; delete options.experimentalInteropTools; - const recordOptions: Record = options; - for (const [key, option] of Object.entries(recordOptions)) { - if (option?.default !== undefined) { - const copy: YargsOptions = { - ...option, - defaultDescription: - option.defaultDescription ?? JSON.stringify(option.default), - }; - delete copy.default; - recordOptions[key] = copy; - } - } - return options; } @@ -465,6 +465,13 @@ const CONFLICTING_ARGS: Array> = [ ['categoryExtensions', 'browserUrl', 'wsEndpoint'], ]; +const IMPLICATIONS: Array<[keyof typeof mcpOptions, keyof typeof mcpOptions]> = + [ + ['wsHeaders', 'wsEndpoint'], + ['experimentalFfmpegPath', 'experimentalScreencast'], + ['experimentalScreencastFps', 'experimentalScreencast'], + ]; + function isPlainObject(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } @@ -473,18 +480,13 @@ function getErrorMessage(err: unknown): string { return err instanceof Error ? err.message : String(err); } -/** - * Exported only for testing to not trigger process exit. - */ -export function parser( +export function buildCliParser>( version: string, - argv = process.argv, - env = process.env, + argv: string[], + options: T, ) { - const isViaCli = argv.includes('--viaCli') || argv.includes('--via-cli'); - const options = isViaCli ? getMcpOptionsForViaCli() : mcpOptions; - - const yargsInstance = yargs(hideBin(argv)) + const yargsInstance = yargs(hideBin(argv)); + return yargsInstance .scriptName('npx chrome-devtools-mcp@latest') .parserConfiguration({ 'strip-aliased': true, @@ -492,126 +494,208 @@ export function parser( }) .options(options) .showHelpOnFail(false, 'Specify --help for available options') - .check(args => { - const activeArgs = new Set(); + .example(CLI_EXAMPLES) + .wrap(Math.min(120, yargsInstance.terminalWidth())) + .help() + .version(version); +} - for (const [key, val] of Object.entries(args)) { - if (val !== undefined && val !== false) { - activeArgs.add(key); - } - } +function withoutDefaults( + options: Record, +): Record { + const result: Record = {}; + for (const [key, option] of Object.entries(options)) { + const copy: YargsOptions = {...option}; + if (copy.default !== undefined) { + copy.defaultDescription ??= JSON.stringify(copy.default); + delete copy.default; + } + result[key] = copy; + } + return result; +} - for (const group of CONFLICTING_ARGS) { - // Find all active arguments within this conflict group - const activeInGroup = group.filter(arg => activeArgs.has(arg)); +function stripYargsPositionalArgs( + parsed: T, +): Omit { + const {_: _positionals, $0: _scriptName, ...rest} = parsed; + return rest; +} - if (activeInGroup.length > 1) { - const [arg1, arg2] = activeInGroup; - throw new Error( - `Arguments ${arg1} and ${arg2} are mutually exclusive`, - ); - } - } +/** + * Step 1: parses the CLI flags without applying defaults, so the result only + * contains what the user passed. `--help` and `--version` print and exit the + * process; parse errors throw. + */ +export function parseCliArgs( + version: string, + argv: string[], +): Partial { + const parsed = buildCliParser(version, argv, withoutDefaults(mcpOptions)) + .fail(false) + .parseSync(); + return stripYargsPositionalArgs(parsed); +} - return true; - }) - .middleware(args => { - if (isViaCli) { - if (args.filesystemRoot === DEFAULT_FILESYSTEM_ROOT) { - const cliFilesystemArgs: { - allowUnrestrictedPaths?: boolean; - filesystemRoot?: unknown; - } = args; - cliFilesystemArgs.allowUnrestrictedPaths = true; - cliFilesystemArgs.filesystemRoot = undefined; - } - // Defaults that cannot be set in options without affecting yargs conflict resolution. - const connectsToExistingBrowser = - args.autoConnect || args.browserUrl || args.wsEndpoint; - if ( - args.isolated === undefined && - args.userDataDir === undefined && - !connectsToExistingBrowser - ) { - args.isolated = true; - } - if ( - args.categoryExtensions === undefined && - !connectsToExistingBrowser - ) { - args.categoryExtensions = true; - } - } - // Only fall back to stable when Chrome is launched by channel. Leaving it - // unset otherwise keeps it out of telemetry (computeFlagUsage) for - // browserUrl, wsEndpoint and executablePath. - if ( - !args.channel && - !args.browserUrl && - !args.wsEndpoint && - !args.executablePath - ) { - args.channel = 'stable'; - } - if (env['CI'] || env['CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS']) { - console.error( - "turning off usage statistics. process.env['CI'] || process.env['CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS'] is set.", - ); - args.usageStatistics = false; - } +/** + * Step 2: reads the JSON config file and runs it through yargs to reject + * unknown keys and apply coercions, without applying defaults. + */ +export function parseConfigFile(configPath: string): Partial { + try { + const fileContent: unknown = JSON.parse(readFileSync(configPath, 'utf-8')); + if (!isPlainObject(fileContent)) { + throw new Error('Config must be a JSON object'); + } + const parsed = yargs([]) + .parserConfiguration({ + 'strip-aliased': true, + 'camel-case-expansion': false, + }) + .options(withoutDefaults(mcpOptions)) + .config(fileContent) + .strict() + .fail(false) + .exitProcess(false) + .parseSync([]); + return stripYargsPositionalArgs(parsed); + } catch (err) { + throw new Error(`Invalid JSON config file: ${getErrorMessage(err)}`); + } +} - const cliOptionsAllowedArgs = [ - ...Object.keys(options), - // Yargs populated with positional args - '_', - '$0', - ]; +function warnUnknownArgs(cliArgs: Partial): void { + const allowedArgs = new Set(Object.keys(mcpOptions)); + const unknownArgs = Object.keys(cliArgs).filter(arg => !allowedArgs.has(arg)); + if (unknownArgs.length > 0) { + console.error(`Unknown arguments: ${unknownArgs.map(arg => `--${arg}`)}`); + } +} - const unknownArgs = Object.keys(args).filter( - arg => !cliOptionsAllowedArgs.includes(arg), - ); +/** + * Step 4: rejects mutually exclusive inputs. Only explicit inputs are checked, + * so defaults never conflict. + */ +export function validateConflicts( + explicitArgs: Partial, +): void { + const activeArgs = new Set(); + for (const [key, val] of Object.entries(explicitArgs)) { + if (val !== undefined && val !== false) { + activeArgs.add(key); + } + } + for (const group of CONFLICTING_ARGS) { + const activeInGroup = group.filter(arg => activeArgs.has(arg)); + if (activeInGroup.length > 1) { + const [arg1, arg2] = activeInGroup; + throw new Error(`Arguments ${arg1} and ${arg2} are mutually exclusive`); + } + } +} - if (unknownArgs.length > 0) { - console.error( - `Unknown arguments: ${unknownArgs.map(arg => `--${arg}`)}`, - ); - } - }) - .example(CLI_EXAMPLES); +export function validateImplications( + explicitArgs: Partial, +): void { + for (const [key, implied] of IMPLICATIONS) { + const isKeySet = + explicitArgs[key] !== undefined && explicitArgs[key] !== false; + const isImpliedSet = + explicitArgs[implied] !== undefined && explicitArgs[implied] !== false; + if (isKeySet && !isImpliedSet) { + throw new Error(`Implications failed:\n ${key} -> ${implied}`); + } + } +} - return yargsInstance - .config('config', 'Path to JSON configuration file', configPath => { - try { - const parsed: unknown = JSON.parse(readFileSync(configPath, 'utf-8')); - if (!isPlainObject(parsed)) { - throw new Error('Config must be a JSON object'); - } - - yargs() - .parserConfiguration({ - 'strip-aliased': true, - 'camel-case-expansion': false, - }) - .options(options) - .config(parsed) - .strict() - .fail(false) - .exitProcess(false) - .parseSync([]); - return parsed; - } catch (err) { - throw new Error(`Invalid JSON config file: ${getErrorMessage(err)}`); - } - }) - .wrap(Math.min(120, yargsInstance.terminalWidth())) - .help() - .version(version); +/** + * Step 5: fills in defaults for everything that was not set explicitly. + * `viaCli` is an explicit input like any other; it selects which defaults + * apply. + */ +export function applyDefaults( + explicitArgs: Partial, + env: NodeJS.ProcessEnv, +): Partial { + const isViaCli = explicitArgs.viaCli === true; + const baseOptions = isViaCli ? getMcpOptionsForViaCli() : mcpOptions; + const resolvedArgs = {...explicitArgs}; + // `channel` only applies when Chrome is launched by channel. Leaving it + // unset otherwise keeps it out of telemetry (computeFlagUsage). + const launchesByChannel = + !explicitArgs.browserUrl && + !explicitArgs.wsEndpoint && + !explicitArgs.executablePath; + + for (const [key, option] of Object.entries(baseOptions)) { + if (key === 'channel' && !launchesByChannel) { + continue; + } + if (resolvedArgs[key] === undefined && 'default' in option) { + resolvedArgs[key] = option.default; + } + } + + if (isViaCli) { + if (resolvedArgs.filesystemRoot === DEFAULT_FILESYSTEM_ROOT) { + resolvedArgs.allowUnrestrictedPaths = true; + resolvedArgs.filesystemRoot = undefined; + } + const connectsToExistingBrowser = + resolvedArgs.autoConnect || + resolvedArgs.browserUrl || + resolvedArgs.wsEndpoint; + if ( + explicitArgs.isolated === undefined && + resolvedArgs.userDataDir === undefined && + !connectsToExistingBrowser + ) { + resolvedArgs.isolated = true; + } + if ( + resolvedArgs.categoryExtensions === undefined && + !connectsToExistingBrowser + ) { + resolvedArgs.categoryExtensions = true; + } + } + + if (env['CI'] || env['CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS']) { + console.error( + "turning off usage statistics. process.env['CI'] || process.env['CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS'] is set.", + ); + resolvedArgs.usageStatistics = false; + } + + return resolvedArgs; } export function parseArguments( version: string, argv = process.argv, env = process.env, -) { - return parser(version, argv, env).parseSync(); + exitProcess = true, +): ParsedArguments { + try { + const cliArgs = parseCliArgs(version, argv); + const configFileArgs = + typeof cliArgs.config === 'string' ? parseConfigFile(cliArgs.config) : {}; + // Step 3: merges the explicit inputs. The CLI wins over the config file. + const explicitArgs = {...configFileArgs, ...cliArgs}; + warnUnknownArgs(cliArgs); + validateConflicts(explicitArgs); + validateImplications(explicitArgs); + const resolvedArgs = applyDefaults(explicitArgs, env); + + // The merge and the default loop lose the static type that yargs infers from + // `mcpOptions`. Every value was produced by the same option definitions (CLI + // parser, strict config-file parser, option defaults), so the shape matches. + return resolvedArgs as ParsedArguments; + } catch (error) { + if (exitProcess) { + console.error(getErrorMessage(error)); + process.exit(1); + } + throw error; + } } diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 7d63b7bf4..41e960333 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -11,19 +11,18 @@ import {describe, it} from 'node:test'; import {buildCommand} from '../src/config/cli-commands.js'; import {commands} from '../src/config/cli-options.js'; import { + buildCliParser, DEFAULT_FILESYSTEM_ROOT, getCliOptions, mcpOptions, - parser, + parseArguments as parseArgumentsImpl, } from '../src/config/mcp-options.js'; import {computeFlagUsage} from '../src/telemetry/flagUtils.js'; import {createTempFile} from './utils.js'; function parseArguments(argv: string[], env: NodeJS.ProcessEnv = {}) { - return parser('0.0.0', ['node', 'main.js', ...argv], env) - .exitProcess(false) - .parseSync(); + return parseArgumentsImpl('0.0.0', ['node', 'main.js', ...argv], env, false); } describe('cli args parsing', () => { @@ -37,6 +36,7 @@ describe('cli args parsing', () => { categoryMemory: true, autoConnect: false, headless: false, + isolated: false, acceptInsecureCerts: false, performanceCrux: true, usageStatistics: true, @@ -65,8 +65,6 @@ describe('cli args parsing', () => { const args = parseArguments([]); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', channel: 'stable', }); }); @@ -86,8 +84,6 @@ describe('cli args parsing', () => { const args = parseArguments(['--browserUrl', 'http://localhost:3000']); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', browserUrl: 'http://localhost:3000', }); }); @@ -116,8 +112,6 @@ describe('cli args parsing', () => { const args = parseArguments(['--user-data-dir', '/tmp/chrome-profile']); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', channel: 'stable', userDataDir: '/tmp/chrome-profile', }); @@ -127,8 +121,6 @@ describe('cli args parsing', () => { const args = parseArguments(['--browserUrl', ''], {}); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', channel: 'stable', browserUrl: undefined, }); @@ -138,8 +130,6 @@ describe('cli args parsing', () => { const args = parseArguments(['--executablePath', '/tmp/test 123/chrome']); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', executablePath: '/tmp/test 123/chrome', }); }); @@ -148,8 +138,6 @@ describe('cli args parsing', () => { const args = parseArguments(['--viewport', '888x777']); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', channel: 'stable', viewport: { width: 888, @@ -165,8 +153,6 @@ describe('cli args parsing', () => { ]); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', channel: 'stable', chromeArg: ['--no-sandbox', '--disable-setuid-sandbox'], }); @@ -219,8 +205,6 @@ describe('cli args parsing', () => { ]); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', channel: 'stable', ignoreDefaultChromeArg: [ '--disable-extensions', @@ -236,8 +220,6 @@ describe('cli args parsing', () => { ]); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', wsEndpoint: 'ws://127.0.0.1:9222/devtools/browser/abc123', }); }); @@ -249,8 +231,6 @@ describe('cli args parsing', () => { ]); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', wsEndpoint: 'wss://example.com:9222/devtools/browser/abc123', }); }); @@ -272,8 +252,6 @@ describe('cli args parsing', () => { const args = parseArguments(['--no-category-emulation']); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', channel: 'stable', categoryEmulation: false, }); @@ -282,8 +260,6 @@ describe('cli args parsing', () => { const args = parseArguments(['--auto-connect'], {}); assert.deepStrictEqual(args, { ...defaultArgs, - _: [], - $0: 'npx chrome-devtools-mcp@latest', channel: 'stable', autoConnect: true, }); @@ -470,6 +446,44 @@ describe('cli args parsing', () => { ); }); + it('strips an empty blocked-url-pattern', async () => { + const args = parseArguments(['--blocked-url-pattern']); + assert.strictEqual(args.blockedUrlPattern, undefined); + }); + + it('allows an empty config blockedUrlPattern with allowed-url-pattern', async () => { + using testConfig = createTempFile( + JSON.stringify({blockedUrlPattern: []}), + 'cd4a.test.config.empty-blocked.json', + ); + const args = parseArguments([ + '--config', + testConfig.path, + '--allowed-url-pattern', + 'https://a.com/*', + ]); + assert.strictEqual(args.blockedUrlPattern, undefined); + assert.deepStrictEqual(args.allowedUrlPattern, ['https://a.com/*']); + }); + + it('rejects an empty allowed-url-pattern', async () => { + assert.throws( + () => parseArguments(['--allowed-url-pattern']), + /Invalid --allowedUrlPattern: at least one pattern is required/, + ); + }); + + it('rejects an empty config allowedUrlPattern', async () => { + using testConfig = createTempFile( + JSON.stringify({allowedUrlPattern: []}), + 'cd4a.test.config.empty-allowed.json', + ); + assert.throws( + () => parseArguments(['--config', testConfig.path]), + /Invalid JSON config file: Invalid --allowedUrlPattern: at least one pattern is required/, + ); + }); + it('parses source-maps flag', async () => { const defaultParsed = parseArguments(['main.js']); assert.strictEqual(defaultParsed.sourceMaps, true); @@ -561,7 +575,7 @@ describe('cli args parsing', () => { const args = parseArguments(['--viaCli', '--config', relativePath]); assert.strictEqual(args.config, testConfig.path); assert.strictEqual(args.userDataDir, '/tmp/custom-profile'); - assert.strictEqual(args.isolated, undefined); + assert.strictEqual(args.isolated, false); assert.strictEqual(args.headless, false); }); @@ -604,11 +618,145 @@ describe('cli args parsing', () => { ); }); + it('rejects a config file with malformed JSON', async () => { + using testConfig = createTempFile( + '{"headless": true,', + 'cd4a.test.config.malformed.json', + ); + assert.throws( + () => parseArguments(['--config', testConfig.path]), + /Invalid JSON config file:/, + ); + }); + + it('rejects a config file that is not a JSON object', async () => { + using testConfig = createTempFile( + JSON.stringify(['--headless']), + 'cd4a.test.config.array.json', + ); + assert.throws( + () => parseArguments(['--config', testConfig.path]), + /Invalid JSON config file: Config must be a JSON object/, + ); + }); + + it('rejects a missing config file', async () => { + assert.throws( + () => parseArguments(['--config', 'cd4a.test.config.missing.json']), + /Invalid JSON config file: ENOENT/, + ); + }); + + it('replaces config arrays with cli arrays instead of merging', async () => { + using testConfig = createTempFile( + JSON.stringify({chromeArg: ['--a']}), + 'cd4a.test.config.array-replace.json', + ); + const args = parseArguments([ + '--config', + testConfig.path, + '--chrome-arg=--b', + ]); + assert.deepStrictEqual(args.chromeArg, ['--b']); + }); + + it('lets the CI env disable usage statistics enabled in config', async () => { + using testConfig = createTempFile( + JSON.stringify({usageStatistics: true}), + 'cd4a.test.config.usage-statistics.json', + ); + const args = parseArguments(['--config', testConfig.path], {CI: 'true'}); + assert.strictEqual(args.usageStatistics, false); + }); + + it('lets explicit cli flags override viaCli dynamic defaults', async () => { + const args = parseArguments(['--viaCli', '--no-headless']); + assert.strictEqual(args.headless, false); + }); + + describe('viaCli defaults', () => { + for (const flag of ['--viaCli=true', '--via-cli=true']) { + it(`applies viaCli defaults for ${flag}`, async () => { + const args = parseArguments([flag]); + assert.strictEqual(args.viaCli, true); + assert.strictEqual(args.headless, true); + assert.strictEqual(args.isolated, true); + assert.strictEqual(args.memoryDebugging, true); + }); + } + + it('does not apply viaCli defaults for --viaCli false', async () => { + const args = parseArguments(['--viaCli', 'false']); + assert.strictEqual(args.viaCli, false); + assert.strictEqual(args.headless, false); + assert.strictEqual(args.isolated, false); + assert.strictEqual(args.memoryDebugging, false); + }); + + it('applies viaCli defaults when viaCli is set in the config file', async () => { + using testConfig = createTempFile( + JSON.stringify({viaCli: true}), + 'cd4a.test.config.via-cli.json', + ); + const args = parseArguments(['--config', testConfig.path]); + assert.strictEqual(args.viaCli, true); + assert.strictEqual(args.headless, true); + assert.strictEqual(args.isolated, true); + }); + + it('lets cli viaCli=false override config viaCli', async () => { + using testConfig = createTempFile( + JSON.stringify({viaCli: true}), + 'cd4a.test.config.via-cli-override.json', + ); + const args = parseArguments([ + '--config', + testConfig.path, + '--viaCli=false', + ]); + assert.strictEqual(args.viaCli, false); + assert.strictEqual(args.headless, false); + assert.strictEqual(args.isolated, false); + }); + + for (const connectArgs of [ + ['--auto-connect'], + ['--browserUrl', 'http://localhost:9222'], + ['--wsEndpoint', 'ws://localhost:9222'], + ['--user-data-dir', '/tmp/chrome-profile'], + ]) { + it(`does not default to isolated with ${connectArgs[0]}`, async () => { + const args = parseArguments(['--viaCli', ...connectArgs]); + assert.strictEqual(args.isolated, false); + }); + } + + it('does not default to isolated with autoConnect from the config file', async () => { + using testConfig = createTempFile( + JSON.stringify({autoConnect: true}), + 'cd4a.test.config.via-cli-auto-connect.json', + ); + const args = parseArguments(['--viaCli', '--config', testConfig.path]); + assert.strictEqual(args.autoConnect, true); + assert.strictEqual(args.isolated, false); + }); + }); + it('parses with devtoolsComments enabled', async () => { const args = parseArguments(['--devtoolsComments']); assert.strictEqual(args.devtoolsComments, true); }); + it('includes usage examples in help output', async () => { + const help = await buildCliParser( + '0.0.0', + ['node', 'main.js'], + mcpOptions, + ).getHelp(); + assert.match(help, /Examples:/); + assert.match(help, /--browserUrl http:\/\/127\.0\.0\.1:9222/); + }); + it('clears default values and populates defaultDescription in getCliOptions', () => { const cliOptions = getCliOptions(); @@ -832,6 +980,73 @@ describe('cli args parsing', () => { ); }); + it('rejects cli channel with config-based browserUrl', async () => { + using testConfig = createTempFile( + JSON.stringify({browserUrl: 'http://localhost:9222'}), + 'cd4a.test.config.browser-url.json', + ); + assert.throws( + () => + parseArguments(['--config', testConfig.path, '--channel', 'canary']), + /Arguments channel and browserUrl are mutually exclusive/, + ); + }); + + it('rejects conflicting arguments within a config file', async () => { + using testConfig = createTempFile( + JSON.stringify({ + browserUrl: 'http://localhost:9222', + wsEndpoint: 'ws://localhost:9222', + }), + 'cd4a.test.config.conflict.json', + ); + assert.throws( + () => parseArguments(['--config', testConfig.path]), + /Arguments browserUrl and wsEndpoint are mutually exclusive/, + ); + }); + + it('allows explicitly disabled isolated with userDataDir', async () => { + const args = parseArguments([ + '--isolated=false', + '--user-data-dir', + '/tmp/chrome-profile', + ]); + assert.strictEqual(args.isolated, false); + assert.strictEqual(args.userDataDir, '/tmp/chrome-profile'); + }); + + it('allows a config value set to false alongside a conflicting arg', async () => { + using testConfig = createTempFile( + JSON.stringify({autoConnect: false}), + 'cd4a.test.config.false-no-conflict.json', + ); + const args = parseArguments([ + '--config', + testConfig.path, + '--executablePath', + '/bin/chrome', + ]); + assert.strictEqual(args.autoConnect, false); + assert.strictEqual(args.executablePath, '/bin/chrome'); + }); + + it('lets a cli false override a conflicting config value', async () => { + using testConfig = createTempFile( + JSON.stringify({autoConnect: true}), + 'cd4a.test.config.false-override.json', + ); + const args = parseArguments([ + '--config', + testConfig.path, + '--autoConnect=false', + '--executablePath', + '/bin/chrome', + ]); + assert.strictEqual(args.autoConnect, false); + assert.strictEqual(args.executablePath, '/bin/chrome'); + }); + it('rejects categoryExtensions with autoConnect', async () => { assert.throws( () => parseArguments(['--category-extensions', '--auto-connect']), @@ -882,7 +1097,7 @@ describe('cli args parsing', () => { it(`allows viaCli with ${connectArgs[0]} without enabling extensions`, async () => { const args = parseArguments(['--viaCli', ...connectArgs]); assert.strictEqual(args.categoryExtensions, undefined); - assert.strictEqual(args.isolated, undefined); + assert.strictEqual(args.isolated, false); }); } @@ -975,8 +1190,8 @@ describe('cli command strings', () => { describe('flag usage telemetry', () => { it('reports the stable channel for a default launch', async () => { const usage = computeFlagUsage(parseArguments([]), mcpOptions); - assert.strictEqual(usage.isolated_present, false); - assert.strictEqual(usage.channel_present, true); + assert.strictEqual(usage.isolated_present, undefined); + assert.strictEqual(usage.channel_present, undefined); assert.strictEqual(usage.channel, 'CHANNEL_STABLE'); }); @@ -987,7 +1202,7 @@ describe('flag usage telemetry', () => { ]) { it(`does not report channel with ${connectArgs[0]}`, async () => { const usage = computeFlagUsage(parseArguments(connectArgs), mcpOptions); - assert.strictEqual(usage.isolated_present, false); + assert.strictEqual(usage.isolated_present, undefined); assert.strictEqual(usage.channel_present, false); assert.strictEqual(usage.channel, undefined); }); diff --git a/tests/config/mcp-options.test.ts b/tests/config/mcp-options.test.ts new file mode 100644 index 000000000..8b043edc0 --- /dev/null +++ b/tests/config/mcp-options.test.ts @@ -0,0 +1,174 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'node:assert'; +import {afterEach, describe, it} from 'node:test'; + +import sinon from 'sinon'; + +import { + applyDefaults, + DEFAULT_FILESYSTEM_ROOT, + parseCliArgs, + parseConfigFile, + validateConflicts, + validateImplications, +} from '../../src/config/mcp-options.js'; +import {createTempFile} from '../utils.js'; + +describe('mcp-options steps', () => { + afterEach(() => sinon.restore()); + + describe('parseCliArgs', () => { + it('returns only explicitly passed flags', () => { + const args = parseCliArgs('0.0.0', ['node', 'main.js', '--headless']); + assert.deepStrictEqual(args, { + headless: true, + }); + }); + }); + + describe('parseConfigFile', () => { + it('returns only the keys from the file', () => { + using configFile = createTempFile( + JSON.stringify({ + headless: true, + blockedUrlPattern: ['https://a.com/*'], + }), + 'cd4a.steps.config.json', + ); + const args = parseConfigFile(configFile.path); + assert.strictEqual(args.headless, true); + assert.deepStrictEqual(args.blockedUrlPattern, ['https://a.com/*']); + assert.strictEqual(args.isolated, undefined); + assert.strictEqual(args.channel, undefined); + }); + + it('rejects unknown keys', () => { + using configFile = createTempFile( + JSON.stringify({notAnOption: true}), + 'cd4a.steps.config.unknown.json', + ); + assert.throws( + () => parseConfigFile(configFile.path), + /Invalid JSON config file: .*notAnOption/, + ); + }); + + it('rejects non-object JSON', () => { + using configFile = createTempFile( + JSON.stringify([]), + 'cd4a.steps.config.array.json', + ); + assert.throws( + () => parseConfigFile(configFile.path), + /Invalid JSON config file: Config must be a JSON object/, + ); + }); + }); + + describe('validateConflicts', () => { + it('accepts a single argument from a conflict group', () => { + validateConflicts({browserUrl: 'http://localhost:9222'}); + }); + + it('ignores false and undefined values', () => { + validateConflicts({ + browserUrl: 'http://localhost:9222', + wsEndpoint: undefined, + categoryExtensions: false, + }); + }); + + it('rejects two arguments from the same group', () => { + assert.throws( + () => + validateConflicts({ + browserUrl: 'http://localhost:9222', + channel: 'canary', + }), + /Arguments channel and browserUrl are mutually exclusive/, + ); + }); + + describe('validateImplications', () => { + it('accepts when implying key is not set', () => { + validateImplications({wsEndpoint: 'ws://localhost:9222'}); + }); + + it('accepts when both implying and implied keys are set', () => { + validateImplications({ + wsHeaders: {Auth: 'token'}, + wsEndpoint: 'ws://localhost:9222', + }); + }); + + it('rejects when implying key is set but implied key is missing', () => { + assert.throws( + () => validateImplications({wsHeaders: {Auth: 'token'}}), + /Implications failed:\n {2}wsHeaders -> wsEndpoint/, + ); + }); + + it('rejects when implying key is set but implied key is negated', () => { + assert.throws( + () => + validateImplications({ + experimentalFfmpegPath: '/bin/ffmpeg', + experimentalScreencast: false, + }), + /Implications failed:\n {2}experimentalFfmpegPath -> experimentalScreencast/, + ); + }); + }); + }); + describe('applyDefaults', () => { + it('keeps explicit values and fills in defaults', () => { + const args = applyDefaults({headless: true}, {}); + assert.strictEqual(args.headless, true); + assert.strictEqual(args.isolated, false); + assert.strictEqual(args.channel, 'stable'); + assert.strictEqual(args.categoryExtensions, undefined); + }); + + for (const explicitArgs of [ + {browserUrl: 'http://localhost:9222'}, + {wsEndpoint: 'ws://localhost:9222'}, + {executablePath: '/tmp/chrome'}, + ]) { + it(`does not default channel with ${Object.keys(explicitArgs)[0]}`, () => { + assert.strictEqual(applyDefaults(explicitArgs, {}).channel, undefined); + }); + } + + it('applies viaCli defaults when launching a browser', () => { + const args = applyDefaults( + {viaCli: true, filesystemRoot: DEFAULT_FILESYSTEM_ROOT}, + {}, + ); + assert.strictEqual(args.headless, true); + assert.strictEqual(args.isolated, true); + assert.strictEqual(args.categoryExtensions, true); + assert.strictEqual(args.allowUnrestrictedPaths, true); + assert.strictEqual(args.filesystemRoot, undefined); + }); + + it('does not enable isolated or extensions for viaCli with browserUrl', () => { + const args = applyDefaults( + {viaCli: true, browserUrl: 'http://localhost:9222'}, + {}, + ); + assert.strictEqual(args.isolated, false); + assert.strictEqual(args.categoryExtensions, undefined); + }); + + it('turns off usage statistics in CI', () => { + sinon.stub(console, 'error'); + const args = applyDefaults({usageStatistics: true}, {CI: 'true'}); + assert.strictEqual(args.usageStatistics, false); + }); + }); +}); diff --git a/tests/mocks.ts b/tests/mocks.ts index 4c09435e2..a009898de 100644 --- a/tests/mocks.ts +++ b/tests/mocks.ts @@ -25,7 +25,10 @@ import type {Frame} from 'puppeteer-core'; import sinon from 'sinon'; -import {type ParsedArguments, parser} from '../src/config/mcp-options.js'; +import { + type ParsedArguments, + parseArguments, +} from '../src/config/mcp-options.js'; import {McpContext} from '../src/McpContext.js'; import {McpPage} from '../src/McpPage.js'; import {McpResponse} from '../src/McpResponse.js'; @@ -1064,7 +1067,12 @@ export function createMockContextAnalysisResult(): DevTools.HeapSnapshotModel.He export function createMockParsedArguments( options: Partial = {}, ): ParsedArguments { - const defaultArgs = parser('0.0.0', ['node', 'main.js']).parseSync(); + const defaultArgs = parseArguments( + '0.0.0', + ['node', 'main.js'], + process.env, + false, + ); return {...defaultArgs, ...options}; }