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.

Author: Lightning00Blade

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',

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,7 +324,9 @@ export const mcpOptions = {
   },
 } satisfies Record<string, YargsOptions>;
 
-export type ParsedArguments = ReturnType<typeof parseArguments>;
+export type ParsedArguments = ReturnType<
+  ReturnType<typeof parser>['parseSync']
+>;
 
 export function getMcpOptionsForViaCli(): typeof mcpOptions {
   if (!('default' in mcpOptions.headless)) {
@@ -466,140 +473,165 @@ function isPlainObject(value: unknown): value is Record<string, unknown> {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
 }
 
+function isParsedArguments(value: unknown): value is ParsedArguments {
+  return isPlainObject(value) && Array.isArray(value['_']);
+}
+
 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<T extends Record<string, YargsOptions>>(
   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,
       'strip-dashed': true,
     })
     .options(options)
     .showHelpOnFail(false, 'Specify --help for available options')
-    .check(args => {
-      const activeArgs = new Set<string>();
-
-      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 => {
-      args.channel = args.channel ?? 'stable';
-      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.
-        if (
-          args.isolated === undefined &&
-          args.userDataDir === undefined &&
-          !args.autoConnect &&
-          !args.browserUrl &&
-          !args.wsEndpoint
-        ) {
-          args.isolated = true;
-        }
-      }
-      args.isolated = args.isolated ?? false;
-      if (
-        args.experimentalToonFormat &&
-        args.experimentalDataFormat === 'default'
-      ) {
-        args.experimentalDataFormat = 'toon';
-      }
-      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<string, YargsOptions> = {};
+  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<string, unknown> = yargsInstance.parseSync();
+
+  // Config file parsing
+  let parsedConfigFile: Record<string, unknown> = {};
+  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};
+
+  for (const [key, option] of Object.entries(baseOptions)) {
+    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;
+    }
+    if (
+      explicitConfig.isolated === undefined &&
+      resolvedConfig.userDataDir === undefined &&
+      !resolvedConfig.autoConnect &&
+      !resolvedConfig.browserUrl &&
+      !resolvedConfig.wsEndpoint
+    ) {
+      resolvedConfig.isolated = true;
+    }
+  }
+
+  if (
+    resolvedConfig.experimentalToonFormat &&
+    resolvedConfig.experimentalDataFormat === 'default'
+  ) {
+    resolvedConfig.experimentalDataFormat = 'toon';
+  }
+  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<string>();
+  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`);
+    }
+  }
+
+  if (!isParsedArguments(resolvedConfig)) {
+    throw new Error('Failed to resolve configuration');
+  }
+  return resolvedConfig;
 }