Read CLI options from testconfig.json via IConfiguration (#6349)

[Amaury Levé (Evangelink)]

Summary

Resolves part of #6349: let users specify CLI options inside testconfig.json by routing ICommandLineOptions through IConfiguration.

Approach

1. New CommandLineConfigurationSource (Order = 0) wraps the parsed CLI into an IConfigurationProvider that flattens each option under commandLineOptions:<name> (zero-arity) or commandLineOptions:<name>:<index> (arg-bearing).
2. The CLI source is registered in TestHostBuilder.CommonServicesbefore the JSON source, so CLI keeps highest precedence.
3. AggregatedConfiguration.TryGetCommandLineOptionFromProviders does a provider-aware lookup: it walks providers in registration order, and the first provider with any data for that option wins outright (no per-key cross-provider merging). This avoids cases where IConfiguration[key] accidentally returns the JSON :0 while the CLI set the zero-arity bare key, or vice versa.
4. The public extension helpers (IsCommandLineOptionSet, TryGetCommandLineOptionArguments) delegate to that method when given an AggregatedConfiguration, and CommandLineHandler becomes a thin facade that does the same. End result: every consumer of ICommandLineOptions transparently sees JSON-sourced options.
5. JsonConfigurationProvider exposes a typed, schema-validated enumeration of commandLineOptions:* entries. CommandLineOptionsValidator runs three extra passes over those entries (unknown-option, arity, per-arg validation) so testconfig.json typos surface during startup instead of crashing inside option handlers.
6. DisplayBannerIfEnabledAsync reads --no-banner from the unified ICommandLineOptions, so banner suppression honors testconfig.json as well as the CLI.

What works

• testconfig.json keys under commandLineOptions are surfaced everywhere ICommandLineOptions is consumed.
• CLI still wins over JSON for the same option.
• --results-directory defined in JSON is honored (was previously ignored - the legacy GetResultsDirectoryCore path was bypassing JSON).
• --no-banner defined in JSON suppresses the banner.
• Invalid JSON entries (unknown option, wrong arity, bad arg) fail at startup with a clear error referencing testconfig.json, instead of crashing later. Concrete cases now caught:
  • --timeout: true (was: IndexOutOfRangeException in args[0])
  • --exit-on-process-exit: "abc" (was: FormatException in int.Parse(args[0]))
  • typos like "timeoutt": "30s" (was: silently ignored)
• Back-compat ctor on CommandLineHandler is kept so external code keeps compiling.

Rough edges still on the table

These are not addressed in this PR:

1. Bootstrap readers.--diagnostic* and --config-file are read off parseResultbeforeIConfiguration is built. Honoring them from JSON requires either a two-phase config build or rewriting those bootstrap paths.
2. Scalar bool ambiguity. For arg-bearing options, JSON "true" / "false" is ambiguous with the zero-arity presence marker. Callers must use the array form ("foo": ["true"]) for non-boolean string values that happen to be "true" / "false". No code enforces this today.
3. API visibility. New helpers are kept internal for now. If we ship this we need to decide whether they become public surface.

Tests

• CommandLineConfigurationProviderTests covers the source/provider flattening + the provider-aware invariants (including ProviderAwareResolution_CliZeroArityShadowsJsonIndexedArgs end-to-end).
• CommandLineConfigurationExtensionsTests covers the helper-level and handler-end-to-end behavior.
• JsonCommandLineOptionsTests (new) covers the JSON enumeration schema, the three new validator passes, disabled entries, shadowing, and case-insensitivity.
• Full UT project green on net8.0 / net9.0 / net462.

[Copilot]

Pull request overview

This prototype routes command-line options through the platform IConfiguration model so testconfig.json can provide values consumed by existing ICommandLineOptions users, while preserving CLI precedence.

Changes:

• Adds a CLI-backed configuration source/provider under commandLineOptions:*.
• Adds provider-aware command-line option lookup helpers on AggregatedConfiguration/ConfigurationExtensions.
• Wires CommandLineHandler to use the unified configuration view and adds unit coverage for precedence and lookup behavior.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Registers the CLI configuration source and passes built configuration into command-line handling.
src/Platform/Microsoft.Testing.Platform/Configurations/PlatformConfigurationConstants.cs	Adds the commandLineOptions section constant.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationExtensions.cs	Adds unified command-line option lookup helpers.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationSource.cs	Adds the configuration source for parsed CLI options.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationProvider.cs	Flattens parsed CLI options into configuration keys.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs	Adds provider-aware option resolution and uses it for results-directory.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs	Documents current validation gaps for JSON-sourced options.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineManager.cs	Passes configuration into CommandLineHandler.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineHandler.cs	Delegates option reads to configuration when available.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationProviderTests.cs	Adds tests for CLI provider flattening and precedence behavior.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationExtensionsTests.cs	Adds tests for helper and handler behavior over configuration-backed options.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Round-2 review summary

All four round-1 fixes are correctly applied and the build is clean (0 warnings, 0 errors; 1034 pass / 0 fail). Two minor correctness observations and three test-hygiene NITs left inline. Nothing blocking.

Fix validation

Fix	Status
Case-insensitive duplicate detection (3 dicts + ToDictionary)	✅ Applied correctly. One remaining edge case at validator.cs:62 — see inline.
FormatException catch around EnumerateJsonCommandLineOptions	✅ Mirrors the standard failure path, no double-banner risk. One asymmetry re HasTool — see inline.
[DoesNotReturn] on the two throw helpers	✅ Both helpers carry the attribute.
Rewritten XML doc on TryGetCommandLineOptionArguments	✅ No more stale "validation on parseResult only" wording.

New test adequacy

Test	Verdict
EnumerateCommandLineOptions_SectionNameCaseInsensitive_Honored	✅ Strong assertions (option name + argument value).
Validator_EmptyOptionNameInJson_FailsWithJsonPrefix	⚠️ Only checks "testconfig.json" substring — wouldn't catch a resource string change. Advisory.
Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully	⚠️ Test comment misrepresents pre-fix behavior; assertion only checks IsValid == false. See inline.
Validator_JsonSparseIndexedEntry_DefensiveSchemaRejection	⚠️ Test name doesn't match what's tested. See inline.

Public API hygiene

✅ PublicAPI.Unshipped.txt only has #nullable enable — all new types (JsonCommandLineOptionEntry, CommandLineConfigurationSource, CommandLineConfigurationProvider) and methods (EnumerateJsonCommandLineOptions, TryGetCommandLineOptionArguments, IsCommandLineOptionSet) are correctly internal.
✅ JsonCommandLineOptionEntry uses plain get; properties — no init accessors on any new API.
✅ New internalCommandLineHandler constructor accepting IConfiguration? preserves source/binary compatibility (the existing public constructor delegates with configuration: null).

Summary table

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 LOW (system+system dictionary, FormatException + HasTool)
13	Test Completeness & Coverage	⚪ 1 NIT (weaker substring assertions on 3 new tests)
16	Naming & Conventions	⚪ 1 NIT (Validator_JsonSparseIndexedEntry_* name)
17	Documentation Accuracy	⚪ 1 NIT (test comment misrepresents pre-fix behavior)

✅ 17/21 dimensions clean.

Threading & Concurrency, Security & IPC, Public API & Binary Compatibility, Performance & Allocations, Cross-TFM Compatibility, Resource & IDisposable, Defensive Coding at Boundaries, Localization, Test Isolation, Assertion Quality, Flakiness Patterns, Data-Driven Coverage, Code Structure, Analyzer Quality (N/A), IPC Wire Compatibility (N/A), Build Infrastructure, Scope Discipline — all clean.

Noted for follow-up (out-of-scope for this PR)

• Env-var injection of commandLineOptions:* keys: now that IsCommandLineOptionSet/TryGetCommandLineOptionArguments route through IConfiguration, an env var spelled commandLineOptions__timeout=30s (with __→: normalization in EnvironmentVariablesConfigurationProvider) will silently shadow JSON values and be invisible to CommandLineOptionsValidator (which only sees CLI + JSON entries). This is an existing platform concern that the unification only exposes — not introduced here — but worth tracking.
• Behavioral break for extension authors who intentionally registered case-differing option names (e.g. "Timeout" for one provider, "timeout" for another). They were previously accepted as distinct (silently mis-treated as one by every downstream case-insensitive lookup); they now fail validation up front. This is the correct fix, but worth a CHANGELOG entry if not already planned.

[Amaury Levé (Evangelink)]

Round 3 review — bcb797345

✅ 21/21 dimensions clean — no blocking, medium, or low findings.

Round-2 fixes validation

Fix #1 — ValidateOptionsAreNotDuplicated extended to cover system providers: CORRECT and complete.

I traced every collision shape through the new pass and confirmed no legitimate existing behavior changes:

• Extension-vs-extension (case-differing). Previously crashed with ArgumentException at the providerAndOptionByOptionName.ToDictionary(..., StringComparer.OrdinalIgnoreCase) lookup on line 60-62. Now caught with the friendly "declared by multiple" error. ✅
• Extension-vs-system (any casing). Already caught earlier by ValidateExtensionOptionsDoNotContainReservedOptions (line 107-152), which uses OrdinalIgnoreCase and returns early. The new Concat-over-both-dictionaries iteration is reachable in theory for this case but practically pre-empted. ✅
• System-vs-system (case-differing). Previously crashed at the same ToDictionary lookup. Now caught with the friendly error. ✅ (Pinned by the new Validator_DuplicateOptionNamesAcrossSystemAndSystem_FailsGracefully test.)

I considered whether two providers could legitimately register the same option name (e.g. a system provider exposing a public alias also added by an extension). The contract is that each ICommandLineOptionsProvider.GetCommandLineOptions() returns options it owns, and the pre-existing ValidateExtensionOptionsDoNotContainReservedOptions pass already treats extension-vs-system collisions as errors (any casing). So no legitimate scenario is regressed.

Minor cosmetic observation (NIT, not blocking): line 60 still uses LINQ Union over the two dictionaries, while the new code at line 165 uses Concat. Both produce the same result here (provider keys are reference-equal across the two source dictionaries, and there are no duplicates by reference), but slight stylistic inconsistency. Not worth a follow-up.

---

Fix #2 — FormatException exception filter + rich error display: CORRECT and complete.

• The when (!loggingState.CommandLineParseResult.HasTool) exception filter is standard C# 6+ and works on all four TFMs.
• The silent-degrade-to-empty-list path for tools is consistent with the existing pipeline convention: validation errors are already gated by !HasTool on line 230, so tools that have a structurally-broken commandLineOptions section already wouldn't see validation surface the issue. The FormatException fallback mirrors this.
• I verified HasTool semantics: ParseResult.HasTool => ToolName is not null. --help and --info are regular options handled via IsHelpInvoked / IsInfoInvoked on CommandLineHandler (lines 240-247), not tools. So --help and --info go through the normal !HasTool path and do surface FormatException with the rich InvalidCommandLineArguments header.
• Only server-mode tools (those that set ToolName via the tool entry, e.g. --server) silently degrade. Even for these, the rest of testconfig.json (e.g. results-directory, custom IConfiguration[key] lookups) remains accessible because the throw fires only during the typed-schema enumeration of the commandLineOptions section — direct provider.TryGet calls bypass it.
• The rich-error formatting (StringBuilder + PlatformResources.InvalidCommandLineArguments header + "\t- {ex.Message}" + TrimEnd()) matches CommandLineOptionsValidator (line 23-28). Environment.NewLine is correctly avoided (confirmed via grep). StringBuilder resolves via the global System.Text using in Directory.Build.props.
• DisplayBannerIfEnabledAsync call inside the catch (line 205) safely reads --no-banner via commandLineOptions.IsOptionSet, which routes through provider.TryGet on the already-flattened key-value store, not through EnumerateCommandLineOptions. No re-throw risk.

---

Fresh-pass net-new findings

Nothing blocking, medium, or low. Three honest NIT-level observations, all noted-for-follow-up only:

1. NIT (style) — CommandLineOptionsValidator.cs line 60 uses Union where Concat would be slightly clearer and marginally cheaper (the new code at line 165 already uses Concat).
2. NIT (test redundancy) — Validator_JsonEntryWithTooFewArguments_FailsArityCheck (line 454) overlaps with Validator_JsonArityTooFew_Fails (line 200); only the arity shape differs (ArgumentArity(2,2) vs ExactlyOne). Defensible as defensive coverage of a non-trivial arity.
3. NIT (docs precision) — The comment on Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully (lines 410-415) cites line 62, which will become stale on future edits to the validator. Consider replacing with a symbolic reference ("the OrdinalIgnoreCaseToDictionary lookup").

---

Other dimensions checked clean

• Public API & init accessors — All new types/members are internal. No PublicAPI.Unshipped.txt churn. ✅
• Cross-TFM — Exception filters (C# 6+), StringBuilder, CultureInfo, OrdinalIgnoreCase all available on net462 / netstandard2.0 / net8.0 / net9.0. ✅
• Localization — Two new resx strings (JsonCommandLineOptionsEntryMustBeScalarOrArrayErrorMessage, JsonCommandLineOptionsValidationErrorPrefix) with <comment> metadata. .xlf deltas look auto-generated (stub-per-resource pattern, +10 per language file). No manual xlf edits. ✅
• Threading — All affected code paths run on the single-threaded host startup pipeline. No new shared mutable state. ✅
• IPC wire compatibility — N/A; no serialized type changes. ✅
• Resource management — N/A; no new disposables. ✅
• Defensive coding — Trust boundary (user testconfig.json) correctly guarded; internal invariants (e.g. sparse-indexed defensive guard in JsonConfigurationProvider) correctly preserved. ✅
• Test isolation — Tests use isolated TestProvider instances per [TestMethod]; no shared static mutable state. ✅
• Flakiness — No Thread.Sleep, no wall-clock assertions, no hard-coded ports in new tests. ✅

---

Verdict

Ready to merge. Round-2 fixes are correct, complete, and free of new bugs. The three NIT observations above are stylistic/documentation polish and do not affect users. Items explicitly deferred in the PR description (scalar-bool ambiguity, --diagnostic*/--config-file bootstrap reads, IsCommandLineOptionSet API visibility) remain out of scope for this PR as agreed.

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 31/31 changed files
• Comments generated: 0 new

[Amaury Levé (Evangelink)]

Summary

Resolves part of #6349: let users specify CLI options inside testconfig.json by routing ICommandLineOptions through IConfiguration.

Approach

1. New CommandLineConfigurationSource (Order = 0) wraps the parsed CLI into an IConfigurationProvider that flattens each option under commandLineOptions:<name> (zero-arity) or commandLineOptions:<name>:<index> (arg-bearing).
2. The CLI source is registered in TestHostBuilder.CommonServicesbefore the JSON source, so CLI keeps highest precedence.
3. AggregatedConfiguration.TryGetCommandLineOptionFromProviders does a provider-aware lookup: it walks providers in registration order, and the first provider with any data for that option wins outright (no per-key cross-provider merging). This avoids cases where IConfiguration[key] accidentally returns the JSON :0 while the CLI set the zero-arity bare key, or vice versa.
4. The public extension helpers (IsCommandLineOptionSet, TryGetCommandLineOptionArguments) delegate to that method when given an AggregatedConfiguration, and CommandLineHandler becomes a thin facade that does the same. End result: every consumer of ICommandLineOptions transparently sees JSON-sourced options.
5. JsonConfigurationProvider exposes a typed, schema-validated enumeration of commandLineOptions:* entries. CommandLineOptionsValidator runs three extra passes over those entries (unknown-option, arity, per-arg validation) so testconfig.json typos surface during startup instead of crashing inside option handlers.
6. DisplayBannerIfEnabledAsync reads --no-banner from the unified ICommandLineOptions, so banner suppression honors testconfig.json as well as the CLI.

What works

• testconfig.json keys under commandLineOptions are surfaced everywhere ICommandLineOptions is consumed.
• CLI still wins over JSON for the same option.
• --results-directory defined in JSON is honored (was previously ignored - the legacy GetResultsDirectoryCore path was bypassing JSON).
• --no-banner defined in JSON suppresses the banner.
• Invalid JSON entries (unknown option, wrong arity, bad arg) fail at startup with a clear error referencing testconfig.json, instead of crashing later. Concrete cases now caught:
  • --timeout: true (was: IndexOutOfRangeException in args[0])
  • --exit-on-process-exit: "abc" (was: FormatException in int.Parse(args[0]))
  • typos like "timeoutt": "30s" (was: silently ignored)
• Back-compat ctor on CommandLineHandler is kept so external code keeps compiling.

Rough edges still on the table

These are not addressed in this PR:

1. Bootstrap readers.--diagnostic* and --config-file are read off parseResultbeforeIConfiguration is built. Honoring them from JSON requires either a two-phase config build or rewriting those bootstrap paths.
2. Scalar bool ambiguity. For arg-bearing options, JSON "true" / "false" is ambiguous with the zero-arity presence marker. Callers must use the array form ("foo": ["true"]) for non-boolean string values that happen to be "true" / "false". No code enforces this today.
3. API visibility. New helpers are kept internal for now. If we ship this we need to decide whether they become public surface.

Tests

• CommandLineConfigurationProviderTests covers the source/provider flattening + the provider-aware invariants (including ProviderAwareResolution_CliZeroArityShadowsJsonIndexedArgs end-to-end).
• CommandLineConfigurationExtensionsTests covers the helper-level and handler-end-to-end behavior.
• JsonCommandLineOptionsTests (new) covers the JSON enumeration schema, the three new validator passes, disabled entries, shadowing, and case-insensitivity.
• Full UT project green on net8.0 / net9.0 / net462.

[Copilot]

Pull request overview

This prototype routes command-line options through the platform IConfiguration model so testconfig.json can provide values consumed by existing ICommandLineOptions users, while preserving CLI precedence.

Changes:

• Adds a CLI-backed configuration source/provider under commandLineOptions:*.
• Adds provider-aware command-line option lookup helpers on AggregatedConfiguration/ConfigurationExtensions.
• Wires CommandLineHandler to use the unified configuration view and adds unit coverage for precedence and lookup behavior.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Registers the CLI configuration source and passes built configuration into command-line handling.
src/Platform/Microsoft.Testing.Platform/Configurations/PlatformConfigurationConstants.cs	Adds the commandLineOptions section constant.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationExtensions.cs	Adds unified command-line option lookup helpers.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationSource.cs	Adds the configuration source for parsed CLI options.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationProvider.cs	Flattens parsed CLI options into configuration keys.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs	Adds provider-aware option resolution and uses it for results-directory.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs	Documents current validation gaps for JSON-sourced options.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineManager.cs	Passes configuration into CommandLineHandler.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineHandler.cs	Delegates option reads to configuration when available.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationProviderTests.cs	Adds tests for CLI provider flattening and precedence behavior.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationExtensionsTests.cs	Adds tests for helper and handler behavior over configuration-backed options.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Round-2 review summary

All four round-1 fixes are correctly applied and the build is clean (0 warnings, 0 errors; 1034 pass / 0 fail). Two minor correctness observations and three test-hygiene NITs left inline. Nothing blocking.

Fix validation

Fix	Status
Case-insensitive duplicate detection (3 dicts + ToDictionary)	✅ Applied correctly. One remaining edge case at validator.cs:62 — see inline.
FormatException catch around EnumerateJsonCommandLineOptions	✅ Mirrors the standard failure path, no double-banner risk. One asymmetry re HasTool — see inline.
[DoesNotReturn] on the two throw helpers	✅ Both helpers carry the attribute.
Rewritten XML doc on TryGetCommandLineOptionArguments	✅ No more stale "validation on parseResult only" wording.

New test adequacy

Test	Verdict
EnumerateCommandLineOptions_SectionNameCaseInsensitive_Honored	✅ Strong assertions (option name + argument value).
Validator_EmptyOptionNameInJson_FailsWithJsonPrefix	⚠️ Only checks "testconfig.json" substring — wouldn't catch a resource string change. Advisory.
Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully	⚠️ Test comment misrepresents pre-fix behavior; assertion only checks IsValid == false. See inline.
Validator_JsonSparseIndexedEntry_DefensiveSchemaRejection	⚠️ Test name doesn't match what's tested. See inline.

Public API hygiene

✅ PublicAPI.Unshipped.txt only has #nullable enable — all new types (JsonCommandLineOptionEntry, CommandLineConfigurationSource, CommandLineConfigurationProvider) and methods (EnumerateJsonCommandLineOptions, TryGetCommandLineOptionArguments, IsCommandLineOptionSet) are correctly internal.
✅ JsonCommandLineOptionEntry uses plain get; properties — no init accessors on any new API.
✅ New internalCommandLineHandler constructor accepting IConfiguration? preserves source/binary compatibility (the existing public constructor delegates with configuration: null).

Summary table

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 LOW (system+system dictionary, FormatException + HasTool)
13	Test Completeness & Coverage	⚪ 1 NIT (weaker substring assertions on 3 new tests)
16	Naming & Conventions	⚪ 1 NIT (Validator_JsonSparseIndexedEntry_* name)
17	Documentation Accuracy	⚪ 1 NIT (test comment misrepresents pre-fix behavior)

✅ 17/21 dimensions clean.

Threading & Concurrency, Security & IPC, Public API & Binary Compatibility, Performance & Allocations, Cross-TFM Compatibility, Resource & IDisposable, Defensive Coding at Boundaries, Localization, Test Isolation, Assertion Quality, Flakiness Patterns, Data-Driven Coverage, Code Structure, Analyzer Quality (N/A), IPC Wire Compatibility (N/A), Build Infrastructure, Scope Discipline — all clean.

Noted for follow-up (out-of-scope for this PR)

• Env-var injection of commandLineOptions:* keys: now that IsCommandLineOptionSet/TryGetCommandLineOptionArguments route through IConfiguration, an env var spelled commandLineOptions__timeout=30s (with __→: normalization in EnvironmentVariablesConfigurationProvider) will silently shadow JSON values and be invisible to CommandLineOptionsValidator (which only sees CLI + JSON entries). This is an existing platform concern that the unification only exposes — not introduced here — but worth tracking.
• Behavioral break for extension authors who intentionally registered case-differing option names (e.g. "Timeout" for one provider, "timeout" for another). They were previously accepted as distinct (silently mis-treated as one by every downstream case-insensitive lookup); they now fail validation up front. This is the correct fix, but worth a CHANGELOG entry if not already planned.

[Amaury Levé (Evangelink)]

Round 3 review — bcb797345

✅ 21/21 dimensions clean — no blocking, medium, or low findings.

Round-2 fixes validation

Fix #1 — ValidateOptionsAreNotDuplicated extended to cover system providers: CORRECT and complete.

I traced every collision shape through the new pass and confirmed no legitimate existing behavior changes:

• Extension-vs-extension (case-differing). Previously crashed with ArgumentException at the providerAndOptionByOptionName.ToDictionary(..., StringComparer.OrdinalIgnoreCase) lookup on line 60-62. Now caught with the friendly "declared by multiple" error. ✅
• Extension-vs-system (any casing). Already caught earlier by ValidateExtensionOptionsDoNotContainReservedOptions (line 107-152), which uses OrdinalIgnoreCase and returns early. The new Concat-over-both-dictionaries iteration is reachable in theory for this case but practically pre-empted. ✅
• System-vs-system (case-differing). Previously crashed at the same ToDictionary lookup. Now caught with the friendly error. ✅ (Pinned by the new Validator_DuplicateOptionNamesAcrossSystemAndSystem_FailsGracefully test.)

I considered whether two providers could legitimately register the same option name (e.g. a system provider exposing a public alias also added by an extension). The contract is that each ICommandLineOptionsProvider.GetCommandLineOptions() returns options it owns, and the pre-existing ValidateExtensionOptionsDoNotContainReservedOptions pass already treats extension-vs-system collisions as errors (any casing). So no legitimate scenario is regressed.

Minor cosmetic observation (NIT, not blocking): line 60 still uses LINQ Union over the two dictionaries, while the new code at line 165 uses Concat. Both produce the same result here (provider keys are reference-equal across the two source dictionaries, and there are no duplicates by reference), but slight stylistic inconsistency. Not worth a follow-up.

---

Fix #2 — FormatException exception filter + rich error display: CORRECT and complete.

• The when (!loggingState.CommandLineParseResult.HasTool) exception filter is standard C# 6+ and works on all four TFMs.
• The silent-degrade-to-empty-list path for tools is consistent with the existing pipeline convention: validation errors are already gated by !HasTool on line 230, so tools that have a structurally-broken commandLineOptions section already wouldn't see validation surface the issue. The FormatException fallback mirrors this.
• I verified HasTool semantics: ParseResult.HasTool => ToolName is not null. --help and --info are regular options handled via IsHelpInvoked / IsInfoInvoked on CommandLineHandler (lines 240-247), not tools. So --help and --info go through the normal !HasTool path and do surface FormatException with the rich InvalidCommandLineArguments header.
• Only server-mode tools (those that set ToolName via the tool entry, e.g. --server) silently degrade. Even for these, the rest of testconfig.json (e.g. results-directory, custom IConfiguration[key] lookups) remains accessible because the throw fires only during the typed-schema enumeration of the commandLineOptions section — direct provider.TryGet calls bypass it.
• The rich-error formatting (StringBuilder + PlatformResources.InvalidCommandLineArguments header + "\t- {ex.Message}" + TrimEnd()) matches CommandLineOptionsValidator (line 23-28). Environment.NewLine is correctly avoided (confirmed via grep). StringBuilder resolves via the global System.Text using in Directory.Build.props.
• DisplayBannerIfEnabledAsync call inside the catch (line 205) safely reads --no-banner via commandLineOptions.IsOptionSet, which routes through provider.TryGet on the already-flattened key-value store, not through EnumerateCommandLineOptions. No re-throw risk.

---

Fresh-pass net-new findings

Nothing blocking, medium, or low. Three honest NIT-level observations, all noted-for-follow-up only:

1. NIT (style) — CommandLineOptionsValidator.cs line 60 uses Union where Concat would be slightly clearer and marginally cheaper (the new code at line 165 already uses Concat).
2. NIT (test redundancy) — Validator_JsonEntryWithTooFewArguments_FailsArityCheck (line 454) overlaps with Validator_JsonArityTooFew_Fails (line 200); only the arity shape differs (ArgumentArity(2,2) vs ExactlyOne). Defensible as defensive coverage of a non-trivial arity.
3. NIT (docs precision) — The comment on Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully (lines 410-415) cites line 62, which will become stale on future edits to the validator. Consider replacing with a symbolic reference ("the OrdinalIgnoreCaseToDictionary lookup").

---

Other dimensions checked clean

• Public API & init accessors — All new types/members are internal. No PublicAPI.Unshipped.txt churn. ✅
• Cross-TFM — Exception filters (C# 6+), StringBuilder, CultureInfo, OrdinalIgnoreCase all available on net462 / netstandard2.0 / net8.0 / net9.0. ✅
• Localization — Two new resx strings (JsonCommandLineOptionsEntryMustBeScalarOrArrayErrorMessage, JsonCommandLineOptionsValidationErrorPrefix) with <comment> metadata. .xlf deltas look auto-generated (stub-per-resource pattern, +10 per language file). No manual xlf edits. ✅
• Threading — All affected code paths run on the single-threaded host startup pipeline. No new shared mutable state. ✅
• IPC wire compatibility — N/A; no serialized type changes. ✅
• Resource management — N/A; no new disposables. ✅
• Defensive coding — Trust boundary (user testconfig.json) correctly guarded; internal invariants (e.g. sparse-indexed defensive guard in JsonConfigurationProvider) correctly preserved. ✅
• Test isolation — Tests use isolated TestProvider instances per [TestMethod]; no shared static mutable state. ✅
• Flakiness — No Thread.Sleep, no wall-clock assertions, no hard-coded ports in new tests. ✅

---

Verdict

Ready to merge. Round-2 fixes are correct, complete, and free of new bugs. The three NIT observations above are stylistic/documentation polish and do not affect users. Items explicitly deferred in the PR description (scalar-bool ambiguity, --diagnostic*/--config-file bootstrap reads, IsCommandLineOptionSet API visibility) remain out of scope for this PR as agreed.

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 31/31 changed files
• Comments generated: 0 new

[Amaury Levé (Evangelink)]

Summary

Resolves part of #6349: let users specify CLI options inside testconfig.json by routing ICommandLineOptions through IConfiguration.

Approach

1. New CommandLineConfigurationSource (Order = 0) wraps the parsed CLI into an IConfigurationProvider that flattens each option under commandLineOptions:<name> (zero-arity) or commandLineOptions:<name>:<index> (arg-bearing).
2. The CLI source is registered in TestHostBuilder.CommonServicesbefore the JSON source, so CLI keeps highest precedence.
3. AggregatedConfiguration.TryGetCommandLineOptionFromProviders does a provider-aware lookup: it walks providers in registration order, and the first provider with any data for that option wins outright (no per-key cross-provider merging). This avoids cases where IConfiguration[key] accidentally returns the JSON :0 while the CLI set the zero-arity bare key, or vice versa.
4. The public extension helpers (IsCommandLineOptionSet, TryGetCommandLineOptionArguments) delegate to that method when given an AggregatedConfiguration, and CommandLineHandler becomes a thin facade that does the same. End result: every consumer of ICommandLineOptions transparently sees JSON-sourced options.
5. JsonConfigurationProvider exposes a typed, schema-validated enumeration of commandLineOptions:* entries. CommandLineOptionsValidator runs three extra passes over those entries (unknown-option, arity, per-arg validation) so testconfig.json typos surface during startup instead of crashing inside option handlers.
6. DisplayBannerIfEnabledAsync reads --no-banner from the unified ICommandLineOptions, so banner suppression honors testconfig.json as well as the CLI.

What works

• testconfig.json keys under commandLineOptions are surfaced everywhere ICommandLineOptions is consumed.
• CLI still wins over JSON for the same option.
• --results-directory defined in JSON is honored (was previously ignored - the legacy GetResultsDirectoryCore path was bypassing JSON).
• --no-banner defined in JSON suppresses the banner.
• Invalid JSON entries (unknown option, wrong arity, bad arg) fail at startup with a clear error referencing testconfig.json, instead of crashing later. Concrete cases now caught:
  • --timeout: true (was: IndexOutOfRangeException in args[0])
  • --exit-on-process-exit: "abc" (was: FormatException in int.Parse(args[0]))
  • typos like "timeoutt": "30s" (was: silently ignored)
• Back-compat ctor on CommandLineHandler is kept so external code keeps compiling.

Rough edges still on the table

These are not addressed in this PR:

1. Bootstrap readers.--diagnostic* and --config-file are read off parseResultbeforeIConfiguration is built. Honoring them from JSON requires either a two-phase config build or rewriting those bootstrap paths.
2. Scalar bool ambiguity. For arg-bearing options, JSON "true" / "false" is ambiguous with the zero-arity presence marker. Callers must use the array form ("foo": ["true"]) for non-boolean string values that happen to be "true" / "false". No code enforces this today.
3. API visibility. New helpers are kept internal for now. If we ship this we need to decide whether they become public surface.

Tests

• CommandLineConfigurationProviderTests covers the source/provider flattening + the provider-aware invariants (including ProviderAwareResolution_CliZeroArityShadowsJsonIndexedArgs end-to-end).
• CommandLineConfigurationExtensionsTests covers the helper-level and handler-end-to-end behavior.
• JsonCommandLineOptionsTests (new) covers the JSON enumeration schema, the three new validator passes, disabled entries, shadowing, and case-insensitivity.
• Full UT project green on net8.0 / net9.0 / net462.

[Copilot]

Pull request overview

This prototype routes command-line options through the platform IConfiguration model so testconfig.json can provide values consumed by existing ICommandLineOptions users, while preserving CLI precedence.

Changes:

• Adds a CLI-backed configuration source/provider under commandLineOptions:*.
• Adds provider-aware command-line option lookup helpers on AggregatedConfiguration/ConfigurationExtensions.
• Wires CommandLineHandler to use the unified configuration view and adds unit coverage for precedence and lookup behavior.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Registers the CLI configuration source and passes built configuration into command-line handling.
src/Platform/Microsoft.Testing.Platform/Configurations/PlatformConfigurationConstants.cs	Adds the commandLineOptions section constant.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationExtensions.cs	Adds unified command-line option lookup helpers.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationSource.cs	Adds the configuration source for parsed CLI options.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationProvider.cs	Flattens parsed CLI options into configuration keys.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs	Adds provider-aware option resolution and uses it for results-directory.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs	Documents current validation gaps for JSON-sourced options.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineManager.cs	Passes configuration into CommandLineHandler.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineHandler.cs	Delegates option reads to configuration when available.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationProviderTests.cs	Adds tests for CLI provider flattening and precedence behavior.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationExtensionsTests.cs	Adds tests for helper and handler behavior over configuration-backed options.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Round-2 review summary

All four round-1 fixes are correctly applied and the build is clean (0 warnings, 0 errors; 1034 pass / 0 fail). Two minor correctness observations and three test-hygiene NITs left inline. Nothing blocking.

Fix validation

Fix	Status
Case-insensitive duplicate detection (3 dicts + ToDictionary)	✅ Applied correctly. One remaining edge case at validator.cs:62 — see inline.
FormatException catch around EnumerateJsonCommandLineOptions	✅ Mirrors the standard failure path, no double-banner risk. One asymmetry re HasTool — see inline.
[DoesNotReturn] on the two throw helpers	✅ Both helpers carry the attribute.
Rewritten XML doc on TryGetCommandLineOptionArguments	✅ No more stale "validation on parseResult only" wording.

New test adequacy

Test	Verdict
EnumerateCommandLineOptions_SectionNameCaseInsensitive_Honored	✅ Strong assertions (option name + argument value).
Validator_EmptyOptionNameInJson_FailsWithJsonPrefix	⚠️ Only checks "testconfig.json" substring — wouldn't catch a resource string change. Advisory.
Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully	⚠️ Test comment misrepresents pre-fix behavior; assertion only checks IsValid == false. See inline.
Validator_JsonSparseIndexedEntry_DefensiveSchemaRejection	⚠️ Test name doesn't match what's tested. See inline.

Public API hygiene

✅ PublicAPI.Unshipped.txt only has #nullable enable — all new types (JsonCommandLineOptionEntry, CommandLineConfigurationSource, CommandLineConfigurationProvider) and methods (EnumerateJsonCommandLineOptions, TryGetCommandLineOptionArguments, IsCommandLineOptionSet) are correctly internal.
✅ JsonCommandLineOptionEntry uses plain get; properties — no init accessors on any new API.
✅ New internalCommandLineHandler constructor accepting IConfiguration? preserves source/binary compatibility (the existing public constructor delegates with configuration: null).

Summary table

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 LOW (system+system dictionary, FormatException + HasTool)
13	Test Completeness & Coverage	⚪ 1 NIT (weaker substring assertions on 3 new tests)
16	Naming & Conventions	⚪ 1 NIT (Validator_JsonSparseIndexedEntry_* name)
17	Documentation Accuracy	⚪ 1 NIT (test comment misrepresents pre-fix behavior)

✅ 17/21 dimensions clean.

Threading & Concurrency, Security & IPC, Public API & Binary Compatibility, Performance & Allocations, Cross-TFM Compatibility, Resource & IDisposable, Defensive Coding at Boundaries, Localization, Test Isolation, Assertion Quality, Flakiness Patterns, Data-Driven Coverage, Code Structure, Analyzer Quality (N/A), IPC Wire Compatibility (N/A), Build Infrastructure, Scope Discipline — all clean.

Noted for follow-up (out-of-scope for this PR)

• Env-var injection of commandLineOptions:* keys: now that IsCommandLineOptionSet/TryGetCommandLineOptionArguments route through IConfiguration, an env var spelled commandLineOptions__timeout=30s (with __→: normalization in EnvironmentVariablesConfigurationProvider) will silently shadow JSON values and be invisible to CommandLineOptionsValidator (which only sees CLI + JSON entries). This is an existing platform concern that the unification only exposes — not introduced here — but worth tracking.
• Behavioral break for extension authors who intentionally registered case-differing option names (e.g. "Timeout" for one provider, "timeout" for another). They were previously accepted as distinct (silently mis-treated as one by every downstream case-insensitive lookup); they now fail validation up front. This is the correct fix, but worth a CHANGELOG entry if not already planned.

[Amaury Levé (Evangelink)]

Round 3 review — bcb797345

✅ 21/21 dimensions clean — no blocking, medium, or low findings.

Round-2 fixes validation

Fix #1 — ValidateOptionsAreNotDuplicated extended to cover system providers: CORRECT and complete.

I traced every collision shape through the new pass and confirmed no legitimate existing behavior changes:

• Extension-vs-extension (case-differing). Previously crashed with ArgumentException at the providerAndOptionByOptionName.ToDictionary(..., StringComparer.OrdinalIgnoreCase) lookup on line 60-62. Now caught with the friendly "declared by multiple" error. ✅
• Extension-vs-system (any casing). Already caught earlier by ValidateExtensionOptionsDoNotContainReservedOptions (line 107-152), which uses OrdinalIgnoreCase and returns early. The new Concat-over-both-dictionaries iteration is reachable in theory for this case but practically pre-empted. ✅
• System-vs-system (case-differing). Previously crashed at the same ToDictionary lookup. Now caught with the friendly error. ✅ (Pinned by the new Validator_DuplicateOptionNamesAcrossSystemAndSystem_FailsGracefully test.)

I considered whether two providers could legitimately register the same option name (e.g. a system provider exposing a public alias also added by an extension). The contract is that each ICommandLineOptionsProvider.GetCommandLineOptions() returns options it owns, and the pre-existing ValidateExtensionOptionsDoNotContainReservedOptions pass already treats extension-vs-system collisions as errors (any casing). So no legitimate scenario is regressed.

Minor cosmetic observation (NIT, not blocking): line 60 still uses LINQ Union over the two dictionaries, while the new code at line 165 uses Concat. Both produce the same result here (provider keys are reference-equal across the two source dictionaries, and there are no duplicates by reference), but slight stylistic inconsistency. Not worth a follow-up.

---

Fix #2 — FormatException exception filter + rich error display: CORRECT and complete.

• The when (!loggingState.CommandLineParseResult.HasTool) exception filter is standard C# 6+ and works on all four TFMs.
• The silent-degrade-to-empty-list path for tools is consistent with the existing pipeline convention: validation errors are already gated by !HasTool on line 230, so tools that have a structurally-broken commandLineOptions section already wouldn't see validation surface the issue. The FormatException fallback mirrors this.
• I verified HasTool semantics: ParseResult.HasTool => ToolName is not null. --help and --info are regular options handled via IsHelpInvoked / IsInfoInvoked on CommandLineHandler (lines 240-247), not tools. So --help and --info go through the normal !HasTool path and do surface FormatException with the rich InvalidCommandLineArguments header.
• Only server-mode tools (those that set ToolName via the tool entry, e.g. --server) silently degrade. Even for these, the rest of testconfig.json (e.g. results-directory, custom IConfiguration[key] lookups) remains accessible because the throw fires only during the typed-schema enumeration of the commandLineOptions section — direct provider.TryGet calls bypass it.
• The rich-error formatting (StringBuilder + PlatformResources.InvalidCommandLineArguments header + "\t- {ex.Message}" + TrimEnd()) matches CommandLineOptionsValidator (line 23-28). Environment.NewLine is correctly avoided (confirmed via grep). StringBuilder resolves via the global System.Text using in Directory.Build.props.
• DisplayBannerIfEnabledAsync call inside the catch (line 205) safely reads --no-banner via commandLineOptions.IsOptionSet, which routes through provider.TryGet on the already-flattened key-value store, not through EnumerateCommandLineOptions. No re-throw risk.

---

Fresh-pass net-new findings

Nothing blocking, medium, or low. Three honest NIT-level observations, all noted-for-follow-up only:

1. NIT (style) — CommandLineOptionsValidator.cs line 60 uses Union where Concat would be slightly clearer and marginally cheaper (the new code at line 165 already uses Concat).
2. NIT (test redundancy) — Validator_JsonEntryWithTooFewArguments_FailsArityCheck (line 454) overlaps with Validator_JsonArityTooFew_Fails (line 200); only the arity shape differs (ArgumentArity(2,2) vs ExactlyOne). Defensible as defensive coverage of a non-trivial arity.
3. NIT (docs precision) — The comment on Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully (lines 410-415) cites line 62, which will become stale on future edits to the validator. Consider replacing with a symbolic reference ("the OrdinalIgnoreCaseToDictionary lookup").

---

Other dimensions checked clean

• Public API & init accessors — All new types/members are internal. No PublicAPI.Unshipped.txt churn. ✅
• Cross-TFM — Exception filters (C# 6+), StringBuilder, CultureInfo, OrdinalIgnoreCase all available on net462 / netstandard2.0 / net8.0 / net9.0. ✅
• Localization — Two new resx strings (JsonCommandLineOptionsEntryMustBeScalarOrArrayErrorMessage, JsonCommandLineOptionsValidationErrorPrefix) with <comment> metadata. .xlf deltas look auto-generated (stub-per-resource pattern, +10 per language file). No manual xlf edits. ✅
• Threading — All affected code paths run on the single-threaded host startup pipeline. No new shared mutable state. ✅
• IPC wire compatibility — N/A; no serialized type changes. ✅
• Resource management — N/A; no new disposables. ✅
• Defensive coding — Trust boundary (user testconfig.json) correctly guarded; internal invariants (e.g. sparse-indexed defensive guard in JsonConfigurationProvider) correctly preserved. ✅
• Test isolation — Tests use isolated TestProvider instances per [TestMethod]; no shared static mutable state. ✅
• Flakiness — No Thread.Sleep, no wall-clock assertions, no hard-coded ports in new tests. ✅

---

Verdict

Ready to merge. Round-2 fixes are correct, complete, and free of new bugs. The three NIT observations above are stylistic/documentation polish and do not affect users. Items explicitly deferred in the PR description (scalar-bool ambiguity, --diagnostic*/--config-file bootstrap reads, IsCommandLineOptionSet API visibility) remain out of scope for this PR as agreed.

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 31/31 changed files
• Comments generated: 0 new

[Amaury Levé (Evangelink)]

Summary

Resolves part of #6349: let users specify CLI options inside testconfig.json by routing ICommandLineOptions through IConfiguration.

Approach

1. New CommandLineConfigurationSource (Order = 0) wraps the parsed CLI into an IConfigurationProvider that flattens each option under commandLineOptions:<name> (zero-arity) or commandLineOptions:<name>:<index> (arg-bearing).
2. The CLI source is registered in TestHostBuilder.CommonServicesbefore the JSON source, so CLI keeps highest precedence.
3. AggregatedConfiguration.TryGetCommandLineOptionFromProviders does a provider-aware lookup: it walks providers in registration order, and the first provider with any data for that option wins outright (no per-key cross-provider merging). This avoids cases where IConfiguration[key] accidentally returns the JSON :0 while the CLI set the zero-arity bare key, or vice versa.
4. The public extension helpers (IsCommandLineOptionSet, TryGetCommandLineOptionArguments) delegate to that method when given an AggregatedConfiguration, and CommandLineHandler becomes a thin facade that does the same. End result: every consumer of ICommandLineOptions transparently sees JSON-sourced options.
5. JsonConfigurationProvider exposes a typed, schema-validated enumeration of commandLineOptions:* entries. CommandLineOptionsValidator runs three extra passes over those entries (unknown-option, arity, per-arg validation) so testconfig.json typos surface during startup instead of crashing inside option handlers.
6. DisplayBannerIfEnabledAsync reads --no-banner from the unified ICommandLineOptions, so banner suppression honors testconfig.json as well as the CLI.

What works

• testconfig.json keys under commandLineOptions are surfaced everywhere ICommandLineOptions is consumed.
• CLI still wins over JSON for the same option.
• --results-directory defined in JSON is honored (was previously ignored - the legacy GetResultsDirectoryCore path was bypassing JSON).
• --no-banner defined in JSON suppresses the banner.
• Invalid JSON entries (unknown option, wrong arity, bad arg) fail at startup with a clear error referencing testconfig.json, instead of crashing later. Concrete cases now caught:
  • --timeout: true (was: IndexOutOfRangeException in args[0])
  • --exit-on-process-exit: "abc" (was: FormatException in int.Parse(args[0]))
  • typos like "timeoutt": "30s" (was: silently ignored)
• Back-compat ctor on CommandLineHandler is kept so external code keeps compiling.

Rough edges still on the table

These are not addressed in this PR:

1. Bootstrap readers.--diagnostic* and --config-file are read off parseResultbeforeIConfiguration is built. Honoring them from JSON requires either a two-phase config build or rewriting those bootstrap paths.
2. Scalar bool ambiguity. For arg-bearing options, JSON "true" / "false" is ambiguous with the zero-arity presence marker. Callers must use the array form ("foo": ["true"]) for non-boolean string values that happen to be "true" / "false". No code enforces this today.
3. API visibility. New helpers are kept internal for now. If we ship this we need to decide whether they become public surface.

Tests

• CommandLineConfigurationProviderTests covers the source/provider flattening + the provider-aware invariants (including ProviderAwareResolution_CliZeroArityShadowsJsonIndexedArgs end-to-end).
• CommandLineConfigurationExtensionsTests covers the helper-level and handler-end-to-end behavior.
• JsonCommandLineOptionsTests (new) covers the JSON enumeration schema, the three new validator passes, disabled entries, shadowing, and case-insensitivity.
• Full UT project green on net8.0 / net9.0 / net462.

[Copilot]

Pull request overview

This prototype routes command-line options through the platform IConfiguration model so testconfig.json can provide values consumed by existing ICommandLineOptions users, while preserving CLI precedence.

Changes:

• Adds a CLI-backed configuration source/provider under commandLineOptions:*.
• Adds provider-aware command-line option lookup helpers on AggregatedConfiguration/ConfigurationExtensions.
• Wires CommandLineHandler to use the unified configuration view and adds unit coverage for precedence and lookup behavior.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Registers the CLI configuration source and passes built configuration into command-line handling.
src/Platform/Microsoft.Testing.Platform/Configurations/PlatformConfigurationConstants.cs	Adds the commandLineOptions section constant.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationExtensions.cs	Adds unified command-line option lookup helpers.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationSource.cs	Adds the configuration source for parsed CLI options.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationProvider.cs	Flattens parsed CLI options into configuration keys.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs	Adds provider-aware option resolution and uses it for results-directory.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs	Documents current validation gaps for JSON-sourced options.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineManager.cs	Passes configuration into CommandLineHandler.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineHandler.cs	Delegates option reads to configuration when available.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationProviderTests.cs	Adds tests for CLI provider flattening and precedence behavior.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationExtensionsTests.cs	Adds tests for helper and handler behavior over configuration-backed options.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Round-2 review summary

All four round-1 fixes are correctly applied and the build is clean (0 warnings, 0 errors; 1034 pass / 0 fail). Two minor correctness observations and three test-hygiene NITs left inline. Nothing blocking.

Fix validation

Fix	Status
Case-insensitive duplicate detection (3 dicts + ToDictionary)	✅ Applied correctly. One remaining edge case at validator.cs:62 — see inline.
FormatException catch around EnumerateJsonCommandLineOptions	✅ Mirrors the standard failure path, no double-banner risk. One asymmetry re HasTool — see inline.
[DoesNotReturn] on the two throw helpers	✅ Both helpers carry the attribute.
Rewritten XML doc on TryGetCommandLineOptionArguments	✅ No more stale "validation on parseResult only" wording.

New test adequacy

Test	Verdict
EnumerateCommandLineOptions_SectionNameCaseInsensitive_Honored	✅ Strong assertions (option name + argument value).
Validator_EmptyOptionNameInJson_FailsWithJsonPrefix	⚠️ Only checks "testconfig.json" substring — wouldn't catch a resource string change. Advisory.
Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully	⚠️ Test comment misrepresents pre-fix behavior; assertion only checks IsValid == false. See inline.
Validator_JsonSparseIndexedEntry_DefensiveSchemaRejection	⚠️ Test name doesn't match what's tested. See inline.

Public API hygiene

✅ PublicAPI.Unshipped.txt only has #nullable enable — all new types (JsonCommandLineOptionEntry, CommandLineConfigurationSource, CommandLineConfigurationProvider) and methods (EnumerateJsonCommandLineOptions, TryGetCommandLineOptionArguments, IsCommandLineOptionSet) are correctly internal.
✅ JsonCommandLineOptionEntry uses plain get; properties — no init accessors on any new API.
✅ New internalCommandLineHandler constructor accepting IConfiguration? preserves source/binary compatibility (the existing public constructor delegates with configuration: null).

Summary table

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 LOW (system+system dictionary, FormatException + HasTool)
13	Test Completeness & Coverage	⚪ 1 NIT (weaker substring assertions on 3 new tests)
16	Naming & Conventions	⚪ 1 NIT (Validator_JsonSparseIndexedEntry_* name)
17	Documentation Accuracy	⚪ 1 NIT (test comment misrepresents pre-fix behavior)

✅ 17/21 dimensions clean.

Threading & Concurrency, Security & IPC, Public API & Binary Compatibility, Performance & Allocations, Cross-TFM Compatibility, Resource & IDisposable, Defensive Coding at Boundaries, Localization, Test Isolation, Assertion Quality, Flakiness Patterns, Data-Driven Coverage, Code Structure, Analyzer Quality (N/A), IPC Wire Compatibility (N/A), Build Infrastructure, Scope Discipline — all clean.

Noted for follow-up (out-of-scope for this PR)

• Env-var injection of commandLineOptions:* keys: now that IsCommandLineOptionSet/TryGetCommandLineOptionArguments route through IConfiguration, an env var spelled commandLineOptions__timeout=30s (with __→: normalization in EnvironmentVariablesConfigurationProvider) will silently shadow JSON values and be invisible to CommandLineOptionsValidator (which only sees CLI + JSON entries). This is an existing platform concern that the unification only exposes — not introduced here — but worth tracking.
• Behavioral break for extension authors who intentionally registered case-differing option names (e.g. "Timeout" for one provider, "timeout" for another). They were previously accepted as distinct (silently mis-treated as one by every downstream case-insensitive lookup); they now fail validation up front. This is the correct fix, but worth a CHANGELOG entry if not already planned.

[Amaury Levé (Evangelink)]

Round 3 review — bcb797345

✅ 21/21 dimensions clean — no blocking, medium, or low findings.

Round-2 fixes validation

Fix #1 — ValidateOptionsAreNotDuplicated extended to cover system providers: CORRECT and complete.

I traced every collision shape through the new pass and confirmed no legitimate existing behavior changes:

• Extension-vs-extension (case-differing). Previously crashed with ArgumentException at the providerAndOptionByOptionName.ToDictionary(..., StringComparer.OrdinalIgnoreCase) lookup on line 60-62. Now caught with the friendly "declared by multiple" error. ✅
• Extension-vs-system (any casing). Already caught earlier by ValidateExtensionOptionsDoNotContainReservedOptions (line 107-152), which uses OrdinalIgnoreCase and returns early. The new Concat-over-both-dictionaries iteration is reachable in theory for this case but practically pre-empted. ✅
• System-vs-system (case-differing). Previously crashed at the same ToDictionary lookup. Now caught with the friendly error. ✅ (Pinned by the new Validator_DuplicateOptionNamesAcrossSystemAndSystem_FailsGracefully test.)

I considered whether two providers could legitimately register the same option name (e.g. a system provider exposing a public alias also added by an extension). The contract is that each ICommandLineOptionsProvider.GetCommandLineOptions() returns options it owns, and the pre-existing ValidateExtensionOptionsDoNotContainReservedOptions pass already treats extension-vs-system collisions as errors (any casing). So no legitimate scenario is regressed.

Minor cosmetic observation (NIT, not blocking): line 60 still uses LINQ Union over the two dictionaries, while the new code at line 165 uses Concat. Both produce the same result here (provider keys are reference-equal across the two source dictionaries, and there are no duplicates by reference), but slight stylistic inconsistency. Not worth a follow-up.

---

Fix #2 — FormatException exception filter + rich error display: CORRECT and complete.

• The when (!loggingState.CommandLineParseResult.HasTool) exception filter is standard C# 6+ and works on all four TFMs.
• The silent-degrade-to-empty-list path for tools is consistent with the existing pipeline convention: validation errors are already gated by !HasTool on line 230, so tools that have a structurally-broken commandLineOptions section already wouldn't see validation surface the issue. The FormatException fallback mirrors this.
• I verified HasTool semantics: ParseResult.HasTool => ToolName is not null. --help and --info are regular options handled via IsHelpInvoked / IsInfoInvoked on CommandLineHandler (lines 240-247), not tools. So --help and --info go through the normal !HasTool path and do surface FormatException with the rich InvalidCommandLineArguments header.
• Only server-mode tools (those that set ToolName via the tool entry, e.g. --server) silently degrade. Even for these, the rest of testconfig.json (e.g. results-directory, custom IConfiguration[key] lookups) remains accessible because the throw fires only during the typed-schema enumeration of the commandLineOptions section — direct provider.TryGet calls bypass it.
• The rich-error formatting (StringBuilder + PlatformResources.InvalidCommandLineArguments header + "\t- {ex.Message}" + TrimEnd()) matches CommandLineOptionsValidator (line 23-28). Environment.NewLine is correctly avoided (confirmed via grep). StringBuilder resolves via the global System.Text using in Directory.Build.props.
• DisplayBannerIfEnabledAsync call inside the catch (line 205) safely reads --no-banner via commandLineOptions.IsOptionSet, which routes through provider.TryGet on the already-flattened key-value store, not through EnumerateCommandLineOptions. No re-throw risk.

---

Fresh-pass net-new findings

Nothing blocking, medium, or low. Three honest NIT-level observations, all noted-for-follow-up only:

1. NIT (style) — CommandLineOptionsValidator.cs line 60 uses Union where Concat would be slightly clearer and marginally cheaper (the new code at line 165 already uses Concat).
2. NIT (test redundancy) — Validator_JsonEntryWithTooFewArguments_FailsArityCheck (line 454) overlaps with Validator_JsonArityTooFew_Fails (line 200); only the arity shape differs (ArgumentArity(2,2) vs ExactlyOne). Defensible as defensive coverage of a non-trivial arity.
3. NIT (docs precision) — The comment on Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully (lines 410-415) cites line 62, which will become stale on future edits to the validator. Consider replacing with a symbolic reference ("the OrdinalIgnoreCaseToDictionary lookup").

---

Other dimensions checked clean

• Public API & init accessors — All new types/members are internal. No PublicAPI.Unshipped.txt churn. ✅
• Cross-TFM — Exception filters (C# 6+), StringBuilder, CultureInfo, OrdinalIgnoreCase all available on net462 / netstandard2.0 / net8.0 / net9.0. ✅
• Localization — Two new resx strings (JsonCommandLineOptionsEntryMustBeScalarOrArrayErrorMessage, JsonCommandLineOptionsValidationErrorPrefix) with <comment> metadata. .xlf deltas look auto-generated (stub-per-resource pattern, +10 per language file). No manual xlf edits. ✅
• Threading — All affected code paths run on the single-threaded host startup pipeline. No new shared mutable state. ✅
• IPC wire compatibility — N/A; no serialized type changes. ✅
• Resource management — N/A; no new disposables. ✅
• Defensive coding — Trust boundary (user testconfig.json) correctly guarded; internal invariants (e.g. sparse-indexed defensive guard in JsonConfigurationProvider) correctly preserved. ✅
• Test isolation — Tests use isolated TestProvider instances per [TestMethod]; no shared static mutable state. ✅
• Flakiness — No Thread.Sleep, no wall-clock assertions, no hard-coded ports in new tests. ✅

---

Verdict

Ready to merge. Round-2 fixes are correct, complete, and free of new bugs. The three NIT observations above are stylistic/documentation polish and do not affect users. Items explicitly deferred in the PR description (scalar-bool ambiguity, --diagnostic*/--config-file bootstrap reads, IsCommandLineOptionSet API visibility) remain out of scope for this PR as agreed.

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 31/31 changed files
• Comments generated: 0 new

[Amaury Levé (Evangelink)]

Summary

Resolves part of #6349: let users specify CLI options inside testconfig.json by routing ICommandLineOptions through IConfiguration.

Approach

1. New CommandLineConfigurationSource (Order = 0) wraps the parsed CLI into an IConfigurationProvider that flattens each option under commandLineOptions:<name> (zero-arity) or commandLineOptions:<name>:<index> (arg-bearing).
2. The CLI source is registered in TestHostBuilder.CommonServicesbefore the JSON source, so CLI keeps highest precedence.
3. AggregatedConfiguration.TryGetCommandLineOptionFromProviders does a provider-aware lookup: it walks providers in registration order, and the first provider with any data for that option wins outright (no per-key cross-provider merging). This avoids cases where IConfiguration[key] accidentally returns the JSON :0 while the CLI set the zero-arity bare key, or vice versa.
4. The public extension helpers (IsCommandLineOptionSet, TryGetCommandLineOptionArguments) delegate to that method when given an AggregatedConfiguration, and CommandLineHandler becomes a thin facade that does the same. End result: every consumer of ICommandLineOptions transparently sees JSON-sourced options.
5. JsonConfigurationProvider exposes a typed, schema-validated enumeration of commandLineOptions:* entries. CommandLineOptionsValidator runs three extra passes over those entries (unknown-option, arity, per-arg validation) so testconfig.json typos surface during startup instead of crashing inside option handlers.
6. DisplayBannerIfEnabledAsync reads --no-banner from the unified ICommandLineOptions, so banner suppression honors testconfig.json as well as the CLI.

What works

• testconfig.json keys under commandLineOptions are surfaced everywhere ICommandLineOptions is consumed.
• CLI still wins over JSON for the same option.
• --results-directory defined in JSON is honored (was previously ignored - the legacy GetResultsDirectoryCore path was bypassing JSON).
• --no-banner defined in JSON suppresses the banner.
• Invalid JSON entries (unknown option, wrong arity, bad arg) fail at startup with a clear error referencing testconfig.json, instead of crashing later. Concrete cases now caught:
  • --timeout: true (was: IndexOutOfRangeException in args[0])
  • --exit-on-process-exit: "abc" (was: FormatException in int.Parse(args[0]))
  • typos like "timeoutt": "30s" (was: silently ignored)
• Back-compat ctor on CommandLineHandler is kept so external code keeps compiling.

Rough edges still on the table

These are not addressed in this PR:

1. Bootstrap readers.--diagnostic* and --config-file are read off parseResultbeforeIConfiguration is built. Honoring them from JSON requires either a two-phase config build or rewriting those bootstrap paths.
2. Scalar bool ambiguity. For arg-bearing options, JSON "true" / "false" is ambiguous with the zero-arity presence marker. Callers must use the array form ("foo": ["true"]) for non-boolean string values that happen to be "true" / "false". No code enforces this today.
3. API visibility. New helpers are kept internal for now. If we ship this we need to decide whether they become public surface.

Tests

• CommandLineConfigurationProviderTests covers the source/provider flattening + the provider-aware invariants (including ProviderAwareResolution_CliZeroArityShadowsJsonIndexedArgs end-to-end).
• CommandLineConfigurationExtensionsTests covers the helper-level and handler-end-to-end behavior.
• JsonCommandLineOptionsTests (new) covers the JSON enumeration schema, the three new validator passes, disabled entries, shadowing, and case-insensitivity.
• Full UT project green on net8.0 / net9.0 / net462.

[Copilot]

Pull request overview

This prototype routes command-line options through the platform IConfiguration model so testconfig.json can provide values consumed by existing ICommandLineOptions users, while preserving CLI precedence.

Changes:

• Adds a CLI-backed configuration source/provider under commandLineOptions:*.
• Adds provider-aware command-line option lookup helpers on AggregatedConfiguration/ConfigurationExtensions.
• Wires CommandLineHandler to use the unified configuration view and adds unit coverage for precedence and lookup behavior.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Registers the CLI configuration source and passes built configuration into command-line handling.
src/Platform/Microsoft.Testing.Platform/Configurations/PlatformConfigurationConstants.cs	Adds the commandLineOptions section constant.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationExtensions.cs	Adds unified command-line option lookup helpers.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationSource.cs	Adds the configuration source for parsed CLI options.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationProvider.cs	Flattens parsed CLI options into configuration keys.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs	Adds provider-aware option resolution and uses it for results-directory.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs	Documents current validation gaps for JSON-sourced options.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineManager.cs	Passes configuration into CommandLineHandler.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineHandler.cs	Delegates option reads to configuration when available.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationProviderTests.cs	Adds tests for CLI provider flattening and precedence behavior.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationExtensionsTests.cs	Adds tests for helper and handler behavior over configuration-backed options.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Round-2 review summary

All four round-1 fixes are correctly applied and the build is clean (0 warnings, 0 errors; 1034 pass / 0 fail). Two minor correctness observations and three test-hygiene NITs left inline. Nothing blocking.

Fix validation

Fix	Status
Case-insensitive duplicate detection (3 dicts + ToDictionary)	✅ Applied correctly. One remaining edge case at validator.cs:62 — see inline.
FormatException catch around EnumerateJsonCommandLineOptions	✅ Mirrors the standard failure path, no double-banner risk. One asymmetry re HasTool — see inline.
[DoesNotReturn] on the two throw helpers	✅ Both helpers carry the attribute.
Rewritten XML doc on TryGetCommandLineOptionArguments	✅ No more stale "validation on parseResult only" wording.

New test adequacy

Test	Verdict
EnumerateCommandLineOptions_SectionNameCaseInsensitive_Honored	✅ Strong assertions (option name + argument value).
Validator_EmptyOptionNameInJson_FailsWithJsonPrefix	⚠️ Only checks "testconfig.json" substring — wouldn't catch a resource string change. Advisory.
Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully	⚠️ Test comment misrepresents pre-fix behavior; assertion only checks IsValid == false. See inline.
Validator_JsonSparseIndexedEntry_DefensiveSchemaRejection	⚠️ Test name doesn't match what's tested. See inline.

Public API hygiene

✅ PublicAPI.Unshipped.txt only has #nullable enable — all new types (JsonCommandLineOptionEntry, CommandLineConfigurationSource, CommandLineConfigurationProvider) and methods (EnumerateJsonCommandLineOptions, TryGetCommandLineOptionArguments, IsCommandLineOptionSet) are correctly internal.
✅ JsonCommandLineOptionEntry uses plain get; properties — no init accessors on any new API.
✅ New internalCommandLineHandler constructor accepting IConfiguration? preserves source/binary compatibility (the existing public constructor delegates with configuration: null).

Summary table

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 LOW (system+system dictionary, FormatException + HasTool)
13	Test Completeness & Coverage	⚪ 1 NIT (weaker substring assertions on 3 new tests)
16	Naming & Conventions	⚪ 1 NIT (Validator_JsonSparseIndexedEntry_* name)
17	Documentation Accuracy	⚪ 1 NIT (test comment misrepresents pre-fix behavior)

✅ 17/21 dimensions clean.

Threading & Concurrency, Security & IPC, Public API & Binary Compatibility, Performance & Allocations, Cross-TFM Compatibility, Resource & IDisposable, Defensive Coding at Boundaries, Localization, Test Isolation, Assertion Quality, Flakiness Patterns, Data-Driven Coverage, Code Structure, Analyzer Quality (N/A), IPC Wire Compatibility (N/A), Build Infrastructure, Scope Discipline — all clean.

Noted for follow-up (out-of-scope for this PR)

• Env-var injection of commandLineOptions:* keys: now that IsCommandLineOptionSet/TryGetCommandLineOptionArguments route through IConfiguration, an env var spelled commandLineOptions__timeout=30s (with __→: normalization in EnvironmentVariablesConfigurationProvider) will silently shadow JSON values and be invisible to CommandLineOptionsValidator (which only sees CLI + JSON entries). This is an existing platform concern that the unification only exposes — not introduced here — but worth tracking.
• Behavioral break for extension authors who intentionally registered case-differing option names (e.g. "Timeout" for one provider, "timeout" for another). They were previously accepted as distinct (silently mis-treated as one by every downstream case-insensitive lookup); they now fail validation up front. This is the correct fix, but worth a CHANGELOG entry if not already planned.

[Amaury Levé (Evangelink)]

Round 3 review — bcb797345

✅ 21/21 dimensions clean — no blocking, medium, or low findings.

Round-2 fixes validation

Fix #1 — ValidateOptionsAreNotDuplicated extended to cover system providers: CORRECT and complete.

I traced every collision shape through the new pass and confirmed no legitimate existing behavior changes:

• Extension-vs-extension (case-differing). Previously crashed with ArgumentException at the providerAndOptionByOptionName.ToDictionary(..., StringComparer.OrdinalIgnoreCase) lookup on line 60-62. Now caught with the friendly "declared by multiple" error. ✅
• Extension-vs-system (any casing). Already caught earlier by ValidateExtensionOptionsDoNotContainReservedOptions (line 107-152), which uses OrdinalIgnoreCase and returns early. The new Concat-over-both-dictionaries iteration is reachable in theory for this case but practically pre-empted. ✅
• System-vs-system (case-differing). Previously crashed at the same ToDictionary lookup. Now caught with the friendly error. ✅ (Pinned by the new Validator_DuplicateOptionNamesAcrossSystemAndSystem_FailsGracefully test.)

I considered whether two providers could legitimately register the same option name (e.g. a system provider exposing a public alias also added by an extension). The contract is that each ICommandLineOptionsProvider.GetCommandLineOptions() returns options it owns, and the pre-existing ValidateExtensionOptionsDoNotContainReservedOptions pass already treats extension-vs-system collisions as errors (any casing). So no legitimate scenario is regressed.

Minor cosmetic observation (NIT, not blocking): line 60 still uses LINQ Union over the two dictionaries, while the new code at line 165 uses Concat. Both produce the same result here (provider keys are reference-equal across the two source dictionaries, and there are no duplicates by reference), but slight stylistic inconsistency. Not worth a follow-up.

---

Fix #2 — FormatException exception filter + rich error display: CORRECT and complete.

• The when (!loggingState.CommandLineParseResult.HasTool) exception filter is standard C# 6+ and works on all four TFMs.
• The silent-degrade-to-empty-list path for tools is consistent with the existing pipeline convention: validation errors are already gated by !HasTool on line 230, so tools that have a structurally-broken commandLineOptions section already wouldn't see validation surface the issue. The FormatException fallback mirrors this.
• I verified HasTool semantics: ParseResult.HasTool => ToolName is not null. --help and --info are regular options handled via IsHelpInvoked / IsInfoInvoked on CommandLineHandler (lines 240-247), not tools. So --help and --info go through the normal !HasTool path and do surface FormatException with the rich InvalidCommandLineArguments header.
• Only server-mode tools (those that set ToolName via the tool entry, e.g. --server) silently degrade. Even for these, the rest of testconfig.json (e.g. results-directory, custom IConfiguration[key] lookups) remains accessible because the throw fires only during the typed-schema enumeration of the commandLineOptions section — direct provider.TryGet calls bypass it.
• The rich-error formatting (StringBuilder + PlatformResources.InvalidCommandLineArguments header + "\t- {ex.Message}" + TrimEnd()) matches CommandLineOptionsValidator (line 23-28). Environment.NewLine is correctly avoided (confirmed via grep). StringBuilder resolves via the global System.Text using in Directory.Build.props.
• DisplayBannerIfEnabledAsync call inside the catch (line 205) safely reads --no-banner via commandLineOptions.IsOptionSet, which routes through provider.TryGet on the already-flattened key-value store, not through EnumerateCommandLineOptions. No re-throw risk.

---

Fresh-pass net-new findings

Nothing blocking, medium, or low. Three honest NIT-level observations, all noted-for-follow-up only:

1. NIT (style) — CommandLineOptionsValidator.cs line 60 uses Union where Concat would be slightly clearer and marginally cheaper (the new code at line 165 already uses Concat).
2. NIT (test redundancy) — Validator_JsonEntryWithTooFewArguments_FailsArityCheck (line 454) overlaps with Validator_JsonArityTooFew_Fails (line 200); only the arity shape differs (ArgumentArity(2,2) vs ExactlyOne). Defensible as defensive coverage of a non-trivial arity.
3. NIT (docs precision) — The comment on Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully (lines 410-415) cites line 62, which will become stale on future edits to the validator. Consider replacing with a symbolic reference ("the OrdinalIgnoreCaseToDictionary lookup").

---

Other dimensions checked clean

• Public API & init accessors — All new types/members are internal. No PublicAPI.Unshipped.txt churn. ✅
• Cross-TFM — Exception filters (C# 6+), StringBuilder, CultureInfo, OrdinalIgnoreCase all available on net462 / netstandard2.0 / net8.0 / net9.0. ✅
• Localization — Two new resx strings (JsonCommandLineOptionsEntryMustBeScalarOrArrayErrorMessage, JsonCommandLineOptionsValidationErrorPrefix) with <comment> metadata. .xlf deltas look auto-generated (stub-per-resource pattern, +10 per language file). No manual xlf edits. ✅
• Threading — All affected code paths run on the single-threaded host startup pipeline. No new shared mutable state. ✅
• IPC wire compatibility — N/A; no serialized type changes. ✅
• Resource management — N/A; no new disposables. ✅
• Defensive coding — Trust boundary (user testconfig.json) correctly guarded; internal invariants (e.g. sparse-indexed defensive guard in JsonConfigurationProvider) correctly preserved. ✅
• Test isolation — Tests use isolated TestProvider instances per [TestMethod]; no shared static mutable state. ✅
• Flakiness — No Thread.Sleep, no wall-clock assertions, no hard-coded ports in new tests. ✅

---

Verdict

Ready to merge. Round-2 fixes are correct, complete, and free of new bugs. The three NIT observations above are stylistic/documentation polish and do not affect users. Items explicitly deferred in the PR description (scalar-bool ambiguity, --diagnostic*/--config-file bootstrap reads, IsCommandLineOptionSet API visibility) remain out of scope for this PR as agreed.

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 31/31 changed files
• Comments generated: 0 new

[Amaury Levé (Evangelink)]

Summary

Resolves part of #6349: let users specify CLI options inside testconfig.json by routing ICommandLineOptions through IConfiguration.

Approach

1. New CommandLineConfigurationSource (Order = 0) wraps the parsed CLI into an IConfigurationProvider that flattens each option under commandLineOptions:<name> (zero-arity) or commandLineOptions:<name>:<index> (arg-bearing).
2. The CLI source is registered in TestHostBuilder.CommonServicesbefore the JSON source, so CLI keeps highest precedence.
3. AggregatedConfiguration.TryGetCommandLineOptionFromProviders does a provider-aware lookup: it walks providers in registration order, and the first provider with any data for that option wins outright (no per-key cross-provider merging). This avoids cases where IConfiguration[key] accidentally returns the JSON :0 while the CLI set the zero-arity bare key, or vice versa.
4. The public extension helpers (IsCommandLineOptionSet, TryGetCommandLineOptionArguments) delegate to that method when given an AggregatedConfiguration, and CommandLineHandler becomes a thin facade that does the same. End result: every consumer of ICommandLineOptions transparently sees JSON-sourced options.
5. JsonConfigurationProvider exposes a typed, schema-validated enumeration of commandLineOptions:* entries. CommandLineOptionsValidator runs three extra passes over those entries (unknown-option, arity, per-arg validation) so testconfig.json typos surface during startup instead of crashing inside option handlers.
6. DisplayBannerIfEnabledAsync reads --no-banner from the unified ICommandLineOptions, so banner suppression honors testconfig.json as well as the CLI.

What works

• testconfig.json keys under commandLineOptions are surfaced everywhere ICommandLineOptions is consumed.
• CLI still wins over JSON for the same option.
• --results-directory defined in JSON is honored (was previously ignored - the legacy GetResultsDirectoryCore path was bypassing JSON).
• --no-banner defined in JSON suppresses the banner.
• Invalid JSON entries (unknown option, wrong arity, bad arg) fail at startup with a clear error referencing testconfig.json, instead of crashing later. Concrete cases now caught:
  • --timeout: true (was: IndexOutOfRangeException in args[0])
  • --exit-on-process-exit: "abc" (was: FormatException in int.Parse(args[0]))
  • typos like "timeoutt": "30s" (was: silently ignored)
• Back-compat ctor on CommandLineHandler is kept so external code keeps compiling.

Rough edges still on the table

These are not addressed in this PR:

1. Bootstrap readers.--diagnostic* and --config-file are read off parseResultbeforeIConfiguration is built. Honoring them from JSON requires either a two-phase config build or rewriting those bootstrap paths.
2. Scalar bool ambiguity. For arg-bearing options, JSON "true" / "false" is ambiguous with the zero-arity presence marker. Callers must use the array form ("foo": ["true"]) for non-boolean string values that happen to be "true" / "false". No code enforces this today.
3. API visibility. New helpers are kept internal for now. If we ship this we need to decide whether they become public surface.

Tests

• CommandLineConfigurationProviderTests covers the source/provider flattening + the provider-aware invariants (including ProviderAwareResolution_CliZeroArityShadowsJsonIndexedArgs end-to-end).
• CommandLineConfigurationExtensionsTests covers the helper-level and handler-end-to-end behavior.
• JsonCommandLineOptionsTests (new) covers the JSON enumeration schema, the three new validator passes, disabled entries, shadowing, and case-insensitivity.
• Full UT project green on net8.0 / net9.0 / net462.

[Copilot]

Pull request overview

This prototype routes command-line options through the platform IConfiguration model so testconfig.json can provide values consumed by existing ICommandLineOptions users, while preserving CLI precedence.

Changes:

• Adds a CLI-backed configuration source/provider under commandLineOptions:*.
• Adds provider-aware command-line option lookup helpers on AggregatedConfiguration/ConfigurationExtensions.
• Wires CommandLineHandler to use the unified configuration view and adds unit coverage for precedence and lookup behavior.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Registers the CLI configuration source and passes built configuration into command-line handling.
src/Platform/Microsoft.Testing.Platform/Configurations/PlatformConfigurationConstants.cs	Adds the commandLineOptions section constant.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationExtensions.cs	Adds unified command-line option lookup helpers.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationSource.cs	Adds the configuration source for parsed CLI options.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationProvider.cs	Flattens parsed CLI options into configuration keys.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs	Adds provider-aware option resolution and uses it for results-directory.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs	Documents current validation gaps for JSON-sourced options.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineManager.cs	Passes configuration into CommandLineHandler.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineHandler.cs	Delegates option reads to configuration when available.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationProviderTests.cs	Adds tests for CLI provider flattening and precedence behavior.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationExtensionsTests.cs	Adds tests for helper and handler behavior over configuration-backed options.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Round-2 review summary

All four round-1 fixes are correctly applied and the build is clean (0 warnings, 0 errors; 1034 pass / 0 fail). Two minor correctness observations and three test-hygiene NITs left inline. Nothing blocking.

Fix validation

Fix	Status
Case-insensitive duplicate detection (3 dicts + ToDictionary)	✅ Applied correctly. One remaining edge case at validator.cs:62 — see inline.
FormatException catch around EnumerateJsonCommandLineOptions	✅ Mirrors the standard failure path, no double-banner risk. One asymmetry re HasTool — see inline.
[DoesNotReturn] on the two throw helpers	✅ Both helpers carry the attribute.
Rewritten XML doc on TryGetCommandLineOptionArguments	✅ No more stale "validation on parseResult only" wording.

New test adequacy

Test	Verdict
EnumerateCommandLineOptions_SectionNameCaseInsensitive_Honored	✅ Strong assertions (option name + argument value).
Validator_EmptyOptionNameInJson_FailsWithJsonPrefix	⚠️ Only checks "testconfig.json" substring — wouldn't catch a resource string change. Advisory.
Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully	⚠️ Test comment misrepresents pre-fix behavior; assertion only checks IsValid == false. See inline.
Validator_JsonSparseIndexedEntry_DefensiveSchemaRejection	⚠️ Test name doesn't match what's tested. See inline.

Public API hygiene

✅ PublicAPI.Unshipped.txt only has #nullable enable — all new types (JsonCommandLineOptionEntry, CommandLineConfigurationSource, CommandLineConfigurationProvider) and methods (EnumerateJsonCommandLineOptions, TryGetCommandLineOptionArguments, IsCommandLineOptionSet) are correctly internal.
✅ JsonCommandLineOptionEntry uses plain get; properties — no init accessors on any new API.
✅ New internalCommandLineHandler constructor accepting IConfiguration? preserves source/binary compatibility (the existing public constructor delegates with configuration: null).

Summary table

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 LOW (system+system dictionary, FormatException + HasTool)
13	Test Completeness & Coverage	⚪ 1 NIT (weaker substring assertions on 3 new tests)
16	Naming & Conventions	⚪ 1 NIT (Validator_JsonSparseIndexedEntry_* name)
17	Documentation Accuracy	⚪ 1 NIT (test comment misrepresents pre-fix behavior)

✅ 17/21 dimensions clean.

Threading & Concurrency, Security & IPC, Public API & Binary Compatibility, Performance & Allocations, Cross-TFM Compatibility, Resource & IDisposable, Defensive Coding at Boundaries, Localization, Test Isolation, Assertion Quality, Flakiness Patterns, Data-Driven Coverage, Code Structure, Analyzer Quality (N/A), IPC Wire Compatibility (N/A), Build Infrastructure, Scope Discipline — all clean.

Noted for follow-up (out-of-scope for this PR)

• Env-var injection of commandLineOptions:* keys: now that IsCommandLineOptionSet/TryGetCommandLineOptionArguments route through IConfiguration, an env var spelled commandLineOptions__timeout=30s (with __→: normalization in EnvironmentVariablesConfigurationProvider) will silently shadow JSON values and be invisible to CommandLineOptionsValidator (which only sees CLI + JSON entries). This is an existing platform concern that the unification only exposes — not introduced here — but worth tracking.
• Behavioral break for extension authors who intentionally registered case-differing option names (e.g. "Timeout" for one provider, "timeout" for another). They were previously accepted as distinct (silently mis-treated as one by every downstream case-insensitive lookup); they now fail validation up front. This is the correct fix, but worth a CHANGELOG entry if not already planned.

[Amaury Levé (Evangelink)]

Round 3 review — bcb797345

✅ 21/21 dimensions clean — no blocking, medium, or low findings.

Round-2 fixes validation

Fix #1 — ValidateOptionsAreNotDuplicated extended to cover system providers: CORRECT and complete.

I traced every collision shape through the new pass and confirmed no legitimate existing behavior changes:

• Extension-vs-extension (case-differing). Previously crashed with ArgumentException at the providerAndOptionByOptionName.ToDictionary(..., StringComparer.OrdinalIgnoreCase) lookup on line 60-62. Now caught with the friendly "declared by multiple" error. ✅
• Extension-vs-system (any casing). Already caught earlier by ValidateExtensionOptionsDoNotContainReservedOptions (line 107-152), which uses OrdinalIgnoreCase and returns early. The new Concat-over-both-dictionaries iteration is reachable in theory for this case but practically pre-empted. ✅
• System-vs-system (case-differing). Previously crashed at the same ToDictionary lookup. Now caught with the friendly error. ✅ (Pinned by the new Validator_DuplicateOptionNamesAcrossSystemAndSystem_FailsGracefully test.)

I considered whether two providers could legitimately register the same option name (e.g. a system provider exposing a public alias also added by an extension). The contract is that each ICommandLineOptionsProvider.GetCommandLineOptions() returns options it owns, and the pre-existing ValidateExtensionOptionsDoNotContainReservedOptions pass already treats extension-vs-system collisions as errors (any casing). So no legitimate scenario is regressed.

Minor cosmetic observation (NIT, not blocking): line 60 still uses LINQ Union over the two dictionaries, while the new code at line 165 uses Concat. Both produce the same result here (provider keys are reference-equal across the two source dictionaries, and there are no duplicates by reference), but slight stylistic inconsistency. Not worth a follow-up.

---

Fix #2 — FormatException exception filter + rich error display: CORRECT and complete.

• The when (!loggingState.CommandLineParseResult.HasTool) exception filter is standard C# 6+ and works on all four TFMs.
• The silent-degrade-to-empty-list path for tools is consistent with the existing pipeline convention: validation errors are already gated by !HasTool on line 230, so tools that have a structurally-broken commandLineOptions section already wouldn't see validation surface the issue. The FormatException fallback mirrors this.
• I verified HasTool semantics: ParseResult.HasTool => ToolName is not null. --help and --info are regular options handled via IsHelpInvoked / IsInfoInvoked on CommandLineHandler (lines 240-247), not tools. So --help and --info go through the normal !HasTool path and do surface FormatException with the rich InvalidCommandLineArguments header.
• Only server-mode tools (those that set ToolName via the tool entry, e.g. --server) silently degrade. Even for these, the rest of testconfig.json (e.g. results-directory, custom IConfiguration[key] lookups) remains accessible because the throw fires only during the typed-schema enumeration of the commandLineOptions section — direct provider.TryGet calls bypass it.
• The rich-error formatting (StringBuilder + PlatformResources.InvalidCommandLineArguments header + "\t- {ex.Message}" + TrimEnd()) matches CommandLineOptionsValidator (line 23-28). Environment.NewLine is correctly avoided (confirmed via grep). StringBuilder resolves via the global System.Text using in Directory.Build.props.
• DisplayBannerIfEnabledAsync call inside the catch (line 205) safely reads --no-banner via commandLineOptions.IsOptionSet, which routes through provider.TryGet on the already-flattened key-value store, not through EnumerateCommandLineOptions. No re-throw risk.

---

Fresh-pass net-new findings

Nothing blocking, medium, or low. Three honest NIT-level observations, all noted-for-follow-up only:

1. NIT (style) — CommandLineOptionsValidator.cs line 60 uses Union where Concat would be slightly clearer and marginally cheaper (the new code at line 165 already uses Concat).
2. NIT (test redundancy) — Validator_JsonEntryWithTooFewArguments_FailsArityCheck (line 454) overlaps with Validator_JsonArityTooFew_Fails (line 200); only the arity shape differs (ArgumentArity(2,2) vs ExactlyOne). Defensible as defensive coverage of a non-trivial arity.
3. NIT (docs precision) — The comment on Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully (lines 410-415) cites line 62, which will become stale on future edits to the validator. Consider replacing with a symbolic reference ("the OrdinalIgnoreCaseToDictionary lookup").

---

Other dimensions checked clean

• Public API & init accessors — All new types/members are internal. No PublicAPI.Unshipped.txt churn. ✅
• Cross-TFM — Exception filters (C# 6+), StringBuilder, CultureInfo, OrdinalIgnoreCase all available on net462 / netstandard2.0 / net8.0 / net9.0. ✅
• Localization — Two new resx strings (JsonCommandLineOptionsEntryMustBeScalarOrArrayErrorMessage, JsonCommandLineOptionsValidationErrorPrefix) with <comment> metadata. .xlf deltas look auto-generated (stub-per-resource pattern, +10 per language file). No manual xlf edits. ✅
• Threading — All affected code paths run on the single-threaded host startup pipeline. No new shared mutable state. ✅
• IPC wire compatibility — N/A; no serialized type changes. ✅
• Resource management — N/A; no new disposables. ✅
• Defensive coding — Trust boundary (user testconfig.json) correctly guarded; internal invariants (e.g. sparse-indexed defensive guard in JsonConfigurationProvider) correctly preserved. ✅
• Test isolation — Tests use isolated TestProvider instances per [TestMethod]; no shared static mutable state. ✅
• Flakiness — No Thread.Sleep, no wall-clock assertions, no hard-coded ports in new tests. ✅

---

Verdict

Ready to merge. Round-2 fixes are correct, complete, and free of new bugs. The three NIT observations above are stylistic/documentation polish and do not affect users. Items explicitly deferred in the PR description (scalar-bool ambiguity, --diagnostic*/--config-file bootstrap reads, IsCommandLineOptionSet API visibility) remain out of scope for this PR as agreed.

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 31/31 changed files
• Comments generated: 0 new

[Amaury Levé (Evangelink)]

Summary

Resolves part of #6349: let users specify CLI options inside testconfig.json by routing ICommandLineOptions through IConfiguration.

Approach

1. New CommandLineConfigurationSource (Order = 0) wraps the parsed CLI into an IConfigurationProvider that flattens each option under commandLineOptions:<name> (zero-arity) or commandLineOptions:<name>:<index> (arg-bearing).
2. The CLI source is registered in TestHostBuilder.CommonServicesbefore the JSON source, so CLI keeps highest precedence.
3. AggregatedConfiguration.TryGetCommandLineOptionFromProviders does a provider-aware lookup: it walks providers in registration order, and the first provider with any data for that option wins outright (no per-key cross-provider merging). This avoids cases where IConfiguration[key] accidentally returns the JSON :0 while the CLI set the zero-arity bare key, or vice versa.
4. The public extension helpers (IsCommandLineOptionSet, TryGetCommandLineOptionArguments) delegate to that method when given an AggregatedConfiguration, and CommandLineHandler becomes a thin facade that does the same. End result: every consumer of ICommandLineOptions transparently sees JSON-sourced options.
5. JsonConfigurationProvider exposes a typed, schema-validated enumeration of commandLineOptions:* entries. CommandLineOptionsValidator runs three extra passes over those entries (unknown-option, arity, per-arg validation) so testconfig.json typos surface during startup instead of crashing inside option handlers.
6. DisplayBannerIfEnabledAsync reads --no-banner from the unified ICommandLineOptions, so banner suppression honors testconfig.json as well as the CLI.

What works

• testconfig.json keys under commandLineOptions are surfaced everywhere ICommandLineOptions is consumed.
• CLI still wins over JSON for the same option.
• --results-directory defined in JSON is honored (was previously ignored - the legacy GetResultsDirectoryCore path was bypassing JSON).
• --no-banner defined in JSON suppresses the banner.
• Invalid JSON entries (unknown option, wrong arity, bad arg) fail at startup with a clear error referencing testconfig.json, instead of crashing later. Concrete cases now caught:
  • --timeout: true (was: IndexOutOfRangeException in args[0])
  • --exit-on-process-exit: "abc" (was: FormatException in int.Parse(args[0]))
  • typos like "timeoutt": "30s" (was: silently ignored)
• Back-compat ctor on CommandLineHandler is kept so external code keeps compiling.

Rough edges still on the table

These are not addressed in this PR:

1. Bootstrap readers.--diagnostic* and --config-file are read off parseResultbeforeIConfiguration is built. Honoring them from JSON requires either a two-phase config build or rewriting those bootstrap paths.
2. Scalar bool ambiguity. For arg-bearing options, JSON "true" / "false" is ambiguous with the zero-arity presence marker. Callers must use the array form ("foo": ["true"]) for non-boolean string values that happen to be "true" / "false". No code enforces this today.
3. API visibility. New helpers are kept internal for now. If we ship this we need to decide whether they become public surface.

Tests

• CommandLineConfigurationProviderTests covers the source/provider flattening + the provider-aware invariants (including ProviderAwareResolution_CliZeroArityShadowsJsonIndexedArgs end-to-end).
• CommandLineConfigurationExtensionsTests covers the helper-level and handler-end-to-end behavior.
• JsonCommandLineOptionsTests (new) covers the JSON enumeration schema, the three new validator passes, disabled entries, shadowing, and case-insensitivity.
• Full UT project green on net8.0 / net9.0 / net462.

[Copilot]

Pull request overview

This prototype routes command-line options through the platform IConfiguration model so testconfig.json can provide values consumed by existing ICommandLineOptions users, while preserving CLI precedence.

Changes:

• Adds a CLI-backed configuration source/provider under commandLineOptions:*.
• Adds provider-aware command-line option lookup helpers on AggregatedConfiguration/ConfigurationExtensions.
• Wires CommandLineHandler to use the unified configuration view and adds unit coverage for precedence and lookup behavior.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Registers the CLI configuration source and passes built configuration into command-line handling.
src/Platform/Microsoft.Testing.Platform/Configurations/PlatformConfigurationConstants.cs	Adds the commandLineOptions section constant.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationExtensions.cs	Adds unified command-line option lookup helpers.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationSource.cs	Adds the configuration source for parsed CLI options.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationProvider.cs	Flattens parsed CLI options into configuration keys.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs	Adds provider-aware option resolution and uses it for results-directory.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs	Documents current validation gaps for JSON-sourced options.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineManager.cs	Passes configuration into CommandLineHandler.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineHandler.cs	Delegates option reads to configuration when available.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationProviderTests.cs	Adds tests for CLI provider flattening and precedence behavior.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationExtensionsTests.cs	Adds tests for helper and handler behavior over configuration-backed options.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Round-2 review summary

All four round-1 fixes are correctly applied and the build is clean (0 warnings, 0 errors; 1034 pass / 0 fail). Two minor correctness observations and three test-hygiene NITs left inline. Nothing blocking.

Fix validation

Fix	Status
Case-insensitive duplicate detection (3 dicts + ToDictionary)	✅ Applied correctly. One remaining edge case at validator.cs:62 — see inline.
FormatException catch around EnumerateJsonCommandLineOptions	✅ Mirrors the standard failure path, no double-banner risk. One asymmetry re HasTool — see inline.
[DoesNotReturn] on the two throw helpers	✅ Both helpers carry the attribute.
Rewritten XML doc on TryGetCommandLineOptionArguments	✅ No more stale "validation on parseResult only" wording.

New test adequacy

Test	Verdict
EnumerateCommandLineOptions_SectionNameCaseInsensitive_Honored	✅ Strong assertions (option name + argument value).
Validator_EmptyOptionNameInJson_FailsWithJsonPrefix	⚠️ Only checks "testconfig.json" substring — wouldn't catch a resource string change. Advisory.
Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully	⚠️ Test comment misrepresents pre-fix behavior; assertion only checks IsValid == false. See inline.
Validator_JsonSparseIndexedEntry_DefensiveSchemaRejection	⚠️ Test name doesn't match what's tested. See inline.

Public API hygiene

✅ PublicAPI.Unshipped.txt only has #nullable enable — all new types (JsonCommandLineOptionEntry, CommandLineConfigurationSource, CommandLineConfigurationProvider) and methods (EnumerateJsonCommandLineOptions, TryGetCommandLineOptionArguments, IsCommandLineOptionSet) are correctly internal.
✅ JsonCommandLineOptionEntry uses plain get; properties — no init accessors on any new API.
✅ New internalCommandLineHandler constructor accepting IConfiguration? preserves source/binary compatibility (the existing public constructor delegates with configuration: null).

Summary table

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 LOW (system+system dictionary, FormatException + HasTool)
13	Test Completeness & Coverage	⚪ 1 NIT (weaker substring assertions on 3 new tests)
16	Naming & Conventions	⚪ 1 NIT (Validator_JsonSparseIndexedEntry_* name)
17	Documentation Accuracy	⚪ 1 NIT (test comment misrepresents pre-fix behavior)

✅ 17/21 dimensions clean.

Threading & Concurrency, Security & IPC, Public API & Binary Compatibility, Performance & Allocations, Cross-TFM Compatibility, Resource & IDisposable, Defensive Coding at Boundaries, Localization, Test Isolation, Assertion Quality, Flakiness Patterns, Data-Driven Coverage, Code Structure, Analyzer Quality (N/A), IPC Wire Compatibility (N/A), Build Infrastructure, Scope Discipline — all clean.

Noted for follow-up (out-of-scope for this PR)

• Env-var injection of commandLineOptions:* keys: now that IsCommandLineOptionSet/TryGetCommandLineOptionArguments route through IConfiguration, an env var spelled commandLineOptions__timeout=30s (with __→: normalization in EnvironmentVariablesConfigurationProvider) will silently shadow JSON values and be invisible to CommandLineOptionsValidator (which only sees CLI + JSON entries). This is an existing platform concern that the unification only exposes — not introduced here — but worth tracking.
• Behavioral break for extension authors who intentionally registered case-differing option names (e.g. "Timeout" for one provider, "timeout" for another). They were previously accepted as distinct (silently mis-treated as one by every downstream case-insensitive lookup); they now fail validation up front. This is the correct fix, but worth a CHANGELOG entry if not already planned.

[Amaury Levé (Evangelink)]

Round 3 review — bcb797345

✅ 21/21 dimensions clean — no blocking, medium, or low findings.

Round-2 fixes validation

Fix #1 — ValidateOptionsAreNotDuplicated extended to cover system providers: CORRECT and complete.

I traced every collision shape through the new pass and confirmed no legitimate existing behavior changes:

• Extension-vs-extension (case-differing). Previously crashed with ArgumentException at the providerAndOptionByOptionName.ToDictionary(..., StringComparer.OrdinalIgnoreCase) lookup on line 60-62. Now caught with the friendly "declared by multiple" error. ✅
• Extension-vs-system (any casing). Already caught earlier by ValidateExtensionOptionsDoNotContainReservedOptions (line 107-152), which uses OrdinalIgnoreCase and returns early. The new Concat-over-both-dictionaries iteration is reachable in theory for this case but practically pre-empted. ✅
• System-vs-system (case-differing). Previously crashed at the same ToDictionary lookup. Now caught with the friendly error. ✅ (Pinned by the new Validator_DuplicateOptionNamesAcrossSystemAndSystem_FailsGracefully test.)

I considered whether two providers could legitimately register the same option name (e.g. a system provider exposing a public alias also added by an extension). The contract is that each ICommandLineOptionsProvider.GetCommandLineOptions() returns options it owns, and the pre-existing ValidateExtensionOptionsDoNotContainReservedOptions pass already treats extension-vs-system collisions as errors (any casing). So no legitimate scenario is regressed.

Minor cosmetic observation (NIT, not blocking): line 60 still uses LINQ Union over the two dictionaries, while the new code at line 165 uses Concat. Both produce the same result here (provider keys are reference-equal across the two source dictionaries, and there are no duplicates by reference), but slight stylistic inconsistency. Not worth a follow-up.

---

Fix #2 — FormatException exception filter + rich error display: CORRECT and complete.

• The when (!loggingState.CommandLineParseResult.HasTool) exception filter is standard C# 6+ and works on all four TFMs.
• The silent-degrade-to-empty-list path for tools is consistent with the existing pipeline convention: validation errors are already gated by !HasTool on line 230, so tools that have a structurally-broken commandLineOptions section already wouldn't see validation surface the issue. The FormatException fallback mirrors this.
• I verified HasTool semantics: ParseResult.HasTool => ToolName is not null. --help and --info are regular options handled via IsHelpInvoked / IsInfoInvoked on CommandLineHandler (lines 240-247), not tools. So --help and --info go through the normal !HasTool path and do surface FormatException with the rich InvalidCommandLineArguments header.
• Only server-mode tools (those that set ToolName via the tool entry, e.g. --server) silently degrade. Even for these, the rest of testconfig.json (e.g. results-directory, custom IConfiguration[key] lookups) remains accessible because the throw fires only during the typed-schema enumeration of the commandLineOptions section — direct provider.TryGet calls bypass it.
• The rich-error formatting (StringBuilder + PlatformResources.InvalidCommandLineArguments header + "\t- {ex.Message}" + TrimEnd()) matches CommandLineOptionsValidator (line 23-28). Environment.NewLine is correctly avoided (confirmed via grep). StringBuilder resolves via the global System.Text using in Directory.Build.props.
• DisplayBannerIfEnabledAsync call inside the catch (line 205) safely reads --no-banner via commandLineOptions.IsOptionSet, which routes through provider.TryGet on the already-flattened key-value store, not through EnumerateCommandLineOptions. No re-throw risk.

---

Fresh-pass net-new findings

Nothing blocking, medium, or low. Three honest NIT-level observations, all noted-for-follow-up only:

1. NIT (style) — CommandLineOptionsValidator.cs line 60 uses Union where Concat would be slightly clearer and marginally cheaper (the new code at line 165 already uses Concat).
2. NIT (test redundancy) — Validator_JsonEntryWithTooFewArguments_FailsArityCheck (line 454) overlaps with Validator_JsonArityTooFew_Fails (line 200); only the arity shape differs (ArgumentArity(2,2) vs ExactlyOne). Defensible as defensive coverage of a non-trivial arity.
3. NIT (docs precision) — The comment on Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully (lines 410-415) cites line 62, which will become stale on future edits to the validator. Consider replacing with a symbolic reference ("the OrdinalIgnoreCaseToDictionary lookup").

---

Other dimensions checked clean

• Public API & init accessors — All new types/members are internal. No PublicAPI.Unshipped.txt churn. ✅
• Cross-TFM — Exception filters (C# 6+), StringBuilder, CultureInfo, OrdinalIgnoreCase all available on net462 / netstandard2.0 / net8.0 / net9.0. ✅
• Localization — Two new resx strings (JsonCommandLineOptionsEntryMustBeScalarOrArrayErrorMessage, JsonCommandLineOptionsValidationErrorPrefix) with <comment> metadata. .xlf deltas look auto-generated (stub-per-resource pattern, +10 per language file). No manual xlf edits. ✅
• Threading — All affected code paths run on the single-threaded host startup pipeline. No new shared mutable state. ✅
• IPC wire compatibility — N/A; no serialized type changes. ✅
• Resource management — N/A; no new disposables. ✅
• Defensive coding — Trust boundary (user testconfig.json) correctly guarded; internal invariants (e.g. sparse-indexed defensive guard in JsonConfigurationProvider) correctly preserved. ✅
• Test isolation — Tests use isolated TestProvider instances per [TestMethod]; no shared static mutable state. ✅
• Flakiness — No Thread.Sleep, no wall-clock assertions, no hard-coded ports in new tests. ✅

---

Verdict

Ready to merge. Round-2 fixes are correct, complete, and free of new bugs. The three NIT observations above are stylistic/documentation polish and do not affect users. Items explicitly deferred in the PR description (scalar-bool ambiguity, --diagnostic*/--config-file bootstrap reads, IsCommandLineOptionSet API visibility) remain out of scope for this PR as agreed.

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 31/31 changed files
• Comments generated: 0 new

[Amaury Levé (Evangelink)]

Summary

Resolves part of #6349: let users specify CLI options inside testconfig.json by routing ICommandLineOptions through IConfiguration.

Approach

1. New CommandLineConfigurationSource (Order = 0) wraps the parsed CLI into an IConfigurationProvider that flattens each option under commandLineOptions:<name> (zero-arity) or commandLineOptions:<name>:<index> (arg-bearing).
2. The CLI source is registered in TestHostBuilder.CommonServicesbefore the JSON source, so CLI keeps highest precedence.
3. AggregatedConfiguration.TryGetCommandLineOptionFromProviders does a provider-aware lookup: it walks providers in registration order, and the first provider with any data for that option wins outright (no per-key cross-provider merging). This avoids cases where IConfiguration[key] accidentally returns the JSON :0 while the CLI set the zero-arity bare key, or vice versa.
4. The public extension helpers (IsCommandLineOptionSet, TryGetCommandLineOptionArguments) delegate to that method when given an AggregatedConfiguration, and CommandLineHandler becomes a thin facade that does the same. End result: every consumer of ICommandLineOptions transparently sees JSON-sourced options.
5. JsonConfigurationProvider exposes a typed, schema-validated enumeration of commandLineOptions:* entries. CommandLineOptionsValidator runs three extra passes over those entries (unknown-option, arity, per-arg validation) so testconfig.json typos surface during startup instead of crashing inside option handlers.
6. DisplayBannerIfEnabledAsync reads --no-banner from the unified ICommandLineOptions, so banner suppression honors testconfig.json as well as the CLI.

What works

• testconfig.json keys under commandLineOptions are surfaced everywhere ICommandLineOptions is consumed.
• CLI still wins over JSON for the same option.
• --results-directory defined in JSON is honored (was previously ignored - the legacy GetResultsDirectoryCore path was bypassing JSON).
• --no-banner defined in JSON suppresses the banner.
• Invalid JSON entries (unknown option, wrong arity, bad arg) fail at startup with a clear error referencing testconfig.json, instead of crashing later. Concrete cases now caught:
  • --timeout: true (was: IndexOutOfRangeException in args[0])
  • --exit-on-process-exit: "abc" (was: FormatException in int.Parse(args[0]))
  • typos like "timeoutt": "30s" (was: silently ignored)
• Back-compat ctor on CommandLineHandler is kept so external code keeps compiling.

Rough edges still on the table

These are not addressed in this PR:

1. Bootstrap readers.--diagnostic* and --config-file are read off parseResultbeforeIConfiguration is built. Honoring them from JSON requires either a two-phase config build or rewriting those bootstrap paths.
2. Scalar bool ambiguity. For arg-bearing options, JSON "true" / "false" is ambiguous with the zero-arity presence marker. Callers must use the array form ("foo": ["true"]) for non-boolean string values that happen to be "true" / "false". No code enforces this today.
3. API visibility. New helpers are kept internal for now. If we ship this we need to decide whether they become public surface.

Tests

• CommandLineConfigurationProviderTests covers the source/provider flattening + the provider-aware invariants (including ProviderAwareResolution_CliZeroArityShadowsJsonIndexedArgs end-to-end).
• CommandLineConfigurationExtensionsTests covers the helper-level and handler-end-to-end behavior.
• JsonCommandLineOptionsTests (new) covers the JSON enumeration schema, the three new validator passes, disabled entries, shadowing, and case-insensitivity.
• Full UT project green on net8.0 / net9.0 / net462.

[Copilot]

Pull request overview

This prototype routes command-line options through the platform IConfiguration model so testconfig.json can provide values consumed by existing ICommandLineOptions users, while preserving CLI precedence.

Changes:

• Adds a CLI-backed configuration source/provider under commandLineOptions:*.
• Adds provider-aware command-line option lookup helpers on AggregatedConfiguration/ConfigurationExtensions.
• Wires CommandLineHandler to use the unified configuration view and adds unit coverage for precedence and lookup behavior.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Registers the CLI configuration source and passes built configuration into command-line handling.
src/Platform/Microsoft.Testing.Platform/Configurations/PlatformConfigurationConstants.cs	Adds the commandLineOptions section constant.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationExtensions.cs	Adds unified command-line option lookup helpers.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationSource.cs	Adds the configuration source for parsed CLI options.
src/Platform/Microsoft.Testing.Platform/Configurations/CommandLineConfigurationProvider.cs	Flattens parsed CLI options into configuration keys.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs	Adds provider-aware option resolution and uses it for results-directory.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs	Documents current validation gaps for JSON-sourced options.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineManager.cs	Passes configuration into CommandLineHandler.
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineHandler.cs	Delegates option reads to configuration when available.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationProviderTests.cs	Adds tests for CLI provider flattening and precedence behavior.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/CommandLineConfigurationExtensionsTests.cs	Adds tests for helper and handler behavior over configuration-backed options.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Round-2 review summary

All four round-1 fixes are correctly applied and the build is clean (0 warnings, 0 errors; 1034 pass / 0 fail). Two minor correctness observations and three test-hygiene NITs left inline. Nothing blocking.

Fix validation

Fix	Status
Case-insensitive duplicate detection (3 dicts + ToDictionary)	✅ Applied correctly. One remaining edge case at validator.cs:62 — see inline.
FormatException catch around EnumerateJsonCommandLineOptions	✅ Mirrors the standard failure path, no double-banner risk. One asymmetry re HasTool — see inline.
[DoesNotReturn] on the two throw helpers	✅ Both helpers carry the attribute.
Rewritten XML doc on TryGetCommandLineOptionArguments	✅ No more stale "validation on parseResult only" wording.

New test adequacy

Test	Verdict
EnumerateCommandLineOptions_SectionNameCaseInsensitive_Honored	✅ Strong assertions (option name + argument value).
Validator_EmptyOptionNameInJson_FailsWithJsonPrefix	⚠️ Only checks "testconfig.json" substring — wouldn't catch a resource string change. Advisory.
Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully	⚠️ Test comment misrepresents pre-fix behavior; assertion only checks IsValid == false. See inline.
Validator_JsonSparseIndexedEntry_DefensiveSchemaRejection	⚠️ Test name doesn't match what's tested. See inline.

Public API hygiene

✅ PublicAPI.Unshipped.txt only has #nullable enable — all new types (JsonCommandLineOptionEntry, CommandLineConfigurationSource, CommandLineConfigurationProvider) and methods (EnumerateJsonCommandLineOptions, TryGetCommandLineOptionArguments, IsCommandLineOptionSet) are correctly internal.
✅ JsonCommandLineOptionEntry uses plain get; properties — no init accessors on any new API.
✅ New internalCommandLineHandler constructor accepting IConfiguration? preserves source/binary compatibility (the existing public constructor delegates with configuration: null).

Summary table

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 LOW (system+system dictionary, FormatException + HasTool)
13	Test Completeness & Coverage	⚪ 1 NIT (weaker substring assertions on 3 new tests)
16	Naming & Conventions	⚪ 1 NIT (Validator_JsonSparseIndexedEntry_* name)
17	Documentation Accuracy	⚪ 1 NIT (test comment misrepresents pre-fix behavior)

✅ 17/21 dimensions clean.

Threading & Concurrency, Security & IPC, Public API & Binary Compatibility, Performance & Allocations, Cross-TFM Compatibility, Resource & IDisposable, Defensive Coding at Boundaries, Localization, Test Isolation, Assertion Quality, Flakiness Patterns, Data-Driven Coverage, Code Structure, Analyzer Quality (N/A), IPC Wire Compatibility (N/A), Build Infrastructure, Scope Discipline — all clean.

Noted for follow-up (out-of-scope for this PR)

• Env-var injection of commandLineOptions:* keys: now that IsCommandLineOptionSet/TryGetCommandLineOptionArguments route through IConfiguration, an env var spelled commandLineOptions__timeout=30s (with __→: normalization in EnvironmentVariablesConfigurationProvider) will silently shadow JSON values and be invisible to CommandLineOptionsValidator (which only sees CLI + JSON entries). This is an existing platform concern that the unification only exposes — not introduced here — but worth tracking.
• Behavioral break for extension authors who intentionally registered case-differing option names (e.g. "Timeout" for one provider, "timeout" for another). They were previously accepted as distinct (silently mis-treated as one by every downstream case-insensitive lookup); they now fail validation up front. This is the correct fix, but worth a CHANGELOG entry if not already planned.

[Amaury Levé (Evangelink)]

Round 3 review — bcb797345

✅ 21/21 dimensions clean — no blocking, medium, or low findings.

Round-2 fixes validation

Fix #1 — ValidateOptionsAreNotDuplicated extended to cover system providers: CORRECT and complete.

I traced every collision shape through the new pass and confirmed no legitimate existing behavior changes:

• Extension-vs-extension (case-differing). Previously crashed with ArgumentException at the providerAndOptionByOptionName.ToDictionary(..., StringComparer.OrdinalIgnoreCase) lookup on line 60-62. Now caught with the friendly "declared by multiple" error. ✅
• Extension-vs-system (any casing). Already caught earlier by ValidateExtensionOptionsDoNotContainReservedOptions (line 107-152), which uses OrdinalIgnoreCase and returns early. The new Concat-over-both-dictionaries iteration is reachable in theory for this case but practically pre-empted. ✅
• System-vs-system (case-differing). Previously crashed at the same ToDictionary lookup. Now caught with the friendly error. ✅ (Pinned by the new Validator_DuplicateOptionNamesAcrossSystemAndSystem_FailsGracefully test.)

I considered whether two providers could legitimately register the same option name (e.g. a system provider exposing a public alias also added by an extension). The contract is that each ICommandLineOptionsProvider.GetCommandLineOptions() returns options it owns, and the pre-existing ValidateExtensionOptionsDoNotContainReservedOptions pass already treats extension-vs-system collisions as errors (any casing). So no legitimate scenario is regressed.

Minor cosmetic observation (NIT, not blocking): line 60 still uses LINQ Union over the two dictionaries, while the new code at line 165 uses Concat. Both produce the same result here (provider keys are reference-equal across the two source dictionaries, and there are no duplicates by reference), but slight stylistic inconsistency. Not worth a follow-up.

---

Fix #2 — FormatException exception filter + rich error display: CORRECT and complete.

• The when (!loggingState.CommandLineParseResult.HasTool) exception filter is standard C# 6+ and works on all four TFMs.
• The silent-degrade-to-empty-list path for tools is consistent with the existing pipeline convention: validation errors are already gated by !HasTool on line 230, so tools that have a structurally-broken commandLineOptions section already wouldn't see validation surface the issue. The FormatException fallback mirrors this.
• I verified HasTool semantics: ParseResult.HasTool => ToolName is not null. --help and --info are regular options handled via IsHelpInvoked / IsInfoInvoked on CommandLineHandler (lines 240-247), not tools. So --help and --info go through the normal !HasTool path and do surface FormatException with the rich InvalidCommandLineArguments header.
• Only server-mode tools (those that set ToolName via the tool entry, e.g. --server) silently degrade. Even for these, the rest of testconfig.json (e.g. results-directory, custom IConfiguration[key] lookups) remains accessible because the throw fires only during the typed-schema enumeration of the commandLineOptions section — direct provider.TryGet calls bypass it.
• The rich-error formatting (StringBuilder + PlatformResources.InvalidCommandLineArguments header + "\t- {ex.Message}" + TrimEnd()) matches CommandLineOptionsValidator (line 23-28). Environment.NewLine is correctly avoided (confirmed via grep). StringBuilder resolves via the global System.Text using in Directory.Build.props.
• DisplayBannerIfEnabledAsync call inside the catch (line 205) safely reads --no-banner via commandLineOptions.IsOptionSet, which routes through provider.TryGet on the already-flattened key-value store, not through EnumerateCommandLineOptions. No re-throw risk.

---

Fresh-pass net-new findings

Nothing blocking, medium, or low. Three honest NIT-level observations, all noted-for-follow-up only:

1. NIT (style) — CommandLineOptionsValidator.cs line 60 uses Union where Concat would be slightly clearer and marginally cheaper (the new code at line 165 already uses Concat).
2. NIT (test redundancy) — Validator_JsonEntryWithTooFewArguments_FailsArityCheck (line 454) overlaps with Validator_JsonArityTooFew_Fails (line 200); only the arity shape differs (ArgumentArity(2,2) vs ExactlyOne). Defensible as defensive coverage of a non-trivial arity.
3. NIT (docs precision) — The comment on Validator_DuplicateOptionNamesDifferingByCase_FailsGracefully (lines 410-415) cites line 62, which will become stale on future edits to the validator. Consider replacing with a symbolic reference ("the OrdinalIgnoreCaseToDictionary lookup").

---

Other dimensions checked clean

• Public API & init accessors — All new types/members are internal. No PublicAPI.Unshipped.txt churn. ✅
• Cross-TFM — Exception filters (C# 6+), StringBuilder, CultureInfo, OrdinalIgnoreCase all available on net462 / netstandard2.0 / net8.0 / net9.0. ✅
• Localization — Two new resx strings (JsonCommandLineOptionsEntryMustBeScalarOrArrayErrorMessage, JsonCommandLineOptionsValidationErrorPrefix) with <comment> metadata. .xlf deltas look auto-generated (stub-per-resource pattern, +10 per language file). No manual xlf edits. ✅
• Threading — All affected code paths run on the single-threaded host startup pipeline. No new shared mutable state. ✅
• IPC wire compatibility — N/A; no serialized type changes. ✅
• Resource management — N/A; no new disposables. ✅
• Defensive coding — Trust boundary (user testconfig.json) correctly guarded; internal invariants (e.g. sparse-indexed defensive guard in JsonConfigurationProvider) correctly preserved. ✅
• Test isolation — Tests use isolated TestProvider instances per [TestMethod]; no shared static mutable state. ✅
• Flakiness — No Thread.Sleep, no wall-clock assertions, no hard-coded ports in new tests. ✅

---

Verdict

Ready to merge. Round-2 fixes are correct, complete, and free of new bugs. The three NIT observations above are stylistic/documentation polish and do not affect users. Items explicitly deferred in the PR description (scalar-bool ambiguity, --diagnostic*/--config-file bootstrap reads, IsCommandLineOptionSet API visibility) remain out of scope for this PR as agreed.

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 29/29 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 31/31 changed files
• Comments generated: 0 new