Let human-readable output be routed to stdout

[Alexandre Zollinger Chohfi (azchohfi)]

Fixes #161

Problem

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.

Documenting failOnStderr: false does not fix this: Azure DevOps still prints ##[error] for stderr lines even when the task is configured not to fail on them (azure-pipelines-tasks#16825, #13097).

Why this is opt-in rather than a changed default

The stderr routing is deliberate (commit ca1d6ab, PR #158 / issue #150): 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).

Prior art agrees the default is right: gh puts progress on stderr (gated on IsStdoutTTY()), and az, terraform, aws, npm, pip, docker and dotnet all do the same with no way to switch streams. GitHub Actions is unaffected either way — it fails on exit code only and never annotates stderr.

Change

A global --output-stream <stderr|stdout> option, backed by the MSSTORE_OUTPUT_STREAM environment variable so a pipeline can opt in once at job scope.

# per call
- script: msstore publish ./MyApp --output-stream stdout

# or once for the whole job
variables:
  MSSTORE_OUTPUT_STREAM: stdout

Resolution is option > environment variable > stderr. The option deliberately wins, because a job-wide environment variable is the one real hazard here — it would interleave human output with the payload on the commands that emit one. Those calls opt back out locally:

- script: msstore submission get $(AppId) --output-stream stderr   # clean JSON on stdout

StandardOutput is untouched, so payloads always go to stdout either way.

Implementation notes:

• The console must exist before the command line is parsed, since the host builder needs it in the service collection. OutputStreamResolver therefore reads the raw args, the same way --verbose already does — stopping at -- so it agrees with System.CommandLine's end-of-options semantics.
• The option uses a CustomParser backed by the same TryParse as the resolver, so both sides share one contract and neither accepts the enum's underlying numbers.
• ConsoleFactory.CreateAndInstall builds the console and installs it as the static AnsiConsole.Console, so the call sites that still use the static console honour the selected stream.
• Interactivity probes whichever stream is actually in use (Console.IsOutputRedirected vs Console.IsErrorRedirected).

⚠️ Behavior change

Nine call sites reached for the static AnsiConsole, which writes to stdout: the apps list / flights list / info tables, the BrowserLauncher prompt, and every ConsoleReader prompt. Human-readable output therefore did not all go to stderr by default, and --output-stream did not control those paths at all.

Command	Before	After
apps list, flights list, info tables	stdout	stderr (or stdout with --output-stream stdout)
interactive prompts	stdout	stderr
apps get, flights get, submission get, package payloads	stdout	stdout — unchanged

This makes the documented contract actually true and --output-stream authoritative over all human-readable output.

Scope

--help stays on stdout so msstore --help | more keeps working, and parse errors stay on stderr because they accompany a non-zero exit code. Both are written through InvocationConfiguration, which is deliberately left at its defaults.

Verification

Behaviour confirmed against the built binary, comparing separately-redirected stdout/stderr:

Invocation	Human output lands on
(default)	stderr
--output-stream stdout	stdout
--output-stream=stdout / --output-stream:stdout	stdout
--output-stream STDOUT	stdout
MSSTORE_OUTPUT_STREAM=stdout	stdout
MSSTORE_OUTPUT_STREAM=stdout + --output-stream stderr	stderr
MSSTORE_OUTPUT_STREAM=bogus or =1	stderr, with a warning
-- --output-stream=stdout (after end-of-options)	stderr — ignored, matching the parser

44 new tests across three classes:

• resolver/parser unit tests
• ConsoleFactoryUnitTests for the stream selection and the static-console install
• OutputStreamProcessTests, which runs the built executable and reads the two streams separately

OutputStreamProcessTests is opt-in through MSSTORE_RUN_PROCESS_TESTS, which both CI definitions set on every test step. Running the real CLI also runs CreateTelemetryClientAsync, which can rewrite telemetrySettings.json, and ConfigurationManager resolves that path via Environment.GetFolderPath, which ignores LOCALAPPDATA/HOME on Windows — so it cannot be redirected at a temp profile. Rather than snapshot and restore a developer's real config (which is not crash-safe if the host is killed mid-run), these only run where the mutation is harmless. All runners used here are ephemeral and hosted (windows-latest / ubuntu-latest / macos-latest, and 1ES hosted pools). Verified from the CI TRX artifacts that all 9 run and pass on Windows, Ubuntu and macOS.

Both new test classes were mutation-checked: reverting the Out selector to Console.Error fails 4 process tests, and deleting the AnsiConsole.Console assignment fails 3 factory tests. Removing the built executable fails all 9 process tests with the probed path, rather than skipping.

Full suite is 237/237 on net10.0-windows10.0.17763.0. On net10.0 the only 2 failures (PublishCommandFor{WinUI,Maui}AppsShouldCallMSBuildIfWindows) reproduce identically on a stashed clean tree, so they are pre-existing and environmental — both pass in CI. Release build is warning-free under TreatWarningsAsErrors, and ILC reports no trim/AOT warnings.

[Copilot]

🟡 Changes recommended

Static console writes bypass the selected stream, and raw argument parsing has validation and delimiter inconsistencies.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds opt-in routing of human-readable CLI output to stdout for Azure DevOps while preserving machine-readable stdout payloads.

Changes:

• Adds --output-stream and MSSTORE_OUTPUT_STREAM resolution.
• Configures console routing and stream-aware interactivity.
• Documents behavior and adds resolver/parser tests.

File summaries

File	Description
README.md	Documents stream behavior and Azure DevOps usage.
MSStore.CLI/StoreHostBuilderExtensions.cs	Registers the option across commands.
MSStore.CLI/Services/EnvironmentInfo.cs	Defines the environment variable.
MSStore.CLI/Program.cs	Selects and configures the human-output stream.
MSStore.CLI/MicrosoftStoreCLI.cs	Defines the global option.
MSStore.CLI/Helpers/StandardOutput.cs	Clarifies payload routing.
MSStore.CLI/Helpers/OutputStreamResolver.cs	Resolves option and environment values.
MSStore.CLI/Helpers/OutputStream.cs	Defines supported streams.
MSStore.CLI.UnitTests/OutputStreamUnitTests.cs	Tests resolution and parser acceptance.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 4
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

System.CommandLine help and diagnostics bypass the configured Spectre stream, contradicting the advertised authoritative routing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 13/13 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Numeric enum option values are accepted by the command parser but interpreted differently by the raw stream resolver.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

MSStore.CLI/MicrosoftStoreCLI.cs:37

• The default enum converter accepts numeric values, but the raw resolver intentionally rejects them. As a result, --output-stream 1 is accepted by System.CommandLine as Stdout while OutputStreamResolver falls back to Stderr, so the command silently writes to the opposite stream. Use the same TryParse logic as a custom option parser (and cover 0/1 as option values) so parsing and pre-host resolution share one contract.

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The central end-to-end stream-routing behavior lacks automated coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 13/13 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Response-file arguments can make parsing and actual stream routing disagree, and process tests can modify real user configuration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

MSStore.CLI/Helpers/OutputStreamResolver.cs:64

• System.CommandLine 2.0.10 expands @response-file tokens before parsing, but this pre-host scan only sees the literal @file argument. A response file containing --output-stream stdout is therefore accepted by the command parser while the console remains on stderr. Expand response-file arguments with the same parser configuration before resolving, or explicitly prevent this option from being supplied that way so parsing and routing cannot disagree.

MSStore.CLI/Program.cs:66

• The process tests only use the verbose logger marker, which writes through the injected console; the in-process harness assigns AnsiConsole.Console independently. Consequently, removing this assignment leaves the suite green even though tables and prompts stop honoring --output-stream. Add coverage that executes a static AnsiConsole writer through the real startup configuration for both default and stdout routing.

            AnsiConsole.Console = ansiConsole;

• Files reviewed: 14/14 changed files
• Comments generated: 1
• Review effort level: Balanced

[Alexandre Zollinger Chohfi (azchohfi)]

Picking up the two suppressed comments from the last review, since they have no threads to reply in.

Static AnsiConsole coverage — valid, fixed

Correct: removing AnsiConsole.Console = ansiConsole left the suite green. The process tests assert on the --verbose logger, which writes through the injected console, and BaseCommandLineTest assigns the static console itself, so nothing exercised the production assignment.

The console construction is now extracted into ConsoleFactory.Create, which Program.Main calls, and ConsoleFactoryUnitTests exercises it directly for both streams through both the injected console and a static AnsiConsole.WriteLine. Confirmed the coverage bites — deleting the assignment fails three of the five:

failed CreateInstallsTheConsoleAsTheStaticConsole
failed StaticWritesFollowStderr
failed StaticWritesFollowStdout
total: 5, failed: 3

Response files — does not reproduce on System.CommandLine 2.0.10

This one I could not confirm, and the evidence says the two sides do not disagree. Response-file token replacement is not enabled in 2.0.10, so @file reaches the parser as a literal argument, exactly as it reaches OutputStreamResolver.

Tested against the built binary with a response file containing --output-stream bogus:

Invocation	Exit	Parse error
msstore --output-stream bogus	1	Cannot parse argument 'bogus' for option '--output-stream'.
msstore @bad.rsp --help	0	none

If the file were expanded, the second row would have produced the same parse error. And with a valid --output-stream stdout inside a response file, output stayed on stderr — the parser did not pick it up either, so parsing and routing agree.

Happy to revisit if response file support gets enabled later; the fix would be to expand tokens with the same configuration before resolving. As it stands there is no divergence to fix, so I have not added speculative handling.

Full suite is 237/237 on net10.0-windows10.0.17763.0.

[Copilot]

🟢 Approval recommended

The implementation matches the documented precedence and includes comprehensive unit and process-level coverage.

Review details

• Files reviewed: 16/16 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The process tests still modify the real telemetry configuration without the promised snapshot and restoration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 18/18 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Process tests can mutate real telemetry settings and do not fully validate parser acceptance.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

MSStore.CLI.UnitTests/OutputStreamProcessTests.cs:21

• The PR description says these tests snapshot and restore telemetrySettings.json, but this implementation only gates them behind an environment variable. Once enabled, each process runs LoadAsync(true)/CreateTelemetryClientAsync against the real profile and can permanently replace the user's telemetry settings; this also affects persistent CI agents. Please restore the file snapshot (or delete a newly created file) in class cleanup as described.

    /// <c>CreateTelemetryClientAsync</c>, which rewrites <c>telemetrySettings.json</c> whenever the telemetry
    /// GUID is missing or older than 24 hours, and <c>ConfigurationManager</c> resolves that path through
    /// <see cref="Environment.GetFolderPath(Environment.SpecialFolder)"/>, which ignores <c>LOCALAPPDATA</c>
    /// and <c>HOME</c> 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.

• Files reviewed: 18/18 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The routing contract is consistently implemented, documented, and covered by focused unit and process tests.

Review details

• Files reviewed: 18/18 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The resolver mishandles an end-of-options marker used where the option value is missing, diverging from parser behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

MSStore.CLI/Helpers/OutputStreamResolver.cs:151

• When -- immediately follows this option (for example, --output-stream -- --output-stream=stdout), the resolver consumes the marker as the option value and skips over it, then treats the trailing literal as another option. System.CommandLine instead keeps -- as the end-of-options marker and reports the first option's missing value, so the pre-parse console can be routed differently from the parser contract. Stop scanning when the prospective value is the marker.

MSStore.CLI.UnitTests/ConsoleFactoryUnitTests.cs:83

• This test comment still names the apps/flights/info tables as static-console consumers, but those call sites are migrated to _ansiConsole in this PR. The static assertion remains useful for the browser launcher and prompts; update the rationale accordingly.

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

• Files reviewed: 21/21 changed files
• Comments generated: 2
• Review effort level: Balanced

[Alexandre Zollinger Chohfi (azchohfi)]

Picking up the suppressed comment about the end-of-options marker, since it has no thread to reply in.

--output-stream -- ... — valid, reproduced, fixed

This one was real and I could reproduce it exactly as described. --output-stream immediately followed by -- meant the resolver took the marker as the option's value, skipped past it, and then matched the trailing literal:

msstore --verbose --output-stream -- --output-stream=stdout

	Before	After
Exit code	1	1
Parser	Required argument missing for option: '--output-stream' Unrecognized command or argument '--output-stream=stdout'	same
Human output routed to	stdout ❌	stderr ✅

So a command line the parser rejected outright was still steering the console — exactly the parser/resolver divergence this option is supposed to avoid.

FindOptionValue now stops when the prospective value is the marker, matching System.CommandLine's treatment of -- as end-of-options rather than a value.

Two regression tests added, and I confirmed they bite by neutralising the guard:

failed ResolveDoesNotConsumeTheEndOfOptionsMarkerAsTheOptionValue
failed ResolveKeepsAnEarlierValueWhenTheOptionLaterPrecedesTheEndOfOptionsMarker

Checked the neighbouring behaviour is unaffected: --output-stream stdout --help still routes to stdout and exits 0, and a value supplied before a later -- still wins.

A methodology note

My first mutation attempt replaced the guard with if (false), which tripped CS0162 under TreatWarningsAsErrors. The build failed, the test run used the stale binary, and everything came back green — a false negative. Re-ran it with a sentinel comparison that keeps the branch reachable, which is where the two failures above came from. Worth knowing if anyone repeats this on a Release build.

Suite is 245/245 on net10.0-windows10.0.17763.0.

[Copilot]

🟢 Approval recommended

Stream precedence, payload isolation, static-console behavior, documentation, and cross-platform CI coverage are consistent and adequately tested.

Review details

• Files reviewed: 21/21 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[isourabh]

This hand-rolls a chunk of System.CommandLine's tokenizer — inline =/: separators, last-occurrence-wins, -- end-of-options, and the "don't consume -- as the value" case. That's ~90 lines plus ~15 tests to read one flag, and two of the commits on this branch (c6c2cf8, f671704) were spent reconciling it with the real parser. Every future parser nuance is a divergence that only surfaces as the option and the resolver disagreeing.

I don't think the circular dependency this works around actually exists. MicrosoftStoreCLI.OutputStreamOption is a static built in a static ctor with no DI, so it can be parsed against a bare RootCommand before the host is built:

private static OutputStream? FromArgs(IReadOnlyList<string> args)
{
    var probe = new RootCommand { TreatUnmatchedTokensAsErrors = false };
    probe.Options.Add(MicrosoftStoreCLI.OutputStreamOption);

    var result = probe.Parse([.. args]);
    var optionResult = result.GetResult(MicrosoftStoreCLI.OutputStreamOption);

    return optionResult is null || optionResult.Errors.Count > 0
        ? null
        : result.GetValue(MicrosoftStoreCLI.OutputStreamOption);
}

TreatUnmatchedTokensAsErrors = false makes subcommands and positionals harmless, and post--- tokens land in UnmatchedTokens for free — so the two end-of-options cases this method handles explicitly need no code at all. Scope the error check to optionResult.Errors rather than result.Errors, otherwise an unrelated parse error elsewhere on the line would silently drop a valid --output-stream.

I prototyped this locally and ran it against all 18 edge cases covered by OutputStreamUnitTests (inline separators, casing, repeats, --output-stream --, --output-streamer, --output, missing value, invalid value, -- in every position). All 18 resolve identically to the current implementation, so the existing tests can stay exactly as they are and serve as the regression net for the swap.

Worth noting the --verbose comparison, since this is the local precedent: Program.cs:63 gets away with args.Contains only because a boolean has no value to locate — and it's already quietly wrong in the same way (msstore package -- --verbose enables verbose logging even though the parser treats it as a literal). Nobody notices because the cost is a few extra log lines, whereas getting --output-stream wrong defeats the whole feature. So the extra rigor here is right; it just doesn't have to be hand-written. If you like the approach, the same probe could resolve both options and retire the Contains hack too — probably a follow-up rather than this PR.

Non-blocking either way — the current code is correct as far as I can tell, this is about the maintenance surface.

[isourabh]

This section reads as if the stdout/stderr split had always been the design, but it's a behavior change. Installing the stderr console as the static AnsiConsole.Console in ConsoleFactory.CreateAndInstall, plus switching the three commands to the injected console, moves everything that previously went through the static console off stdout: the apps list and flights list tables, the info table, ConsoleReader prompts, and the BrowserLauncher confirmation. The test churn in AppsCommandUnitTests, FlightsCommandUnitTests and EmptyCommandUnitTests (result.Output → result.Error) is exactly this, and it's the part a user upgrading will feel — msstore apps list | grep ... and msstore info > file start returning nothing, with a zero exit code and no diagnostic.

Worth stating outright, along with the rollback. Suggested addition just before ### Azure DevOps:

> [!WARNING]
> **Changed in <version>.** Human-readable output previously went to stdout. It now
> defaults to stderr, including the `apps list`, `flights list` and `info` tables,
> prompts, and status messages. Scripts that piped or captured that output need one of:
>
> * `--output-stream stdout` (or `MSSTORE_OUTPUT_STREAM=stdout`) to restore the
>   previous routing, or
> * `2>&1` to merge the streams.
>
> Machine-readable payloads (`submission get`, `apps get`, `package`) were already on
> stdout and are unaffected.

The point I'd most want made is that --output-stream stdout is a complete one-flag rollback — that's what makes this a manageable change rather than an alarming one, and the section never says it today. There's no CHANGELOG in the repo, so the README and the GitHub release notes are the only places this can land.

Two related follow-ups:

1. The option's Description in MicrosoftStoreCLI.cs is 426 characters on one line. In the --help options table that wraps into a paragraph next to --verbose's "Verbose output". Since the README now covers the Azure DevOps rationale and the override precedence properly, the help text can shrink to one sentence naming the two values and the environment variable.
2. The README links aka.ms/msstoredevcli/docs as canonical for CI/CD setup, and its Azure DevOps guidance goes stale when this merges. Probably an issue against the docs rather than anything in this PR, but the two shouldn't disagree.