From c873516eb657e68c93bf7dc40143c5d98fcf5159 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 00:09:12 +0200 Subject: [PATCH 01/26] Add the Notice message level, and align both reporters' mappings MessageLevel had no rung meaning "significant, but not a warning, and visible at minimal verbosity" -- the thing MSBuild has always had as MessageImportance.High and syslog calls Notice. Its absence is why the two IReporter implementations disagreed about the same call: TaskLoggingHelperReporter had nothing to put in MSBuild's High slot, so it shifted the whole ladder down a notch and mapped Info there, while ConsoleReporter hid Info at minimal. Notice goes between Warning and Info. Six levels no longer fit five thresholds injectively, so IsEnabled can no longer compare the enums' underlying values; MessageLevelExtensions.MinimumVerbosity states the mapping instead, and is now the single authority both reporters answer to. The enums' docs lose the one-to-one claim they can no longer make, and the Notice-versus-Info criterion goes on the enum member itself, where the next call-site author will meet it. TaskLoggingHelperReporter's outbound map shifts by one rung and its Verbosity getter with it, collapsing to two branches: minimal and quiet now answer the same. Both remaining judgment calls in that getter over-claim on purpose, and the remarks say why -- an honest Detailed would make the formatting overloads drop Trace under -v:diag, and an honest Quiet would make them drop warnings MSBuild still prints. The remarks also record that Detail and Trace share MessageImportance.Low because MSBuild's ladder has three rungs and ends there: EngineServices exposes no verbosity to read, by design, so detailed and diagnostic are indistinguishable from inside a task. Activity lines stay gated at Info on both sides, which is the agreement they already had -- the SDK side logs them at Normal importance so they stay hidden at MSBuild's default verbosity. Co-Authored-By: Claude Opus 5 (1M context) --- .../ConsoleOutput/MessageLevel.cs | 33 ++++++++++++--- .../ConsoleOutput/MessageLevelExtensions.cs | 41 +++++++++++++++++++ .../ConsoleOutput/ReporterExtensions.cs | 13 +++++- .../ConsoleOutput/Verbosity.cs | 13 +++--- .../TextWriterReporter.cs | 1 + .../TaskLoggingHelperReporter.cs | 40 +++++++++++++----- .../MessageLevelExtensionsTests.cs | 38 +++++++++++++++++ .../TextWriterReporterTests.cs | 17 ++++++++ .../TaskLoggingHelperReporterTests.cs | 14 ++++--- 9 files changed, 182 insertions(+), 28 deletions(-) create mode 100644 src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs create mode 100644 tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevel.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevel.cs index df74212e..aa083c65 100644 --- a/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevel.cs +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevel.cs @@ -8,20 +8,41 @@ namespace Buildvana.Core.ConsoleOutput; /// message and, together with the reporter's , decides whether it is shown. /// /// -/// Members are ordered from highest to lowest severity, mapping one-to-one onto the -/// thresholds: ↔, ↔, -/// ↔, ↔, -/// ↔. +/// Members are ordered from highest to lowest severity. There are more levels than there are +/// thresholds, so the two do not map one-to-one: the verbosity from which each level +/// becomes visible is stated by , which is the single +/// authority on the matter and the one every implementation agrees on. /// public enum MessageLevel { /// An error: something went wrong. Shown at every verbosity. Error, - /// A warning: something looks off but is not fatal. + /// + /// A warning: something looks off but is not fatal. Shown at and above. + /// Warning, - /// An informational milestone. Shown at and above. + /// + /// A record of a fact: something changed, something was decided, something was deliberately skipped. + /// Shown at and above. + /// + /// + /// Use this level for what the reader would want to know afterwards — the version spec changed, N files + /// were rewritten, a step was skipped and why — and for narration of what the tool is doing + /// right now. The two are not a loudness ranking: this is a quieter , not a louder + /// , and it should feel like it costs something. A message at this level survives the default + /// verbosity, so promoting narration to it makes the default as noisy as and + /// leaves the ladder with no rung meaning what this one means. + /// + Notice, + + /// + /// Narration of what the tool is doing right now. Shown at and above. + /// + /// + /// See for the criterion that separates the two levels. + /// Info, /// A detail useful when following along closely. Shown at and above. diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs new file mode 100644 index 00000000..4b8a6c85 --- /dev/null +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs @@ -0,0 +1,41 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System; + +namespace Buildvana.Core.ConsoleOutput; + +/// +/// Provides extension methods for values. +/// +#pragma warning disable CA1034 // Nested types should not be visible — false positive on C# 14 extension blocks; fixed in .NET 11, backport to .NET 10 requested in https://github.com/dotnet/sdk/issues/53984 +#pragma warning disable CA1708 // Identifiers should differ by more than case — false positive on classes with C# 14 extension blocks; fixed in .NET 11, https://github.com/dotnet/sdk/issues/51716 +public static class MessageLevelExtensions +{ + extension(MessageLevel @this) + { + /// + /// Gets the least verbose at which a message of this level is shown. + /// + /// The minimum that enables this level. + /// This level is not a known . + /// + /// This method is the single authority on when a level becomes visible. Every + /// implementation answers the question through it — directly, or by mapping the + /// answer onto the visibility rules of whatever it renders through — so that a given level becomes + /// visible at the same point no matter which reporter is in play. + /// The mapping cannot be a comparison of the two enums' underlying values: there are more levels + /// than there are thresholds, so no ordering of the members makes such a comparison give the right + /// answer for all of them. + /// + public Verbosity MinimumVerbosity() => @this switch + { + MessageLevel.Error => Verbosity.Quiet, + MessageLevel.Warning or MessageLevel.Notice => Verbosity.Minimal, + MessageLevel.Info => Verbosity.Normal, + MessageLevel.Detail => Verbosity.Detailed, + MessageLevel.Trace => Verbosity.Diagnostic, + _ => throw new ArgumentOutOfRangeException(nameof(@this), @this, "Unknown message level."), + }; + } +} diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/ReporterExtensions.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/ReporterExtensions.cs index 6ca74fc0..f2ccf268 100644 --- a/src/Buildvana.Core.Abstractions/ConsoleOutput/ReporterExtensions.cs +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/ReporterExtensions.cs @@ -29,6 +29,10 @@ public static class ReporterExtensions /// The message text. public void Warning(string message) => @this.Report(MessageLevel.Warning, message); + /// Reports a message. + /// The message text. + public void Notice(string message) => @this.Report(MessageLevel.Notice, message); + /// Reports an message. /// The message text. public void Info(string message) => @this.Report(MessageLevel.Info, message); @@ -53,6 +57,12 @@ public void Error(CompositeFormat format, params ReadOnlySpan args) public void Warning(CompositeFormat format, params ReadOnlySpan args) => @this.Report(MessageLevel.Warning, format, args); + /// Formats and reports a message. + /// The composite format string. + /// The arguments to format. + public void Notice(CompositeFormat format, params ReadOnlySpan args) + => @this.Report(MessageLevel.Notice, format, args); + /// Formats and reports an message. /// The composite format string. /// The arguments to format. @@ -95,7 +105,8 @@ public void Report(MessageLevel level, CompositeFormat format, params ReadOnlySp /// /// The level to test. /// if the level is enabled; otherwise, . - public bool IsEnabled(MessageLevel level) => (int)level <= (int)@this.Verbosity; + /// is not a known . + public bool IsEnabled(MessageLevel level) => @this.IsVerbosityAtLeast(level.MinimumVerbosity()); /// /// Determines whether the reporter's is at least the given diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs index d468d597..194267c4 100644 --- a/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs @@ -5,22 +5,23 @@ namespace Buildvana.Core.ConsoleOutput; /// /// Controls how much of a reporter's output reaches the user. Each level enables all the -/// s enabled by the levels below it (see for the mapping). +/// s enabled by the levels below it. /// /// -/// The members mirror bv's --verbosity command-line vocabulary and are ordered from least to most -/// verbose, so a message at a given is shown when -/// (int)level <= (int)verbosity. +/// The members mirror bv's --verbosity command-line vocabulary and are ordered from least to +/// most verbose. Which levels each one enables is stated by +/// : a message is shown when its level's minimum verbosity +/// is at most the one in effect. /// public enum Verbosity { /// Only errors are shown. Quiet, - /// Errors and warnings are shown. + /// Errors, warnings, and notices are shown. Minimal, - /// Errors, warnings, and informational messages are shown. This is the default. + /// Everything shows, plus informational messages. Normal, /// Everything shows, plus detail messages. diff --git a/src/Buildvana.Core.ConsoleOutput/TextWriterReporter.cs b/src/Buildvana.Core.ConsoleOutput/TextWriterReporter.cs index f3d92188..14d40ce6 100644 --- a/src/Buildvana.Core.ConsoleOutput/TextWriterReporter.cs +++ b/src/Buildvana.Core.ConsoleOutput/TextWriterReporter.cs @@ -133,6 +133,7 @@ public void ChildError(string line, Verbosity? minimumVerbosity) { MessageLevel.Error => (ConsoleColor.Red, "error"), MessageLevel.Warning => (ConsoleColor.Yellow, "warning"), + MessageLevel.Notice => (null, "notice"), MessageLevel.Info => (null, "info"), MessageLevel.Detail => (null, "detail"), MessageLevel.Trace => (null, "trace"), diff --git a/src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs b/src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs index be95cf61..5231f746 100644 --- a/src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs +++ b/src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs @@ -16,18 +16,36 @@ namespace Buildvana.Sdk; /// human-facing output through the build's own loggers. /// /// -/// Message levels map to MSBuild severities as follows: and -/// become build errors and warnings; , -/// , and become messages of -/// , , and -/// importance respectively. +/// Message levels map to MSBuild severities so that each level becomes visible at the verbosity named by +/// , the same one at which bv's console shows it: +/// and become build errors and warnings; +/// becomes a message of importance, which +/// MSBuild shows from minimal verbosity up; becomes +/// , shown from normal verbosity up; and both +/// and become +/// , shown from detailed verbosity up. +/// The last two share a rung because MSBuild's ladder has three and ends there. A task cannot do better: +/// exposes no verbosity to read, and its +/// is a union query across every registered logger, each +/// with a verbosity of its own — so detailed and diagnostic are indistinguishable from in here, and +/// surfaces one rung earlier than it would on the console. /// Messages are always forwarded, regardless of : visibility is governed by /// MSBuild's own verbosity and importance filtering, exactly like any other message logged by a task. /// is derived from when available /// (falling back to fully permissive otherwise), so that callers checking it — such as the formatting /// helpers in — skip work whose output MSBuild would discard anyway. +/// That derivation over-claims at both ends of the ladder, deliberately, because its only consumers are +/// short-circuit checks and the cost of guessing low is a dropped message. When low-importance messages are +/// logged it reports rather than : the honest +/// answer would make Report(Trace, format, args) short-circuit and drop every formatted +/// message even under -v:diag. When nothing at all is logged it floors at +/// rather than : the honest answer would make +/// Warning(format, args) short-circuit and drop warnings that MSBuild's own quiet verbosity still +/// prints. /// Activity start and outcome lines are logged at importance, -/// so they are hidden at MSBuild's default (minimal) verbosity and visible from normal verbosity up. +/// so they are hidden at MSBuild's default (minimal) verbosity and visible from normal verbosity up. That is the +/// importance maps to, and the console side gates the same lines on +/// too, so the two agree on when an activity is narrated. /// Child-process output and error lines are both forwarded as low-importance messages: MSBuild has no /// neutral standard-error channel, and logging a build error or warning would misrepresent — and, given how /// task success is determined, potentially fail the build over — stderr lines that many tools use for @@ -48,9 +66,10 @@ public TaskLoggingHelperReporter(TaskLoggingHelper log, IBuildEngine engine) _engineServices = (engine as IBuildEngine10)?.EngineServices; } + // The two remaining rungs of MSBuild's ladder, minimal and quiet, both answer Minimal here: see the + // over-claim paragraph in this class's remarks for why quiet does not answer Quiet. public Verbosity Verbosity => LogsMessagesOfImportance(MessageImportance.Low) ? Verbosity.Diagnostic - : LogsMessagesOfImportance(MessageImportance.Normal) ? Verbosity.Detailed - : LogsMessagesOfImportance(MessageImportance.High) ? Verbosity.Normal + : LogsMessagesOfImportance(MessageImportance.Normal) ? Verbosity.Normal : Verbosity.Minimal; public void Report(MessageLevel level, string message) @@ -66,12 +85,13 @@ public void Report(MessageLevel level, string message) case MessageLevel.Warning: _log.LogWarning("{0}", message); break; - case MessageLevel.Info: + case MessageLevel.Notice: _log.LogMessage(MessageImportance.High, "{0}", message); break; - case MessageLevel.Detail: + case MessageLevel.Info: _log.LogMessage(MessageImportance.Normal, "{0}", message); break; + case MessageLevel.Detail: case MessageLevel.Trace: _log.LogMessage(MessageImportance.Low, "{0}", message); break; diff --git a/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs b/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs new file mode 100644 index 00000000..3323c4b9 --- /dev/null +++ b/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs @@ -0,0 +1,38 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using Buildvana.Core.ConsoleOutput; + +internal sealed class MessageLevelExtensionsTests +{ + [Test] + [Arguments(MessageLevel.Error, Verbosity.Quiet)] + [Arguments(MessageLevel.Warning, Verbosity.Minimal)] + [Arguments(MessageLevel.Notice, Verbosity.Minimal)] + [Arguments(MessageLevel.Info, Verbosity.Normal)] + [Arguments(MessageLevel.Detail, Verbosity.Detailed)] + [Arguments(MessageLevel.Trace, Verbosity.Diagnostic)] + public async Task MinimumVerbosity_KnownLevel_ReturnsThreshold(MessageLevel level, Verbosity expected) + { + await Assert.That(level.MinimumVerbosity()).IsEqualTo(expected); + } + + [Test] + public async Task MinimumVerbosity_CoversEveryLevel() + { + // Guards the switch against a level added without a threshold: the arm would be missing, not wrong, + // and no per-level test above would notice. + foreach (var level in Enum.GetValues()) + { + await Assert.That(() => level.MinimumVerbosity()).ThrowsNothing(); + } + } + + [Test] + [Arguments(-1)] + [Arguments(42)] + public async Task MinimumVerbosity_UnknownLevel_Throws(int level) + { + await Assert.That(() => ((MessageLevel)level).MinimumVerbosity()).Throws(); + } +} diff --git a/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs b/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs index 3009308a..79bc15d6 100644 --- a/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs +++ b/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs @@ -9,6 +9,7 @@ internal sealed class TextWriterReporterTests [Test] [Arguments(MessageLevel.Error, "error")] [Arguments(MessageLevel.Warning, "warning")] + [Arguments(MessageLevel.Notice, "notice")] [Arguments(MessageLevel.Info, "info")] [Arguments(MessageLevel.Detail, "detail")] [Arguments(MessageLevel.Trace, "trace")] @@ -22,6 +23,7 @@ public async Task Report_EnabledLevel_WritesLabeledLineToErrorWriter(MessageLeve [Test] [Arguments(Verbosity.Quiet, MessageLevel.Warning)] + [Arguments(Verbosity.Quiet, MessageLevel.Notice)] [Arguments(Verbosity.Minimal, MessageLevel.Info)] [Arguments(Verbosity.Normal, MessageLevel.Detail)] [Arguments(Verbosity.Detailed, MessageLevel.Trace)] @@ -33,6 +35,21 @@ public async Task Report_DisabledLevel_WritesNothing(Verbosity verbosity, Messag await Assert.That(reporter.OutputText).IsEmpty(); } + [Test] + [Arguments(MessageLevel.Error, Verbosity.Quiet)] + [Arguments(MessageLevel.Warning, Verbosity.Minimal)] + [Arguments(MessageLevel.Notice, Verbosity.Minimal)] + [Arguments(MessageLevel.Info, Verbosity.Normal)] + [Arguments(MessageLevel.Detail, Verbosity.Detailed)] + [Arguments(MessageLevel.Trace, Verbosity.Diagnostic)] + public async Task Report_AtLevelMinimumVerbosity_Writes(MessageLevel level, Verbosity verbosity) + { + using var reporter = new StringWriterReporter(verbosity); + reporter.Report(level, "something happened"); + await Assert.That(reporter.ErrorText).EndsWith($"something happened{Environment.NewLine}"); + await Assert.That(reporter.OutputText).IsEmpty(); + } + [Test] [Arguments(MessageLevel.Error, "\e[91m", "error")] [Arguments(MessageLevel.Warning, "\e[93m", "warning")] diff --git a/tests/Buildvana.Sdk.Tasks.Tests/TaskLoggingHelperReporterTests.cs b/tests/Buildvana.Sdk.Tasks.Tests/TaskLoggingHelperReporterTests.cs index 6cb56751..1668e47f 100644 --- a/tests/Buildvana.Sdk.Tasks.Tests/TaskLoggingHelperReporterTests.cs +++ b/tests/Buildvana.Sdk.Tasks.Tests/TaskLoggingHelperReporterTests.cs @@ -44,8 +44,9 @@ public async Task Report_Warning_LogsBuildWarning() } [Test] - [Arguments(MessageLevel.Info, MessageImportance.High)] - [Arguments(MessageLevel.Detail, MessageImportance.Normal)] + [Arguments(MessageLevel.Notice, MessageImportance.High)] + [Arguments(MessageLevel.Info, MessageImportance.Normal)] + [Arguments(MessageLevel.Detail, MessageImportance.Low)] [Arguments(MessageLevel.Trace, MessageImportance.Low)] public async Task Report_MessageLevel_LogsMessageWithExpectedImportance( MessageLevel level, @@ -81,10 +82,13 @@ public async Task Verbosity_WithoutEngineServices_IsDiagnostic() await Assert.That(reporter.Verbosity).IsEqualTo(Verbosity.Diagnostic); } + // Both ends of this table over-claim on purpose: Low answers Diagnostic rather than Detailed so that + // formatted Trace messages are not short-circuited away under -v:diag, and an engine that logs nothing + // floors at Minimal rather than Quiet so that formatted warnings survive MSBuild's own quiet verbosity. [Test] [Arguments(MessageImportance.Low, Verbosity.Diagnostic)] - [Arguments(MessageImportance.Normal, Verbosity.Detailed)] - [Arguments(MessageImportance.High, Verbosity.Normal)] + [Arguments(MessageImportance.Normal, Verbosity.Normal)] + [Arguments(MessageImportance.High, Verbosity.Minimal)] [Arguments(null, Verbosity.Minimal)] public async Task Verbosity_WithEngineServices_TracksImportanceFiltering( MessageImportance? minimumImportance, @@ -193,7 +197,7 @@ public async Task ChildError_LogsLowImportanceMessage_NotABuildError() [Test] public async Task ChildLines_WhenEngineDiscardsLowImportance_AreNotForwarded() { - // Dropped twice over: the reporter's minimumVerbosity gate (verbosity here is Normal) and + // Dropped twice over: the reporter's minimumVerbosity gate (verbosity here is Minimal) and // TaskLoggingHelper's own importance filtering both consult the same EngineServices. var (reporter, engine) = CreateReporter(MessageImportance.High); reporter.ChildOutput("out", null); From 3045b0d38e636add89eb242be0ec7f14f47bbe29 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 00:13:56 +0200 Subject: [PATCH 02/26] Report facts as Notice, keeping narration at Info The lines `bv release` uses to record what it did to the repository -- version spec changed, N public API files modified, changelog substituted, N self-referenced files rewritten, N packages pushed, and every "skipped, because" line -- are the audit trail of a CI-only operation, so they move to Notice and survive the minimal verbosity the next commit makes the default. What the command is doing at a given moment stays at Info: "Reading release asset lists...", the changelog and public-API services' narration, the hook runner's, the Git service's. Three lines outside `release` come along: - VersioningService's "Version X (height N, publicity)". This one is collateral of the remap rather than a call-site judgment: it is Core code that also runs inside SDK tasks, where Info used to mean MessageImportance.High, so the line shows on every plain `dotnet build` today. Notice is where it belongs on the criterion anyway -- it records the version this build decided to stamp -- and keeps `dotnet build` output exactly as it is. - ServerRelease's "Repository unchanged, no commit to push.", a release outcome with no aggregate counterpart in ReleaseCommand to carry it. - All three of `version advance`'s outcome lines, not just the one naming the new spec. The command has no deliverable stream of its own, so at Info the no-op case would print nothing at all, and promoting the outcome without the "review and commit" line would strand it: leaving the change uncommitted for review is what the command is for. Two candidates stay at Info deliberately. The fallback-push-credentials line is a setup fact rather than an outcome, and the case worth surfacing at minimal is already covered by the warning in its else branch; and the successful changelog check records nothing and changes nothing. Co-Authored-By: Claude Opus 5 (1M context) --- .../VersioningService.cs | 2 +- src/Buildvana.Tool/Services/DotNetService.cs | 2 +- .../Services/ServerAdapters/ServerRelease.cs | 2 +- .../Subcommands/ReleaseCommand.cs | 36 +++++++++---------- .../Subcommands/VersionAdvanceCommand.cs | 6 ++-- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/Buildvana.Core.Versioning/VersioningService.cs b/src/Buildvana.Core.Versioning/VersioningService.cs index 5e7ab55b..18c97978 100644 --- a/src/Buildvana.Core.Versioning/VersioningService.cs +++ b/src/Buildvana.Core.Versioning/VersioningService.cs @@ -59,7 +59,7 @@ public VersioningService( FileVersion = FormattableString.Invariant($"{SimpleVersion}.0"); InformationalVersion = ComputeInformationalVersion(SemVer, Spec.Prerelease, IsPublicRelease, CommitId); var publicity = IsPublicRelease ? "public release" : "not a public release"; - reporter.Info(FormattableString.Invariant($"Version {SemVer} (height {Height}, {publicity})")); + reporter.Notice(FormattableString.Invariant($"Version {SemVer} (height {Height}, {publicity})")); } /// diff --git a/src/Buildvana.Tool/Services/DotNetService.cs b/src/Buildvana.Tool/Services/DotNetService.cs index e8e9de34..d27a96f7 100644 --- a/src/Buildvana.Tool/Services/DotNetService.cs +++ b/src/Buildvana.Tool/Services/DotNetService.cs @@ -268,7 +268,7 @@ await RunDotNetAsync( cancellationToken: cancellationToken).ConfigureAwait(false); } - _reporter.Info($"Pushed {packages.Length} packages to {target.Source}."); + _reporter.Notice($"Pushed {packages.Length} packages to {target.Source}."); } /// diff --git a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs index 1f2d1190..6bce60ff 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs @@ -188,7 +188,7 @@ public void PushUpdates() if (!_repositoryUpdated) { - _reporter.Info("Repository unchanged, no commit to push."); + _reporter.Notice("Repository unchanged, no commit to push."); return; } diff --git a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs index d1707c2d..0431fa64 100644 --- a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs +++ b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs @@ -64,7 +64,7 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) // Ensure that the CI bot identity is used for commits, if not already set. git.CommitterIdentity ??= server.CIBotIdentity ?? throw new BuildFailedException("Cannot determine a committer identity for release commits. Configure git config user.name/user.email before running this task."); - reporter.Info($"Using committer identity: {git.CommitterIdentity.Name} <{git.CommitterIdentity.Email}>"); + reporter.Notice($"Using committer identity: {git.CommitterIdentity.Name} <{git.CommitterIdentity.Email}>"); // Set fallback Git credentials if the server adapter can provide them. var pushUsername = server.PushUsername; @@ -103,20 +103,20 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) var previousVersionSpec = versionFile.Spec; if (versionFile.ApplyChange(versionSpecChange)) { - reporter.Info($"Version spec changed from {previousVersionSpec} to {versionFile.Spec}."); + reporter.Notice($"Version spec changed from {previousVersionSpec} to {versionFile.Spec}."); versionFile.Save(versioningSettings.PrereleaseTag); release.UpdateRepository(versionFile.Path); } else { - reporter.Info("Version spec not changed."); + reporter.Notice("Version spec not changed."); } } // Update public API files only when releasing a stable version if (version.IsPrerelease) { - reporter.Info("Public API update skipped: not needed on prerelease."); + reporter.Notice("Public API update skipped: not needed on prerelease."); } else { @@ -124,13 +124,13 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) switch (modified.Length) { case 0: - reporter.Info("No public API files were modified."); + reporter.Notice("No public API files were modified."); break; case 1: - reporter.Info("1 public API file was modified."); + reporter.Notice("1 public API file was modified."); break; default: - reporter.Info(string.Create(CultureInfo.InvariantCulture, $"{modified.Length} public API files were modified.")); + reporter.Notice(string.Create(CultureInfo.InvariantCulture, $"{modified.Length} public API files were modified.")); break; } @@ -147,14 +147,14 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) && (changelogUpdates == ChangelogUpdates.All || !version.IsPrerelease); if (!changelog.Exists) { - reporter.Info($"Changelog update skipped: {ChangelogService.FileName} not found."); + reporter.Notice($"Changelog update skipped: {ChangelogService.FileName} not found."); } else if (!shouldUpdateChangelog) { var reason = changelogUpdates == ChangelogUpdates.None ? "changelog updates are disabled (release.changelogUpdates is 'none')." : "not needed on prerelease."; - reporter.Info($"Changelog update skipped: {reason}"); + reporter.Notice($"Changelog update skipped: {reason}"); } else { @@ -172,7 +172,7 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) emptyChangelogSubstitute is null, "Changelog check failed: the \"Unreleased changes\" section is empty or only contains sub-section headings, and no substitute text is configured (release.emptyChangelog)."); - reporter.Info("Changelog \"Unreleased changes\" section is empty; substituting the configured release.emptyChangelog text."); + reporter.Notice("Changelog \"Unreleased changes\" section is empty; substituting the configured release.emptyChangelog text."); } // Update the changelog and commit the change before building. @@ -209,7 +209,7 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) } else { - reporter.Info("Changelog section title update skipped: changelog has not been updated."); + reporter.Notice("Changelog section title update skipped: changelog has not been updated."); } // Discover the packages produced by the pack step; both the post-release hook args and @@ -241,13 +241,13 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) switch (hookUpdates.Count) { case 0: - reporter.Info("The post-release hook modified no files."); + reporter.Notice("The post-release hook modified no files."); break; case 1: - reporter.Info("The post-release hook modified 1 file."); + reporter.Notice("The post-release hook modified 1 file."); break; default: - reporter.Info(string.Create(CultureInfo.InvariantCulture, $"The post-release hook modified {hookUpdates.Count} files.")); + reporter.Notice(string.Create(CultureInfo.InvariantCulture, $"The post-release hook modified {hookUpdates.Count} files.")); break; } } @@ -265,19 +265,19 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) switch (selfReferenceUpdates.Count) { case 0: - reporter.Info("No self-referenced files were modified."); + reporter.Notice("No self-referenced files were modified."); break; case 1: - reporter.Info("1 self-referenced file was modified."); + reporter.Notice("1 self-referenced file was modified."); break; default: - reporter.Info(string.Create(CultureInfo.InvariantCulture, $"{selfReferenceUpdates.Count} self-referenced files were modified.")); + reporter.Notice(string.Create(CultureInfo.InvariantCulture, $"{selfReferenceUpdates.Count} self-referenced files were modified.")); break; } } else { - reporter.Info("Self-reference update skipped: option 'dogfood' is false."); + reporter.Notice("Self-reference update skipped: option 'dogfood' is false."); } // Assemble the post-release commit from the self-reference rewrites and the hook's changes. diff --git a/src/Buildvana.Tool/Subcommands/VersionAdvanceCommand.cs b/src/Buildvana.Tool/Subcommands/VersionAdvanceCommand.cs index d3042886..bf191831 100644 --- a/src/Buildvana.Tool/Subcommands/VersionAdvanceCommand.cs +++ b/src/Buildvana.Tool/Subcommands/VersionAdvanceCommand.cs @@ -33,12 +33,12 @@ public Task ExecuteAsync(CancellationToken cancellationToken) if (versionFile.ApplyChange(change)) { versionFile.Save(versioningSettings.PrereleaseTag); - reporter.Info($"Version spec changed from {previousSpec} to {versionFile.Spec}."); - reporter.Info($"Review and commit the modified {VersionFile.FileName} file."); + reporter.Notice($"Version spec changed from {previousSpec} to {versionFile.Spec}."); + reporter.Notice($"Review and commit the modified {VersionFile.FileName} file."); } else { - reporter.Info("Version spec not changed."); + reporter.Notice("Version spec not changed."); } return Task.FromResult(0); From a763056579628d46886134f95a8f7c1a07968588 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 00:23:23 +0200 Subject: [PATCH 03/26] Default to minimal verbosity for every command bv defaulted to normal while `dotnet restore`/`build`/`test`/`pack` -- the commands the build pipeline wraps, and forwards its own verbosity to verbatim -- default to minimal, so plain `bv build` produced a markedly noisier MSBuild log than plain `dotnet build`. Buildvana is a component of a .NET toolchain: where the toolchain has settled a question of behavior, we settle it the same way rather than on the merits. The default is uniform across commands, so the per-command defaultVerbosity goes with it, along with the ImplementsCommandAttribute parameter, the CommandRegistration member, the CommandRegistry plumbing, and the single call site on `version show`. That carve-out existed to give a query command a quieter default than normal; with minimal as the base there is nothing left for it to do. Nothing replaces it: verbosity is process-wide, and a per-command exception would restore the noisy MSBuild log in `release`, which runs the build pipeline itself and has the longest log of all. Program's DefaultVerbosity is a constant because the pre-verbosity error path needs the same value: a reporter built before --verbosity is parsed must filter like the one built after it. Two unreleased changelog entries described behavior this commit changes -- `version show`'s own minimal default, and the resolved verbosity forwarded to `dotnet` -- and are corrected in place rather than left to contradict the new entries in the same release. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 6 ++++-- .../ConsoleOutput/Verbosity.cs | 2 +- .../Infrastructure/Execution/CommandRegistration.cs | 5 +---- .../Infrastructure/Execution/CommandRegistry.cs | 1 - .../Execution/ImplementsCommandAttribute.cs | 13 ------------- src/Buildvana.Tool/Program.cs | 12 +++++++++--- src/Buildvana.Tool/Subcommands/GlobalSettings.cs | 2 +- .../Subcommands/VersionShowCommand.cs | 3 +-- .../ImplementsCommandAttributeTests.cs | 8 -------- 9 files changed, 17 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95188ed2..f4d558fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,10 +43,12 @@ See the Nerdbank.GitVersioning removal entry under _Changes to existing features - Before running any command that uses Buildvana SDK (`restore`, `build`, `test`, `pack`, and `release`), `bv` now verifies that the repository pins the SDK (the `Buildvana.Sdk` entry under `msbuild-sdks` in `global.json`) at its own version: `bv`, `Buildvana.Sdk`, and `Buildvana.Runtime` are released in lockstep, and a version mismatch — a half-updated repository, a newer globally-installed tool against an older pin — would otherwise produce silent behavior drift. A missing `global.json`, section, or entry counts as a mismatch. On mismatch, the command fails with a message naming both versions and the ways to align them. Versions are compared by SemVer precedence, ignoring build metadata. The new global option `--skip-sdk-check` skips the check, for scenarios that require a deliberate mismatch (e.g. bisecting an SDK regression in CI). - `bv` now delegates to the repository's pinned version: whenever the tool manifest (`.config/dotnet-tools.json`) pins `bv`, the pinned version is the one that runs, no matter which `bv` is invoked — like the Angular CLI, where the global `ng` always hands over to the project-local install. The invoked `bv` makes sure the pinned version is installed — probing the SDK's tool resolver cache the same way `dotnet tool run` does, and running `dotnet tool restore` only when needed; a failed restore is reported but does not block the attempt — and hands it the entire original command line (`dotnet tool run bv`) with inherited standard streams, forwarding its exit code. The delegated `bv` runs from the home directory, so a relative path inside forwarded arguments resolves against the home directory rather than the invocation directory; and `--version` answers for the pinned `bv` (pass `--skip-delegation` to ask the invoked binary). When the versions differ, an info line on standard error names the version that runs. A delegating `bv` does not judge the command line beyond the minimal split that finds the subcommand and the global options (only a value-bearing global option with no following value, such as a trailing `-v`, is rejected before delegation, with the same message in every version), and does not read the configuration file — both may be valid for the pinned version and not for the invoked one, and judging them is the pinned version's job. The new `update` subcommand is exempt (see below); the new global option `--skip-delegation` runs the exact binary invoked; and the `BV_DELEGATED` environment variable, set on the delegated child, guarantees that a delegated invocation never delegates again. The variable is removed from the environment of every other child process `bv` spawns, so a `bv` reached through a hook or a build makes its own delegation decision. A `bv` invoked outside a repository, or in a repository whose tool manifest does not pin `bv` (the entry is matched case-insensitively, like the dotnet CLI matches it; an entry with an unusable version is reported and treated as no pin), runs in place as before. - A new `bv update` command updates the repository's entire Buildvana surface to the running `bv`'s version in one operation: the `bv` pin in the tool manifest, via `dotnet tool update` (or `dotnet tool install --create-manifest-if-needed` when there is no entry yet), which also downloads the version; the `Buildvana.Sdk` pin in `global.json`, creating the file and/or the `msbuild-sdks` section if needed and preserving the file's formatting otherwise; and the version segment of the configuration file's `$schema` reference, when it points at the canonical `Tenacom/Buildvana//schemas/` URL. Afterwards, the configuration file is loaded with the new version's model, and any problems are reported as warnings for review. `update` is exempt from delegation — it updates the repository to the `bv` actually invoked ("bring this repository to me"), so the usual upgrade flow is `dotnet tool update -g bv` followed by `bv update`, and `dnx bv@ update` targets any specific version — and it refuses to downgrade a repository whose pins are newer than the running `bv`, unless `--force` is passed. A manifest whose `bv` entry pins an invalid version is beyond the dotnet CLI's reach entirely (the CLI cannot parse such a manifest), so `bv update` fails up front with a message naming the entry to fix. -- `bv` has a new `version` command group for working with native versioning outside of a release. `bv version show` (also reachable as plain `bv version`) prints the computed current version alongside the latest and latest stable published versions, the public-release and prerelease flags, and the current branch; the report is the command's deliverable, printed regardless of verbosity, and the command defaults to minimal verbosity so that by default the report is all there is (pass `--verbosity normal` or higher for diagnostics). `bv version advance [CHANGE]` applies a version-spec change (`none`, `unstable`, `stable`, `minor`, or `major`, the same values as `bv release --bump`) to the `VERSION` file, running the change through the same analysis as `bv release` (published-version comparison plus public API check, the latter controlled by `--check-public-api` and `release.checkPublicApi`); pass `--force` to apply the requested change verbatim, skipping the analysis. The modified `VERSION` file is left uncommitted for review. +- `bv` has a new `version` command group for working with native versioning outside of a release. `bv version show` (also reachable as plain `bv version`) prints the computed current version alongside the latest and latest stable published versions, the public-release and prerelease flags, and the current branch; the report is the command's deliverable, printed regardless of verbosity, so that at the default verbosity the report is all there is (pass `--verbosity normal` or higher for diagnostics). `bv version advance [CHANGE]` applies a version-spec change (`none`, `unstable`, `stable`, `minor`, or `major`, the same values as `bv release --bump`) to the `VERSION` file, running the change through the same analysis as `bv release` (published-version comparison plus public API check, the latter controlled by `--check-public-api` and `release.checkPublicApi`); pass `--force` to apply the requested change verbatim, skipping the analysis. The modified `VERSION` file is left uncommitted for review. ### Changes to existing features +- **BREAKING CHANGE**: `bv` now defaults to `minimal` verbosity, for every command alike; it used to default to `normal`. The build pipeline commands wrap `dotnet restore`/`build`/`test`/`pack`, which default to `minimal` themselves and receive `bv`'s verbosity verbatim, so `bv build` used to produce a markedly noisier MSBuild log than plain `dotnet build`; it now produces a comparable one. Nothing fails and no migration is required: pass `-v normal` for the previous output. Note that `bv`'s own narration — activity start/finish lines and the `info:` lines describing what the tool is doing — is hidden at the new default, while the record of what a command _did_ survives it (see the new `notice:` level below), so `bv release` still logs its complete account of what it changed and published. +- A new message level, rendered as `notice:`, sits between `warning:` and `info:` and is shown from `minimal` verbosity up. It carries the messages that record a fact — something changed, something was decided, something was deliberately skipped — as opposed to the narration of what a command is doing at a given moment, which stays at `info:`. This is also the level at which the version computed for a build is reported, so that line keeps appearing in `dotnet build` output at its default verbosity, as it did before. Buildvana SDK tasks and `bv` now agree on the verbosity at which each level becomes visible: previously a message logged by a task showed up one rung earlier than the same message printed by `bv`. - **BREAKING CHANGE**: The `.buildvana-home` marker file is no longer recognized: home-directory discovery now only looks for a Buildvana configuration file (in the home directory root or in its `.buildvana` subdirectory) and Git markers. `.buildvana-home` predates the configuration file, and a `buildvana.json` containing an empty object (`{}`) does the same job — marking a directory as home without configuring anything — while being what one would naturally reach for. To migrate, replace `.buildvana-home` with a `buildvana.json` file containing `{}`. - **BREAKING CHANGE**: The `JetBrainsAnnotations` module no longer adds any JetBrains annotations package to your project, and the `UseJetBrainsAnnotations` property has been removed. To export ReSharper external annotations, reference an annotations source yourself — the compiled `JetBrains.Annotations` package, `JetBrains.Annotations.Sources`, or your own attributes in the `JetBrains.Annotations` namespace — and set the new boolean `ExportJetBrainsAnnotations` property (default `false`) to `true`. When enabled, Buildvana SDK reads the annotations from source with Roslyn after each build and packs a `{AssemblyName}.ExternalAnnotations.xml` file next to the assembly in `lib/`, one per target framework. The export no longer depends on Mono.Cecil or on a second build pass; and, when the annotation attributes are `[Conditional("JETBRAINS_ANNOTATIONS")]` (as in the JetBrains packages), no JetBrains attribute metadata remains in the compiled assembly, leaving clean IL and AOT output. - **BREAKING CHANGE**: The `JetBrainsAnnotations` module no longer supports Visual Basic projects. `ExportJetBrainsAnnotations` is forced to `false` for any project that is not a C# (`.csproj`) project, because the exporter reads C# source directly. @@ -80,7 +82,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. +- **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 `minimal`) 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. - **BREAKING CHANGE**: `bv release` rejects a `--` separator (and anything after it): unlike the pipeline commands, it has no underlying `dotnet` pass-through to forward arguments to. diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs index 194267c4..b2e2b0e3 100644 --- a/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/Verbosity.cs @@ -18,7 +18,7 @@ public enum Verbosity /// Only errors are shown. Quiet, - /// Errors, warnings, and notices are shown. + /// Errors, warnings, and notices are shown. This is bv's default, as it is the .NET CLI's. Minimal, /// Everything shows, plus informational messages. diff --git a/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistration.cs b/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistration.cs index 180b6591..a2a41954 100644 --- a/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistration.cs +++ b/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistration.cs @@ -3,20 +3,18 @@ using System; using System.Collections.Generic; -using Buildvana.Core.ConsoleOutput; namespace Buildvana.Tool.Infrastructure.Execution; /// /// A discovered bv command: the paths it is registered under, the class that implements it, whether -/// it forwards all of its arguments verbatim, its settings type (if any), and its default verbosity. Produced by +/// it forwards all of its arguments verbatim, and its settings type (if any). Produced by /// from . /// /// The paths the command is invoked under, each as a list of segments. The first path is canonical. /// The class implementing the command. /// Whether the command forwards all of its arguments verbatim. /// The command's *Settings type, or if it has none. -/// The verbosity in effect when --verbosity is not given. /// Whether the command uses the repository's pinned Buildvana SDK and must therefore /// pass the SDK version check before running. internal sealed record CommandRegistration( @@ -24,7 +22,6 @@ internal sealed record CommandRegistration( Type CommandType, bool ConsumesAllArguments, Type? SettingsType, - Verbosity DefaultVerbosity = Verbosity.Normal, bool UsesSdk = false) { /// diff --git a/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistry.cs b/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistry.cs index ebf8faa8..3fe180ad 100644 --- a/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistry.cs +++ b/src/Buildvana.Tool/Infrastructure/Execution/CommandRegistry.cs @@ -190,7 +190,6 @@ private static IReadOnlyList Discover() type, attribute.ConsumesAllArguments, attribute.SettingsType, - attribute.DefaultVerbosity, attribute.UsesSdk)); } } diff --git a/src/Buildvana.Tool/Infrastructure/Execution/ImplementsCommandAttribute.cs b/src/Buildvana.Tool/Infrastructure/Execution/ImplementsCommandAttribute.cs index 5f530b60..0c1b8101 100644 --- a/src/Buildvana.Tool/Infrastructure/Execution/ImplementsCommandAttribute.cs +++ b/src/Buildvana.Tool/Infrastructure/Execution/ImplementsCommandAttribute.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; -using Buildvana.Core.ConsoleOutput; namespace Buildvana.Tool.Infrastructure.Execution; @@ -41,11 +40,6 @@ internal sealed class ImplementsCommandAttribute : Attribute /// -decorated properties the help renderer /// enumerates; for commands with no options or arguments of their own. /// - /// - /// The verbosity in effect when --verbosity is not given. Commands whose deliverable is their console - /// output (query commands) use so that only their result, warnings, and - /// errors are shown by default. - /// /// /// if the command uses the repository's pinned Buildvana SDK — by running MSBuild /// targets on solution projects, or anything else whose outcome depends on the global.json pin — @@ -57,7 +51,6 @@ public ImplementsCommandAttribute( string aliases, bool consumesAllArguments = false, Type? settingsType = null, - Verbosity defaultVerbosity = Verbosity.Normal, bool usesSdk = false) { ArgumentException.ThrowIfNullOrWhiteSpace(aliases); @@ -77,7 +70,6 @@ public ImplementsCommandAttribute( AliasPaths = paths; ConsumesAllArguments = consumesAllArguments; SettingsType = settingsType; - DefaultVerbosity = defaultVerbosity; UsesSdk = usesSdk; } @@ -101,11 +93,6 @@ public ImplementsCommandAttribute( /// public Type? SettingsType { get; } - /// - /// Gets the verbosity in effect when --verbosity is not given. - /// - public Verbosity DefaultVerbosity { get; } - /// /// Gets a value indicating whether the command uses the repository's pinned Buildvana SDK and must /// therefore pass the SDK version check before running. diff --git a/src/Buildvana.Tool/Program.cs b/src/Buildvana.Tool/Program.cs index a203ae8d..b645a90f 100644 --- a/src/Buildvana.Tool/Program.cs +++ b/src/Buildvana.Tool/Program.cs @@ -31,6 +31,10 @@ internal static class Program // 128 + SIGINT (2): the POSIX convention for a process terminated by Ctrl-C. private const int CancelledExitCode = 130; + // The verbosity in effect when --verbosity is not given, for every command alike. It matches the default of + // `dotnet restore`/`build`/`test`/`pack`, which the build pipeline commands wrap and forward it to. + private const Verbosity DefaultVerbosity = Verbosity.Minimal; + public static async Task Main(string[] args) { // Before anything else, including Spectre's console: replacing the console's encoding resets Console.Out @@ -106,8 +110,10 @@ public static async Task Main(string[] args) CommandArgumentValidator.Validate(command, parsed, positionals); // Parse --verbosity eagerly so an invalid value surfaces in the outer catch. - // When absent, the command's own default applies (query commands default to Minimal). - var verbosity = globals.Verbosity is null ? command.DefaultVerbosity : VerbosityParser.Parse(globals.Verbosity); + // The default is the .NET CLI's, so that a bv command's output is comparable to that of the + // dotnet command underneath it. It is uniform across commands: verbosity is process-wide, and a + // command that runs the build pipeline would restore the noisier log through the back door. + var verbosity = globals.Verbosity is null ? DefaultVerbosity : VerbosityParser.Parse(globals.Verbosity); // --color / --no-color win over auto-detection; neither (or both) leaves the reporter to auto-detect. bool? colorOverride = (globals.Color, globals.NoColor) switch @@ -191,7 +197,7 @@ void OnCancel(object? sender, ConsoleCancelEventArgs e) return ex.ExitCode; } - static IReporter CreateDefaultReporter() => new ConsoleReporter(Verbosity.Normal, colorOverride: null); + static IReporter CreateDefaultReporter() => new ConsoleReporter(DefaultVerbosity, colorOverride: null); } private static Task TryDelegateAsync(string[] args, ParsedCommandLine parsed, GlobalSettings globals) diff --git a/src/Buildvana.Tool/Subcommands/GlobalSettings.cs b/src/Buildvana.Tool/Subcommands/GlobalSettings.cs index 1b48f73e..c2873cc5 100644 --- a/src/Buildvana.Tool/Subcommands/GlobalSettings.cs +++ b/src/Buildvana.Tool/Subcommands/GlobalSettings.cs @@ -22,7 +22,7 @@ namespace Buildvana.Tool.Subcommands; /// The constructor parameter order is also the order in which these options appear in bv's help. internal sealed record GlobalSettings( [property: BvOption("-v|--verbosity ")] - [property: Description("Logging verbosity. One of: quiet, minimal, normal, detailed, diagnostic. Defaults to normal.")] + [property: Description("Logging verbosity. One of: quiet, minimal, normal, detailed, diagnostic. Defaults to minimal.")] string? Verbosity, [property: BvOption("--color")] [property: Description("Force ANSI color output even when not connected to a TTY.")] diff --git a/src/Buildvana.Tool/Subcommands/VersionShowCommand.cs b/src/Buildvana.Tool/Subcommands/VersionShowCommand.cs index 363bca88..f1d4f0a6 100644 --- a/src/Buildvana.Tool/Subcommands/VersionShowCommand.cs +++ b/src/Buildvana.Tool/Subcommands/VersionShowCommand.cs @@ -4,7 +4,6 @@ using System.ComponentModel; using System.Threading; using System.Threading.Tasks; -using Buildvana.Core.ConsoleOutput; using Buildvana.Tool.Infrastructure.Execution; using Buildvana.Tool.Services.Git; using Buildvana.Tool.Services.Versioning; @@ -12,7 +11,7 @@ namespace Buildvana.Tool.Subcommands; -[ImplementsCommand("version show | version", defaultVerbosity: Verbosity.Minimal)] +[ImplementsCommand("version show | version")] [Description("Show current and published version information.")] internal sealed class VersionShowCommand(VersionService version, GitService git, IAnsiConsole console) : IBvCommand { diff --git a/tests/Buildvana.Tool.Tests/ImplementsCommandAttributeTests.cs b/tests/Buildvana.Tool.Tests/ImplementsCommandAttributeTests.cs index f46bba73..174a435e 100644 --- a/tests/Buildvana.Tool.Tests/ImplementsCommandAttributeTests.cs +++ b/tests/Buildvana.Tool.Tests/ImplementsCommandAttributeTests.cs @@ -1,7 +1,6 @@ // Copyright (C) Tenacom and Contributors. Licensed under the MIT license. // See the LICENSE file in the project root for full license information. -using Buildvana.Core.ConsoleOutput; using Buildvana.Tool.Infrastructure.Execution; internal sealed class ImplementsCommandAttributeTests @@ -23,13 +22,6 @@ public async Task Aliases_AreNormalizedToLowercase() await Assert.That(string.Join(' ', attribute.AliasPaths[0])).IsEqualTo("version show"); } - [Test] - public async Task DefaultVerbosity_DefaultsToNormal() - { - var attribute = new ImplementsCommandAttribute("foo"); - await Assert.That(attribute.DefaultVerbosity).IsEqualTo(Verbosity.Normal); - } - [Test] [Arguments("")] [Arguments(" ")] From ae1a35b6a451a38ff188241a87d7555e8d652c9e Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 00:24:12 +0200 Subject: [PATCH 04/26] Align the CI workflows with the new default verbosity release.yml offered three of the five verbosity values and defaulted to normal; it now offers all five in ladder order and defaults to minimal. Its `if [ -z "$BV_VERBOSITY" ]` line goes: the input is required and defaulted on a dispatch-only workflow, so it can never be empty. Note that dispatching a release at quiet now hides the release record entirely, Notice being gated at minimal -- that is the level doing its job, but worth knowing for an operation that mutates and pushes. build-test-pack.yml had no such input at all, so its own -z fallback pinned Normal unconditionally on every push and pull request. It now passes no --verbosity and lets bv's default apply. Both keep the RUNNER_DEBUG override, and the verbosity literals are lowercase in both files. No changelog entry: these are Buildvana's own CI workflows, not behavior we ship. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/build-test-pack.yml | 8 +++++--- .github/workflows/release.yml | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build-test-pack.yml b/.github/workflows/build-test-pack.yml index e457a436..ad9947e7 100644 --- a/.github/workflows/build-test-pack.yml +++ b/.github/workflows/build-test-pack.yml @@ -48,9 +48,11 @@ jobs: - name: Build, test, and pack with Buildvana shell: bash run: | - if [ -z "$BV_VERBOSITY" ]; then BV_VERBOSITY=Normal; fi - if [ "$RUNNER_DEBUG" = "1" ]; then BV_VERBOSITY=Diagnostic; fi - dotnet bv pack --verbosity "$BV_VERBOSITY" + if [ "$RUNNER_DEBUG" = "1" ]; then + dotnet bv pack --verbosity diagnostic + else + dotnet bv pack + fi - name: Upload coverage to Codecov if: ${{ hashFiles('TestResults/*.cobertura.xml') != '' }} uses: codecov/codecov-action@v7 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 62f1ffad..3d2aca7b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -26,8 +26,10 @@ on: description: 'Buildvana verbosity' required: true type: choice - default: 'normal' + default: 'minimal' options: + - quiet + - minimal - normal - detailed - diagnostic @@ -81,7 +83,6 @@ jobs: id: build shell: bash run: | - if [ -z "$BV_VERBOSITY" ]; then BV_VERBOSITY=normal; fi if [ "$RUNNER_DEBUG" = "1" ]; then BV_VERBOSITY=diagnostic; fi dotnet bv release --verbosity "$BV_VERBOSITY" --bump "$BV_BUMP" - name: Upload coverage to Codecov From 415896b75bccbc89c7053de998a29fb56d0b9a4e Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 01:03:43 +0200 Subject: [PATCH 05/26] Cover the reporter extension shortcuts with tests The Notice(CompositeFormat, ...) overload added by this branch had no test, and neither did the eleven lines around it: the whole formatting-overload family, Error(string) and Trace(string), and Report's null check and its skip-formatting short-circuit were all uncovered before this branch touched the file. Each of the two table tests calls all six shortcuts against one reporter and asserts the six emitted lines, so what is pinned is the level-to-label mapping, not merely that the lines execute. Co-Authored-By: Claude Opus 5 (1M context) --- .../ReporterExtensionsTests.cs | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 tests/Buildvana.Core.ConsoleOutput.Tests/ReporterExtensionsTests.cs diff --git a/tests/Buildvana.Core.ConsoleOutput.Tests/ReporterExtensionsTests.cs b/tests/Buildvana.Core.ConsoleOutput.Tests/ReporterExtensionsTests.cs new file mode 100644 index 00000000..629bdc8d --- /dev/null +++ b/tests/Buildvana.Core.ConsoleOutput.Tests/ReporterExtensionsTests.cs @@ -0,0 +1,75 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Text; +using Buildvana.Core.ConsoleOutput; +using Buildvana.Core.Testing; + +internal sealed class ReporterExtensionsTests +{ + private static readonly CompositeFormat Format = CompositeFormat.Parse("{0} in {1}"); + + [Test] + public async Task MessageShortcuts_ReportAtTheirOwnLevel() + { + using var reporter = new StringWriterReporter(Verbosity.Diagnostic); + reporter.Error("boom"); + reporter.Warning("careful"); + reporter.Notice("noted"); + reporter.Info("working"); + reporter.Detail("details"); + reporter.Trace("trace"); + string[] expected = + [ + "error: boom", + "warning: careful", + "notice: noted", + "info: working", + "detail: details", + "trace: trace", + ]; + await Assert.That(reporter.ErrorText).IsEqualTo(string.Join(Environment.NewLine, expected) + Environment.NewLine); + await Assert.That(reporter.OutputText).IsEmpty(); + } + + [Test] + public async Task FormattingShortcuts_FormatArgumentsAndReportAtTheirOwnLevel() + { + using var reporter = new StringWriterReporter(Verbosity.Diagnostic); + reporter.Error(Format, "boom", 1); + reporter.Warning(Format, "careful", 2); + reporter.Notice(Format, "noted", 3); + reporter.Info(Format, "working", 4); + reporter.Detail(Format, "details", 5); + reporter.Trace(Format, "trace", 6); + string[] expected = + [ + "error: boom in 1", + "warning: careful in 2", + "notice: noted in 3", + "info: working in 4", + "detail: details in 5", + "trace: trace in 6", + ]; + await Assert.That(reporter.ErrorText).IsEqualTo(string.Join(Environment.NewLine, expected) + Environment.NewLine); + await Assert.That(reporter.OutputText).IsEmpty(); + } + + [Test] + public async Task Report_DisabledLevel_WritesNothing() + { + using var reporter = new StringWriterReporter(Verbosity.Minimal); + reporter.Report(MessageLevel.Info, Format, "working", 1); + await Assert.That(reporter.ErrorText).IsEmpty(); + await Assert.That(reporter.OutputText).IsEmpty(); + } + + [Test] + public async Task Report_NullFormat_Throws() + { + using var reporter = new StringWriterReporter(Verbosity.Diagnostic); + + // ReSharper disable once AccessToDisposedClosure // the assertion invokes the delegate before returning + await Assert.That(() => reporter.Report(MessageLevel.Info, (CompositeFormat)null!)).Throws(); + } +} From 863f7e6e435f7098734b63961e321230be0429c5 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:01:33 +0200 Subject: [PATCH 06/26] Record pushing and publishing as release outcomes At minimal verbosity `bv release` recorded what it had prepared - the version, the changelog edits, the file counts - and the packages it pushed, but never that it had pushed the release commits or published the release itself. The no-op branch of PushUpdates says "Repository unchanged, no commit to push." at notice level; its counterpart said nothing at all. Both outcome lines go in the server-independent layer, next to the facts they complete: ServerRelease.PublishAsync reports the published tag and its asset count once DoPublishAsync returns, and GitService.Push reports branch and remote after the push. Any future server adapter therefore inherits the record instead of reinventing it. The "Pushing..." and "Publishing..." lines stay at info: they narrate what is about to happen in front of a network call, which is not what the reader wants afterwards. The force-push branch is reported too. It runs during rollback, so it is the one push whose omission would leave the trail describing a remote state that was subsequently undone. Co-Authored-By: Claude Opus 5 (1M context) --- src/Buildvana.Tool/Services/Git/GitService.cs | 2 ++ .../Services/ServerAdapters/ServerRelease.cs | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/src/Buildvana.Tool/Services/Git/GitService.cs b/src/Buildvana.Tool/Services/Git/GitService.cs index 41f503f1..fcd82dc9 100644 --- a/src/Buildvana.Tool/Services/Git/GitService.cs +++ b/src/Buildvana.Tool/Services/Git/GitService.cs @@ -290,11 +290,13 @@ public void Push(bool force = false) _reporter.Info($"Force pushing changes to '{remote}'..."); var pushRefSpec = string.Format(CultureInfo.InvariantCulture, "+{0}:{0}", _repository.Head.CanonicalName); _repository.Network.Push(_repository.Network.Remotes[remote], pushRefSpec, pushOptions); + _reporter.Notice($"Force-pushed '{head.FriendlyName}' to '{remote}'."); } else { _reporter.Info($"Pushing changes to '{remote}'..."); _repository.Network.Push(head, pushOptions); + _reporter.Notice($"Pushed '{head.FriendlyName}' to '{remote}'."); } } diff --git a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs index 6bce60ff..4392a14f 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.IO; using System.Threading.Tasks; using Buildvana.Core.ConsoleOutput; @@ -223,6 +224,13 @@ public async Task PublishAsync() EnsurePending(); await DoPublishAsync(_assets).ConfigureAwait(false); + _reporter.Notice(_assets.Count switch + { + 0 => $"Published release {_version.CurrentStr} with no assets.", + 1 => $"Published release {_version.CurrentStr} with 1 asset.", + var count => string.Create(CultureInfo.InvariantCulture, $"Published release {_version.CurrentStr} with {count} assets."), + }); + OnRollback(async () => await UndoPublishAsync().ConfigureAwait(false)); await OnPublishedAsync().ConfigureAwait(false); _published = true; From c159204f3cfe85fafbbc10be8ee90c5bf3792e3c Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:01:49 +0200 Subject: [PATCH 07/26] Report a skipped test run at notice level "No test projects found, skipping tests." is a deliberate skip, which the MessageLevel.Notice criterion covers, and nothing else records it: at the new default, `bv test` on a solution with no test project was indistinguishable from one whose tests all passed. Co-Authored-By: Claude Opus 5 (1M context) --- src/Buildvana.Tool/Services/DotNetService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Buildvana.Tool/Services/DotNetService.cs b/src/Buildvana.Tool/Services/DotNetService.cs index d27a96f7..b31beb5e 100644 --- a/src/Buildvana.Tool/Services/DotNetService.cs +++ b/src/Buildvana.Tool/Services/DotNetService.cs @@ -159,7 +159,7 @@ public async Task TestSolutionAsync(SolutionContext solution, string configurati if (!hasTestProjects) { - _reporter.Info("No test projects found, skipping tests."); + _reporter.Notice("No test projects found, skipping tests."); return; } From 0836f1bc40153eadf249cb0eeaf278e6201feafe Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:02:11 +0200 Subject: [PATCH 08/26] Record that a hook ran Running a repository-owned hook is a fact worth keeping: it can change files, and outside `bv release` - whose caller reports the file count separately - nothing else records that it happened. The absent-hook branch stays at info. Its cost grows with the number of hooks a command raises events for, and a repository with no hooks at all would pay a notice per event to be told about files it never had, which is the reverse of what the level is for. Co-Authored-By: Claude Opus 5 (1M context) --- src/Buildvana.Tool/Services/Hooks/HookRunner.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Buildvana.Tool/Services/Hooks/HookRunner.cs b/src/Buildvana.Tool/Services/Hooks/HookRunner.cs index c179eea6..83cdb27d 100644 --- a/src/Buildvana.Tool/Services/Hooks/HookRunner.cs +++ b/src/Buildvana.Tool/Services/Hooks/HookRunner.cs @@ -112,6 +112,7 @@ private async Task RunHookAsync(string context, string @event, object args path, workingDirectory: _home.HomeDirectory, cancellationToken: cancellationToken).ConfigureAwait(false); + _reporter.Notice($"Hook {hookName}: ran {relativePath}."); return true; } } From df14e81f535300d599a80a0138b2708d8908af86 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:02:22 +0200 Subject: [PATCH 09/26] Demote the hook args dump to trace level A whole JSON document on one line is fine-grained diagnostic chatter, not a detail to follow a release by: it belongs one rung further down. The same document is written to the hook's args file and left there after the run, so nothing is lost by asking for `--verbosity diagnostic` to see it inline. Co-Authored-By: Claude Opus 5 (1M context) --- src/Buildvana.Tool/Services/Hooks/HookRunner.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Buildvana.Tool/Services/Hooks/HookRunner.cs b/src/Buildvana.Tool/Services/Hooks/HookRunner.cs index 83cdb27d..90b5d8fb 100644 --- a/src/Buildvana.Tool/Services/Hooks/HookRunner.cs +++ b/src/Buildvana.Tool/Services/Hooks/HookRunner.cs @@ -103,7 +103,7 @@ private async Task RunHookAsync(string context, string @event, object args } var json = JsonSerializer.Serialize(args, args.GetType(), BuildvanaJsonContext.Default); - _reporter.Detail($"Hook {hookName}: args: {json}"); + _reporter.Trace($"Hook {hookName}: args: {json}"); var argsPath = _home.GetFullPath(WellKnownPaths.GetHookArgsFile(context, @event)); _ = UserDirectory.CreateDirectory(Path.GetDirectoryName(argsPath)!); await UserFile.WriteAllTextAsync(argsPath, json, cancellationToken).ConfigureAwait(false); From 33da375bc93464edc286efee2847c281994fa826 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:04:30 +0200 Subject: [PATCH 10/26] Correct the changelog entry on `bv version show` The entry claimed that at the default verbosity the report is all there is. It no longer is: the version line reporting height and publicity is now a notice, so `bv version show` writes it to standard error at the default too. What the command actually guarantees, and what the entry should have said in the first place, is that the report is alone on standard output - which is the property anyone piping it cares about, and one no verbosity setting can take away. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4d558fe..80e1e7a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,7 @@ See the Nerdbank.GitVersioning removal entry under _Changes to existing features - Before running any command that uses Buildvana SDK (`restore`, `build`, `test`, `pack`, and `release`), `bv` now verifies that the repository pins the SDK (the `Buildvana.Sdk` entry under `msbuild-sdks` in `global.json`) at its own version: `bv`, `Buildvana.Sdk`, and `Buildvana.Runtime` are released in lockstep, and a version mismatch — a half-updated repository, a newer globally-installed tool against an older pin — would otherwise produce silent behavior drift. A missing `global.json`, section, or entry counts as a mismatch. On mismatch, the command fails with a message naming both versions and the ways to align them. Versions are compared by SemVer precedence, ignoring build metadata. The new global option `--skip-sdk-check` skips the check, for scenarios that require a deliberate mismatch (e.g. bisecting an SDK regression in CI). - `bv` now delegates to the repository's pinned version: whenever the tool manifest (`.config/dotnet-tools.json`) pins `bv`, the pinned version is the one that runs, no matter which `bv` is invoked — like the Angular CLI, where the global `ng` always hands over to the project-local install. The invoked `bv` makes sure the pinned version is installed — probing the SDK's tool resolver cache the same way `dotnet tool run` does, and running `dotnet tool restore` only when needed; a failed restore is reported but does not block the attempt — and hands it the entire original command line (`dotnet tool run bv`) with inherited standard streams, forwarding its exit code. The delegated `bv` runs from the home directory, so a relative path inside forwarded arguments resolves against the home directory rather than the invocation directory; and `--version` answers for the pinned `bv` (pass `--skip-delegation` to ask the invoked binary). When the versions differ, an info line on standard error names the version that runs. A delegating `bv` does not judge the command line beyond the minimal split that finds the subcommand and the global options (only a value-bearing global option with no following value, such as a trailing `-v`, is rejected before delegation, with the same message in every version), and does not read the configuration file — both may be valid for the pinned version and not for the invoked one, and judging them is the pinned version's job. The new `update` subcommand is exempt (see below); the new global option `--skip-delegation` runs the exact binary invoked; and the `BV_DELEGATED` environment variable, set on the delegated child, guarantees that a delegated invocation never delegates again. The variable is removed from the environment of every other child process `bv` spawns, so a `bv` reached through a hook or a build makes its own delegation decision. A `bv` invoked outside a repository, or in a repository whose tool manifest does not pin `bv` (the entry is matched case-insensitively, like the dotnet CLI matches it; an entry with an unusable version is reported and treated as no pin), runs in place as before. - A new `bv update` command updates the repository's entire Buildvana surface to the running `bv`'s version in one operation: the `bv` pin in the tool manifest, via `dotnet tool update` (or `dotnet tool install --create-manifest-if-needed` when there is no entry yet), which also downloads the version; the `Buildvana.Sdk` pin in `global.json`, creating the file and/or the `msbuild-sdks` section if needed and preserving the file's formatting otherwise; and the version segment of the configuration file's `$schema` reference, when it points at the canonical `Tenacom/Buildvana//schemas/` URL. Afterwards, the configuration file is loaded with the new version's model, and any problems are reported as warnings for review. `update` is exempt from delegation — it updates the repository to the `bv` actually invoked ("bring this repository to me"), so the usual upgrade flow is `dotnet tool update -g bv` followed by `bv update`, and `dnx bv@ update` targets any specific version — and it refuses to downgrade a repository whose pins are newer than the running `bv`, unless `--force` is passed. A manifest whose `bv` entry pins an invalid version is beyond the dotnet CLI's reach entirely (the CLI cannot parse such a manifest), so `bv update` fails up front with a message naming the entry to fix. -- `bv` has a new `version` command group for working with native versioning outside of a release. `bv version show` (also reachable as plain `bv version`) prints the computed current version alongside the latest and latest stable published versions, the public-release and prerelease flags, and the current branch; the report is the command's deliverable, printed regardless of verbosity, so that at the default verbosity the report is all there is (pass `--verbosity normal` or higher for diagnostics). `bv version advance [CHANGE]` applies a version-spec change (`none`, `unstable`, `stable`, `minor`, or `major`, the same values as `bv release --bump`) to the `VERSION` file, running the change through the same analysis as `bv release` (published-version comparison plus public API check, the latter controlled by `--check-public-api` and `release.checkPublicApi`); pass `--force` to apply the requested change verbatim, skipping the analysis. The modified `VERSION` file is left uncommitted for review. +- `bv` has a new `version` command group for working with native versioning outside of a release. `bv version show` (also reachable as plain `bv version`) prints the computed current version alongside the latest and latest stable published versions, the public-release and prerelease flags, and the current branch; the report is the command's deliverable, printed regardless of verbosity and alone on standard output, so it stays pipeable whatever the verbosity (diagnostics go to standard error; pass `--verbosity normal` or higher for more of them). `bv version advance [CHANGE]` applies a version-spec change (`none`, `unstable`, `stable`, `minor`, or `major`, the same values as `bv release --bump`) to the `VERSION` file, running the change through the same analysis as `bv release` (published-version comparison plus public API check, the latter controlled by `--check-public-api` and `release.checkPublicApi`); pass `--force` to apply the requested change verbatim, skipping the analysis. The modified `VERSION` file is left uncommitted for review. ### Changes to existing features From c28227751d4fc9416758dc5752f9ec1f4ccfe4b7 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:06:35 +0200 Subject: [PATCH 11/26] Name the offending parameter "level" on an unknown message level MinimumVerbosity threw with nameof(@this), and nameof strips the @: the exception carried ParamName "this", a name no caller of the method can see. It also contradicted IsEnabled, which documents the exception as being about its `level` parameter, and it silently renamed what TextWriterReporter.Report used to throw - reporting an unknown level was StyleFor's job before IsEnabled began consulting MinimumVerbosity, and StyleFor named it "level". The throw moves into a static local function whose parameter is actually called `level`, so nameof keeps doing the work and nothing has to be kept in sync by hand. Both tests that exercise the unknown-level path asserted the exception type alone, which is why the change went unnoticed; they now assert ParamName too. Co-Authored-By: Claude Opus 5 (1M context) --- .../ConsoleOutput/MessageLevelExtensions.cs | 25 +++++++++++++------ .../MessageLevelExtensionsTests.cs | 3 ++- .../TextWriterReporterTests.cs | 5 +++- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs index 4b8a6c85..4826db62 100644 --- a/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs +++ b/src/Buildvana.Core.Abstractions/ConsoleOutput/MessageLevelExtensions.cs @@ -28,14 +28,23 @@ public static class MessageLevelExtensions /// than there are thresholds, so no ordering of the members makes such a comparison give the right /// answer for all of them. /// - public Verbosity MinimumVerbosity() => @this switch + public Verbosity MinimumVerbosity() { - MessageLevel.Error => Verbosity.Quiet, - MessageLevel.Warning or MessageLevel.Notice => Verbosity.Minimal, - MessageLevel.Info => Verbosity.Normal, - MessageLevel.Detail => Verbosity.Detailed, - MessageLevel.Trace => Verbosity.Diagnostic, - _ => throw new ArgumentOutOfRangeException(nameof(@this), @this, "Unknown message level."), - }; + return @this switch + { + MessageLevel.Error => Verbosity.Quiet, + MessageLevel.Warning or MessageLevel.Notice => Verbosity.Minimal, + MessageLevel.Info => Verbosity.Normal, + MessageLevel.Detail => Verbosity.Detailed, + MessageLevel.Trace => Verbosity.Diagnostic, + _ => ThrowUnknownLevel(@this), + }; + + // The exception names the offending value "level", as every caller of this method does. The name + // cannot come from the receiver: nameof(@this) yields "this", which names the parameter the + // compiler emits rather than anything a caller can see. + static Verbosity ThrowUnknownLevel(MessageLevel level) + => throw new ArgumentOutOfRangeException(nameof(level), level, "Unknown message level."); + } } } diff --git a/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs b/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs index 3323c4b9..d2e81e80 100644 --- a/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs +++ b/tests/Buildvana.Core.ConsoleOutput.Tests/MessageLevelExtensionsTests.cs @@ -33,6 +33,7 @@ public async Task MinimumVerbosity_CoversEveryLevel() [Arguments(42)] public async Task MinimumVerbosity_UnknownLevel_Throws(int level) { - await Assert.That(() => ((MessageLevel)level).MinimumVerbosity()).Throws(); + var exception = await Assert.That(() => ((MessageLevel)level).MinimumVerbosity()).Throws(); + await Assert.That(exception?.ParamName).IsEqualTo("level"); } } diff --git a/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs b/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs index 79bc15d6..3948bb2a 100644 --- a/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs +++ b/tests/Buildvana.Core.ConsoleOutput.Tests/TextWriterReporterTests.cs @@ -79,7 +79,10 @@ public async Task Report_UnknownLevel_Throws() using var reporter = new StringWriterReporter(Verbosity.Diagnostic); // ReSharper disable once AccessToDisposedClosure // the assertion invokes the delegate before returning - await Assert.That(() => reporter.Report((MessageLevel)(-1), "something happened")).Throws(); + var exception = await Assert.That(() => reporter.Report((MessageLevel)(-1), "something happened")) + .Throws(); + + await Assert.That(exception?.ParamName).IsEqualTo("level"); } [Test] From 394e6a61121dae9ed66e3a0711f02aef22eb4c05 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:08:26 +0200 Subject: [PATCH 12/26] Wrap the over-long lines this branch modified Three notice call sites in ReleaseCommand were already past 140 characters before this branch, and promoting them from Info to Notice pushed them two characters further. Modified lines comply, so they are wrapped here; the lines this branch never touched are dealt with separately. Co-Authored-By: Claude Opus 5 (1M context) --- src/Buildvana.Tool/Subcommands/ReleaseCommand.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs index 0431fa64..1a95c37d 100644 --- a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs +++ b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs @@ -172,7 +172,8 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) emptyChangelogSubstitute is null, "Changelog check failed: the \"Unreleased changes\" section is empty or only contains sub-section headings, and no substitute text is configured (release.emptyChangelog)."); - reporter.Notice("Changelog \"Unreleased changes\" section is empty; substituting the configured release.emptyChangelog text."); + reporter.Notice( + "Changelog \"Unreleased changes\" section is empty; substituting the configured release.emptyChangelog text."); } // Update the changelog and commit the change before building. @@ -247,7 +248,9 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) reporter.Notice("The post-release hook modified 1 file."); break; default: - reporter.Notice(string.Create(CultureInfo.InvariantCulture, $"The post-release hook modified {hookUpdates.Count} files.")); + reporter.Notice(string.Create( + CultureInfo.InvariantCulture, + $"The post-release hook modified {hookUpdates.Count} files.")); break; } } @@ -271,7 +274,9 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) reporter.Notice("1 self-referenced file was modified."); break; default: - reporter.Notice(string.Create(CultureInfo.InvariantCulture, $"{selfReferenceUpdates.Count} self-referenced files were modified.")); + reporter.Notice(string.Create( + CultureInfo.InvariantCulture, + $"{selfReferenceUpdates.Count} self-referenced files were modified.")); break; } } From 57322ee30551be973caf7c52586266d55b798b66 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:11:10 +0200 Subject: [PATCH 13/26] Bring the rest of ReleaseCommand within the line limits The leftover wraps, kept out of the previous commit so that the change under review stays readable: five lines that were already past 140 characters and that this branch never touched, plus the primary constructor, which a declaration's stricter 120-character limit catches. Two of the five are the arguments of calls that take more than one, so they cannot simply span two lines (SA1118). The long error message is hoisted into a local constant, which is also where a reader looking for the text will now find it; the two warnings wrap around string.Create, like every other formatted message in the file. Co-Authored-By: Claude Opus 5 (1M context) --- .../Subcommands/ReleaseCommand.cs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs index 1a95c37d..e284d40e 100644 --- a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs +++ b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs @@ -31,7 +31,10 @@ namespace Buildvana.Tool.Subcommands; [ImplementsCommand("release", settingsType: typeof(ReleaseSettings), usesSdk: true)] [Description("Publish a new public release (CI only).")] -internal sealed class ReleaseCommand(IServiceProvider services, ReleaseSettings settings, BuildPipeline pipeline) : IBvCommand +internal sealed class ReleaseCommand( + IServiceProvider services, + ReleaseSettings settings, + BuildPipeline pipeline) : IBvCommand { public async Task ExecuteAsync(CancellationToken cancellationToken) { @@ -63,7 +66,9 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) BuildFailedException.ThrowIfNot(version.IsPublicRelease, "Cannot create a release from the current branch."); // Ensure that the CI bot identity is used for commits, if not already set. - git.CommitterIdentity ??= server.CIBotIdentity ?? throw new BuildFailedException("Cannot determine a committer identity for release commits. Configure git config user.name/user.email before running this task."); + git.CommitterIdentity ??= server.CIBotIdentity ?? throw new BuildFailedException( + "Cannot determine a committer identity for release commits. " + + "Configure git config user.name/user.email before running this task."); reporter.Notice($"Using committer identity: {git.CommitterIdentity.Name} <{git.CommitterIdentity.Email}>"); // Set fallback Git credentials if the server adapter can provide them. @@ -76,7 +81,9 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) } else { - reporter.Warning("No push credentials provided by the server adapter. Push operations may fail if the repository is not already authenticated."); + reporter.Warning( + "No push credentials provided by the server adapter. " + + "Push operations may fail if the repository is not already authenticated."); } // Perform an initial versioning consistency check. @@ -167,10 +174,11 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) } else { + const string failureMessage = "Changelog check failed: the \"Unreleased changes\" section is empty " + + "or only contains sub-section headings, and no substitute text is configured (release.emptyChangelog)."; + emptyChangelogSubstitute = settings.ResolveEmptyChangelog(); - BuildFailedException.ThrowIf( - emptyChangelogSubstitute is null, - "Changelog check failed: the \"Unreleased changes\" section is empty or only contains sub-section headings, and no substitute text is configured (release.emptyChangelog)."); + BuildFailedException.ThrowIf(emptyChangelogSubstitute is null, failureMessage); reporter.Notice( "Changelog \"Unreleased changes\" section is empty; substituting the configured release.emptyChangelog text."); @@ -309,13 +317,17 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) var parts = line.Split('\t'); if (parts.Length != 3) { - reporter.Warning(string.Create(CultureInfo.InvariantCulture, $"Release asset list {path}, line #{i}: invalid line '{line}'")); + reporter.Warning(string.Create( + CultureInfo.InvariantCulture, + $"Release asset list {path}, line #{i}: invalid line '{line}'")); continue; } if (!File.Exists(parts[0])) { - reporter.Warning(string.Create(CultureInfo.InvariantCulture, $"Release asset list {path}, line #{i}: asset not found '{parts[0]}'")); + reporter.Warning(string.Create( + CultureInfo.InvariantCulture, + $"Release asset list {path}, line #{i}: asset not found '{parts[0]}'")); continue; } From 24c402c2aeab3c2fc1e01d05fc7aaf42aef2245d Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:42:51 +0200 Subject: [PATCH 14/26] Record the publication only once it cannot be undone PublishAsync printed its notice as soon as DoPublishAsync returned, then registered the rollback that undoes the publication and awaited OnPublishedAsync. That last step can still fail - it writes the Actions step output, so an unset GITHUB_OUTPUT or an I/O error is enough - and its failure leaves _published false, so DisposeAsync rolls the release back and deletes the very release and tag the log had just claimed. The notice moves to the end of the method, past the point where the rollback actions are cleared. Nothing changes on the ordinary path: the steps it now follows print nothing of their own. Co-Authored-By: Claude Opus 5 (1M context) --- .../Services/ServerAdapters/ServerRelease.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs index 4392a14f..91ef78a0 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs @@ -224,17 +224,20 @@ public async Task PublishAsync() EnsurePending(); await DoPublishAsync(_assets).ConfigureAwait(false); + OnRollback(async () => await UndoPublishAsync().ConfigureAwait(false)); + await OnPublishedAsync().ConfigureAwait(false); + _published = true; + _rollbackActions.Clear(); + + // The record of the publication comes last, once nothing can undo it any more: OnPublishedAsync + // is the final step that can still fail, and its failure rolls the whole release back - deleting + // the release and the tag that a notice printed any earlier would already have claimed. _reporter.Notice(_assets.Count switch { 0 => $"Published release {_version.CurrentStr} with no assets.", 1 => $"Published release {_version.CurrentStr} with 1 asset.", var count => string.Create(CultureInfo.InvariantCulture, $"Published release {_version.CurrentStr} with {count} assets."), }); - - OnRollback(async () => await UndoPublishAsync().ConfigureAwait(false)); - await OnPublishedAsync().ConfigureAwait(false); - _published = true; - _rollbackActions.Clear(); } public async ValueTask DisposeAsync() From 7b05f70782eab454118124b09570b59b4f3e0bd6 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:43:34 +0200 Subject: [PATCH 15/26] Record what a rolled-back release undoes The argument that promoted the rollback's force push covers the rest of the rollback verbatim: each of its steps undoes state that an earlier notice claimed. Without them, a release that fails and rolls back reads, at the default verbosity, as a pushed branch and a published release followed by an error and a force push, with nothing to say that the release and its tag were deleted and the commits reset. Deleting the release and deleting the tag get one notice each, right after the act, as the pushes do; the paths that find no tag to delete stay at info level, having changed nothing. Co-Authored-By: Claude Opus 5 (1M context) --- src/Buildvana.Tool/Services/Git/GitService.cs | 1 + .../ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/Buildvana.Tool/Services/Git/GitService.cs b/src/Buildvana.Tool/Services/Git/GitService.cs index fcd82dc9..40aad862 100644 --- a/src/Buildvana.Tool/Services/Git/GitService.cs +++ b/src/Buildvana.Tool/Services/Git/GitService.cs @@ -261,6 +261,7 @@ public void UndoLastCommit() var previousCommit = _repository.Head.Tip.Parents.FirstOrDefault(); BuildFailedException.ThrowIf(previousCommit is null, "Git: cannot reset, there is no commit to go back to."); _repository.Reset(ResetMode.Hard, previousCommit); + _reporter.Notice("Undid the last commit."); } /// diff --git a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs index ac5225e2..b6f656a6 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs @@ -188,6 +188,7 @@ public async Task DeleteReleaseAsync(Release release, string? tagName) _reporter.Info("Deleting the previously created release..."); var client = CreateGitHubClient(); await client.Repository.Release.Delete(RepositoryOwner, RepositoryName, release.Id).ConfigureAwait(false); + _reporter.Notice("Deleted the previously created release."); if (string.IsNullOrEmpty(tagName)) { return; @@ -207,6 +208,7 @@ public async Task DeleteReleaseAsync(Release release, string? tagName) _reporter.Info($"Deleting reference '{reference}' in GitHub repository..."); await client.Git.Reference.Delete(RepositoryOwner, RepositoryName, reference).ConfigureAwait(false); + _reporter.Notice($"Deleted tag {tagName}."); } /// From 63dbee98ce0823bcc8d797a532ade63c9265b47a Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:43:50 +0200 Subject: [PATCH 16/26] Name a running hook without repeating its path "Hook release/post-release: ran .buildvana/hooks/release/post-release.cs." says the context and the event twice: the path is built from them, so it carries no information the hook name does not. Harmless while the line was narration; the line is a notice now, and appears in everyone's default output. Both the announcement and the outcome shed the path and read as the plain narration/outcome pair the pushes use. The line reporting an absent hook keeps its path: there, the path is the file one would create. Co-Authored-By: Claude Opus 5 (1M context) --- src/Buildvana.Tool/Services/Hooks/HookRunner.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Buildvana.Tool/Services/Hooks/HookRunner.cs b/src/Buildvana.Tool/Services/Hooks/HookRunner.cs index 90b5d8fb..4aa32094 100644 --- a/src/Buildvana.Tool/Services/Hooks/HookRunner.cs +++ b/src/Buildvana.Tool/Services/Hooks/HookRunner.cs @@ -107,12 +107,12 @@ private async Task RunHookAsync(string context, string @event, object args var argsPath = _home.GetFullPath(WellKnownPaths.GetHookArgsFile(context, @event)); _ = UserDirectory.CreateDirectory(Path.GetDirectoryName(argsPath)!); await UserFile.WriteAllTextAsync(argsPath, json, cancellationToken).ConfigureAwait(false); - _reporter.Info($"Hook {hookName}: running {relativePath}..."); + _reporter.Info($"Running hook {hookName}..."); _ = await _appRunner.RunFileBasedAppAsync( path, workingDirectory: _home.HomeDirectory, cancellationToken: cancellationToken).ConfigureAwait(false); - _reporter.Notice($"Hook {hookName}: ran {relativePath}."); + _reporter.Notice($"Hook {hookName} ran."); return true; } } From 163f84f2faf5df6e8744f7394178ffdc27731bdf Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:45:50 +0200 Subject: [PATCH 17/26] Read GITHUB_OUTPUT before the release exists The Actions step output is written after the release has been published, and SetActionsStepOutput read GITHUB_OUTPUT right there, failing the release if the variable was unset. That is the worst possible moment to discover it: the publication has succeeded, so the failure rolls it back, deleting a release and a tag that were correct. GitHubServerRelease now requires the variable in its factory method, before the draft release is created - nothing exists to be undone yet, and the message names the variable exactly as it would have before - and remembers the path for the whole life of the release. SetActionsStepOutput takes the path from its caller and is left with only the Actions-specific part: the name=value line. Co-Authored-By: Claude Opus 5 (1M context) --- docs/EnvironmentVariables.md | 2 +- .../Internal/GitHub/GitHubServerAdapter.cs | 15 +++++++----- .../Internal/GitHub/GitHubServerRelease.cs | 23 +++++++++++++++---- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/docs/EnvironmentVariables.md b/docs/EnvironmentVariables.md index 874e6dfe..7b0d99df 100644 --- a/docs/EnvironmentVariables.md +++ b/docs/EnvironmentVariables.md @@ -36,7 +36,7 @@ Set to `true` by GitHub Actions on every step. `bv` reads it to recognize that i ### `GITHUB_OUTPUT` -Set by GitHub Actions to the path of the file that collects a step's outputs. `bv release` appends to that file to publish the released version as the `version` step output, so that later steps of the same job can refer to it; the release fails if the variable is unset. `bv` never sets this variable itself. +Set by GitHub Actions to the path of the file that collects a step's outputs. `bv release` appends to that file to publish the released version as the `version` step output, so that later steps of the same job can refer to it; the release fails if the variable is unset, and fails up front, before creating anything, rather than at the moment the output is written. `bv` never sets this variable itself. ### `GITLAB_CI` diff --git a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs index b6f656a6..a26e0aad 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs @@ -94,14 +94,17 @@ private GitHubServerAdapter(IServiceProvider services) /// /// Sets a GitHub Actions step output. /// + /// The path of the file that collects the step's outputs, i.e. the value of + /// the GITHUB_OUTPUT environment variable. /// The output name. /// The output value. - public static void SetActionsStepOutput(string name, string value) - { - var outputFile = Environment.GetEnvironmentVariable("GITHUB_OUTPUT"); - BuildFailedException.ThrowIf(string.IsNullOrEmpty(outputFile), "Cannot set Actions step output: GITHUB_OUTPUT not set."); - UserFile.AppendAllLines(outputFile, [$"{name}={value}"], Encoding.UTF8); - } + /// + /// The caller provides the path, instead of this method reading the environment variable itself, + /// so that a missing variable is discovered while there is still nothing to undo. See + /// . + /// + public static void SetActionsStepOutput(string outputFile, string name, string value) + => UserFile.AppendAllLines(outputFile, [$"{name}={value}"], Encoding.UTF8); /// public override async Task IsPrivateRepositoryAsync() diff --git a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerRelease.cs b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerRelease.cs index 685fd7cb..50161d6c 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerRelease.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerRelease.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Buildvana.Core.ConsoleOutput; using Buildvana.Tool.Services.Versioning; +using Buildvana.Tool.Utilities; using CommunityToolkit.Diagnostics; using Microsoft.Extensions.DependencyInjection; using Octokit; @@ -23,10 +24,15 @@ internal sealed class GitHubServerRelease : ServerRelease private readonly IReporter _reporter; private readonly VersionService _version; private readonly Release _gitHubRelease; + private readonly string _actionsStepOutputFile; private bool _gitHubReleaseDeleted; - private GitHubServerRelease(GitHubServerAdapter server, IServiceProvider services, Release gitHubRelease) + private GitHubServerRelease( + GitHubServerAdapter server, + IServiceProvider services, + Release gitHubRelease, + string actionsStepOutputFile) : base(services) { Guard.IsNotNull(server); @@ -37,6 +43,7 @@ private GitHubServerRelease(GitHubServerAdapter server, IServiceProvider service _reporter = services.GetRequiredService(); _version = services.GetRequiredService(); _gitHubRelease = gitHubRelease; + _actionsStepOutputFile = actionsStepOutputFile; OnRollback(async () => { @@ -48,14 +55,22 @@ private GitHubServerRelease(GitHubServerAdapter server, IServiceProvider service }); } - public static async Task CreateAsync(GitHubServerAdapter server, IServiceProvider services, Func> createGitHubReleaseAsync) + public static async Task CreateAsync( + GitHubServerAdapter server, + IServiceProvider services, + Func> createGitHubReleaseAsync) { Guard.IsNotNull(server); Guard.IsNotNull(services); Guard.IsNotNull(createGitHubReleaseAsync); + // GITHUB_OUTPUT is read here, before the draft release exists, and remembered for the whole + // life of the release. Reading it where it is used - after publication, to write the step + // output - would let a variable that was never set fail a release that had otherwise + // succeeded, and roll back everything it had just done. + var actionsStepOutputFile = EnvVarHelper.Require("GITHUB_OUTPUT"); var gitHubRelease = await createGitHubReleaseAsync().ConfigureAwait(false); - return new(server, services, gitHubRelease); + return new(server, services, gitHubRelease, actionsStepOutputFile); } protected override async Task DoPublishAsync(IReadOnlyList assets) @@ -92,7 +107,7 @@ protected override async Task UndoPublishAsync() protected override Task OnPublishedAsync() { - GitHubServerAdapter.SetActionsStepOutput("version", _version.CurrentStr); + GitHubServerAdapter.SetActionsStepOutput(_actionsStepOutputFile, "version", _version.CurrentStr); return Task.CompletedTask; } } From ce86eee0adce2008bab27136fbb275df72e0015e Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 02:47:30 +0200 Subject: [PATCH 18/26] Bring ServerRelease and GitHubServerAdapter within the line limits The leftover wraps for the two files this round works in, kept apart from the changes under review as the style guide asks. Seven lines, none of them touched by this branch: four internal-error messages, a two-argument guard, a property initializer, and a call to the release-notes endpoint. The four messages are the sole argument of their call, so they simply move to the following line; the rest take the one-per-line treatment. Co-Authored-By: Claude Opus 5 (1M context) --- .../Internal/GitHub/GitHubServerAdapter.cs | 13 ++++++++++--- .../Services/ServerAdapters/ServerRelease.cs | 12 ++++++++---- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs index a26e0aad..a7b9b8e9 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs @@ -36,7 +36,9 @@ private GitHubServerAdapter(IServiceProvider services) _reporter = services.GetRequiredService(); _version = services.GetRequiredService(); _git = services.GetRequiredService(); - BuildFailedException.ThrowIfNot(GitUrlInfo.TryCreate(_git.OriginUrl, out var originInfo), $"Couldn't get information from origin URL '{_git.OriginUrl}'."); + BuildFailedException.ThrowIfNot( + GitUrlInfo.TryCreate(_git.OriginUrl, out var originInfo), + $"Couldn't get information from origin URL '{_git.OriginUrl}'."); BuildFailedException.ThrowIfNot(originInfo.PathSegments.Count == 2, $"'{originInfo.Url}' is not a valid GitHub repository URL."); HostName = originInfo.Host; RepositoryOwner = originInfo.PathSegments[0]; @@ -68,7 +70,9 @@ private GitHubServerAdapter(IServiceProvider services) public override bool IsCloudBuild => true; /// - public override GitIdentity? CIBotIdentity { get; } = new("github-actions[bot]", "41898282+github-actions[bot]@users.noreply.github.com"); + public override GitIdentity? CIBotIdentity { get; } = new( + "github-actions[bot]", + "41898282+github-actions[bot]@users.noreply.github.com"); /// public override string PushUsername => "x-access-token"; @@ -163,7 +167,10 @@ public async Task PublishReleaseAsync(Release release, string targetCommitish) TargetCommitish = targetCommitish, }; - var generateNotesResponse = await client.Repository.Release.GenerateReleaseNotes(RepositoryOwner, RepositoryName, releaseNotesRequest).ConfigureAwait(false); + var generateNotesResponse = await client.Repository.Release.GenerateReleaseNotes( + RepositoryOwner, + RepositoryName, + releaseNotesRequest).ConfigureAwait(false); var body = $"We also have a [human-curated changelog]({GetFileUrl("CHANGELOG.md", _git.CurrentBranch)}).\n\n---\n\n" + generateNotesResponse.Body; diff --git a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs index 91ef78a0..f508b494 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs @@ -77,7 +77,8 @@ public void EnsureReleaseCommit() if (_updatesPushed) { - ThrowHelper.ThrowInvalidOperationException("Internal error: cannot create the release commit when updates have already been pushed."); + ThrowHelper.ThrowInvalidOperationException( + "Internal error: cannot create the release commit when updates have already been pushed."); } if (_repositoryUpdated) @@ -122,7 +123,8 @@ public void UpdateRepository(params string[] files) if (_postReleaseCommits > 0) { - ThrowHelper.ThrowInvalidOperationException("Internal error: cannot update the release commit after a post-release commit has been added."); + ThrowHelper.ThrowInvalidOperationException( + "Internal error: cannot update the release commit after a post-release commit has been added."); } // Staging comes first, so that the files are already in the index when the release commit is @@ -165,12 +167,14 @@ public void AddPostReleaseCommit(string message, params string[] files) if (_updatesPushed) { - ThrowHelper.ThrowInvalidOperationException("Internal error: cannot add a post-release commit when updates have already been pushed."); + ThrowHelper.ThrowInvalidOperationException( + "Internal error: cannot add a post-release commit when updates have already been pushed."); } if (!_repositoryUpdated) { - ThrowHelper.ThrowInvalidOperationException("Internal error: cannot add a post-release commit before the release commit has been created."); + ThrowHelper.ThrowInvalidOperationException( + "Internal error: cannot add a post-release commit before the release commit has been created."); } _git.Stage(files); From 9e77a49cb647c5015eecf356a8ab6c27cb0edd89 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 10:05:52 +0200 Subject: [PATCH 19/26] Record the GITHUB_OUTPUT fix in the changelog The variable's read moved from after publication to before anything exists, which changes released behavior: an unset variable used to fail - and therefore roll back - a release that had already been published correctly, and the failure message changed with the move. The entry joins the other "bv release no longer publishes the wrong thing" siblings under Bugs fixed in this release. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80e1e7a3..75cf72d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,7 @@ See the Nerdbank.GitVersioning removal entry under _Changes to existing features - `bv release` no longer tags and publishes a version one patch above the one its artifacts were built with. The "Prepare release" commit bumps the Git height, hence the version, but it was only created when an earlier step had a file to commit; a release with nothing to commit before the build (typically a prerelease with no version-spec change and `release.changelogUpdates` set to `stable` or `none`) therefore built and pushed its packages at the pre-commit version, then created the commit, and tagged and released the version above. The release commit is now always created before the build. - `bv release` no longer tags and publishes a version one patch _below_ the one its artifacts were built with when the release applies a version-spec change (`--bump minor` or `--bump major`, or a minor bump forced by an additive public API change). The `VERSION` file reached the "Prepare release" commit only after the version had been computed, so at computation time the new version line had no commit carrying it, and its Git height came out as 0 — hence versions such as `2.4.0-preview`, which the height calculation can never legitimately produce, since it counts from 1 at the bump commit and reserves 0 for a version line with no committed history. The artifacts, built after the commit, correctly carried `2.4.1-preview`, while the tag, the release, and the hook args said `2.4.0-preview`; produced-package discovery, which matches packages by version, therefore found none, and the self-reference (dogfood) updates were silently skipped. Files are now staged before the release commit is created, and the version is refreshed after every change to the commit's contents, so what is tagged and published is always what a build of the tagged commit produces. - `bv release` now refuses to publish a version whose Git height is 0, i.e. one whose version line is carried by no commit in the branch's history — typically because `VERSION` has never been committed. Such a version cannot be reproduced: a build of the tagged commit would count the height from 1 as soon as the file reached it, and produce a different version. Building in that state remains perfectly legitimate — it is what a working tree looks like between `bv version advance` and the commit of its result — so only releasing is refused, with a message naming the file to commit. +- `bv release` no longer fails _after_ publishing a release when `GITHUB_OUTPUT` is not set. The variable was read where it is used — appending the `version` step output, which happens once the GitHub release has been published — so an unset variable failed a release that had otherwise succeeded, and the failure rolled it back, deleting the release and the tag it had just created. It is now required up front, before the provisional draft release is created and before any change to the repository, where a failure costs nothing. The message is the usual one for a missing variable (`Required environment variable GITHUB_OUTPUT is not set or empty.`), replacing `Cannot set Actions step output: GITHUB_OUTPUT not set.` - URLs that `bv release` builds from the repository URL are no longer missing the separator before their first path segment: release links (`.../Buildvanareleases/tag/1.1.10`) and file links (`.../Buildvanablob/main/CHANGELOG.md`) now come out as `.../Buildvana/releases/tag/1.1.10` and `.../Buildvana/blob/main/CHANGELOG.md`. This affected the version section titles written into the changelog and the "human-curated changelog" link at the top of every generated release description; the titles already written for 1.0.220, 1.1.4, and 1.1.10 have been corrected in place. - `bv clean` no longer silently ignores unknown options: `bv clean --bogus` now fails with `Unknown option '--bogus' for command 'clean'`. Every `bv` command now rejects options it does not recognize, and does so before anything else runs: previously, commands that parse their own options (e.g. `bv release`) reported an unknown option only after the SDK version check, so a mismatched SDK pin could mask the typo. - A denied or failed file or directory access during a `bv` command (e.g. a locked or read-only `CHANGELOG.md`, `Directory.Packages.props`, or public API file, or a `bin` directory locked by Visual Studio during `bv clean`) no longer surfaces as an unhandled-exception stack trace pointing at `bv` internals. File and directory accesses now report failure as a single clean error line naming the operation, the path, and the operating-system reason (`Could not read from : `), and `bv` exits with its regular failure exit code. This covers failures that happen part-way through reading a file, not just failures to open it: `bv release` reads the whole of `CHANGELOG.md` before rewriting it, so a file yanked mid-read (say, by a cloud-sync provider) is reported the same clean way. From ec6f63471cdc66d92d8c1988a4b8945dbf8a99ed Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 10:06:28 +0200 Subject: [PATCH 20/26] Record a rollback's undone commits once, not one by one The rollback walks back the release commit and any post-release commits on top of it, so a notice inside UndoLastCommit printed the same line as many times as there were commits: accurate, but read as a duplicate rather than as two undos. The record moves to the loop that owns the act - only the caller knows what the set of commits was - and names them the way the rest of the release log does. UndoLastCommit keeps its narration and gains a remark saying why it no longer reports an outcome. Co-Authored-By: Claude Opus 5 (1M context) --- src/Buildvana.Tool/Services/Git/GitService.cs | 3 ++- .../Services/ServerAdapters/ServerRelease.cs | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Buildvana.Tool/Services/Git/GitService.cs b/src/Buildvana.Tool/Services/Git/GitService.cs index 40aad862..af4c2e15 100644 --- a/src/Buildvana.Tool/Services/Git/GitService.cs +++ b/src/Buildvana.Tool/Services/Git/GitService.cs @@ -254,6 +254,8 @@ public void Commit(string message, bool amend = false, bool allowEmpty = false) /// This method's purpose is to undo a commit that was just generated by code and is not a merge commit. /// If the current HEAD has multiple parents, the behavior of this method is undefined. /// If the repository has no commits, or the current HEAD has no parents, this method will fail. + /// Undoing a commit is not an outcome in itself: callers undo a whole set of commits at once, and only + /// the caller knows what that set was, so the record of the undoing belongs to it. This method only narrates. /// public void UndoLastCommit() { @@ -261,7 +263,6 @@ public void UndoLastCommit() var previousCommit = _repository.Head.Tip.Parents.FirstOrDefault(); BuildFailedException.ThrowIf(previousCommit is null, "Git: cannot reset, there is no commit to go back to."); _repository.Reset(ResetMode.Hard, previousCommit); - _reporter.Notice("Undid the last commit."); } /// diff --git a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs index f508b494..f80da846 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/ServerRelease.cs @@ -101,6 +101,15 @@ public void EnsureReleaseCommit() _git.UndoLastCommit(); } + // The whole set of commits is one undoing, recorded once: a notice per commit would read + // as a repeated line rather than as the several commits it actually walks back. + _reporter.Notice(_postReleaseCommits switch + { + 0 => "Undid the release commit.", + 1 => "Undid the release commit and 1 post-release commit.", + var count => string.Create(CultureInfo.InvariantCulture, $"Undid the release commit and {count} post-release commits."), + }); + // If updates have already been pushed... if (_updatesPushed) { From e9477b35c218564d7a6175cf9c6224c336da0fef Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 10:06:59 +0200 Subject: [PATCH 21/26] Name the release a rollback deletes "Deleted the previously created release." named nothing, while its counterpart reads "Published release X with N assets."; and in the rollback that runs before publication the reader has never been told a release existed, the draft's creation being narration. The tag name comes from the caller, not from the release object: the draft is created before the release commit exists, and that commit always moves the version, so the tag name the draft carries is never the one the release ends up published under. A draft has no tag at all, so it is reported as the draft it is rather than named after a version that never existed. Co-Authored-By: Claude Opus 5 (1M context) --- .../Internal/GitHub/GitHubServerAdapter.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs index a7b9b8e9..086adcd0 100644 --- a/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs +++ b/src/Buildvana.Tool/Services/ServerAdapters/Internal/GitHub/GitHubServerAdapter.cs @@ -192,18 +192,26 @@ public async Task PublishReleaseAsync(Release release, string targetCommitish) /// An object representing the release. /// The tag name, or to not delete a tag. /// A representing the ongoing operation. + /// + /// A deleted release is named after , never after + /// : the draft is created before the release commit exists, and that commit + /// always moves the version, so the tag name the draft carries is never the one the release is published + /// under. A draft, having no tag of its own, is reported as the draft it is. + /// public async Task DeleteReleaseAsync(Release release, string? tagName) { Guard.IsNotNull(release); _reporter.Info("Deleting the previously created release..."); var client = CreateGitHubClient(); await client.Repository.Release.Delete(RepositoryOwner, RepositoryName, release.Id).ConfigureAwait(false); - _reporter.Notice("Deleted the previously created release."); if (string.IsNullOrEmpty(tagName)) { + _reporter.Notice("Deleted the provisional draft release."); return; } + _reporter.Notice($"Deleted release {tagName}."); + var reference = "refs/tags/" + tagName; _reporter.Info($"Looking for reference '{reference}' in GitHub repository..."); try From 5664fc280f8f66fa532bc7c11c6c37554dbca9a6 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 10:25:56 +0200 Subject: [PATCH 22/26] Say what the GITHUB_OUTPUT fix actually bought "where a failure costs nothing" claims more than the fix delivers: the variable is required where the release is created, which is after the verification pass, so an unset variable still costs a clean, a build, and a test run. What it no longer costs is an undoing: nothing has been created or changed at that point, which is the whole of the fix and is what the entry now says. docs/EnvironmentVariables.md already puts it that way ("before creating anything") and needs no change. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75cf72d8..bd418c16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,7 +106,7 @@ See the Nerdbank.GitVersioning removal entry under _Changes to existing features - `bv release` no longer tags and publishes a version one patch above the one its artifacts were built with. The "Prepare release" commit bumps the Git height, hence the version, but it was only created when an earlier step had a file to commit; a release with nothing to commit before the build (typically a prerelease with no version-spec change and `release.changelogUpdates` set to `stable` or `none`) therefore built and pushed its packages at the pre-commit version, then created the commit, and tagged and released the version above. The release commit is now always created before the build. - `bv release` no longer tags and publishes a version one patch _below_ the one its artifacts were built with when the release applies a version-spec change (`--bump minor` or `--bump major`, or a minor bump forced by an additive public API change). The `VERSION` file reached the "Prepare release" commit only after the version had been computed, so at computation time the new version line had no commit carrying it, and its Git height came out as 0 — hence versions such as `2.4.0-preview`, which the height calculation can never legitimately produce, since it counts from 1 at the bump commit and reserves 0 for a version line with no committed history. The artifacts, built after the commit, correctly carried `2.4.1-preview`, while the tag, the release, and the hook args said `2.4.0-preview`; produced-package discovery, which matches packages by version, therefore found none, and the self-reference (dogfood) updates were silently skipped. Files are now staged before the release commit is created, and the version is refreshed after every change to the commit's contents, so what is tagged and published is always what a build of the tagged commit produces. - `bv release` now refuses to publish a version whose Git height is 0, i.e. one whose version line is carried by no commit in the branch's history — typically because `VERSION` has never been committed. Such a version cannot be reproduced: a build of the tagged commit would count the height from 1 as soon as the file reached it, and produce a different version. Building in that state remains perfectly legitimate — it is what a working tree looks like between `bv version advance` and the commit of its result — so only releasing is refused, with a message naming the file to commit. -- `bv release` no longer fails _after_ publishing a release when `GITHUB_OUTPUT` is not set. The variable was read where it is used — appending the `version` step output, which happens once the GitHub release has been published — so an unset variable failed a release that had otherwise succeeded, and the failure rolled it back, deleting the release and the tag it had just created. It is now required up front, before the provisional draft release is created and before any change to the repository, where a failure costs nothing. The message is the usual one for a missing variable (`Required environment variable GITHUB_OUTPUT is not set or empty.`), replacing `Cannot set Actions step output: GITHUB_OUTPUT not set.` +- `bv release` no longer fails _after_ publishing a release when `GITHUB_OUTPUT` is not set. The variable was read where it is used — appending the `version` step output, which happens once the GitHub release has been published — so an unset variable failed a release that had otherwise succeeded, and the failure rolled it back, deleting the release and the tag it had just created. It is now required up front, before the provisional draft release is created and before any change to the repository, where nothing has to be undone. The message is the usual one for a missing variable (`Required environment variable GITHUB_OUTPUT is not set or empty.`), replacing `Cannot set Actions step output: GITHUB_OUTPUT not set.` - URLs that `bv release` builds from the repository URL are no longer missing the separator before their first path segment: release links (`.../Buildvanareleases/tag/1.1.10`) and file links (`.../Buildvanablob/main/CHANGELOG.md`) now come out as `.../Buildvana/releases/tag/1.1.10` and `.../Buildvana/blob/main/CHANGELOG.md`. This affected the version section titles written into the changelog and the "human-curated changelog" link at the top of every generated release description; the titles already written for 1.0.220, 1.1.4, and 1.1.10 have been corrected in place. - `bv clean` no longer silently ignores unknown options: `bv clean --bogus` now fails with `Unknown option '--bogus' for command 'clean'`. Every `bv` command now rejects options it does not recognize, and does so before anything else runs: previously, commands that parse their own options (e.g. `bv release`) reported an unknown option only after the SDK version check, so a mismatched SDK pin could mask the typo. - A denied or failed file or directory access during a `bv` command (e.g. a locked or read-only `CHANGELOG.md`, `Directory.Packages.props`, or public API file, or a `bin` directory locked by Visual Studio during `bv clean`) no longer surfaces as an unhandled-exception stack trace pointing at `bv` internals. File and directory accesses now report failure as a single clean error line naming the operation, the path, and the operating-system reason (`Could not read from : `), and `bv` exits with its regular failure exit code. This covers failures that happen part-way through reading a file, not just failures to open it: `bv release` reads the whole of `CHANGELOG.md` before rewriting it, so a file yanked mid-read (say, by a cloud-sync provider) is reported the same clean way. From 77e9318635e5e5b7acec6abc21069b28defd9951 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 11:54:09 +0200 Subject: [PATCH 23/26] Drop the public API count's unreachable singular TransferAllPublicApisToShipped yields both files of every pair it modifies - the unshipped file is emptied into the shipped one, and the release commit carries both - so the count is always even, and "1 public API file was modified." could never be printed. The switch is now an expression over the two cases that exist, with a comment saying why there is no singular. Found while reading what the coverage report says this branch never reaches. Co-Authored-By: Claude Opus 5 (1M context) --- .../Subcommands/ReleaseCommand.cs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs index e284d40e..8bb9a4ae 100644 --- a/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs +++ b/src/Buildvana.Tool/Subcommands/ReleaseCommand.cs @@ -128,18 +128,14 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) else { var modified = publicApiFiles.TransferAllPublicApisToShipped().ToArray(); - switch (modified.Length) + + // Never one: the transfer yields both files of every pair it modifies, so there is no + // singular case to report. + reporter.Notice(modified.Length switch { - case 0: - reporter.Notice("No public API files were modified."); - break; - case 1: - reporter.Notice("1 public API file was modified."); - break; - default: - reporter.Notice(string.Create(CultureInfo.InvariantCulture, $"{modified.Length} public API files were modified.")); - break; - } + 0 => "No public API files were modified.", + var count => string.Create(CultureInfo.InvariantCulture, $"{count} public API files were modified."), + }); if (modified.Length > 0) { From 888654e48ae4249efc719c000e5e302ca6413ced Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 11:54:11 +0200 Subject: [PATCH 24/26] Cover the release command's counted notices The lines that count something - the version spec, the public API files, the hook's changes, the self-reference rewrites - are how a release is read, and every case but the plural one went untested. The branches sit in the middle of a command that only runs end to end, so nothing short of one repository shape per case reaches them, and a wording that reads wrong for a count of one would have gone out unnoticed. The harness grows the one knob that was missing for it: how many of the three self-reference targets the repository contains, so that the update can find none of them, one, or all three. Its new Notices view is what the tests assert on - the messages of the level a default run shows. Co-Authored-By: Claude Opus 5 (1M context) --- .../ReleaseCommandReportingTests.cs | 135 ++++++++++++++++++ tests/Buildvana.Tool.Tests/ReleaseHarness.cs | 24 +++- .../ReleaseHarnessOptions.cs | 8 ++ 3 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs diff --git a/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs b/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs new file mode 100644 index 00000000..4cda6b50 --- /dev/null +++ b/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs @@ -0,0 +1,135 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +/// +/// End-to-end tests of what a successful release records at notice level: the lines a run at the +/// default verbosity shows, which are the whole account of the release a user gets. +/// +/// +/// Counted lines are asserted one repository shape at a time, singular and plural alike. Which shape +/// produces which wording is invisible from the code — the branches sit in the middle of a command that only +/// runs end to end — and a count that reads wrong is exactly the kind of thing that rots unnoticed. +/// The harness anchors the process's current directory and sets an environment variable, so these tests +/// cannot share the process with others that run at the same time. +/// +[NotInParallel] +internal sealed class ReleaseCommandReportingTests +{ + // Asking for a prerelease line that is already a prerelease line changes nothing, which is a decision + // worth recording: the release goes on, and the version file it leaves behind is the one it found. + [Test] + public async Task Release_WithBumpToUnstable_OnAPrereleaseLine_RecordsTheSpecUnchanged() + { + using var harness = new ReleaseHarness(new() { Dogfood = false }); + + var exitCode = await harness.RunAsync("--bump", "unstable").ConfigureAwait(false); + + await Assert.That(exitCode).IsEqualTo(0); + await Assert.That(harness.Notices).Contains("Version spec not changed."); + await Assert.That(harness.ReadFile("VERSION").Trim()).IsEqualTo("2.3-"); + } + + // Public API files are shipped in pairs - the unshipped file is emptied into the shipped one - so the + // count is always even, and the singular case the other counted lines have does not exist here. + [Test] + public async Task Release_ShippingPublicApis_CountsBothFilesOfThePair() + { + using var harness = new ReleaseHarness(new() + { + VersionSpec = "2.3", + Dogfood = false, + CheckPublicApi = false, + UnshippedPublicApi = "#nullable enable\nTest.Thing\n", + }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("2 public API files were modified."); + } + + [Test] + public async Task Release_WithoutPublicApis_RecordsThatNoneWereModified() + { + using var harness = new ReleaseHarness(new() { VersionSpec = "2.3", Dogfood = false }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("No public API files were modified."); + } + + // A hook that changes nothing is not a hook that did not run, and the two have to look different: + // the release still commits and publishes, and the line is what says the hook had its turn. + [Test] + public async Task Release_WithHookThatChangesNothing_RecordsNoModifiedFiles() + { + using var harness = new ReleaseHarness(new() { WithHook = true, Dogfood = false }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.AppRunner.Runs.Count).IsEqualTo(1); + await Assert.That(harness.Notices).Contains("The post-release hook modified no files."); + } + + [Test] + public async Task Release_WithHookThatChangesOneFile_RecordsItInTheSingular() + { + using var harness = new ReleaseHarness(new() { WithHook = true, Dogfood = false }); + + // ReSharper disable once AccessToDisposedClosure // False positive: the hook runs before the harness is disposed + harness.HookBehavior = () => harness.WriteFile("docs/release-notes.md", "Released.\n"); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("The post-release hook modified 1 file."); + } + + [Test] + public async Task Release_WithHookThatChangesSeveralFiles_RecordsTheirNumber() + { + using var harness = new ReleaseHarness(new() { WithHook = true, Dogfood = false }); + + // ReSharper disable once AccessToDisposedClosure // False positive: the hook runs before the harness is disposed + harness.HookBehavior = () => + { + harness.WriteFile("docs/release-notes.md", "Released.\n"); + harness.WriteFile("docs/announcement.md", "Announcing the release.\n"); + }; + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("The post-release hook modified 2 files."); + } + + // Dogfooding that rewrites nothing is the failure mode the self-reference update is most likely to have + // - a name that matches no reference rewrites just as silently as a repository that has none - so the + // line has to be there even when the answer is zero. + [Test] + public async Task Release_WithNoSelfReferenceTargets_RecordsThatNoneWereModified() + { + using var harness = new ReleaseHarness(new() { SelfReferenceTargets = 0 }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("No self-referenced files were modified."); + } + + [Test] + public async Task Release_WithOneSelfReferenceTarget_RecordsItInTheSingular() + { + using var harness = new ReleaseHarness(new() { SelfReferenceTargets = 1 }); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("1 self-referenced file was modified."); + } + + [Test] + public async Task Release_WithEverySelfReferenceTarget_RecordsTheirNumber() + { + using var harness = new ReleaseHarness(); + + _ = await harness.RunAsync().ConfigureAwait(false); + + await Assert.That(harness.Notices).Contains("3 self-referenced files were modified."); + } +} diff --git a/tests/Buildvana.Tool.Tests/ReleaseHarness.cs b/tests/Buildvana.Tool.Tests/ReleaseHarness.cs index 237ef681..2d567a1a 100644 --- a/tests/Buildvana.Tool.Tests/ReleaseHarness.cs +++ b/tests/Buildvana.Tool.Tests/ReleaseHarness.cs @@ -117,6 +117,13 @@ public ReleaseHarness(ReleaseHarnessOptions? options = null) /// public FakeFileBasedAppRunner AppRunner { get; } = new(); + /// + /// Gets the messages the release recorded at level, in order: the + /// record of what the release did, which is what a run at the default verbosity shows. + /// + public IEnumerable Notices + => Reporter.Messages.Where(x => x.Level == MessageLevel.Notice).Select(x => x.Message); + /// /// Gets the observable steps of the release, in order. /// @@ -317,14 +324,25 @@ private void PopulateRepository() private void WriteVersionFile() => Repo.WriteFile("VERSION", _options.VersionSpec + "\n"); // The three files the self-reference update rewrites, each carrying a reference to one of the packages - // this release produces, at the version released before this one. + // this release produces, at the version released before this one. Only the first + // ReleaseHarnessOptions.SelfReferenceTargets of them are written, so that a test can have the update + // find any number of them, none included. private void WriteSelfReferenceTargets() { + if (_options.SelfReferenceTargets < 1) + { + return; + } + var globalJson = new JsonObject { ["msbuild-sdks"] = new JsonObject { [ProducedPackageIds[0]] = PreviousVersion }, }; WriteFile("global.json", globalJson.ToJsonString(IndentedJson)); + if (_options.SelfReferenceTargets < 2) + { + return; + } var toolManifest = new JsonObject { @@ -340,6 +358,10 @@ private void WriteSelfReferenceTargets() }, }; WriteFile(".config/dotnet-tools.json", toolManifest.ToJsonString(IndentedJson)); + if (_options.SelfReferenceTargets < 3) + { + return; + } var packageVersions = $""" diff --git a/tests/Buildvana.Tool.Tests/ReleaseHarnessOptions.cs b/tests/Buildvana.Tool.Tests/ReleaseHarnessOptions.cs index 6e0f2f3d..a6b56f47 100644 --- a/tests/Buildvana.Tool.Tests/ReleaseHarnessOptions.cs +++ b/tests/Buildvana.Tool.Tests/ReleaseHarnessOptions.cs @@ -39,6 +39,14 @@ internal sealed record ReleaseHarnessOptions /// public bool Dogfood { get; init; } = true; + /// + /// Gets the number of self-reference targets the repository contains, out of the three the updater knows + /// about: global.json, .config/dotnet-tools.json, and Directory.Packages.props, in + /// that order. What is written is a prefix of that list: a repository legitimately has fewer than all + /// three, and one that references none of the packages it produces has none at all. + /// + public int SelfReferenceTargets { get; init; } = 3; + /// /// Gets the value of release.checkPublicApi. /// From c91601c93f66f3c8f26168e01086ed5320f5936e Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 11:54:13 +0200 Subject: [PATCH 25/26] Cover ServerRelease's guards and counted notices ServerRelease is a contract for every server adapter, and the release command walks one path through it: it never calls a method out of order, it always has a commit to push, and it adds at most one post-release commit. So the guards that keep a release rollbackable, and the lines that count what was undone or published, are asserted by driving the class directly over the harness's repository rather than through its one caller. Co-Authored-By: Claude Opus 5 (1M context) --- .../ServerReleaseTests.cs | 190 ++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 tests/Buildvana.Tool.Tests/ServerReleaseTests.cs diff --git a/tests/Buildvana.Tool.Tests/ServerReleaseTests.cs b/tests/Buildvana.Tool.Tests/ServerReleaseTests.cs new file mode 100644 index 00000000..b6c10544 --- /dev/null +++ b/tests/Buildvana.Tool.Tests/ServerReleaseTests.cs @@ -0,0 +1,190 @@ +// Copyright (C) Tenacom and Contributors. Licensed under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Globalization; +using Buildvana.Tool.Services.ServerAdapters; + +/// +/// Tests of driven directly, over the same repository and service graph a real +/// release runs on (see ). +/// +/// +/// The release command walks one path through this class; the class is a contract for every adapter, +/// so what it refuses and what it records are asserted here rather than through the one caller that exists +/// today. The order its methods may be called in is part of that contract: a commit after a push, or a +/// post-release commit before the release commit, would leave a release the rollback cannot undo, and each +/// guard is what turns that into an error at the call that got it wrong. +/// The harness anchors the process's current directory and sets an environment variable, so these tests +/// cannot share the process with others that run at the same time. +/// +[NotInParallel] +internal sealed class ServerReleaseTests +{ + [Test] + public async Task EnsureReleaseCommit_AfterPushingUpdates_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + release.PushUpdates(); + + void Act() => release.EnsureReleaseCommit(); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("already been pushed"); + } + } + + [Test] + public async Task UpdateRepository_AfterPushingUpdates_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + release.PushUpdates(); + harness.WriteFile("docs/late.md", "Too late.\n"); + + void Act() => release.UpdateRepository("docs/late.md"); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("already been pushed"); + } + } + + [Test] + public async Task UpdateRepository_AfterAPostReleaseCommit_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + AddPostReleaseCommit(harness, release, 1); + harness.WriteFile("docs/late.md", "Too late.\n"); + + void Act() => release.UpdateRepository("docs/late.md"); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("after a post-release commit"); + } + } + + [Test] + public async Task AddPostReleaseCommit_AfterPushingUpdates_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + release.PushUpdates(); + harness.WriteFile("docs/late.md", "Too late.\n"); + + void Act() => release.AddPostReleaseCommit("Post-release updates [skip ci]", "docs/late.md"); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("already been pushed"); + } + } + + [Test] + public async Task AddPostReleaseCommit_BeforeTheReleaseCommit_Throws() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + harness.WriteFile("docs/early.md", "Too early.\n"); + + void Act() => release.AddPostReleaseCommit("Post-release updates [skip ci]", "docs/early.md"); + + var exception = await Assert.That(Act).Throws(); + await Assert.That(exception!.Message).Contains("before the release commit"); + } + } + + // Nothing to push is not an error - a release may legitimately change no file at all - so it is recorded + // and the release goes on. The line matters because the push is otherwise invisible: without it, a run + // that pushed nothing and a run that pushed the release commit read exactly alike. + [Test] + public async Task PushUpdates_WithoutAReleaseCommit_RecordsThatThereIsNothingToPush() + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.PushUpdates(); + } + + await Assert.That(harness.Notices).Contains("Repository unchanged, no commit to push."); + await Assert.That(harness.Repo.GetRemoteTipSha()).IsNull(); + } + + [Test] + [Arguments(0, "with no assets.")] + [Arguments(1, "with 1 asset.")] + [Arguments(2, "with 2 assets.")] + public async Task PublishAsync_RecordsTheNumberOfAssets(int assetCount, string expectedEnding) + { + using var harness = new ReleaseHarness(); + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + for (var i = 0; i < assetCount; i++) + { + release.AddAsset(string.Create(CultureInfo.InvariantCulture, $"artifacts/asset-{i}.bin")); + } + + await release.PublishAsync().ConfigureAwait(false); + } + + await Assert.That(harness.Notices).Contains($"Published release {harness.ComputeVersion()} {expectedEnding}"); + } + + // Every commit the release made is walked back by one rollback action, and recorded by one line naming + // how many: the release commit is always there, the post-release commits are however many the caller + // added. The release command adds at most one of them; the class allows any number, so the ladder is + // asserted here rather than through the one caller. + [Test] + [Arguments(0, "Undid the release commit.")] + [Arguments(1, "Undid the release commit and 1 post-release commit.")] + [Arguments(2, "Undid the release commit and 2 post-release commits.")] + public async Task DisposeAsync_RecordsEveryUndoneCommitAtOnce(int postReleaseCommits, string expectedNotice) + { + using var harness = new ReleaseHarness(); + var initialSha = harness.Repo.HeadSha; + var release = await StartReleaseAsync(harness).ConfigureAwait(false); + await using (release.ConfigureAwait(false)) + { + release.EnsureReleaseCommit(); + for (var i = 0; i < postReleaseCommits; i++) + { + AddPostReleaseCommit(harness, release, i + 1); + } + } + + await Assert.That(harness.Notices).Contains(expectedNotice); + await Assert.That(harness.Repo.GetCommits(1)[0].Sha).IsEqualTo(initialSha); + } + + // A release object over the harness's repository, with a committer identity for the commits it makes: + // the command sets one from the server adapter before creating the release, and nothing else does. + private static async Task StartReleaseAsync(ReleaseHarness harness) + { + harness.Repo.SetCommitterIdentity("Buildvana Test Bot", "bot@buildvana.invalid"); + return await harness.Adapter.CreateReleaseAsync().ConfigureAwait(false); + } + + private static void AddPostReleaseCommit(ReleaseHarness harness, ServerRelease release, int ordinal) + { + var path = string.Create(CultureInfo.InvariantCulture, $"docs/post-release-{ordinal}.md"); + harness.WriteFile(path, string.Create(CultureInfo.InvariantCulture, $"Post-release change #{ordinal}.\n")); + release.AddPostReleaseCommit( + string.Create(CultureInfo.InvariantCulture, $"Post-release updates #{ordinal} [skip ci]"), + path); + } +} From ae108644ef888e942c7d1a5f88cf47966c0069b8 Mon Sep 17 00:00:00 2001 From: Riccardo De Agostini Date: Fri, 14 Aug 2026 11:58:11 +0200 Subject: [PATCH 26/26] Keep the hook's two files out of a closure The gate's ReSharper pass flags every capture of the harness in the hook callback, and a "disable once" covers one of them: a two-statement lambda needs a suppression per statement, or a disable/restore pair that then trips StyleCop's rule against a comment followed by a blank line. What the hook writes moves into a method taking the harness as a parameter, leaving the callback the single-expression shape the other hook tests already use. Co-Authored-By: Claude Opus 5 (1M context) --- .../ReleaseCommandReportingTests.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs b/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs index 4cda6b50..5710d00e 100644 --- a/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs +++ b/tests/Buildvana.Tool.Tests/ReleaseCommandReportingTests.cs @@ -89,11 +89,7 @@ public async Task Release_WithHookThatChangesSeveralFiles_RecordsTheirNumber() using var harness = new ReleaseHarness(new() { WithHook = true, Dogfood = false }); // ReSharper disable once AccessToDisposedClosure // False positive: the hook runs before the harness is disposed - harness.HookBehavior = () => - { - harness.WriteFile("docs/release-notes.md", "Released.\n"); - harness.WriteFile("docs/announcement.md", "Announcing the release.\n"); - }; + harness.HookBehavior = () => WriteTwoFiles(harness); _ = await harness.RunAsync().ConfigureAwait(false); @@ -132,4 +128,12 @@ public async Task Release_WithEverySelfReferenceTarget_RecordsTheirNumber() await Assert.That(harness.Notices).Contains("3 self-referenced files were modified."); } + + // A method rather than a two-statement lambda: what the hook writes is captured from the harness, and + // a parameter keeps it out of a closure whose target is disposed by the time the analyzer looks at it. + private static void WriteTwoFiles(ReleaseHarness harness) + { + harness.WriteFile("docs/release-notes.md", "Released.\n"); + harness.WriteFile("docs/announcement.md", "Announcing the release.\n"); + } }