Skip to content

refactor: centralize config defaults and conflict checks - #2753

Merged
Lightning00Blade merged 1 commit into
mainfrom
move-deps-around
Sep 29, 2026
Merged

Lightning00Blade merged 1 commit into
mainfrom
move-deps-around

Conversation

@Lightning00Blade

@Lightning00Blade Lightning00Blade commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

This extracts the conflicting config argument to a separate check, to allow both defaults and conflict on the same option.

@Lightning00Blade Lightning00Blade changed the title Move deps around refactor: how we deal with config Sep 15, 2026
@Lightning00Blade
Lightning00Blade force-pushed the move-deps-around branch 4 times, most recently from be04f2b to abf3135 Compare September 28, 2026 08:33
@Lightning00Blade
Lightning00Blade added this pull request to stack #2851 September 28, 2026 08:33
@Lightning00Blade Lightning00Blade changed the title refactor: how we deal with config refactor: centralize config defaults and conflict checks Sep 28, 2026
@Lightning00Blade
Lightning00Blade force-pushed the move-deps-around branch 2 times, most recently from 305f86e to 04d56ea Compare September 28, 2026 13:29
@Lightning00Blade
Lightning00Blade marked this pull request as ready for review September 28, 2026 13:30

@OrKoN OrKoN left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the following:

  • In src/config/mcp-options.ts (getMcpOptionsForViaCli and parser), apply the --viaCli default for categoryExtensions in .middleware() when !args.autoConnect && !args.browserUrl && !args.wsEndpoint (matching isolated) and update defaultDescription on isolated in getMcpOptionsForViaCli(). Keeping default: true on categoryExtensions in getMcpOptionsForViaCli() causes .check() to throw a mutual-exclusivity error whenever --viaCli (chrome-devtools start) is used with --browserUrl, --wsEndpoint, or --auto-connect, and spreading ...mcpOptions.isolated leaks defaultDescription: 'false' into CLI help.
  • In src/config/browser-options.ts, src/config/mcp-options.ts (parser middleware), and src/telemetry/flagUtils.ts (computeFlagUsage), ensure isolated and channel defaults are recognized by computeFlagUsage and do not set args.channel = 'stable' when browserUrl, wsEndpoint, or executablePath is provided. Because browserOptions.isolated and browserOptions.channel only set defaultDescription (!('default' in config)), unconditionally setting args.isolated = false and args.channel = 'stable' in middleware causes computeFlagUsage to log isolated_present: true on every run and channel_present: true / channel: 'CHANNEL_STABLE' even when connecting via browserUrl/wsEndpoint or launching via executablePath.
  • In src/config/mcp-options.ts (parser middleware) and src/ToolHandler.ts, resolve the legacy experimentalToonFormat fallback in ToolHandler (or before applying the 'default' default) instead of checking args.experimentalDataFormat === 'default' in middleware. Checking args.experimentalDataFormat === 'default' after Yargs applies default: 'default' causes --experimentalToonFormat to override an explicit --experimentalDataFormat=default and causes computeFlagUsage to report experimental_data_format_present: true when only --experimentalToonFormat was passed.

@Lightning00Blade
Lightning00Blade force-pushed the move-deps-around branch 2 times, most recently from 2dfb780 to 7a68a30 Compare September 28, 2026 15:09
paviad added a commit to paviad/chrome-devtools-mcp that referenced this pull request Oct 9, 2026
Remove the categoryExtensions conflicts with autoConnect, browserUrl and
wsEndpoint added in ChromeDevTools#2753. Chrome 149+ supports extension debugging over
these connections, and 1.10.1 accepts the combination.

Update the --categoryExtensions help text, generated docs and the
troubleshooting skill to state the Chrome 149+ requirement for attach modes.

Fixes ChromeDevTools#2989
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants