From 9249a20da7f97b28aea5030b636d40ccd005b7d6 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 25 Sep 2026 15:41:39 +0000 Subject: [PATCH 1/2] feat: resolve config defaults after merging CLI and config file Parse CLI arguments and the JSON config file without yargs defaults so only explicit user intent is captured, merge them with CLI taking precedence, then apply defaults (including the --viaCli ones). Conflict checks now only consider explicitly provided arguments, so implicit defaults such as channel=stable can no longer conflict with explicit flags. Also restores --help/--version exiting via a shared buildCliParser and adds edge-case tests for config parsing, precedence and conflicts. --- src/config/browser-options.ts | 4 +- src/config/mcp-options.ts | 283 +++++++++++++++++++--------------- tests/cli.test.ts | 251 +++++++++++++++++++++++++++++- tests/mocks.ts | 7 +- 4 files changed, 408 insertions(+), 137 deletions(-) diff --git a/src/config/browser-options.ts b/src/config/browser-options.ts index 2b9af492d..d824301d3 100644 --- a/src/config/browser-options.ts +++ b/src/config/browser-options.ts @@ -91,7 +91,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 +103,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..e4fda1203 100644 --- a/src/config/mcp-options.ts +++ b/src/config/mcp-options.ts @@ -144,7 +144,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 +165,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 +324,14 @@ export const mcpOptions = { }, } satisfies Record; -export type ParsedArguments = ReturnType; +export type ParsedArguments = ReturnType< + ReturnType['parseSync'] +>; -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'); } @@ -473,18 +483,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( +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 +497,154 @@ export function parser( }) .options(options) .showHelpOnFail(false, 'Specify --help for available options') - .check(args => { - const activeArgs = new Set(); - - for (const [key, val] of Object.entries(args)) { - if (val !== undefined && val !== false) { - activeArgs.add(key); - } - } - - for (const group of CONFLICTING_ARGS) { - // Find all active arguments within this conflict group - 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`, - ); - } - } - - 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; - } - - const cliOptionsAllowedArgs = [ - ...Object.keys(options), - // Yargs populated with positional args - '_', - '$0', - ]; - - const unknownArgs = Object.keys(args).filter( - arg => !cliOptionsAllowedArgs.includes(arg), - ); - - if (unknownArgs.length > 0) { - console.error( - `Unknown arguments: ${unknownArgs.map(arg => `--${arg}`)}`, - ); - } - }) - .example(CLI_EXAMPLES); - - 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)}`); - } - }) + .example(CLI_EXAMPLES) .wrap(Math.min(120, yargsInstance.terminalWidth())) .help() .version(version); } +export function parser(version: string, argv = process.argv) { + // Used to derive the ParsedArguments type. + return buildCliParser(version, argv, mcpOptions); +} + export function parseArguments( version: string, argv = process.argv, env = process.env, ) { - return parser(version, argv, env).parseSync(); + // Step 1 & 2: Independent Parsing (Bypassing yargs Defaults) + const optionsWithoutDefaults: Record = {}; + for (const [key, option] of Object.entries(mcpOptions)) { + const copy: YargsOptions = {...option}; + if (copy.default !== undefined) { + copy.defaultDescription ??= JSON.stringify(copy.default); + delete copy.default; + } + optionsWithoutDefaults[key] = copy; + } + + // `--help` and `--version` print and exit the process; parse errors throw. + const yargsInstance = buildCliParser( + version, + argv, + optionsWithoutDefaults, + ).fail(false); + + const rawCli: Record = yargsInstance.parseSync(); + + // Config file parsing + let parsedConfigFile: Record = {}; + if (typeof rawCli.config === 'string') { + try { + const fileContent: unknown = JSON.parse( + readFileSync(rawCli.config, 'utf-8'), + ); + if (!isPlainObject(fileContent)) { + throw new Error('Config must be a JSON object'); + } + + // Run the config through yargs to reject unknown keys and apply + // coercions, but without defaults. + parsedConfigFile = yargs([]) + .parserConfiguration({ + 'strip-aliased': true, + 'camel-case-expansion': false, + }) + .options(optionsWithoutDefaults) + .config(fileContent) + .strict() + .fail(false) + .exitProcess(false) + .parseSync([]); + } catch (err) { + throw new Error(`Invalid JSON config file: ${getErrorMessage(err)}`); + } + } + + // Step 3: Cascade Merge of Explicit Inputs + const explicitConfig = {...parsedConfigFile, ...rawCli}; + + // Step 4: Dynamic Default Resolution. `viaCli` is an explicit input like any + // other; it selects which defaults apply. + const isViaCli = explicitConfig.viaCli === true; + const baseOptions = isViaCli ? getMcpOptionsForViaCli() : mcpOptions; + const resolvedConfig = {...explicitConfig}; + // `channel` only applies when Chrome is launched by channel. Leaving it + // unset otherwise keeps it out of telemetry (computeFlagUsage). + const launchesByChannel = + !explicitConfig.browserUrl && + !explicitConfig.wsEndpoint && + !explicitConfig.executablePath; + + for (const [key, option] of Object.entries(baseOptions)) { + if (key === 'channel' && !launchesByChannel) { + continue; + } + if (resolvedConfig[key] === undefined && 'default' in option) { + resolvedConfig[key] = option.default; + } + } + + if (isViaCli) { + if (resolvedConfig.filesystemRoot === DEFAULT_FILESYSTEM_ROOT) { + resolvedConfig.allowUnrestrictedPaths = true; + resolvedConfig.filesystemRoot = undefined; + } + const connectsToExistingBrowser = + resolvedConfig.autoConnect || + resolvedConfig.browserUrl || + resolvedConfig.wsEndpoint; + if ( + explicitConfig.isolated === undefined && + resolvedConfig.userDataDir === undefined && + !connectsToExistingBrowser + ) { + resolvedConfig.isolated = true; + } + if ( + resolvedConfig.categoryExtensions === undefined && + !connectsToExistingBrowser + ) { + resolvedConfig.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.", + ); + resolvedConfig.usageStatistics = false; + } + + const cliOptionsAllowedArgs = [...Object.keys(baseOptions), '_', '$0']; + + const unknownArgs = Object.keys(rawCli).filter( + arg => !cliOptionsAllowedArgs.includes(arg), + ); + + if (unknownArgs.length > 0) { + console.error(`Unknown arguments: ${unknownArgs.map(arg => `--${arg}`)}`); + } + + // Step 5: Validation. Conflicts are only checked against explicit user input + // so that dynamic defaults never conflict. + const activeArgs = new Set(); + for (const [key, val] of Object.entries(explicitConfig)) { + 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`); + } + } + + // 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 resolvedConfig as ParsedArguments; } diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 7d63b7bf4..1be9fdaf2 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -14,6 +14,7 @@ import { DEFAULT_FILESYSTEM_ROOT, getCliOptions, mcpOptions, + parseArguments as parseArgumentsImpl, parser, } from '../src/config/mcp-options.js'; import {computeFlagUsage} from '../src/telemetry/flagUtils.js'; @@ -21,9 +22,7 @@ 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); } 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, @@ -470,6 +470,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 +599,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 +642,141 @@ 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 parser('0.0.0', ['node', 'main.js']).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 +1000,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 +1117,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 +1210,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 +1222,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/mocks.ts b/tests/mocks.ts index 08b97ac86..3a332c5ba 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'; @@ -972,7 +975,7 @@ 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']); return {...defaultArgs, ...options}; } From 915b23b2620b7d3ec7c90a6d1c451b1479f04eb8 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Mon, 28 Sep 2026 08:16:09 +0000 Subject: [PATCH 2/2] refactor: validate resolved config with a zod schema Move the mutually exclusive argument check into a zod superRefine so schema validation is the single final gatekeeper for the resolved configuration. No behavior change. --- src/config/mcp-options.ts | 48 +++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/config/mcp-options.ts b/src/config/mcp-options.ts index e4fda1203..90be59606 100644 --- a/src/config/mcp-options.ts +++ b/src/config/mcp-options.ts @@ -5,7 +5,7 @@ */ import type {YargsOptions} from '../third_party/index.js'; -import {yargs, hideBin} from '../third_party/index.js'; +import {yargs, hideBin, zod as z} from '../third_party/index.js'; import os from 'node:os'; import {readFileSync} from 'node:fs'; import path from 'node:path'; @@ -627,24 +627,38 @@ export function parseArguments( console.error(`Unknown arguments: ${unknownArgs.map(arg => `--${arg}`)}`); } - // Step 5: Validation. Conflicts are only checked against explicit user input - // so that dynamic defaults never conflict. - const activeArgs = new Set(); - for (const [key, val] of Object.entries(explicitConfig)) { - 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`); - } - } + // Step 5: Final Schema Validation & Type Safety. Conflicts are only checked + // against explicit user input so that dynamic defaults never conflict. + const ConfigSchema = z + .object({}) + .passthrough() + .superRefine((_config, ctx) => { + const activeArgs = new Set(); + for (const [key, val] of Object.entries(explicitConfig)) { + 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; + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `Arguments ${arg1} and ${arg2} are mutually exclusive`, + path: [arg1, arg2], + }); + } + } + }); + const result = ConfigSchema.safeParse(resolvedConfig); + if (!result.success) { + throw new Error(result.error.issues[0].message); + } // 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 resolvedConfig as ParsedArguments; + return result.data as ParsedArguments; }