From 32ff7612640bf6fdf2a599a9f6201859e205e8e8 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 2 Sep 2026 15:19:37 -0700 Subject: [PATCH 01/10] Let human-readable output be routed to stdout Program.cs builds the single IAnsiConsole over Console.Error, so every human-facing message - success ticks, status lines, tables, and everything from CustomSpectreConsoleLogger - goes to stderr. Azure DevOps renders any stderr line as ##[error], so a fully successful `msstore publish` reports as a failed or partially failed release stage. Setting `failOnStderr: false` stops the failure but not the error rendering (microsoft/azure-pipelines-tasks#16825), so this cannot be documented away. The stderr routing is deliberate: stdout is reserved for machine-readable payloads, the 19 StandardOutput.WriteLine JSON sites plus the `package` output path. Flipping the default would break `msstore submission get | ConvertFrom-Json` and `$(msstore package)`, so this is opt-in instead. Add a global `--output-stream ` option, backed by MSSTORE_OUTPUT_STREAM so a pipeline can opt in once at job scope. Resolution is option > environment variable > stderr; the option deliberately wins so that a job-wide environment variable can be overridden back to stderr on the individual commands that emit a payload. StandardOutput is untouched, so payloads always go to stdout either way. The console has to exist before the command line is parsed, because the host builder needs it in the service collection, so OutputStreamResolver reads the raw args the same way `--verbose` already does. The option is still registered on every command, otherwise the parser rejects the token. Interactivity now probes whichever stream is in use rather than always probing stderr. Fixes #161 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- .../OutputStreamUnitTests.cs | 194 ++++++++++++++++++ MSStore.CLI/Helpers/OutputStream.cs | 27 +++ MSStore.CLI/Helpers/OutputStreamResolver.cs | 131 ++++++++++++ MSStore.CLI/Helpers/StandardOutput.cs | 7 + MSStore.CLI/MicrosoftStoreCLI.cs | 7 + MSStore.CLI/Program.cs | 11 +- MSStore.CLI/Services/EnvironmentInfo.cs | 3 + MSStore.CLI/StoreHostBuilderExtensions.cs | 2 + README.md | 41 ++++ 9 files changed, 421 insertions(+), 2 deletions(-) create mode 100644 MSStore.CLI.UnitTests/OutputStreamUnitTests.cs create mode 100644 MSStore.CLI/Helpers/OutputStream.cs create mode 100644 MSStore.CLI/Helpers/OutputStreamResolver.cs diff --git a/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs new file mode 100644 index 0000000..31ee514 --- /dev/null +++ b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs @@ -0,0 +1,194 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using MSStore.CLI.Helpers; +using MSStore.CLI.Services; + +namespace MSStore.CLI.UnitTests +{ + [TestClass] + public class OutputStreamUnitTests : BaseCommandLineTest + { + [TestInitialize] + public void Init() + { + FakeLogin(); + AddDefaultFakeAccount(); + AddFakeApps(); + } + + [TestCleanup] + public void ResetOutputStreamEnvironmentVariable() + { + Environment.SetEnvironmentVariable(EnvironmentInfo.OutputStreamEnvironmentVariable, null); + } + + [TestMethod] + public void ResolveDefaultsToStderr() + { + var (stream, warning) = OutputStreamResolver.Resolve([], null); + + stream.Should().Be(OutputStream.Stderr); + warning.Should().BeNull(); + } + + [DataRow("stdout", nameof(OutputStream.Stdout))] + [DataRow("stderr", nameof(OutputStream.Stderr))] + [DataRow("STDOUT", nameof(OutputStream.Stdout))] + [DataRow("StdErr", nameof(OutputStream.Stderr))] + [TestMethod] + public void ResolveReadsTheOptionValueCaseInsensitively(string value, string expected) + { + var (stream, warning) = OutputStreamResolver.Resolve(["publish", "--output-stream", value], null); + + stream.Should().Be(Enum.Parse(expected)); + warning.Should().BeNull(); + } + + [DataRow("--output-stream=stdout")] + [DataRow("--output-stream:stdout")] + [TestMethod] + public void ResolveSupportsInlineValueSeparators(string arg) + { + var (stream, warning) = OutputStreamResolver.Resolve(["publish", arg], null); + + stream.Should().Be(OutputStream.Stdout); + warning.Should().BeNull(); + } + + [TestMethod] + public void ResolveUsesTheLastOccurrenceWhenTheOptionIsRepeated() + { + var (stream, _) = OutputStreamResolver.Resolve( + ["publish", "--output-stream", "stdout", "--output-stream", "stderr"], + null); + + stream.Should().Be(OutputStream.Stderr); + } + + [TestMethod] + public void ResolveIgnoresTheOptionWhenItHasNoValue() + { + var (stream, warning) = OutputStreamResolver.Resolve(["publish", "--output-stream"], null); + + stream.Should().Be(OutputStream.Stderr); + warning.Should().BeNull(); + } + + [TestMethod] + public void ResolveDoesNotMatchOptionsThatMerelyStartWithTheSameText() + { + var (stream, _) = OutputStreamResolver.Resolve(["package", "--output-streamer", "stdout"], null); + + stream.Should().Be(OutputStream.Stderr); + } + + [TestMethod] + public void ResolveDoesNotConfuseTheOptionWithTheOutputDirectoryOption() + { + var (stream, _) = OutputStreamResolver.Resolve(["package", "--output", "C:\\packages"], null); + + stream.Should().Be(OutputStream.Stderr); + } + + [TestMethod] + public void ResolveReadsTheEnvironmentVariableWhenTheOptionIsAbsent() + { + var (stream, warning) = OutputStreamResolver.Resolve(["publish"], "stdout"); + + stream.Should().Be(OutputStream.Stdout); + warning.Should().BeNull(); + } + + [DataRow("stdout", "stderr", nameof(OutputStream.Stderr))] + [DataRow("stderr", "stdout", nameof(OutputStream.Stdout))] + [TestMethod] + public void ResolveLetsTheOptionOverrideTheEnvironmentVariable(string environmentValue, string optionValue, string expected) + { + var (stream, warning) = OutputStreamResolver.Resolve( + ["package", "--output-stream", optionValue], + environmentValue); + + stream.Should().Be(Enum.Parse(expected)); + warning.Should().BeNull(); + } + + [TestMethod] + public void ResolveWarnsAndFallsBackWhenTheEnvironmentVariableIsInvalid() + { + var (stream, warning) = OutputStreamResolver.Resolve(["publish"], "console"); + + stream.Should().Be(OutputStream.Stderr); + warning.Should().Contain("console"); + warning.Should().Contain(EnvironmentInfo.OutputStreamEnvironmentVariable); + } + + [DataRow("")] + [DataRow(" ")] + [TestMethod] + public void ResolveTreatsABlankEnvironmentVariableAsUnset(string environmentValue) + { + var (stream, warning) = OutputStreamResolver.Resolve(["publish"], environmentValue); + + stream.Should().Be(OutputStream.Stderr); + warning.Should().BeNull(); + } + + [TestMethod] + public void ResolveDoesNotWarnForAnInvalidOptionValue() + { + // The parser reports invalid option values, so the resolver just falls through. + var (stream, warning) = OutputStreamResolver.Resolve(["publish", "--output-stream", "console"], null); + + stream.Should().Be(OutputStream.Stderr); + warning.Should().BeNull(); + } + + [TestMethod] + public void ResolveReadsTheRealEnvironmentVariable() + { + Environment.SetEnvironmentVariable(EnvironmentInfo.OutputStreamEnvironmentVariable, "stdout"); + + var (stream, warning) = OutputStreamResolver.Resolve(["publish"]); + + stream.Should().Be(OutputStream.Stdout); + warning.Should().BeNull(); + } + + [DataRow("stdout")] + [DataRow("stderr")] + [TestMethod] + public async Task OutputStreamOptionIsAcceptedByCommands(string value) + { + var appId = FakeApps[2].Id!; + + var result = await ParseAndInvokeAsync( + [ + "apps", + "get", + appId, + "--output-stream", + value + ]); + + // Machine-readable payloads always go to stdout, whichever stream the human-readable + // output was routed to. + result.Output.Should().Contain($"\"Id\": \"{appId}\","); + } + + [TestMethod] + public async Task InvalidOutputStreamOptionValueIsRejectedByTheParser() + { + var result = await ParseAndInvokeAsync( + [ + "apps", + "list", + "--output-stream", + "console" + ], + 1); + + result.Error.Should().Contain("--output-stream"); + } + } +} diff --git a/MSStore.CLI/Helpers/OutputStream.cs b/MSStore.CLI/Helpers/OutputStream.cs new file mode 100644 index 0000000..069f601 --- /dev/null +++ b/MSStore.CLI/Helpers/OutputStream.cs @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +namespace MSStore.CLI.Helpers +{ + /// + /// The standard stream that human-readable console output is written to. + /// + /// + /// This never affects machine-readable payloads, which always go to stdout through + /// . + /// + internal enum OutputStream + { + /// + /// Human-readable output goes to standard error. This is the default, and keeps stdout + /// clean so payloads can be piped or captured. + /// + Stderr, + + /// + /// Human-readable output goes to standard output. Useful on Azure DevOps, which renders + /// every stderr line as ##[error]. + /// + Stdout + } +} diff --git a/MSStore.CLI/Helpers/OutputStreamResolver.cs b/MSStore.CLI/Helpers/OutputStreamResolver.cs new file mode 100644 index 0000000..1dfad1e --- /dev/null +++ b/MSStore.CLI/Helpers/OutputStreamResolver.cs @@ -0,0 +1,131 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using MSStore.CLI.Services; + +namespace MSStore.CLI.Helpers +{ + /// + /// Resolves which standard stream human-readable output should be written to. + /// + /// + /// + /// The resolution order is --output-stream > + /// > . The flag deliberately wins so that a pipeline-wide environment + /// variable can be overridden on the individual commands that emit a machine-readable payload. + /// + /// + /// has to build the before the command line is + /// parsed, because the host builder needs it in the service collection. The raw arguments are therefore + /// inspected here, the same way --verbose is handled. + /// + /// + internal static class OutputStreamResolver + { + internal const string OptionName = "--output-stream"; + + private static readonly char[] InlineValueSeparators = [':', '=']; + + /// + /// Resolves the stream from the raw command line arguments and the environment. + /// + /// The raw command line arguments. + /// The resolved stream, and a warning to surface when the environment variable is malformed. + public static (OutputStream Stream, string? Warning) Resolve(IReadOnlyList args) + { + string? environmentValue; + try + { + environmentValue = Environment.GetEnvironmentVariable(EnvironmentInfo.OutputStreamEnvironmentVariable); + } + catch (Exception) + { + // Reading the environment can throw under restricted hosts. Fall back to the default. + environmentValue = null; + } + + return Resolve(args, environmentValue); + } + + /// + /// Resolves the stream from the raw command line arguments and an explicit environment variable value. + /// + /// The raw command line arguments. + /// The value of the environment variable, or null when it is not set. + /// The resolved stream, and a warning to surface when the environment variable is malformed. + public static (OutputStream Stream, string? Warning) Resolve(IReadOnlyList args, string? environmentValue) + { + ArgumentNullException.ThrowIfNull(args); + + if (TryParse(FindOptionValue(args), out var fromArgs)) + { + return (fromArgs, null); + } + + if (string.IsNullOrWhiteSpace(environmentValue)) + { + return (OutputStream.Stderr, null); + } + + if (TryParse(environmentValue, out var fromEnvironment)) + { + return (fromEnvironment, null); + } + + return ( + OutputStream.Stderr, + $"'{environmentValue}' is not a valid {EnvironmentInfo.OutputStreamEnvironmentVariable} value. Expected '{nameof(OutputStream.Stdout)}' or '{nameof(OutputStream.Stderr)}'. Falling back to '{nameof(OutputStream.Stderr)}'."); + } + + /// + /// Parses a stream name, accepting any casing. + /// + /// The value to parse. + /// The parsed stream. + /// True when the value names a known stream. + public static bool TryParse(string? value, out OutputStream outputStream) + { + outputStream = OutputStream.Stderr; + + return !string.IsNullOrWhiteSpace(value) + && Enum.TryParse(value.Trim(), ignoreCase: true, out outputStream) + && Enum.IsDefined(outputStream); + } + + /// + /// Finds the value of the last --output-stream occurrence, supporting both the + /// --output-stream value and --output-stream=value forms. + /// + /// The raw command line arguments. + /// The value, or null when the option is absent. + private static string? FindOptionValue(IReadOnlyList args) + { + string? value = null; + + for (var i = 0; i < args.Count; i++) + { + var arg = args[i]; + if (arg == null) + { + continue; + } + + if (arg.Length > OptionName.Length + && arg.StartsWith(OptionName, StringComparison.Ordinal) + && Array.IndexOf(InlineValueSeparators, arg[OptionName.Length]) >= 0) + { + value = arg[(OptionName.Length + 1)..]; + } + else if (string.Equals(arg, OptionName, StringComparison.Ordinal) && i + 1 < args.Count) + { + value = args[i + 1]; + i++; + } + } + + return value; + } + } +} diff --git a/MSStore.CLI/Helpers/StandardOutput.cs b/MSStore.CLI/Helpers/StandardOutput.cs index 1de2162..ba29eb2 100644 --- a/MSStore.CLI/Helpers/StandardOutput.cs +++ b/MSStore.CLI/Helpers/StandardOutput.cs @@ -12,9 +12,16 @@ internal static class StandardOutput /// /// The text to write. /// + /// /// This deliberately bypasses Spectre.Console's : its renderer /// word-wraps at the console width (falling back to 80 columns when stdout is redirected), which injects /// raw newline characters inside JSON string values and produces invalid JSON. + /// + /// + /// Machine-readable payloads always go to stdout, regardless of --output-stream. Pass + /// --output-stream stderr on these commands when a pipeline-wide + /// MSSTORE_OUTPUT_STREAM=stdout would otherwise interleave human-readable output with the payload. + /// /// public static void WriteLine(string value) { diff --git a/MSStore.CLI/MicrosoftStoreCLI.cs b/MSStore.CLI/MicrosoftStoreCLI.cs index 1efbdbf..42455b9 100644 --- a/MSStore.CLI/MicrosoftStoreCLI.cs +++ b/MSStore.CLI/MicrosoftStoreCLI.cs @@ -21,6 +21,8 @@ internal class MicrosoftStoreCLI : RootCommand { internal static Option VerboseOption { get; } + internal static Option OutputStreamOption { get; } + static MicrosoftStoreCLI() { VerboseOption = new Option("--verbose", "-v") @@ -28,6 +30,11 @@ static MicrosoftStoreCLI() DefaultValueFactory = _ => false, Description = "Verbose output" }; + + OutputStreamOption = new Option(OutputStreamResolver.OptionName) + { + Description = $"The stream that human-readable output is written to. Defaults to '{nameof(OutputStream.Stderr)}', which keeps stdout free for machine-readable payloads. Use '{nameof(OutputStream.Stdout)}' on Azure DevOps, which reports every stderr line as an error. Also settable through the {EnvironmentInfo.OutputStreamEnvironmentVariable} environment variable, which this option overrides." + }; } internal static void WelcomeMessage(IAnsiConsole ansiConsole) diff --git a/MSStore.CLI/Program.cs b/MSStore.CLI/Program.cs index d969440..4dd49f8 100644 --- a/MSStore.CLI/Program.cs +++ b/MSStore.CLI/Program.cs @@ -52,12 +52,19 @@ public static async Task Main(params string[] args) null); TelemetryConfigurations telemetryConfigurations = await telemetryConfigurationManager.LoadAsync(true, CancellationToken.None); TelemetryClient telemetryClient = await CreateTelemetryClientAsync(telemetryConfigurationManager, telemetryConfigurations); + var (outputStream, outputStreamWarning) = OutputStreamResolver.Resolve(args); + var useStdout = outputStream == OutputStream.Stdout; var ansiConsole = AnsiConsole.Create(new() { - Interactive = Console.IsErrorRedirected ? InteractionSupport.No : InteractionSupport.Yes, - Out = new AnsiConsoleOutput(Console.Error) + Interactive = (useStdout ? Console.IsOutputRedirected : Console.IsErrorRedirected) ? InteractionSupport.No : InteractionSupport.Yes, + Out = new AnsiConsoleOutput(useStdout ? Console.Out : Console.Error) }); + if (outputStreamWarning != null) + { + ansiConsole.MarkupLine($":warning: {outputStreamWarning.EscapeMarkup()}"); + } + if (args.Contains(MicrosoftStoreCLI.VerboseOption.Name) || args.Any(MicrosoftStoreCLI.VerboseOption.Aliases.Contains)) { minimumLogLevel = LogLevel.Information; diff --git a/MSStore.CLI/Services/EnvironmentInfo.cs b/MSStore.CLI/Services/EnvironmentInfo.cs index 543969e..23d16b5 100644 --- a/MSStore.CLI/Services/EnvironmentInfo.cs +++ b/MSStore.CLI/Services/EnvironmentInfo.cs @@ -28,6 +28,9 @@ internal class EnvironmentInfo // Environment variable for client assertion file path, used for authentication. public static readonly string ClientAssertionFileEnvironmentVariable = "MSSTORE_CLIENT_ASSERTION_FILE"; + // Environment variable that selects the standard stream used for human-readable output. + public static readonly string OutputStreamEnvironmentVariable = "MSSTORE_OUTPUT_STREAM"; + // Cached environment information, loaded only once private static readonly Lazy _cachedEnvironmentInfo = new Lazy(ComputeEnvironmentInfo); diff --git a/MSStore.CLI/StoreHostBuilderExtensions.cs b/MSStore.CLI/StoreHostBuilderExtensions.cs index 8670b3f..2c21d40 100644 --- a/MSStore.CLI/StoreHostBuilderExtensions.cs +++ b/MSStore.CLI/StoreHostBuilderExtensions.cs @@ -109,6 +109,7 @@ public static IHostBuilder ConfigureStoreCLICommands(this IHostBuilder builder) { var command = ActivatorUtilities.CreateInstance(sp); command.Options.Add(MicrosoftStoreCLI.VerboseOption); + command.Options.Add(MicrosoftStoreCLI.OutputStreamOption); command.SetAction((parseResult, ct) => sp.GetRequiredService().InvokeAsync(parseResult, ct)); return command; }); @@ -122,6 +123,7 @@ public static IHostBuilder ConfigureStoreCLICommands(this IHostBuilder builder) { var command = ActivatorUtilities.CreateInstance(sp); command.Options.Add(MicrosoftStoreCLI.VerboseOption); + command.Options.Add(MicrosoftStoreCLI.OutputStreamOption); return command; }); } diff --git a/README.md b/README.md index f01df15..28d65c3 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,47 @@ The Microsoft Store Developer Command Line Interface is a cross-platform (Window ## Helpful links * [Documentation](https://aka.ms/msstoredevcli/docs) - Microsoft's official documentation on regards to available commands, installation steps, how to properly setup CI/CD environments, and general guidance. +## Standard output vs. standard error + +The CLI keeps its two output streams separate: + +* **stdout** carries only machine-readable payloads — the JSON emitted by commands such as `submission get`, `apps get` and `submission rollout get`, and the package path printed by `package`. This keeps `msstore submission get ... | ConvertFrom-Json` and `$(msstore package ...)` reliable. +* **stderr** carries everything meant for a human — progress, status, success messages, tables and verbose logging. + +### Azure DevOps + +Azure DevOps reports every stderr line as `##[error]`, even when the command succeeded and even when the task sets `failOnStderr: false`. A successful `msstore publish` therefore shows up as a failed or partially failed stage. + +To avoid this, move the human-readable output to stdout: + +```yaml +- script: msstore publish ./MyApp --output-stream stdout + displayName: Publish to the Microsoft Store +``` + +Or set it once for a whole job, so that every `msstore` call picks it up: + +```yaml +variables: + MSSTORE_OUTPUT_STREAM: stdout +``` + +> [!IMPORTANT] +> Machine-readable payloads always go to stdout. When `MSSTORE_OUTPUT_STREAM` is set for a whole job, the human-readable output is interleaved with the payload, which breaks capturing it. Pass `--output-stream stderr` on those specific calls to opt back out — the option always overrides the environment variable: +> +> ```yaml +> variables: +> MSSTORE_OUTPUT_STREAM: stdout +> +> steps: +> - script: msstore publish ./MyApp # human-readable output on stdout +> - script: msstore submission get $(AppId) --output-stream stderr # clean JSON on stdout +> ``` + +### GitHub Actions + +No change is needed. GitHub Actions fails a step based on its exit code alone and never turns stderr into an error annotation, so the default is already correct. + ## Contributing This project welcomes contributions and suggestions. Most contributions require you to agree to a From 877b4f66f8e14ea307461f157266844d37994b75 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 2 Sep 2026 20:17:40 -0700 Subject: [PATCH 02/10] Address Copilot review feedback - OutputStreamResolver.TryParse matched with Enum.TryParse, which also accepts the underlying numbers, so MSSTORE_OUTPUT_STREAM=1 was silently treated as Stdout instead of warning. Match the two names explicitly. - The raw argument scan ignored System.CommandLine's `--` end-of-options marker, so `msstore package -- --output-stream=stdout` redirected output even though the parser treats that token as a literal path. Stop scanning at `--`. - Nine call sites still reached for the static AnsiConsole (the apps/flights list and info tables, the browser launcher prompt, and every ConsoleReader prompt), which writes to stdout. Human-readable output therefore did not all go to stderr by default, and --output-stream did not control those paths. Point the static console at the configured instance. This moves `apps list`, `flights list` and `info` tables, and interactive prompts, from stdout to stderr, which is what the documented contract already claimed. Their machine-readable counterparts (`apps get`, `flights get`) are unaffected and still emit JSON on stdout. The test harness mirrored the old split, so it is updated alongside the four assertions that depended on it. - README stated the stream separation unconditionally, which the new option contradicts. Qualify it as the default. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- MSStore.CLI.UnitTests/AppsCommandUnitTests.cs | 4 +-- MSStore.CLI.UnitTests/BaseCommandLineTest.cs | 7 ++-- .../EmptyCommandUnitTests.cs | 4 +-- .../FlightsCommandUnitTests.cs | 4 +-- .../OutputStreamUnitTests.cs | 36 +++++++++++++++++++ MSStore.CLI/Helpers/OutputStreamResolver.cs | 32 +++++++++++++++-- MSStore.CLI/Program.cs | 5 +++ README.md | 4 ++- 8 files changed, 83 insertions(+), 13 deletions(-) diff --git a/MSStore.CLI.UnitTests/AppsCommandUnitTests.cs b/MSStore.CLI.UnitTests/AppsCommandUnitTests.cs index f1a73d3..fb7189a 100644 --- a/MSStore.CLI.UnitTests/AppsCommandUnitTests.cs +++ b/MSStore.CLI.UnitTests/AppsCommandUnitTests.cs @@ -23,8 +23,8 @@ public async Task AppsListCommandShouldReturnZero() "list" ]); - result.Output.Should().ContainAll(FakeApps.Select(a => a.Id)); - result.Output.Should().ContainAll(FakeApps.Select(a => a.PrimaryName)); + result.Error.Should().ContainAll(FakeApps.Select(a => a.Id)); + result.Error.Should().ContainAll(FakeApps.Select(a => a.PrimaryName)); } [TestMethod] diff --git a/MSStore.CLI.UnitTests/BaseCommandLineTest.cs b/MSStore.CLI.UnitTests/BaseCommandLineTest.cs index 8a13f5c..6d2a2f0 100644 --- a/MSStore.CLI.UnitTests/BaseCommandLineTest.cs +++ b/MSStore.CLI.UnitTests/BaseCommandLineTest.cs @@ -802,8 +802,9 @@ protected void SetupBasedOnTestDataProjectSubPath(DirectoryInfo dirInfo, string[ var outputCapture = new OutputCapture(Console.Out); var errorCapture = RefreshAnsiConsole(); - // Only stdout is redirected: the error capture is reached exclusively through - // ErrorAnsiConsole, mirroring how Program.cs keeps the two streams apart. + // Only stdout is redirected: it is reserved for StandardOutput payloads. Human-readable writes + // reach the error capture through either ErrorAnsiConsole or the static console, mirroring how + // Program.cs points both at the same instance. Console.SetOut(outputCapture); AnsiConsole.Console = AnsiConsole.Create(new AnsiConsoleSettings @@ -811,7 +812,7 @@ protected void SetupBasedOnTestDataProjectSubPath(DirectoryInfo dirInfo, string[ Ansi = AnsiSupport.Yes, ColorSystem = ColorSystemSupport.TrueColor, Interactive = InteractionSupport.No, - Out = new CustomAnsiConsoleOutput(outputCapture), + Out = new CustomAnsiConsoleOutput(errorCapture), Enrichment = new ProfileEnrichment { UseDefaultEnrichers = false diff --git a/MSStore.CLI.UnitTests/EmptyCommandUnitTests.cs b/MSStore.CLI.UnitTests/EmptyCommandUnitTests.cs index 34409f9..c045c14 100644 --- a/MSStore.CLI.UnitTests/EmptyCommandUnitTests.cs +++ b/MSStore.CLI.UnitTests/EmptyCommandUnitTests.cs @@ -47,7 +47,7 @@ public async Task InfoCommandShouldReturnZero() var result = await ParseAndInvokeAsync(["info"]); - result.Output.Should().Contain("Current Config"); + result.Error.Should().Contain("Current Config"); } [TestMethod] @@ -57,7 +57,7 @@ public async Task InfoCommandShouldReturnZeroWithCert() var result = await ParseAndInvokeAsync(["info"]); - result.Output.Should().Contain("Current Config"); + result.Error.Should().Contain("Current Config"); } } } \ No newline at end of file diff --git a/MSStore.CLI.UnitTests/FlightsCommandUnitTests.cs b/MSStore.CLI.UnitTests/FlightsCommandUnitTests.cs index b8fb4c2..748dca3 100644 --- a/MSStore.CLI.UnitTests/FlightsCommandUnitTests.cs +++ b/MSStore.CLI.UnitTests/FlightsCommandUnitTests.cs @@ -25,8 +25,8 @@ public async Task FlightsListCommandShouldReturnZero() FakeApps[0].Id! ]); - result.Output.Should().ContainAll(FakeFlights.Select(a => a.FlightId)); - result.Output.Should().ContainAll(FakeFlights.Select(a => a.FriendlyName)); + result.Error.Should().ContainAll(FakeFlights.Select(a => a.FlightId)); + result.Error.Should().ContainAll(FakeFlights.Select(a => a.FriendlyName)); } [TestMethod] diff --git a/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs index 31ee514..850145c 100644 --- a/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs +++ b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs @@ -144,6 +144,42 @@ public void ResolveDoesNotWarnForAnInvalidOptionValue() warning.Should().BeNull(); } + [DataRow("0")] + [DataRow("1")] + [DataRow("2")] + [TestMethod] + public void ResolveRejectsNumericEnumValues(string environmentValue) + { + // Only the two names are part of the contract, so the underlying numbers must not be accepted. + var (stream, warning) = OutputStreamResolver.Resolve(["publish"], environmentValue); + + stream.Should().Be(OutputStream.Stderr); + warning.Should().Contain(environmentValue); + } + + [DataRow("--output-stream", "stdout")] + [DataRow("--output-stream=stdout", null)] + [TestMethod] + public void ResolveStopsScanningAtTheEndOfOptionsMarker(string arg, string? value) + { + // System.CommandLine treats everything after `--` as a literal argument, so the resolver must too. + string[] args = value == null ? ["package", "--", arg] : ["package", "--", arg, value]; + + var (stream, _) = OutputStreamResolver.Resolve(args, null); + + stream.Should().Be(OutputStream.Stderr); + } + + [TestMethod] + public void ResolveStillReadsTheOptionBeforeTheEndOfOptionsMarker() + { + var (stream, _) = OutputStreamResolver.Resolve( + ["package", "--output-stream", "stdout", "--", "--output-stream=stderr"], + null); + + stream.Should().Be(OutputStream.Stdout); + } + [TestMethod] public void ResolveReadsTheRealEnvironmentVariable() { diff --git a/MSStore.CLI/Helpers/OutputStreamResolver.cs b/MSStore.CLI/Helpers/OutputStreamResolver.cs index 1dfad1e..600eed8 100644 --- a/MSStore.CLI/Helpers/OutputStreamResolver.cs +++ b/MSStore.CLI/Helpers/OutputStreamResolver.cs @@ -26,6 +26,8 @@ internal static class OutputStreamResolver { internal const string OptionName = "--output-stream"; + private const string EndOfOptions = "--"; + private static readonly char[] InlineValueSeparators = [':', '=']; /// @@ -85,13 +87,28 @@ public static (OutputStream Stream, string? Warning) Resolve(IReadOnlyListThe value to parse. /// The parsed stream. /// True when the value names a known stream. + /// + /// Only the two names are accepted. would also + /// accept the underlying numeric values, which are not part of the documented contract. + /// public static bool TryParse(string? value, out OutputStream outputStream) { outputStream = OutputStream.Stderr; - return !string.IsNullOrWhiteSpace(value) - && Enum.TryParse(value.Trim(), ignoreCase: true, out outputStream) - && Enum.IsDefined(outputStream); + if (string.IsNullOrWhiteSpace(value)) + { + return false; + } + + var name = value.Trim(); + + if (string.Equals(name, nameof(OutputStream.Stdout), StringComparison.OrdinalIgnoreCase)) + { + outputStream = OutputStream.Stdout; + return true; + } + + return string.Equals(name, nameof(OutputStream.Stderr), StringComparison.OrdinalIgnoreCase); } /// @@ -100,6 +117,10 @@ public static bool TryParse(string? value, out OutputStream outputStream) /// /// The raw command line arguments. /// The value, or null when the option is absent. + /// + /// Scanning stops at a standalone --, because System.CommandLine treats everything after it as + /// literal arguments rather than options. + /// private static string? FindOptionValue(IReadOnlyList args) { string? value = null; @@ -112,6 +133,11 @@ public static bool TryParse(string? value, out OutputStream outputStream) continue; } + if (string.Equals(arg, EndOfOptions, StringComparison.Ordinal)) + { + break; + } + if (arg.Length > OptionName.Length && arg.StartsWith(OptionName, StringComparison.Ordinal) && Array.IndexOf(InlineValueSeparators, arg[OptionName.Length]) >= 0) diff --git a/MSStore.CLI/Program.cs b/MSStore.CLI/Program.cs index 4dd49f8..2069689 100644 --- a/MSStore.CLI/Program.cs +++ b/MSStore.CLI/Program.cs @@ -60,6 +60,11 @@ public static async Task Main(params string[] args) Out = new AnsiConsoleOutput(useStdout ? Console.Out : Console.Error) }); + // A handful of call sites (list/info tables, prompts, the browser launcher) still reach for the + // static console. Point it at the same instance so every human-readable write honours the + // selected stream, and stdout is left to StandardOutput. + AnsiConsole.Console = ansiConsole; + if (outputStreamWarning != null) { ansiConsole.MarkupLine($":warning: {outputStreamWarning.EscapeMarkup()}"); diff --git a/README.md b/README.md index 28d65c3..d501cd4 100644 --- a/README.md +++ b/README.md @@ -10,11 +10,13 @@ The Microsoft Store Developer Command Line Interface is a cross-platform (Window ## Standard output vs. standard error -The CLI keeps its two output streams separate: +By default the CLI keeps its two output streams separate: * **stdout** carries only machine-readable payloads — the JSON emitted by commands such as `submission get`, `apps get` and `submission rollout get`, and the package path printed by `package`. This keeps `msstore submission get ... | ConvertFrom-Json` and `$(msstore package ...)` reliable. * **stderr** carries everything meant for a human — progress, status, success messages, tables and verbose logging. +`--output-stream stdout` deliberately breaks that separation: it moves the human-readable half onto stdout, where it is interleaved with any payload. Machine-readable payloads are always written to stdout and are never affected by the option. + ### Azure DevOps Azure DevOps reports every stderr line as `##[error]`, even when the command succeeded and even when the task sets `failOnStderr: false`. A successful `msstore publish` therefore shows up as a failed or partially failed stage. From 9a73976c2e96398306c958d5ef1d73dd87aab24b Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 2 Sep 2026 20:26:43 -0700 Subject: [PATCH 03/10] Scope --output-stream to application output in the docs System.CommandLine writes help through InvocationConfiguration.Output and parse diagnostics through .Error, neither of which is affected by --output-stream. The README claimed stdout carried only machine-readable payloads, which help text contradicts. Rather than redirect them, keep the conventional behaviour and describe it: help belongs on stdout so `msstore --help | more` works, and parse errors belong on stderr because they accompany a non-zero exit code. Document both as deliberately outside the option's scope, and note the exclusion at the InvokeAsync call so it does not read as an oversight. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- MSStore.CLI/MicrosoftStoreCLI.cs | 2 +- MSStore.CLI/Program.cs | 3 +++ README.md | 13 +++++++++---- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/MSStore.CLI/MicrosoftStoreCLI.cs b/MSStore.CLI/MicrosoftStoreCLI.cs index 42455b9..40c5a9f 100644 --- a/MSStore.CLI/MicrosoftStoreCLI.cs +++ b/MSStore.CLI/MicrosoftStoreCLI.cs @@ -33,7 +33,7 @@ static MicrosoftStoreCLI() OutputStreamOption = new Option(OutputStreamResolver.OptionName) { - Description = $"The stream that human-readable output is written to. Defaults to '{nameof(OutputStream.Stderr)}', which keeps stdout free for machine-readable payloads. Use '{nameof(OutputStream.Stdout)}' on Azure DevOps, which reports every stderr line as an error. Also settable through the {EnvironmentInfo.OutputStreamEnvironmentVariable} environment variable, which this option overrides." + Description = $"The stream that human-readable progress and status output is written to. Defaults to '{nameof(OutputStream.Stderr)}', which keeps stdout for machine-readable payloads. Use '{nameof(OutputStream.Stdout)}' on Azure DevOps, which reports every stderr line as an error. Also settable through the {EnvironmentInfo.OutputStreamEnvironmentVariable} environment variable, which this option overrides." }; } diff --git a/MSStore.CLI/Program.cs b/MSStore.CLI/Program.cs index 2069689..48fbb88 100644 --- a/MSStore.CLI/Program.cs +++ b/MSStore.CLI/Program.cs @@ -255,6 +255,9 @@ void AddMSCorrelationId(HttpRequestHeaders defaultRequestHeaders) parseError.ShowHelp = true; } + // InvocationConfiguration is left at its defaults on purpose, so --output-stream does not move it: + // help goes to stdout so that `msstore --help | more` works, and parse diagnostics go to stderr + // because they accompany a non-zero exit code. var result = await parseResult.InvokeAsync(parseResult.InvocationConfiguration, lifetime.ApplicationStopping); await host.StopAsync(); diff --git a/README.md b/README.md index d501cd4..dc87062 100644 --- a/README.md +++ b/README.md @@ -10,12 +10,17 @@ The Microsoft Store Developer Command Line Interface is a cross-platform (Window ## Standard output vs. standard error -By default the CLI keeps its two output streams separate: +By default the CLI splits its output as follows: -* **stdout** carries only machine-readable payloads — the JSON emitted by commands such as `submission get`, `apps get` and `submission rollout get`, and the package path printed by `package`. This keeps `msstore submission get ... | ConvertFrom-Json` and `$(msstore package ...)` reliable. -* **stderr** carries everything meant for a human — progress, status, success messages, tables and verbose logging. +* **stdout** carries the command's result — machine-readable payloads such as the JSON emitted by `submission get`, `apps get` and `submission rollout get`, the package path printed by `package`, and `--help` text. This keeps `msstore submission get ... | ConvertFrom-Json` and `$(msstore package ...)` reliable. +* **stderr** carries everything else meant for a human — progress, status, success messages, tables, prompts and verbose logging. -`--output-stream stdout` deliberately breaks that separation: it moves the human-readable half onto stdout, where it is interleaved with any payload. Machine-readable payloads are always written to stdout and are never affected by the option. +`--output-stream stdout` deliberately breaks that separation: it moves the human-readable half onto stdout, where it is interleaved with any payload. + +Two things sit outside the option's scope on purpose, matching the behavior of other CLIs: + +* Machine-readable payloads are always written to stdout, so they are never affected by the option. +* `--help` is always written to stdout, so that `msstore --help | more` works, and command line parse errors are always written to stderr, because they accompany a non-zero exit code. ### Azure DevOps From c6c2cf8bc5c526ee3def7220f7d1995aeaa28598 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 2 Sep 2026 20:33:58 -0700 Subject: [PATCH 04/10] Share one parse contract between the option and the resolver Tightening OutputStreamResolver.TryParse to the two names left the option itself on System.CommandLine's built-in enum converter, which still accepts the underlying numbers. `--output-stream 1` therefore parsed as Stdout while the resolver, which is what actually selects the stream before the host is built, fell back to Stderr - so the command wrote to the opposite stream of what it accepted. Give the option a CustomParser backed by the same TryParse, following the existing idiom in PublishCommand, so both sides reject anything that is not one of the two names. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- .../OutputStreamUnitTests.cs | 19 +++++++++++++++++++ MSStore.CLI/MicrosoftStoreCLI.cs | 19 ++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs index 850145c..8dc96a7 100644 --- a/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs +++ b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs @@ -226,5 +226,24 @@ public async Task InvalidOutputStreamOptionValueIsRejectedByTheParser() result.Error.Should().Contain("--output-stream"); } + + [DataRow("0")] + [DataRow("1")] + [TestMethod] + public async Task NumericOutputStreamOptionValueIsRejectedByTheParser(string value) + { + // The parser and OutputStreamResolver have to agree: the resolver rejects the underlying + // numbers, so the option must not silently accept them through the built-in enum converter. + var result = await ParseAndInvokeAsync( + [ + "apps", + "list", + "--output-stream", + value + ], + 1); + + result.Error.Should().Contain("--output-stream"); + } } } diff --git a/MSStore.CLI/MicrosoftStoreCLI.cs b/MSStore.CLI/MicrosoftStoreCLI.cs index 40c5a9f..5a36e99 100644 --- a/MSStore.CLI/MicrosoftStoreCLI.cs +++ b/MSStore.CLI/MicrosoftStoreCLI.cs @@ -33,7 +33,24 @@ static MicrosoftStoreCLI() OutputStreamOption = new Option(OutputStreamResolver.OptionName) { - Description = $"The stream that human-readable progress and status output is written to. Defaults to '{nameof(OutputStream.Stderr)}', which keeps stdout for machine-readable payloads. Use '{nameof(OutputStream.Stdout)}' on Azure DevOps, which reports every stderr line as an error. Also settable through the {EnvironmentInfo.OutputStreamEnvironmentVariable} environment variable, which this option overrides." + Description = $"The stream that human-readable progress and status output is written to. Defaults to '{nameof(OutputStream.Stderr)}', which keeps stdout for machine-readable payloads. Use '{nameof(OutputStream.Stdout)}' on Azure DevOps, which reports every stderr line as an error. Also settable through the {EnvironmentInfo.OutputStreamEnvironmentVariable} environment variable, which this option overrides.", + + // The built-in enum converter also accepts the underlying numbers, which OutputStreamResolver + // rejects. That would let "--output-stream 1" parse as Stdout while the resolver, which is what + // actually selects the stream before the host is built, fell back to Stderr. Both sides share + // TryParse so there is a single contract. The arity is ExactlyOne, so a missing value fails to + // parse before this runs. + CustomParser = result => + { + var value = result.Tokens[0].Value; + if (OutputStreamResolver.TryParse(value, out var outputStream)) + { + return outputStream; + } + + result.AddError($"Cannot parse argument '{value}' for option '{OutputStreamResolver.OptionName}'. Expected '{nameof(OutputStream.Stdout)}' or '{nameof(OutputStream.Stderr)}'."); + return OutputStream.Stderr; + } }; } From 00d8d4bc7b9dc432fe26306a66e1d1af8fee4f1d Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 2 Sep 2026 20:42:31 -0700 Subject: [PATCH 05/10] Cover the stream routing with process-level tests The routing configured in Program.Main had no automated coverage. OutputStreamUnitTests exercises the resolver and the parser, but BaseCommandLineTest.ParseAndInvokeAsync builds its own consoles and never runs Main, so pinning the console back to Console.Error left the suite green. Add OutputStreamProcessTests, which runs the built executable and reads stdout and stderr separately, following the existing ExternalCommandExecutorTests precedent. It covers the default, the option in all three spellings, the environment variable, the option overriding the environment variable, the invalid-value warning, and help staying on stdout. Verified the coverage is real: reverting the Out selector to Console.Error fails four of them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- .../OutputStreamProcessTests.cs | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 MSStore.CLI.UnitTests/OutputStreamProcessTests.cs diff --git a/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs b/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs new file mode 100644 index 0000000..32fb6fc --- /dev/null +++ b/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Diagnostics; +using System.Runtime.InteropServices; +using MSStore.CLI.Services; + +namespace MSStore.CLI.UnitTests +{ + /// + /// Covers the stream routing configured in , which the in-process harness cannot + /// reach: builds its own consoles and never runs + /// Main. These run the built executable and read stdout and stderr separately. + /// + [TestClass] + public class OutputStreamProcessTests + { + // Emitted by Program's "Command is {Command}" log, which only reaches the console under --verbose and + // is written through the configured Spectre console, so it lands on whichever stream was selected. + private const string HumanOutputMarker = "Command is"; + + [TestMethod] + public async Task DefaultRoutesHumanReadableOutputToStandardError() + { + var result = await RunCliAsync(["--verbose", "--help"], null); + + result.StdErr.Should().Contain(HumanOutputMarker); + result.StdOut.Should().NotContain(HumanOutputMarker); + } + + [DataRow("--output-stream", "stdout")] + [DataRow("--output-stream=stdout", null)] + [DataRow("--output-stream", "STDOUT")] + [TestMethod] + public async Task OptionRoutesHumanReadableOutputToStandardOutput(string arg, string? value) + { + string[] args = value == null + ? ["--verbose", arg, "--help"] + : ["--verbose", arg, value, "--help"]; + + var result = await RunCliAsync(args, null); + + result.StdOut.Should().Contain(HumanOutputMarker); + result.StdErr.Should().NotContain(HumanOutputMarker); + } + + [TestMethod] + public async Task EnvironmentVariableRoutesHumanReadableOutputToStandardOutput() + { + var result = await RunCliAsync(["--verbose", "--help"], "stdout"); + + result.StdOut.Should().Contain(HumanOutputMarker); + result.StdErr.Should().NotContain(HumanOutputMarker); + } + + [TestMethod] + public async Task OptionOverridesTheEnvironmentVariable() + { + var result = await RunCliAsync(["--verbose", "--output-stream", "stderr", "--help"], "stdout"); + + result.StdErr.Should().Contain(HumanOutputMarker); + result.StdOut.Should().NotContain(HumanOutputMarker); + } + + [TestMethod] + public async Task InvalidEnvironmentVariableFallsBackToStandardErrorWithAWarning() + { + var result = await RunCliAsync(["--verbose", "--help"], "1"); + + result.StdErr.Should().Contain(HumanOutputMarker); + result.StdErr.Should().Contain(EnvironmentInfo.OutputStreamEnvironmentVariable); + result.StdOut.Should().NotContain(HumanOutputMarker); + } + + [DataRow(null)] + [DataRow("stdout")] + [TestMethod] + public async Task HelpAlwaysGoesToStandardOutput(string? environmentValue) + { + // System.CommandLine writes help through InvocationConfiguration.Output, which --output-stream + // deliberately leaves alone so that `msstore --help | more` keeps working. + var result = await RunCliAsync(["--help"], environmentValue); + + result.ExitCode.Should().Be(0); + result.StdOut.Should().Contain("Usage:"); + } + + private static async Task<(int ExitCode, string StdOut, string StdErr)> RunCliAsync(string[] args, string? outputStreamEnvironmentValue) + { + var cliPath = FindCliExecutable(); + if (cliPath == null) + { + Assert.Inconclusive("The MSStore.CLI executable was not found. Build the solution before running this test."); + } + + var startInfo = new ProcessStartInfo(cliPath) + { + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + CreateNoWindow = true + }; + + foreach (var arg in args) + { + startInfo.ArgumentList.Add(arg); + } + + if (outputStreamEnvironmentValue == null) + { + // An ambient value on the machine running the tests must not influence the result. + startInfo.Environment.Remove(EnvironmentInfo.OutputStreamEnvironmentVariable); + } + else + { + startInfo.Environment[EnvironmentInfo.OutputStreamEnvironmentVariable] = outputStreamEnvironmentValue; + } + + using var process = Process.Start(startInfo)!; + + var stdOutTask = process.StandardOutput.ReadToEndAsync(); + var stdErrTask = process.StandardError.ReadToEndAsync(); + + using var timeout = new CancellationTokenSource(TimeSpan.FromMinutes(2)); + try + { + await process.WaitForExitAsync(timeout.Token); + } + catch (OperationCanceledException) + { + process.Kill(entireProcessTree: true); + throw; + } + + return (process.ExitCode, await stdOutTask, await stdErrTask); + } + + private static string? FindCliExecutable() + { + // The test binary lives in /MSStore.CLI.UnitTests/bin//, + // and the CLI is built alongside it under the same configuration and target framework. + var testOutputDirectory = new DirectoryInfo(AppContext.BaseDirectory); + var targetFramework = testOutputDirectory.Name; + var configuration = testOutputDirectory.Parent?.Name; + if (configuration == null) + { + return null; + } + + var repositoryRoot = testOutputDirectory; + while (repositoryRoot != null && !File.Exists(Path.Combine(repositoryRoot.FullName, "MSStore.CLI.sln"))) + { + repositoryRoot = repositoryRoot.Parent; + } + + if (repositoryRoot == null) + { + return null; + } + + var fileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "msstore.exe" : "msstore"; + var cliPath = Path.Combine(repositoryRoot.FullName, "MSStore.CLI", "bin", configuration, targetFramework, fileName); + + return File.Exists(cliPath) ? cliPath : null; + } + } +} From f04809662ed0ef6484c1f4305d6aa020cd122a42 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 2 Sep 2026 20:57:13 -0700 Subject: [PATCH 06/10] Cover the static console assignment and stop touching real config Two gaps from review: - Removing `AnsiConsole.Console = ansiConsole` left the suite green. The process tests only assert on the verbose logger, which writes through the injected console, and the in-process harness assigns the static console itself. Extract the console construction into ConsoleFactory so it can be exercised directly, and add ConsoleFactoryUnitTests covering both streams through the injected and the static console. Deleting the assignment now fails three of them. - OutputStreamProcessTests spawned the CLI, which loads and can rewrite telemetrySettings.json before it parses anything, so running the suite mutated real user configuration. Environment.GetFolderPath ignores LOCALAPPDATA/HOME on Windows, so the child cannot be pointed at a temporary profile; snapshot the file and restore it around the class instead, covering the macOS Application Support location as well. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- .../ConsoleFactoryUnitTests.cs | 103 ++++++++++++++++++ .../OutputStreamProcessTests.cs | 57 ++++++++++ MSStore.CLI/Helpers/ConsoleFactory.cs | 41 +++++++ MSStore.CLI/Program.cs | 12 +- 4 files changed, 202 insertions(+), 11 deletions(-) create mode 100644 MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs create mode 100644 MSStore.CLI/Helpers/ConsoleFactory.cs diff --git a/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs b/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs new file mode 100644 index 0000000..ba7c101 --- /dev/null +++ b/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs @@ -0,0 +1,103 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using MSStore.CLI.Helpers; +using Spectre.Console; + +namespace MSStore.CLI.UnitTests +{ + /// + /// Covers the console that builds, including the static + /// assignment that the tables, prompts and browser launcher depend on. + /// + [TestClass] + public class ConsoleFactoryUnitTests + { + private const string Marker = "console-factory-marker"; + + private IAnsiConsole _previousConsole = null!; + private TextWriter _previousOut = null!; + private TextWriter _previousError = null!; + private StringWriter _stdOut = null!; + private StringWriter _stdError = null!; + + [TestInitialize] + public void Initialize() + { + _previousConsole = AnsiConsole.Console; + _previousOut = Console.Out; + _previousError = Console.Error; + + // ConsoleFactory captures Console.Out/Console.Error when it builds the AnsiConsoleOutput, so the + // redirection has to be in place first. + _stdOut = new StringWriter(); + _stdError = new StringWriter(); + Console.SetOut(_stdOut); + Console.SetError(_stdError); + } + + [TestCleanup] + public void Cleanup() + { + Console.SetOut(_previousOut); + Console.SetError(_previousError); + AnsiConsole.Console = _previousConsole; + _stdOut.Dispose(); + _stdError.Dispose(); + } + + [TestMethod] + public void CreateWritesToStandardErrorForStderr() + { + var console = ConsoleFactory.Create(OutputStream.Stderr); + + console.WriteLine(Marker); + + _stdError.ToString().Should().Contain(Marker); + _stdOut.ToString().Should().NotContain(Marker); + } + + [TestMethod] + public void CreateWritesToStandardOutputForStdout() + { + var console = ConsoleFactory.Create(OutputStream.Stdout); + + console.WriteLine(Marker); + + _stdOut.ToString().Should().Contain(Marker); + _stdError.ToString().Should().NotContain(Marker); + } + + [TestMethod] + public void CreateInstallsTheConsoleAsTheStaticConsole() + { + var console = ConsoleFactory.Create(OutputStream.Stderr); + + AnsiConsole.Console.Should().BeSameAs(console); + } + + [TestMethod] + public void StaticWritesFollowStderr() + { + // The apps/flights/info tables, the browser launcher and every ConsoleReader prompt write through + // the static console, so it has to honour the selected stream too. + ConsoleFactory.Create(OutputStream.Stderr); + + AnsiConsole.WriteLine(Marker); + + _stdError.ToString().Should().Contain(Marker); + _stdOut.ToString().Should().NotContain(Marker); + } + + [TestMethod] + public void StaticWritesFollowStdout() + { + ConsoleFactory.Create(OutputStream.Stdout); + + AnsiConsole.WriteLine(Marker); + + _stdOut.ToString().Should().Contain(Marker); + _stdError.ToString().Should().NotContain(Marker); + } + } +} diff --git a/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs b/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs index 32fb6fc..cbcaf34 100644 --- a/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs +++ b/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs @@ -19,6 +19,63 @@ public class OutputStreamProcessTests // is written through the configured Spectre console, so it lands on whichever stream was selected. private const string HumanOutputMarker = "Command is"; + private static readonly List<(string Path, string? Content)> TelemetrySettingsBackups = []; + + /// + /// Program.Main loads (and may rewrite) telemetrySettings.json before it even parses --help. The + /// location comes from Environment.GetFolderPath, which ignores LOCALAPPDATA/HOME on Windows, so the + /// child cannot simply be pointed at a temporary profile. Snapshot the file instead and put it back, + /// so running the suite leaves the real configuration exactly as it found it. + /// + /// The test context. + [ClassInitialize] + public static void BackUpTelemetrySettings(TestContext context) + { + foreach (var path in TelemetrySettingsPaths()) + { + TelemetrySettingsBackups.Add((path, File.Exists(path) ? File.ReadAllText(path) : null)); + } + } + + [ClassCleanup] + public static void RestoreTelemetrySettings() + { + foreach (var (path, content) in TelemetrySettingsBackups) + { + if (content == null) + { + File.Delete(path); + } + else + { + File.WriteAllText(path, content); + } + } + + TelemetrySettingsBackups.Clear(); + } + + private static IEnumerable TelemetrySettingsPaths() + { + yield return Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), + "Microsoft", + "MSStore.CLI", + "telemetrySettings.json"); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + // ConfigurationManager prefers the native ApplicationSupportDirectory on macOS. + yield return Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), + "Library", + "Application Support", + "Microsoft", + "MSStore.CLI", + "telemetrySettings.json"); + } + } + [TestMethod] public async Task DefaultRoutesHumanReadableOutputToStandardError() { diff --git a/MSStore.CLI/Helpers/ConsoleFactory.cs b/MSStore.CLI/Helpers/ConsoleFactory.cs new file mode 100644 index 0000000..63ea48f --- /dev/null +++ b/MSStore.CLI/Helpers/ConsoleFactory.cs @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using Spectre.Console; + +namespace MSStore.CLI.Helpers +{ + /// + /// Builds the console that every human-readable write goes through. + /// + internal static class ConsoleFactory + { + /// + /// Creates the console for and installs it as the static + /// . + /// + /// The stream human-readable output should be written to. + /// The console, which is also registered in the service collection. + /// + /// A handful of call sites still reach for the static console — the apps, flights and info tables, the + /// browser launcher, and every prompt. Installing the same + /// instance keeps them on the selected stream instead of Spectre's default stdout console, and leaves + /// stdout to . + /// + public static IAnsiConsole Create(OutputStream outputStream) + { + var useStdout = outputStream == OutputStream.Stdout; + + var console = AnsiConsole.Create(new AnsiConsoleSettings + { + Interactive = (useStdout ? Console.IsOutputRedirected : Console.IsErrorRedirected) ? InteractionSupport.No : InteractionSupport.Yes, + Out = new AnsiConsoleOutput(useStdout ? Console.Out : Console.Error) + }); + + AnsiConsole.Console = console; + + return console; + } + } +} diff --git a/MSStore.CLI/Program.cs b/MSStore.CLI/Program.cs index 48fbb88..a5bdb30 100644 --- a/MSStore.CLI/Program.cs +++ b/MSStore.CLI/Program.cs @@ -53,17 +53,7 @@ public static async Task Main(params string[] args) TelemetryConfigurations telemetryConfigurations = await telemetryConfigurationManager.LoadAsync(true, CancellationToken.None); TelemetryClient telemetryClient = await CreateTelemetryClientAsync(telemetryConfigurationManager, telemetryConfigurations); var (outputStream, outputStreamWarning) = OutputStreamResolver.Resolve(args); - var useStdout = outputStream == OutputStream.Stdout; - var ansiConsole = AnsiConsole.Create(new() - { - Interactive = (useStdout ? Console.IsOutputRedirected : Console.IsErrorRedirected) ? InteractionSupport.No : InteractionSupport.Yes, - Out = new AnsiConsoleOutput(useStdout ? Console.Out : Console.Error) - }); - - // A handful of call sites (list/info tables, prompts, the browser launcher) still reach for the - // static console. Point it at the same instance so every human-readable write honours the - // selected stream, and stdout is left to StandardOutput. - AnsiConsole.Console = ansiConsole; + var ansiConsole = ConsoleFactory.Create(outputStream); if (outputStreamWarning != null) { From c13a7ef0bf18745dc1e6df440a97086cbddeed31 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Tue, 8 Sep 2026 13:57:24 -0700 Subject: [PATCH 07/10] Make the process tests opt-in and fail loudly when the CLI is missing - The backup/restore of the real telemetrySettings.json was not crash-safe: a killed test host, a hang past the timeout, or a concurrent run could leave a developer's config deleted or stale. Gate the class behind MSSTORE_RUN_PROCESS_TESTS instead and drop the backup entirely. CI runners are disposable, so the mutation is harmless where the variable is set, and the tests skip everywhere else. Both CI definitions set it on every test step, so the coverage of the real Program.cs stream wiring still runs. - A missing executable used Assert.Inconclusive, so the whole class could silently skip and still report green - dropping the only coverage of the real stream wiring if the build layout ever changed. It is now Assert.Fail naming the probed path, the configuration and the target framework. Deliberate skips (the opt-in gate) and genuine failures (a missing build) are now distinct. - ConsoleFactory.Create also assigned the static AnsiConsole.Console, which the name did not hint at, making the side effect invisible at the call site. Renamed to CreateAndInstall. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- .github/workflows/build.yml | 4 + .pipelines/templates/build-and-tests.yaml | 2 + .../ConsoleFactoryUnitTests.cs | 16 +-- .../OutputStreamProcessTests.cs | 99 +++++++------------ MSStore.CLI/Helpers/ConsoleFactory.cs | 2 +- MSStore.CLI/Program.cs | 2 +- 6 files changed, 49 insertions(+), 76 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 297ee73..789735c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -44,9 +44,13 @@ jobs: - name: Build run: dotnet build MSStore.CLI.sln --no-restore /p:Configuration=Release - name: Test net10.0 + env: + MSSTORE_RUN_PROCESS_TESTS: 'true' run: dotnet run --project MSStore.CLI.UnitTests -f net10.0 --no-build -c Release --coverage --coverage-output-format cobertura --report-trx --results-directory ./TestResults - name: Test net10.0-windows10.0.17763.0 if: ${{ matrix.os == 'windows-latest' }} + env: + MSSTORE_RUN_PROCESS_TESTS: 'true' run: dotnet run --project MSStore.CLI.UnitTests -f net10.0-windows10.0.17763.0 --no-build -c Release --coverage --coverage-output-format cobertura --report-trx --results-directory ./TestResults - name: Publish test results if: ${{ !cancelled() }} diff --git a/.pipelines/templates/build-and-tests.yaml b/.pipelines/templates/build-and-tests.yaml index 2c009e5..9882dbc 100644 --- a/.pipelines/templates/build-and-tests.yaml +++ b/.pipelines/templates/build-and-tests.yaml @@ -39,6 +39,7 @@ steps: displayName: 'Tests net10.0' env: DISPLAY: :0.0 + MSSTORE_RUN_PROCESS_TESTS: 'true' inputs: command: 'run' projects: '**/*[Tt]est*/*.csproj' @@ -48,6 +49,7 @@ steps: condition: startsWith(variables.AgentOS, 'Windows_NT') env: DISPLAY: :0.0 + MSSTORE_RUN_PROCESS_TESTS: 'true' inputs: command: 'run' projects: '**/*[Tt]est*/*.csproj' diff --git a/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs b/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs index ba7c101..7a85eb6 100644 --- a/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs +++ b/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs @@ -47,9 +47,9 @@ public void Cleanup() } [TestMethod] - public void CreateWritesToStandardErrorForStderr() + public void CreateAndInstallWritesToStandardErrorForStderr() { - var console = ConsoleFactory.Create(OutputStream.Stderr); + var console = ConsoleFactory.CreateAndInstall(OutputStream.Stderr); console.WriteLine(Marker); @@ -58,9 +58,9 @@ public void CreateWritesToStandardErrorForStderr() } [TestMethod] - public void CreateWritesToStandardOutputForStdout() + public void CreateAndInstallWritesToStandardOutputForStdout() { - var console = ConsoleFactory.Create(OutputStream.Stdout); + var console = ConsoleFactory.CreateAndInstall(OutputStream.Stdout); console.WriteLine(Marker); @@ -69,9 +69,9 @@ public void CreateWritesToStandardOutputForStdout() } [TestMethod] - public void CreateInstallsTheConsoleAsTheStaticConsole() + public void CreateAndInstallInstallsTheConsoleAsTheStaticConsole() { - var console = ConsoleFactory.Create(OutputStream.Stderr); + var console = ConsoleFactory.CreateAndInstall(OutputStream.Stderr); AnsiConsole.Console.Should().BeSameAs(console); } @@ -81,7 +81,7 @@ public void StaticWritesFollowStderr() { // The apps/flights/info tables, the browser launcher and every ConsoleReader prompt write through // the static console, so it has to honour the selected stream too. - ConsoleFactory.Create(OutputStream.Stderr); + ConsoleFactory.CreateAndInstall(OutputStream.Stderr); AnsiConsole.WriteLine(Marker); @@ -92,7 +92,7 @@ public void StaticWritesFollowStderr() [TestMethod] public void StaticWritesFollowStdout() { - ConsoleFactory.Create(OutputStream.Stdout); + ConsoleFactory.CreateAndInstall(OutputStream.Stdout); AnsiConsole.WriteLine(Marker); diff --git a/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs b/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs index cbcaf34..315d69d 100644 --- a/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs +++ b/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs @@ -12,6 +12,14 @@ namespace MSStore.CLI.UnitTests /// reach: builds its own consoles and never runs /// Main. These run the built executable and read stdout and stderr separately. /// + /// + /// Opt-in through MSSTORE_RUN_PROCESS_TESTS, which CI sets. Running the real executable also runs + /// CreateTelemetryClientAsync, which rewrites telemetrySettings.json whenever the telemetry + /// GUID is missing or older than 24 hours, and ConfigurationManager resolves that path through + /// , which ignores LOCALAPPDATA + /// and HOME on Windows. There is no way to redirect it at a temporary profile, so rather than + /// mutate a developer's real configuration these only run where that is harmless. + /// [TestClass] public class OutputStreamProcessTests { @@ -19,60 +27,15 @@ public class OutputStreamProcessTests // is written through the configured Spectre console, so it lands on whichever stream was selected. private const string HumanOutputMarker = "Command is"; - private static readonly List<(string Path, string? Content)> TelemetrySettingsBackups = []; + private const string OptInEnvironmentVariable = "MSSTORE_RUN_PROCESS_TESTS"; - /// - /// Program.Main loads (and may rewrite) telemetrySettings.json before it even parses --help. The - /// location comes from Environment.GetFolderPath, which ignores LOCALAPPDATA/HOME on Windows, so the - /// child cannot simply be pointed at a temporary profile. Snapshot the file instead and put it back, - /// so running the suite leaves the real configuration exactly as it found it. - /// - /// The test context. - [ClassInitialize] - public static void BackUpTelemetrySettings(TestContext context) - { - foreach (var path in TelemetrySettingsPaths()) - { - TelemetrySettingsBackups.Add((path, File.Exists(path) ? File.ReadAllText(path) : null)); - } - } - - [ClassCleanup] - public static void RestoreTelemetrySettings() - { - foreach (var (path, content) in TelemetrySettingsBackups) - { - if (content == null) - { - File.Delete(path); - } - else - { - File.WriteAllText(path, content); - } - } - - TelemetrySettingsBackups.Clear(); - } - - private static IEnumerable TelemetrySettingsPaths() + [TestInitialize] + public void SkipUnlessOptedIn() { - yield return Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), - "Microsoft", - "MSStore.CLI", - "telemetrySettings.json"); - - if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + if (string.IsNullOrEmpty(Environment.GetEnvironmentVariable(OptInEnvironmentVariable))) { - // ConfigurationManager prefers the native ApplicationSupportDirectory on macOS. - yield return Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), - "Library", - "Application Support", - "Microsoft", - "MSStore.CLI", - "telemetrySettings.json"); + Assert.Inconclusive( + $"Set {OptInEnvironmentVariable} to run these tests. They execute the real CLI, which rewrites the telemetry settings of whoever runs them."); } } @@ -144,13 +107,7 @@ public async Task HelpAlwaysGoesToStandardOutput(string? environmentValue) private static async Task<(int ExitCode, string StdOut, string StdErr)> RunCliAsync(string[] args, string? outputStreamEnvironmentValue) { - var cliPath = FindCliExecutable(); - if (cliPath == null) - { - Assert.Inconclusive("The MSStore.CLI executable was not found. Build the solution before running this test."); - } - - var startInfo = new ProcessStartInfo(cliPath) + var startInfo = new ProcessStartInfo(ResolveCliExecutable()) { RedirectStandardOutput = true, RedirectStandardError = true, @@ -192,17 +149,22 @@ public async Task HelpAlwaysGoesToStandardOutput(string? environmentValue) return (process.ExitCode, await stdOutTask, await stdErrTask); } - private static string? FindCliExecutable() + /// + /// Locates the CLI built alongside this test assembly. + /// + /// The full path to the executable. + /// + /// A miss is a failure rather than a skip: these are the only tests covering the real + /// stream wiring, so quietly reporting green would drop that coverage the moment + /// the build layout changes. + /// + private static string ResolveCliExecutable() { // The test binary lives in /MSStore.CLI.UnitTests/bin//, // and the CLI is built alongside it under the same configuration and target framework. var testOutputDirectory = new DirectoryInfo(AppContext.BaseDirectory); var targetFramework = testOutputDirectory.Name; var configuration = testOutputDirectory.Parent?.Name; - if (configuration == null) - { - return null; - } var repositoryRoot = testOutputDirectory; while (repositoryRoot != null && !File.Exists(Path.Combine(repositoryRoot.FullName, "MSStore.CLI.sln"))) @@ -210,15 +172,20 @@ public async Task HelpAlwaysGoesToStandardOutput(string? environmentValue) repositoryRoot = repositoryRoot.Parent; } - if (repositoryRoot == null) + if (configuration == null || repositoryRoot == null) { - return null; + Assert.Fail($"Could not locate MSStore.CLI.sln or the build configuration by walking up from '{AppContext.BaseDirectory}'."); } var fileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "msstore.exe" : "msstore"; var cliPath = Path.Combine(repositoryRoot.FullName, "MSStore.CLI", "bin", configuration, targetFramework, fileName); - return File.Exists(cliPath) ? cliPath : null; + if (!File.Exists(cliPath)) + { + Assert.Fail($"The MSStore.CLI executable was not found at '{cliPath}'. Build MSStore.CLI.sln for configuration '{configuration}' and target framework '{targetFramework}' before running these tests."); + } + + return cliPath; } } } diff --git a/MSStore.CLI/Helpers/ConsoleFactory.cs b/MSStore.CLI/Helpers/ConsoleFactory.cs index 63ea48f..730aa73 100644 --- a/MSStore.CLI/Helpers/ConsoleFactory.cs +++ b/MSStore.CLI/Helpers/ConsoleFactory.cs @@ -23,7 +23,7 @@ internal static class ConsoleFactory /// instance keeps them on the selected stream instead of Spectre's default stdout console, and leaves /// stdout to . /// - public static IAnsiConsole Create(OutputStream outputStream) + public static IAnsiConsole CreateAndInstall(OutputStream outputStream) { var useStdout = outputStream == OutputStream.Stdout; diff --git a/MSStore.CLI/Program.cs b/MSStore.CLI/Program.cs index a5bdb30..5f6d1cd 100644 --- a/MSStore.CLI/Program.cs +++ b/MSStore.CLI/Program.cs @@ -53,7 +53,7 @@ public static async Task Main(params string[] args) TelemetryConfigurations telemetryConfigurations = await telemetryConfigurationManager.LoadAsync(true, CancellationToken.None); TelemetryClient telemetryClient = await CreateTelemetryClientAsync(telemetryConfigurationManager, telemetryConfigurations); var (outputStream, outputStreamWarning) = OutputStreamResolver.Resolve(args); - var ansiConsole = ConsoleFactory.Create(outputStream); + var ansiConsole = ConsoleFactory.CreateAndInstall(outputStream); if (outputStreamWarning != null) { From 82601742a3c022c0f14b2939df2ed6563a9f3550 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Tue, 8 Sep 2026 14:22:32 -0700 Subject: [PATCH 08/10] Assert exit codes and cover parser acceptance of every option spelling The process tests asserted only on the "Command is" marker, which Program logs before InvokeAsync, so a run that emitted the marker on the right stream and then failed would still have passed. Assert the exit code in every one, and add a negative case for an invalid option value. The colon spelling was also only covered at the resolver level, never end to end, so add it to the routing cases. Note that the exit code alone does not prove a spelling parsed: --help takes precedence over parse errors, so an unrecognized token still exits 0 with no error text on either stream. Parser acceptance of the inline forms is therefore asserted in-process, where no help action is involved and ParseAndInvokeAsync already requires an exit code of 0. Verified by pointing one case at a separator System.CommandLine does not accept, which fails only there. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- .../OutputStreamProcessTests.cs | 22 +++++++++++++++++ .../OutputStreamUnitTests.cs | 24 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs b/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs index 315d69d..aad757f 100644 --- a/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs +++ b/MSStore.CLI.UnitTests/OutputStreamProcessTests.cs @@ -44,12 +44,14 @@ public async Task DefaultRoutesHumanReadableOutputToStandardError() { var result = await RunCliAsync(["--verbose", "--help"], null); + result.ExitCode.Should().Be(0); result.StdErr.Should().Contain(HumanOutputMarker); result.StdOut.Should().NotContain(HumanOutputMarker); } [DataRow("--output-stream", "stdout")] [DataRow("--output-stream=stdout", null)] + [DataRow("--output-stream:stdout", null)] [DataRow("--output-stream", "STDOUT")] [TestMethod] public async Task OptionRoutesHumanReadableOutputToStandardOutput(string arg, string? value) @@ -60,6 +62,11 @@ public async Task OptionRoutesHumanReadableOutputToStandardOutput(string arg, st var result = await RunCliAsync(args, null); + // Guards against the command failing for an unrelated reason while the marker still lands on the + // right stream. It does not prove the spelling parsed: --help takes precedence over parse errors, + // so an unrecognized token here would still exit 0. Parser acceptance of each spelling is covered + // in-process by OutputStreamUnitTests, where no help action is involved. + result.ExitCode.Should().Be(0); result.StdOut.Should().Contain(HumanOutputMarker); result.StdErr.Should().NotContain(HumanOutputMarker); } @@ -69,6 +76,7 @@ public async Task EnvironmentVariableRoutesHumanReadableOutputToStandardOutput() { var result = await RunCliAsync(["--verbose", "--help"], "stdout"); + result.ExitCode.Should().Be(0); result.StdOut.Should().Contain(HumanOutputMarker); result.StdErr.Should().NotContain(HumanOutputMarker); } @@ -78,6 +86,7 @@ public async Task OptionOverridesTheEnvironmentVariable() { var result = await RunCliAsync(["--verbose", "--output-stream", "stderr", "--help"], "stdout"); + result.ExitCode.Should().Be(0); result.StdErr.Should().Contain(HumanOutputMarker); result.StdOut.Should().NotContain(HumanOutputMarker); } @@ -87,11 +96,24 @@ public async Task InvalidEnvironmentVariableFallsBackToStandardErrorWithAWarning { var result = await RunCliAsync(["--verbose", "--help"], "1"); + // A malformed environment variable warns and falls back; it must not fail the command. + result.ExitCode.Should().Be(0); result.StdErr.Should().Contain(HumanOutputMarker); result.StdErr.Should().Contain(EnvironmentInfo.OutputStreamEnvironmentVariable); result.StdOut.Should().NotContain(HumanOutputMarker); } + [TestMethod] + public async Task InvalidOptionValueIsRejected() + { + // No --help here: System.CommandLine gives the help action precedence over the parse error, so + // `--output-stream console --help` exits 0. The rejection is only observable without it. + var result = await RunCliAsync(["--verbose", "--output-stream", "console"], null); + + result.ExitCode.Should().NotBe(0); + result.StdErr.Should().Contain("--output-stream"); + } + [DataRow(null)] [DataRow("stdout")] [TestMethod] diff --git a/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs index 8dc96a7..ec0260f 100644 --- a/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs +++ b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs @@ -212,6 +212,30 @@ public async Task OutputStreamOptionIsAcceptedByCommands(string value) result.Output.Should().Contain($"\"Id\": \"{appId}\","); } + [DataRow("--output-stream=stdout")] + [DataRow("--output-stream:stdout")] + [DataRow("--output-stream=stderr")] + [DataRow("--output-stream:stderr")] + [TestMethod] + public async Task InlineOutputStreamOptionFormsAreAcceptedByCommands(string arg) + { + // OutputStreamResolver accepts both inline separators, so the parser has to as well. This is + // asserted here rather than in the process tests because those pass --help, which takes + // precedence over parse errors and would mask an unrecognized token. ParseAndInvokeAsync + // asserts an exit code of 0, so a rejected spelling fails. + var appId = FakeApps[2].Id!; + + var result = await ParseAndInvokeAsync( + [ + "apps", + "get", + appId, + arg + ]); + + result.Output.Should().Contain($"\"Id\": \"{appId}\","); + } + [TestMethod] public async Task InvalidOutputStreamOptionValueIsRejectedByTheParser() { From 33db995d56b6cb3953c380ab4681a1e754e39944 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 9 Sep 2026 13:54:09 -0700 Subject: [PATCH 09/10] Use the injected console for the list and info tables apps list, flights list and info mixed both consoles in one command: the spinner and status messages went through the injected IAnsiConsole while the table itself went through the static AnsiConsole. apps list also split its "no Managed apps" message that way, and flights list was internally inconsistent, using the static console for the table but the injected one for its "no Flights" message. Installing the configured console as the static one made these behave correctly, but only as a side effect - the call sites still read as though they write somewhere else. Use the injected console directly, and inject one into InfoCommand.Handler, which did not take one. No behaviour change, since both already resolve to the same instance. It does remove the dependency on the static install: before this, `info --output-stream stdout` put the table on stdout only because of the assignment; now it does so on its own. The remaining static call sites are the ConsoleReader prompts and the BrowserLauncher confirmation. Those are interactive prompt paths that the static install still covers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- MSStore.CLI/Commands/Apps/ListCommand.cs | 4 ++-- MSStore.CLI/Commands/Flights/ListCommand.cs | 2 +- MSStore.CLI/Commands/InfoCommand.cs | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/MSStore.CLI/Commands/Apps/ListCommand.cs b/MSStore.CLI/Commands/Apps/ListCommand.cs index d90083f..bdd1678 100644 --- a/MSStore.CLI/Commands/Apps/ListCommand.cs +++ b/MSStore.CLI/Commands/Apps/ListCommand.cs @@ -67,11 +67,11 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio i++; } - AnsiConsole.Write(table); + _ansiConsole.Write(table); return await _telemetryClient.TrackCommandEventAsync(0, ct); } - AnsiConsole.MarkupLine("Your account has [bold][u]no[/] Managed apps[/]."); + _ansiConsole.MarkupLine("Your account has [bold][u]no[/] Managed apps[/]."); return await _telemetryClient.TrackCommandEventAsync(-1, ct); } } diff --git a/MSStore.CLI/Commands/Flights/ListCommand.cs b/MSStore.CLI/Commands/Flights/ListCommand.cs index c83e625..a6c3f93 100644 --- a/MSStore.CLI/Commands/Flights/ListCommand.cs +++ b/MSStore.CLI/Commands/Flights/ListCommand.cs @@ -79,7 +79,7 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio i++; } - AnsiConsole.Write(table); + _ansiConsole.Write(table); return await _telemetryClient.TrackCommandEventAsync(0, ct); } else diff --git a/MSStore.CLI/Commands/InfoCommand.cs b/MSStore.CLI/Commands/InfoCommand.cs index d4fe0f7..5c9df65 100644 --- a/MSStore.CLI/Commands/InfoCommand.cs +++ b/MSStore.CLI/Commands/InfoCommand.cs @@ -21,9 +21,10 @@ public InfoCommand() { } - public class Handler(IConfigurationManager configurationManager, TelemetryClient telemetryClient, ILogger logger) : AsynchronousCommandLineAction + public class Handler(IConfigurationManager configurationManager, IAnsiConsole ansiConsole, TelemetryClient telemetryClient, ILogger logger) : AsynchronousCommandLineAction { private readonly IConfigurationManager _configurationManager = configurationManager ?? throw new ArgumentNullException(nameof(configurationManager)); + private readonly IAnsiConsole _ansiConsole = ansiConsole ?? throw new ArgumentNullException(nameof(ansiConsole)); private readonly TelemetryClient _telemetryClient = telemetryClient ?? throw new ArgumentNullException(nameof(telemetryClient)); private readonly ILogger _logger = logger ?? throw new ArgumentNullException(nameof(logger)); @@ -79,7 +80,7 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio _logger.LogInformation("Settings File Path: {@SettingsFilePath}", _configurationManager.ConfigPath); } - AnsiConsole.Write(table); + _ansiConsole.Write(table); return await _telemetryClient.TrackCommandEventAsync(0, ct); } From f671704f226617cc5c3dd88c3c1f655bde3a3438 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 9 Sep 2026 14:06:33 -0700 Subject: [PATCH 10/10] Do not consume the end-of-options marker as the option value `--output-stream -- --output-stream=stdout` diverged from the parser. The raw scan took `--` as the value for the first occurrence, skipped past it, and then matched the trailing literal, so the console was routed to stdout. System. CommandLine instead keeps `--` as the end-of-options marker and reports the first option's value as missing, failing the command. Stop scanning when the prospective value is the marker, so a rejected command line can no longer redirect output. Also correct two comments left stale by the previous commit: the apps, flights and info tables no longer go through the static console, so only the ConsoleReader prompts and the BrowserLauncher confirmation justify installing it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1d98dd1-7d74-456f-8392-bf210b4400c0 --- .../ConsoleFactoryUnitTests.cs | 7 +++--- .../OutputStreamUnitTests.cs | 22 +++++++++++++++++++ MSStore.CLI/Helpers/ConsoleFactory.cs | 8 +++---- MSStore.CLI/Helpers/OutputStreamResolver.cs | 8 +++++++ 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs b/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs index 7a85eb6..6418113 100644 --- a/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs +++ b/MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs @@ -8,7 +8,8 @@ namespace MSStore.CLI.UnitTests { /// /// Covers the console that builds, including the static - /// assignment that the tables, prompts and browser launcher depend on. + /// assignment that the ConsoleReader prompts and the browser launcher + /// confirmation depend on. /// [TestClass] public class ConsoleFactoryUnitTests @@ -79,8 +80,8 @@ public void CreateAndInstallInstallsTheConsoleAsTheStaticConsole() [TestMethod] public void StaticWritesFollowStderr() { - // The apps/flights/info tables, the browser launcher and every ConsoleReader prompt write through - // the static console, so it has to honour the selected stream too. + // The ConsoleReader prompts and the BrowserLauncher confirmation write through the static + // console, so it has to honour the selected stream too. ConsoleFactory.CreateAndInstall(OutputStream.Stderr); AnsiConsole.WriteLine(Marker); diff --git a/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs index ec0260f..eb2bac1 100644 --- a/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs +++ b/MSStore.CLI.UnitTests/OutputStreamUnitTests.cs @@ -180,6 +180,28 @@ public void ResolveStillReadsTheOptionBeforeTheEndOfOptionsMarker() stream.Should().Be(OutputStream.Stdout); } + [TestMethod] + public void ResolveDoesNotConsumeTheEndOfOptionsMarkerAsTheOptionValue() + { + // System.CommandLine reports a missing value here and treats the rest as literals, so the + // resolver must not take `--` as the value and then pick up the trailing token. + var (stream, _) = OutputStreamResolver.Resolve( + ["package", "--output-stream", "--", "--output-stream=stdout"], + null); + + stream.Should().Be(OutputStream.Stderr); + } + + [TestMethod] + public void ResolveKeepsAnEarlierValueWhenTheOptionLaterPrecedesTheEndOfOptionsMarker() + { + var (stream, _) = OutputStreamResolver.Resolve( + ["package", "--output-stream=stdout", "--output-stream", "--", "stderr"], + null); + + stream.Should().Be(OutputStream.Stdout); + } + [TestMethod] public void ResolveReadsTheRealEnvironmentVariable() { diff --git a/MSStore.CLI/Helpers/ConsoleFactory.cs b/MSStore.CLI/Helpers/ConsoleFactory.cs index 730aa73..ea4acc1 100644 --- a/MSStore.CLI/Helpers/ConsoleFactory.cs +++ b/MSStore.CLI/Helpers/ConsoleFactory.cs @@ -18,10 +18,10 @@ internal static class ConsoleFactory /// The stream human-readable output should be written to. /// The console, which is also registered in the service collection. /// - /// A handful of call sites still reach for the static console — the apps, flights and info tables, the - /// browser launcher, and every prompt. Installing the same - /// instance keeps them on the selected stream instead of Spectre's default stdout console, and leaves - /// stdout to . + /// The prompts and the + /// confirmation still reach for the static console. Installing the same instance keeps them on the + /// selected stream instead of Spectre's default stdout console, and leaves stdout to + /// . /// public static IAnsiConsole CreateAndInstall(OutputStream outputStream) { diff --git a/MSStore.CLI/Helpers/OutputStreamResolver.cs b/MSStore.CLI/Helpers/OutputStreamResolver.cs index 600eed8..ff9301d 100644 --- a/MSStore.CLI/Helpers/OutputStreamResolver.cs +++ b/MSStore.CLI/Helpers/OutputStreamResolver.cs @@ -146,6 +146,14 @@ public static bool TryParse(string? value, out OutputStream outputStream) } else if (string.Equals(arg, OptionName, StringComparison.Ordinal) && i + 1 < args.Count) { + // System.CommandLine treats a following `--` as the end-of-options marker rather than a + // value, and reports the option's value as missing. Stop rather than consuming the marker + // and carrying on past it, which would pick up a later literal the parser never accepts. + if (string.Equals(args[i + 1], EndOfOptions, StringComparison.Ordinal)) + { + break; + } + value = args[i + 1]; i++; }