Auto-promote scalar JSON values for arg-bearing CLI options (fixes #8830) (#8836)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: 3 people

src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using Microsoft.Testing.Platform.CommandLine;
+using Microsoft.Testing.Platform.Extensions.CommandLine;
 using Microsoft.Testing.Platform.Helpers;
 using Microsoft.Testing.Platform.Logging;
 using Microsoft.Testing.Platform.Services;
@@ -170,6 +171,28 @@ internal IReadOnlyList<JsonCommandLineOptionEntry> EnumerateJsonCommandLineOptio
         return jsonProvider?.EnumerateCommandLineOptions() ?? [];
     }
 
+    /// <summary>
+    /// Normalizes JSON-sourced scalar command-line option entries to the indexed shape for any
+    /// option in <paramref name="optionByName"/> with <see cref="ArgumentArity.Min"/> &gt;= 1,
+    /// so that subsequent <see cref="TryGetCommandLineOptionFromProviders"/> and validator passes
+    /// see a uniform representation. See
+    /// <see cref="JsonConfigurationSource.JsonConfigurationProvider.NormalizeCommandLineOptionScalars"/>
+    /// for the full rationale.
+    /// </summary>
+    /// <remarks>
+    /// A no-op when no JSON configuration source is registered. Safe to call at most once after
+    /// the option registry is fully assembled and before any consumer reads command-line option
+    /// values from <see cref="IConfiguration"/>.
+    /// </remarks>
+    internal void NormalizeJsonCommandLineOptionScalars(IReadOnlyDictionary<string, CommandLineOption> optionByName)
+    {
+        JsonConfigurationSource.JsonConfigurationProvider? jsonProvider = _configurationProviders
+            .OfType<JsonConfigurationSource.JsonConfigurationProvider>()
+            .FirstOrDefault();
+
+        jsonProvider?.NormalizeCommandLineOptionScalars(optionByName);
+    }
+
     public async Task CheckTestResultsDirectoryOverrideAndCreateItAsync(IFileLoggerProvider? fileLoggerProvider)
     {
         _resultsDirectory = _fileSystem.CreateDirectory(this[PlatformConfigurationConstants.PlatformResultDirectory]!);

src/Platform/Microsoft.Testing.Platform/Configurations/JsonConfigurationProvider.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using Microsoft.Testing.Platform.CommandLine;
+using Microsoft.Testing.Platform.Extensions.CommandLine;
 using Microsoft.Testing.Platform.Helpers;
 using Microsoft.Testing.Platform.Logging;
 using Microsoft.Testing.Platform.Resources;
@@ -410,6 +411,108 @@ internal IReadOnlyList<JsonCommandLineOptionEntry> EnumerateCommandLineOptions()
             return result;
         }
 
+        /// <summary>
+        /// Rewrites scalar <c>commandLineOptions:&lt;name&gt;</c> entries to the indexed shape
+        /// (<c>commandLineOptions:&lt;name&gt;:0</c>) for options registered with
+        /// <see cref="ArgumentArity.Min"/> &gt;= 1, using <paramref name="optionByName"/> as the
+        /// option registry.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// The CLI-backed configuration provider stores zero-arity flags at the bare option key and
+        /// arg-bearing options under indexed keys (one per argument). The JSON provider cannot
+        /// distinguish these shapes at parse time because the user freely writes either
+        /// <c>"foo": "value"</c> (scalar) or <c>"foo": ["value"]</c> (array). For arg-bearing
+        /// options that always require at least one argument, a scalar value is always the first
+        /// argument — never a presence marker. Storing it under the indexed key normalizes the
+        /// shape so both <see cref="EnumerateCommandLineOptions"/> and
+        /// <see cref="AggregatedConfiguration.TryGetCommandLineOptionFromProviders"/> see one
+        /// consistent representation.
+        /// </para>
+        /// <para>
+        /// In particular, this fixes the JSON scalar/bool ambiguity from #6349/#8830: a value like
+        /// <c>"my-option": "true"</c> for an arg-bearing option used to be misinterpreted as a
+        /// presence marker (zero arguments) instead of being passed as the first argument value.
+        /// </para>
+        /// <para>
+        /// Optional-arg options (<c>Min == 0 &amp;&amp; Max &gt;= 1</c>) are left untouched because
+        /// either interpretation (presence vs. scalar argument) is semantically valid; users who
+        /// need to disambiguate should use the explicit array form.
+        /// </para>
+        /// </remarks>
+        internal void NormalizeCommandLineOptionScalars(IReadOnlyDictionary<string, CommandLineOption> optionByName)
+        {
+            if (_singleValueData is null || _singleValueData.Count == 0)
+            {
+                return;
+            }
+
+            const string sectionName = PlatformConfigurationConstants.CommandLineOptionsSectionName;
+            string sectionPrefix = sectionName + PlatformConfigurationConstants.KeyDelimiter;
+
+            List<(string OldKey, string NewKey, string Value)>? rewrites = null;
+
+            foreach (KeyValuePair<string, string?> kvp in _singleValueData)
+            {
+                if (!kvp.Key.StartsWith(sectionPrefix, StringComparison.OrdinalIgnoreCase))
+                {
+                    continue;
+                }
+
+                // Bare key only: "commandLineOptions:<name>" with no further colon. Indexed entries
+                // already use the canonical shape and are not subject to the scalar/bool ambiguity.
+                string remainder = kvp.Key.Substring(sectionPrefix.Length);
+                if (remainder.Length == 0
+                    || remainder.IndexOf(PlatformConfigurationConstants.KeyDelimiter, StringComparison.Ordinal) >= 0)
+                {
+                    continue;
+                }
+
+                // A null value at the bare key represents an empty object/array; the schema
+                // validator in EnumerateCommandLineOptions will reject it later. Skip here so we
+                // never promote a placeholder to an indexed slot.
+                if (kvp.Value is null)
+                {
+                    continue;
+                }
+
+                if (!optionByName.TryGetValue(remainder, out CommandLineOption? option))
+                {
+                    // Unknown option name. Leave alone so the unknown-option validator pass can
+                    // surface a clear error referencing testconfig.json.
+                    continue;
+                }
+
+                if (option.Arity.Min < 1)
+                {
+                    continue;
+                }
+
+                string newKey = kvp.Key + PlatformConfigurationConstants.KeyDelimiter + "0";
+
+                // Defensive: if the indexed slot already exists, the JSON contained both a scalar
+                // and an array entry for the same option, which the schema validator will flag as
+                // malformed. Don't clobber.
+                if (_singleValueData.ContainsKey(newKey))
+                {
+                    continue;
+                }
+
+                (rewrites ??= []).Add((kvp.Key, newKey, kvp.Value));
+            }
+
+            if (rewrites is null)
+            {
+                return;
+            }
+
+            foreach ((string oldKey, string newKey, string value) in rewrites)
+            {
+                _singleValueData.Remove(oldKey);
+                _singleValueData[newKey] = value;
+            }
+        }
+
         private static bool StartsWithChar(string text, char c)
         {
             for (int i = 0; i < text.Length; i++)

src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs

@@ -5,6 +5,7 @@
 using Microsoft.Testing.Platform.CommandLine;
 using Microsoft.Testing.Platform.Configurations;
 using Microsoft.Testing.Platform.Extensions;
+using Microsoft.Testing.Platform.Extensions.CommandLine;
 using Microsoft.Testing.Platform.Helpers;
 using Microsoft.Testing.Platform.Logging;
 using Microsoft.Testing.Platform.OutputDevice;
@@ -207,6 +208,29 @@ private async Task<BuildContext> SetupCommonServicesAsync(
         IReadOnlyList<JsonCommandLineOptionEntry> jsonCommandLineOptions;
         try
         {
+            // Normalize JSON-sourced scalar option entries to the indexed shape for arg-bearing
+            // options before any consumer reads them. This makes "foo": "value" in testconfig.json
+            // behave identically to "foo": ["value"], removing the foot-gun where a scalar like
+            // "timeout": "true" was misinterpreted as a presence marker. See #6349/#8830.
+            //
+            // Defensive: option providers may register duplicate names; CommandLineOptionsValidator
+            // catches that later (ValidateOptionsAreNotDuplicated) and reports a clear error. Skip
+            // duplicates here so the normalization step itself doesn't crash for that malformed setup.
+            var optionByName = new Dictionary<string, CommandLineOption>(StringComparer.OrdinalIgnoreCase);
+            foreach (ICommandLineOptionsProvider optionsProvider in context.CommandLineHandler.SystemCommandLineOptionsProviders
+                .Concat(context.CommandLineHandler.ExtensionsCommandLineOptionsProviders))
+            {
+                foreach (CommandLineOption option in optionsProvider.GetCommandLineOptions())
+                {
+                    if (!optionByName.ContainsKey(option.Name))
+                    {
+                        optionByName.Add(option.Name, option);
+                    }
+                }
+            }
+
+            context.Configuration.NormalizeJsonCommandLineOptionScalars(optionByName);
+
             jsonCommandLineOptions = context.Configuration.EnumerateJsonCommandLineOptions();
         }
         catch (FormatException ex) when (!loggingState.CommandLineParseResult.HasTool)