Skip to content

feat: resolve config defaults after merging CLI and config file - #2849

Merged
Lightning00Blade merged 3 commits into
mainfrom
cd4a-2-late-defaults
Sep 29, 2026
Merged

Lightning00Blade merged 3 commits into
mainfrom
cd4a-2-late-defaults

Conversation

@Lightning00Blade

Copy link
Copy Markdown
Collaborator

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.


Stack created with GitHub Stacks CLI • Give Feedback 💬

@Lightning00Blade
Lightning00Blade added this pull request to stack #2851 September 28, 2026 08:33
@Lightning00Blade
Lightning00Blade force-pushed the cd4a-2-late-defaults branch 3 times, most recently from e17e547 to 93637ef Compare September 28, 2026 12:43
@Lightning00Blade
Lightning00Blade force-pushed the cd4a-2-late-defaults branch 3 times, most recently from 129749b to a1e6200 Compare September 28, 2026 14:53
@Lightning00Blade
Lightning00Blade force-pushed the cd4a-2-late-defaults branch 2 times, most recently from 9249a20 to 1be1667 Compare September 28, 2026 20:58
Base automatically changed from cd4a-1-parser-refactor to main September 29, 2026 08:45
@Lightning00Blade
Lightning00Blade force-pushed the cd4a-2-late-defaults branch 2 times, most recently from 40253b7 to 69f5c32 Compare September 29, 2026 11:13
@Lightning00Blade
Lightning00Blade marked this pull request as ready for review September 29, 2026 11:16

@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.

let's consider the following:

  • In src/config/mcp-options.ts (stripYargsPositionalArgs and ParsedArguments), remove the (parsed as {_?: unknown}) and (parsed as {$0?: unknown}) casts (for example by using delete parsed._; delete parsed.$0; or destructuring) and omit '_' | '$0' from ParsedArguments. AGENTS.md forbids as type casting, Partial<ParsedArguments> already defines _ and $0, and keeping them on ParsedArguments misrepresents the runtime object after stripYargsPositionalArgs deletes them.
  • In src/config/mcp-options.ts (parseCliArgs, parseConfigFile, and withoutDefaults), strip implies from the per-source yargs passes and validate implications (wsHeaders -> wsEndpoint, experimentalFfmpegPath -> experimentalScreencast, experimentalScreencastFps -> experimentalScreencast) on the merged explicitArgs. Evaluating implies on cliArgs and configFileArgs in isolation rejects valid split configurations (e.g., wsEndpoint in the config file with --wsHeaders on the CLI) and misses cases where the implied flag is set in the config file but negated on the CLI (--no-experimental-screencast).
  • In src/config/mcp-options.ts (getCliOptions), reuse withoutDefaults(getMcpOptionsForViaCli()) before deleting the CLI-excluded options instead of duplicating the loop that populates defaultDescription and deletes default. This keeps the default-stripping logic in a single place.

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.
Comment thread src/config/mcp-options.ts
@Lightning00Blade
Lightning00Blade added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main with commit bdeb70d Sep 29, 2026
23 checks passed
@Lightning00Blade
Lightning00Blade deleted the cd4a-2-late-defaults branch September 29, 2026 13:18
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