Route bv narration to standard error, keeping standard output for deliverables

.claude/rules/testing.md

@@ -0,0 +1,16 @@
+# Testing and code coverage
+
+Coverage reports are produced by `bv test` (Microsoft.Testing.Extensions.CodeCoverage, cobertura format, one report per test project in `TestResults/`) and uploaded to Codecov by CI.
+
+## Coverage exclusion policy
+
+- **Default: code gets tested.** If something is hard to cover, first ask whether a small, honest design change makes it testable — e.g., extracting pure logic (parsing, mapping, formatting) out of plumbing into its own type. Do NOT create abstractions whose only purpose is to let a test mock the environment and assert the mock: that is coverage theater, not testing.
+- **`[ExcludeFromCodeCoverage]` is reserved for code whose behavior is owned by the environment**, not by the code itself: P/Invoke wrappers, process composition roots (`Program`), console/process plumbing that reads global process state. Never use it for logic that is merely untested.
+- **`Justification` is mandatory**, phrased as _why a test cannot honestly exercise this code_ ("behavior depends on the console attached to the process"), never as a restatement of the exclusion ("not tested").
+- **Smallest scope that fits**: method over class, class over assembly; never assembly-wide.
+- **Exclude in source, not in Codecov configuration.** The collector honors the attribute and removes excluded code from the report's denominator, so local numbers match the badge, and the exclusion lives next to the code together with its reason. Do not add `ignore:` path lists to a `codecov.yml`.
+
+## Test conventions
+
+- Test framework is TUnit on Microsoft.Testing.Platform; use TUnit's built-in assertions (`await Assert.That(...)`), never FluentAssertions.
+- Tests that swap process-global state (console writers, current directory) must be marked `[NotInParallel]`.

Buildvana.slnx

@@ -55,6 +55,7 @@
   <Folder Name="/tests/">
     <File Path="tests/Common.targets" />
     <Project Path="tests/Buildvana.Core.Configuration.Tests/Buildvana.Core.Configuration.Tests.csproj" />
+    <Project Path="tests/Buildvana.Core.ConsoleOutput.Tests/Buildvana.Core.ConsoleOutput.Tests.csproj" />
     <Project Path="tests/Buildvana.Core.HomeDirectory.Tests/Buildvana.Core.HomeDirectory.Tests.csproj" />
     <Project Path="tests/Buildvana.Core.Json.Tests/Buildvana.Core.Json.Tests.csproj" />
     <Project Path="tests/Buildvana.Core.JsonSchema.Tests/Buildvana.Core.JsonSchema.Tests.csproj" />

CHANGELOG.md

@@ -78,6 +78,7 @@ See the Nerdbank.GitVersioning removal entry under _Changes to existing features
 
   Cake verbosity values (e.g., `verbose`) are no longer accepted.
 - `bv` no longer prefixes its console output with a log level and a class-name category (e.g. `info: Buildvana.Tool.Services.DotNetService: ...`). Messages now render as clean, color-coded lines: errors in red and warnings in yellow, each line tagged with a short level label (`error:`/`warning:`/`info:`/`detail:`/`trace:`). In addition, `dotnet`/MSBuild output is now streamed through live (standard output to `bv`'s standard output, standard error to its standard error) instead of being hidden unless the build fails; on failure, the first and last lines of the captured output are still included in the error message. Verbosity behavior is unchanged (`--verbosity quiet|minimal|normal|detailed|diagnostic`), as is the handling of `--color`/`--no-color` (with the [`NO_COLOR` environment variable](https://no-color.org) now honored as well).
+- **BREAKING CHANGE**: `bv` now writes all of its own narration to standard error, keeping standard output for actual results, per the prevailing CLI convention (git, npm, cargo, etc.). Narration comprises the leveled diagnostic lines (`error:`/`warning:`/`info:`/`detail:`/`trace:`), activity start/finish lines, and the startup logo. Standard output now carries only command deliverables (e.g. `bv version show`'s report) and the standard output of child `dotnet` processes, which is the payload of the build commands. Results thus stay pipeable at any verbosity: `bv version show | some-parser` receives only the report, and `bv build 2>bv.log` separates `bv`'s narration from `dotnet`'s output. Scripts and CI steps that captured diagnostics from `bv`'s standard output must now capture standard error instead (e.g. via `2>&1`). Color auto-detection consequently probes standard error: a redirected standard error disables color, while a redirected standard output no longer does.
 - **BREAKING CHANGE**: `bv restore`, `bv build`, `bv test`, and `bv pack` forward extra command-line arguments to the underlying `dotnet` invocation(s) only after a `--` separator: everything after the first `--` is passed through verbatim, in the order given, and `bv` no longer parses or validates it. A non-global, option-looking token _before_ `--` is now an error that points you at the separator. Malformed or unknown forwarded arguments produce an error from `dotnet` (or, for `bv test`, from the Microsoft.Testing.Platform test application) rather than from `bv`. Previously only `-p:`/`/p:` MSBuild properties were forwarded. `bv` also always forwards `--nologo` and its resolved `--verbosity` (default `normal`) to those invocations.
   - `bv build -- -m:8 -v:minimal` forwards `-m:8 -v:minimal` to `dotnet build`.
   - `bv test -- --report-trx` reaches the test application.

src/Buildvana.Core.ConsoleOutput/AnsiEscapes.cs

@@ -0,0 +1,54 @@
+// Copyright (C) Tenacom and Contributors. Licensed under the MIT license.
+// See the LICENSE file in the project root for full license information.
+
+using System;
+using System.Globalization;
+using CommunityToolkit.Diagnostics;
+
+namespace Buildvana.Core.ConsoleOutput;
+
+/// <summary>
+/// Provides ANSI (virtual terminal) escape sequences for console text styling.
+/// </summary>
+/// <remarks>
+/// <para>Unlike <see cref="Console.ForegroundColor"/>, whose implementation is tied to the standard output
+/// stream (on Unix it emits escape sequences to standard output even when the write targets standard error),
+/// these sequences can be written to any stream, so the caller decides where styling goes.</para>
+/// <para>The terminal attached to the target stream must interpret virtual terminal sequences; on Windows, see
+/// <see cref="VirtualTerminal"/>.</para>
+/// </remarks>
+public static class AnsiEscapes
+{
+    /// <summary>
+    /// The escape sequence that resets all text attributes to the terminal's defaults.
+    /// </summary>
+    public const string Reset = "\e[0m";
+
+    /// <summary>
+    /// Gets the escape sequence that sets the foreground to the ANSI color corresponding to the specified
+    /// <see cref="ConsoleColor"/>.
+    /// </summary>
+    /// <param name="color">The console color to translate.</param>
+    /// <returns>The escape sequence for <paramref name="color"/>.</returns>
+    /// <remarks>
+    /// <para>The mapping follows the same correspondence the BCL uses on Unix: the eight "dark" colors map to
+    /// standard-intensity ANSI colors (SGR 30-37) and the remaining eight to high-intensity colors (SGR 90-97),
+    /// so a label rendered through these sequences selects the same color indices as one rendered by setting
+    /// <see cref="Console.ForegroundColor"/> (whose Unix implementation may emit terminfo-derived sequences
+    /// rather than these literal codes).</para>
+    /// </remarks>
+    public static string Foreground(ConsoleColor color)
+    {
+        var value = (int)color;
+        if (value is < 0 or > 15)
+        {
+            return ThrowHelper.ThrowArgumentOutOfRangeException<string>(nameof(color), color, "Unknown console color.");
+        }
+
+        // ConsoleColor packs channels blue-lowest (B=1, G=2, R=4); SGR packs them red-lowest (R=1, G=2, B=4):
+        // swap the red and blue bits, then offset into the standard- (30) or high-intensity (90) SGR range.
+        var rgb = ((value & 0b100) >> 2) | (value & 0b010) | ((value & 0b001) << 2);
+        var code = (value < 8 ? 30 : 90) + rgb;
+        return string.Create(CultureInfo.InvariantCulture, $"\e[{code}m");
+    }
+}

src/Buildvana.Core.ConsoleOutput/Buildvana.Core.ConsoleOutput.csproj

@@ -4,8 +4,14 @@
     <Title>Buildvana console output</Title>
     <Description>System.Console-backed IReporter: leveled human-facing console output with activity grouping and child-process passthrough.</Description>
     <TargetFramework>$(StandardTfm)</TargetFramework>
+    <!-- Required by the LibraryImport source generator (VirtualTerminal's console-mode P/Invokes). -->
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
   </PropertyGroup>
 
+  <ItemGroup>
+    <InternalsVisibleTo Include="Buildvana.Core.ConsoleOutput.Tests" />
+  </ItemGroup>
+
   <ItemGroup>
     <ProjectReference Include="..\Buildvana.Core.Abstractions\Buildvana.Core.Abstractions.csproj" />
   </ItemGroup>

src/Buildvana.Core.ConsoleOutput/ConsoleReporter.cs

@@ -3,19 +3,25 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Threading;
 using CommunityToolkit.Diagnostics;
 
 namespace Buildvana.Core.ConsoleOutput;
 
 /// <summary>
-/// An <see cref="IReporter"/> that writes to the process's standard output via <see cref="Console"/>.
+/// An <see cref="IReporter"/> that writes to the process's standard streams via <see cref="Console"/>.
 /// </summary>
 /// <remarks>
+/// <para>Diagnostics (leveled messages and activity header/outcome lines) go to standard error, so that
+/// standard output carries only command deliverables and child-process standard output
+/// (<see cref="ChildOutput"/>) and stays pipeable at any verbosity, per the prevailing CLI convention.</para>
 /// <para>Color is a function of message level, decided here and never by the caller: <c>error:</c> renders in
 /// red and <c>warning:</c> in yellow (foreground only, no background fill); the remaining levels are uncolored
-/// so they inherit the terminal's theme. The message body is never colored.</para>
+/// so they inherit the terminal's theme. The message body is never colored. Coloring uses
+/// <see cref="AnsiEscapes"/> rather than <see cref="Console.ForegroundColor"/>, because the latter is tied to
+/// standard output and would corrupt a redirected deliverable stream.</para>
 /// <para>Output is serialized through an internal lock so that lines streamed from a child process's standard
 /// output and standard error (which arrive on background threads) never interleave mid-line with each other or
 /// with narration.</para>
@@ -32,14 +38,24 @@ public sealed partial class ConsoleReporter : IReporter
     /// <param name="verbosity">The verbosity that gates which message levels are rendered.</param>
     /// <param name="colorOverride">
     /// <see langword="true"/> to force color on, <see langword="false"/> to force it off, or
-    /// <see langword="null"/> to auto-detect (color on unless output is redirected or the <c>NO_COLOR</c>
-    /// environment variable is set). A non-<see langword="null"/> value wins over both, so <c>--color</c>
-    /// overrides <c>NO_COLOR</c>.
+    /// <see langword="null"/> to auto-detect (color on unless standard error is redirected, the <c>NO_COLOR</c>
+    /// environment variable is set, or the console cannot interpret ANSI escape sequences). A
+    /// non-<see langword="null"/> value wins over all three, so <c>--color</c> overrides <c>NO_COLOR</c>.
+    /// Forcing color on also attempts to enable escape-sequence interpretation on the console, but wins even
+    /// when that fails.
     /// </param>
     public ConsoleReporter(Verbosity verbosity, bool? colorOverride)
     {
         Verbosity = verbosity;
-        _useColor = colorOverride ?? (!IsNoColorSet() && !Console.IsOutputRedirected);
+        _useColor = colorOverride ?? DetectColor();
+
+        // Auto-detection folds the virtual-terminal attempt into DetectColor; an explicit "on" override skips
+        // detection, so the attempt must happen here or legacy conhost would print raw escape sequences. The
+        // override stays authoritative even when enabling fails: the user asked for color.
+        if (colorOverride == true)
+        {
+            _ = VirtualTerminal.TryEnableOnStandardError();
+        }
     }
 
     /// <inheritdoc/>
@@ -71,7 +87,7 @@ public IActivityScope BeginActivity(string title)
             _activityStack.Push(scope);
             if (this.IsEnabled(MessageLevel.Info))
             {
-                Console.WriteLine(FormatActivityLine(depth, title, elapsed: null, outcomeMessage: null));
+                Console.Error.WriteLine(FormatActivityLine(depth, title, elapsed: null, outcomeMessage: null));
             }
 
             return scope;
@@ -114,6 +130,10 @@ public void ChildError(string line, Verbosity? minimumVerbosity)
         }
     }
 
+    [ExcludeFromCodeCoverage(Justification = "Reads process-global console state; under a test runner standard error is always redirected, so only one outcome is ever reachable.")]
+    private static bool DetectColor()
+        => !IsNoColorSet() && !Console.IsErrorRedirected && VirtualTerminal.TryEnableOnStandardError();
+
     /// <summary>
     /// Determines whether the <c>NO_COLOR</c> environment variable is set.
     /// </summary>
@@ -122,6 +142,7 @@ public void ChildError(string line, Verbosity? minimumVerbosity)
     /// <para>The <c>NO_COLOR</c> environment variable is a widely-adopted convention for opting out of color in command-line applications.
     /// See <a href="https://no-color.org">https://no-color.org</a> for more information.</para>
     /// </remarks>
+    [ExcludeFromCodeCoverage(Justification = "Reads the process environment; covering both outcomes would require mutating process-global state under a parallel test runner.")]
     private static bool IsNoColorSet() => Environment.GetEnvironmentVariable("NO_COLOR") is { Length: > 0 };
 
     private static (ConsoleColor? Color, string Word) StyleFor(MessageLevel level) => level switch
@@ -140,24 +161,16 @@ private static string FormatActivityLine(int depth, string title, TimeSpan? elap
             ? string.Format(CultureInfo.InvariantCulture, "[{0}] {1}: done ({2:F1}s){3}{4}", depth, title, e.TotalSeconds, outcomeMessage is null ? string.Empty : " - ", outcomeMessage)
             : string.Format(CultureInfo.InvariantCulture, "[{0}] {1}: starting...", depth, title);
 
+    // A single WriteLine per message: Console.Error auto-flushes, so composing first keeps the line atomic at
+    // the OS level (the lock only serializes this reporter, not the logo or Spectre's stdout writes) and costs
+    // one write syscall instead of up to five.
     private void WriteLeveledLine(MessageLevel level, string message)
     {
         var (color, word) = StyleFor(level);
-        if (_useColor && color is { } foreground)
-        {
-            Console.ForegroundColor = foreground;
-            Console.Write(word);
-            Console.Write(':');
-            Console.ResetColor();
-        }
-        else
-        {
-            Console.Write(word);
-            Console.Write(':');
-        }
-
-        Console.Write(' ');
-        Console.WriteLine(message);
+        var line = _useColor && color is { } foreground
+            ? $"{AnsiEscapes.Foreground(foreground)}{word}:{AnsiEscapes.Reset} {message}"
+            : $"{word}: {message}";
+        Console.Error.WriteLine(line);
     }
 
     private void EndActivity(ActivityScope scope, bool completed)
@@ -172,7 +185,7 @@ private void EndActivity(ActivityScope scope, bool completed)
             // No outcome line unless the activity was explicitly completed (e.g. the work threw before Complete).
             if (completed && this.IsEnabled(MessageLevel.Info))
             {
-                Console.WriteLine(FormatActivityLine(scope.Depth, scope.Title, scope.Elapsed, scope.OutcomeMessage));
+                Console.Error.WriteLine(FormatActivityLine(scope.Depth, scope.Title, scope.Elapsed, scope.OutcomeMessage));
             }
         }
     }