Default to minimal verbosity, and add the missing "Notice" message level

[rdeago]

Checklist of related issues / discussions

• Closes Default to minimal verbosity, and add the missing "Notice" message level #346
• Partially closes #
• Related discussion(s): #

Proposed changes

Two problems, independently real, coupled tightly enough that fixing either alone makes things worse.

bv's default verbosity did not match dotnet's. bv defaulted to normal; the pipeline commands it wraps default to minimal, and bv forwards its own verbosity to them verbatim — so plain bv build produced a markedly noisier MSBuild log than plain dotnet build. Where the toolchain we belong to has already settled a question of behavior, we settle it the same way.

The MessageLevel ladder was missing a rung. There was no level 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 was visible in the tree: 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. Two implementations of one contract disagreed about when the same call is visible.

They are coupled because bv release records what it did to the repository through some twenty Info calls, and on CI that log is the audit trail: flipping the default without a level that survives it would silence exactly the output that most needs to survive.

Four commits:

1. Add the level; fix both reporters' mappings. MessageLevel.Notice goes between Warning and Info. Six levels no longer fit five thresholds injectively, so IsEnabled can no longer compare the enums' underlying values; the new MessageLevelExtensions.MinimumVerbosity() states the mapping and is now the single authority both reporters answer to. TaskLoggingHelperReporter's outbound map shifts by one rung and its Verbosity getter with it. Both 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.
2. Reclassify the call sites. Messages recording a fact — something changed, was decided, or was deliberately skipped — become Notice; messages narrating what the tool is doing right now stay Info.
3. Flip the default, and remove the per-command defaultVerbosity plumbing, whose only carve-out disappears with minimal as the base.
4. Align the CI workflows with the new default.

At the default verbosity, bv build / restore / test / pack now produce output comparable to the dotnet command underneath, bv release still prints its complete record, and a given MessageLevel becomes visible at the same point whether bv's console or an MSBuild task renders it. bv build -v normal reproduces the old default output.

Two judgment calls in TaskLoggingHelperReporter are documented in its remarks rather than left to be rediscovered: the Verbosity getter over-claims at both ends of the ladder on purpose (an honest Detailed would make the formatting overloads drop Trace under -v:diag; an honest Quiet would make them drop warnings MSBuild still prints), and 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.

Additional changes

Beyond what the issue laid out:

• More call sites are promoted to Notice than the issue enumerates: VersioningService's "Version X (height N, publicity)" line, ServerRelease's "Repository unchanged, no commit to push.", ReleaseCommand's committer-identity and empty-changelog-substitution lines, DotNetService's "No test projects found, skipping tests." — which nothing else recorded, leaving bv test on a solution with no test project indistinguishable from one whose tests all passed — and all three of bv version advance's outcome lines rather than the one the issue names, the command having no deliverable stream of its own to carry the rest. Each records a fact, which is where the criterion puts it. VersioningService's is also collateral of the remap: it is Core code that runs inside SDK tasks too, where Info used to mean MessageImportance.High, so leaving it would have removed it from every plain dotnet build. Side effect worth knowing: bv version show now emits that one line on standard error alongside its report, which is untouched and still pipes clean.
• bv release gains the outcome lines it never had. The reclassification could only promote messages that already existed, and neither the push, nor the publication, nor the rollback had one: at the new default, a successful release announced everything it had prepared and nothing it had published. GitService.Push now records the branch and the remote, ServerRelease.PublishAsync the published tag and its asset count, and each rollback step what it undoes — all in the server-independent layer, so a future adapter inherits the record instead of reinventing it. The publication line comes last, past the point where the rollback actions are cleared, so it cannot claim a release that a later failure would delete.
• HookRunner states that a hook ran, which nothing recorded. The absent-hook message stays at Info: its cost scales with the number of hook events a command raises, and a repository with no hooks at all would pay a notice per event to be told about files it never had.
• The hook args dump drops from Detail to Trace. A whole JSON document on one line is diagnostic chatter, and the same document is written to the hook's args file and left there after the run.
• GITHUB_OUTPUT is read before the release exists. The Actions step output is written after publication, and reading the variable there let one that was never set fail — and therefore roll back — a release that had otherwise succeeded. GitHubServerRelease now requires it in its factory method, before the draft release is created, and remembers the path for the life of the release; SetActionsStepOutput takes the path from its caller and keeps only the Actions-specific part. docs/EnvironmentVariables.md states the new guarantee, and the changelog carries the entry: this changes released behavior.
• The public API count loses its unreachable singular. TransferAllPublicApisToShipped yields both files of every pair it modifies, 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.
• TaskLoggingHelperReporter.Verbosity collapses from three branches to two. The issue's inverse table has four rows, but its bottom two both yield Minimal, which makes the High probe dead code. Same semantics.
• Two unreleased CHANGELOG.md entries are corrected in place rather than left to contradict the new ones in the same release: bv version show's own minimal default, and the resolved --verbosity forwarded to dotnet.
• Code this branch did not otherwise change gets tests. ReporterExtensions' whole shortcut-and-formatting family was uncovered before the Notice overload joined it, Report's null check and skip-formatting short-circuit included. So was every counted line a release prints except its plural case, in ReleaseCommand and in ServerRelease, along with the guards that keep a release rollbackable — its one caller never calls a method out of order, always has a commit to push, and adds at most one post-release commit. The command's cases go through ReleaseHarness, which grows the one knob they needed: how many of the three self-reference targets the repository has.
• Lines already over the limits are brought within them, in the files this branch works in: ReleaseCommand, ServerRelease, and GitHubServerAdapter, plus ReleaseCommand's primary constructor under the stricter 120-character limit on declarations. Each such sweep is its own commit, so the change under review stays readable.
• MessageLevelExtensions throws a plain ArgumentOutOfRangeException instead of using CommunityToolkit.Diagnostics' ThrowHelper: Buildvana.Core.Abstractions has no package references at all, and a throw helper is not reason enough to add one.

Nothing is done about GitHubServerAdapter and GitHubServerRelease, whose Octokit calls account for most of what Codecov's patch check still misses: neither file has ever been covered on main either. [ExcludeFromCodeCoverage] on the methods that do nothing but make those calls is defensible — that is behavior the environment owns — but it moves the badge, and it is a policy decision of its own rather than something to settle at this PR's tail.

Types of changes

This pull request introduces the following types of changes:

• Bug fix
• New feature
• Test addition / update (no changes to non-test code)
• Refactor (no changes in public API syntax or semantics)
• Performance improvement (no changes in public API syntax or semantics)
• Documentation (docs directory) update
• Dependency addition / update
• Changes to the build scripts
• Changes to CI (workflows, bot / app configurations)
• Changes to repository files (.gitattributes, .gitignore)
• Other

Breaking changes

This pull request introduces breaking changes:

• Yes
• No

Every command's default output changes. CI logs get quieter; anyone who wants today's behavior passes -v normal. No migration is required and nothing fails, but the change is user-visible and gets a **BREAKING CHANGE**: entry under Changes to existing features.

Checklist

• For all types of changes:
  • I have read the Code of Conduct and agree to be bound by it
  • I have read the Contributor Guide and agree to follow it
• For code changes only:
  • The project builds on my machine, via the provided build script, with zero warnings
  • I have added tests that prove my feature works / my fix is effective
  • I have added / modified XML documentation according to changes in code
  • I have checked that all the links I added or modified in XML documentation point to their intended destination
• For documentation changes (docs directory) only:
  • I have built and tested documentation locally
  • I have checked that all the links I added or modified point to their intended destination

[codecov]

Codecov Report

❌ Patch coverage is 80.90909% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.57%. Comparing base (2787117) to head (ae10864).

Files with missing lines	Patch %	Lines
...verAdapters/Internal/GitHub/GitHubServerAdapter.cs	0.00%	14 Missing ⚠️
...verAdapters/Internal/GitHub/GitHubServerRelease.cs	0.00%	4 Missing ⚠️
...uildvana.Tool/Subcommands/VersionAdvanceCommand.cs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #347      +/-   ##
==========================================
+ Coverage   73.66%   74.57%   +0.90%     
==========================================
  Files         161      162       +1     
  Lines        4530     4582      +52     
  Branches      785      788       +3     
==========================================
+ Hits         3337     3417      +80     
+ Misses       1033     1014      -19     
+ Partials      160      151       -9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rdeago]

@claude please review this PR.

[claude]

Claude finished @rdeago's task in 8m 4s —— View job

---

Review: PR #347 — default to minimal verbosity, add Notice

I walked the whole ladder — every MSBuild verbosity state through LogsMessagesOfImportance → derived Verbosity → IsEnabled → outbound importance — plus every reclassified and non-reclassified call site. The core of the change is right, and the two reporters do now agree rung by rung. Findings below, correctness first.

---

1. The release log records the no-op but not the act

ServerRelease.PushUpdates promotes only the negative outcome:

• ServerRelease.cs:191 — Notice("Repository unchanged, no commit to push.") ✅
• GitService.cs:290,296 — Info("Pushing changes to '{remote}'..."), no outcome line after it
• GitHubServerAdapter.cs:167 — Info("Publishing the previously created release as {tag} (target {targetCommitish})...")
• GitHubServerRelease.cs:70,78 — asset uploads and "Asset upload skipped: no release assets defined.", both Info

So at the new default, a release run that pushes commits, publishes the GitHub release under tag X, and uploads N assets says nothing at all about any of it, while a run where the repository was unchanged says so. The NuGet half is covered (DotNetService.cs:271, Notice($"Pushed {packages.Length} packages to {target.Source}.")); the GitHub half has no counterpart.

That inverts the PR's own premise — "bv release records what it did to the repository … on CI that log is the audit trail". The version spec notices and the Version X (height N, …) line tell you what was prepared, never what was published.

Smallest fix that closes it: one outcome line after _server.PublishReleaseAsync(...) in GitHubServerRelease.DoPublishAsync — Notice($"Published release {tag} with {assetCount} asset(s).") — plus promoting the Asset upload skipped line. Fix this →

2. "Deliberately skipped" is applied in ReleaseCommand but not one level down

The criterion now lives on the enum member itself — "something changed, something was decided, something was deliberately skipped" — so it is global, but commit 2 only swept the command. Left at Info:

• DotNetService.cs:162 — "No test projects found, skipping tests." The strongest one: nothing else covers it, so bv test on a solution with no MTP test project is indistinguishable at the default from one whose tests all passed.
• HookRunner.cs:101 — "Hook {hookName}: skipped: no {relativePath} file." When hookRan is false, ReleaseCommand.cs:239 prints no aggregate either, so "no hook ran" is unrecorded.
• GitHubServerRelease.cs:78 — as above.

SelfReferenceUpdater.cs:72 is genuinely covered by ReleaseCommand's case 0, and HookRunner.cs:84 (build-cache cleaning) is internal narration — both correctly left alone. The three above are not. Fix this →

3. CHANGELOG.md — the reworded bv version show entry is now false

The edited sentence reads "the report is the command's deliverable, printed regardless of verbosity, so that at the default verbosity the report is all there is". But this PR promotes VersioningService.cs:62 to Notice, so at the default the command also writes Version X (height N, publicity) to standard error — which the PR body itself flags under Additional changes. The entry and the PR description contradict each other.

What is actually true, and worth saying instead: the report is all that reaches standard output, so it still pipes clean. Fix this →

4. ArgumentOutOfRangeException.ParamName regresses to "this"

MessageLevelExtensions.cs:36 throws new ArgumentOutOfRangeException(nameof(@this), …), and nameof strips the @ — so ParamName is "this". Two consequences:

• ReporterExtensions.cs:108 documents the exception as <paramref name="level"/> is not a known MessageLevel, which the thrown instance contradicts.
• Before this change, TextWriterReporter.Report((MessageLevel)(-1), …) threw from StyleFor with ParamName == "level" — matching the public signature the caller used. IsEnabled now throws first, so the name changed. TextWriterReporterTests.cs:77 asserts only the type, so nothing caught it.

Cheapest fix is to validate in IsEnabled and throw with nameof(level). Side note, no action needed: StyleFor's default arm is now unreachable through TextWriterReporter.Report (still required for exhaustiveness), whereas TaskLoggingHelperReporter's stays live since it does not consult IsEnabled.

5. Style — three modified lines exceed 140 characters

ReleaseCommand.cs:175, :250, :274. All three were already over before the change (Notice is two characters longer than Info, so they went 143→145, 145→147, 152→154), but the rule in csharp-style-guide.md is that modified lines always comply.

6. release.yml — quiet in the dispatch dropdown

Adding quiet to the choice list hands a release run a verbosity at which the entire Notice audit trail disappears — the exact output this PR exists to preserve. minimal is already the floor that keeps the record. Worth dropping quiet from that one list. (I can't edit .github/workflows myself — GitHub App permissions.)

Dropping the [ -z "$BV_VERBOSITY" ] fallback is safe, though: the workflow is workflow_dispatch-only and the input is required with a default, so the variable can't be empty.

---

Checked and correct

• The two ladders agree. MSBuild quiet → Minimal; minimal (High only) → Minimal; normal → Normal; detailed/diag → Diagnostic. Each derived value gates exactly the levels whose outbound importance MSBuild would print, with the single documented exception (Trace one rung early). Both over-claims are load-bearing, and the remarks explain why.
• The Verbosity getter collapse is a fix, not a refactor. On a High-only engine main answered Normal; the new code answers Minimal, which is what MessageImportance.High ↔ Notice requires. TaskLoggingHelperReporterTests.cs:88 pins it.
• Renumbering MessageLevel is safe. It has no reference from Buildvana.Runtime, so it is outside the additive-only packaged contract, and PublicAPI.*.txt tracking covers only that project.
• defaultVerbosity removal is complete — no leftovers in src/, tests/, docs/, workflows, or the config schema; GlobalSettings's help text was updated.
• MinimumVerbosity_CoversEveryLevel is the right shape: per-level [Arguments] can't catch a level added without a threshold, and this does.
• Both new .cs files carry the BOM; the extension block follows the mandated template verbatim (@this, both pragmas, non-partial).
• CI is green on ae1a35b (build/test/pack, CodeQL).

Findings 1–3 are the ones I'd act on before merge; 4–6 are cheap and in-scope per Small changes out of scope. Happy to implement any subset — say which.
· branch dev/346-default-minimal-verbosity

[rdeago]

Finding 1 was the real one, and it would have shipped. Six commits, one per addressed finding.

1. The release log records the no-op but not the act — fixed, one level up

The outcome lines go in the server-independent layer rather than in the GitHub adapter: ServerRelease.PublishAsync reports the published tag and its asset count once DoPublishAsync returns, and GitService.Push reports branch and remote after the push. Putting them in GitHubServerRelease would have made the audit trail a GitHub feature that the next adapter has to reinvent. (863f7e6)

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.

Publishing… and Pushing… stay at Info — they narrate what is about to happen in front of a network call, which is not what the reader wants afterwards. Asset upload skipped: no release assets defined. stays at Info as well: Published release X with no assets. already records that fact, so promoting the skip line would say it twice.

2. "Deliberately skipped" one level down — two of three

• DotNetService.cs:162 → Notice. Agreed, and it was the strongest of the set: bv test on a solution with no test project was indistinguishable from one whose tests all passed. (c159204)
• HookRunner.cs:101 stays at Info. Its cost scales with the number of hook events a command raises, and a repository with no hooks at all would pay a notice per event to be told about files it never had. What was actually missing is the positive: HookRunner now records at Notice that a hook ran, which is the fact worth keeping and which nothing outside bv release was recording. (0836f1b)
• GitHubServerRelease.cs:78 — see above.

3. CHANGELOG.md — the bv version show entry is now false

Fixed, and rephrased around what the command actually guarantees rather than around a verbosity level: the report is alone on standard output, so it stays pipeable whatever the verbosity. (33da375)

4. ArgumentOutOfRangeException.ParamName regresses to "this"

Fixed. The throw moves into a static local function whose parameter really is called level, so nameof keeps doing the work and no literal has to be kept in sync by hand. Both unknown-level tests asserted the exception type alone — which is exactly why this went unnoticed — and now assert ParamName too. (c282277)

5. Style — three modified lines exceed 140 characters

Wrapped. (394e6a6) The style guide also asks that a file one is actually working in comes fully within the limits, so a separate commit takes care of the five pre-existing over-long lines and of the primary constructor, which the stricter 120-character limit on declarations catches. (57322ee)

6. release.yml — quiet in the dispatch dropdown

Left in place, by @rdeago's call: it is a knob he rarely turns but wants available. Errors are logged at quiet too, and a release run's outcome is independently visible as a tag, two commits, and a published release.

Additional change, not from the review

The hook args dump drops from Detail to Trace (df14e81): a whole JSON document on one line is diagnostic chatter, and 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.

---

dotnet bv pack and ReSharper at warning severity are both clean. @claude, ready for another look.

[claude]

Claude finished @rdeago's task in 4m 50s —— View job

---

Re-review: PR #347 — the six follow-up commits

Walked all eight commits since 415896b. Findings 1–5 are addressed; two of them are addressed better than what I proposed. One residual gap falls out of the new outcome lines, plus a wording nit. CI is green on 57322ee (build/test/pack + CodeQL); I could not build locally — the sandbox denied dotnet build — so the compile/analyzer status here is CI's, not mine.

• Read the eight follow-up commits
• Verify finding 1 (release outcome lines)
• Verify finding 2 (deliberate skips)
• Verify findings 3–5 (changelog, ParamName, line lengths)
• Check the unrequested change (hook args dump → Trace)
• Post verdict

---

Verified

1 — fixed, and one layer better than I proposed. ServerRelease.PublishAsync and GitService.Push are the right homes: every future adapter inherits the record instead of reinventing it, which the GitHub-adapter placement I suggested would not have given you. _version.CurrentStr is exactly the tag PublishReleaseAsync sets (GitHubServerAdapter.cs:155), so "Published release X" names the real thing. PushUpdates is now symmetric — both branches leave a notice — and the CHANGELOG's claim that bv release "still logs its complete account of what it changed and published" is true for the first time on this branch.

2 — the strongest one taken, the other two declined on grounds I buy. DotNetService.cs:162 was the one that mattered. On HookRunner: your read is right and mine was not — RunHookAsync fires per event, so a hook-less repo would pay a notice per event to be told about files it never had, and promoting the positive (ran) records the fact that actually changes files. On GitHubServerRelease.cs:78: agreed, Published release X with no assets. already says it, and says it in the server-independent layer.

3 — fixed, and rephrased around the right invariant. VersionShowCommand writes the report through IAnsiConsole unconditionally while narration goes to standard error, so "alone on standard output … stays pipeable whatever the verbosity" is precisely what the code guarantees. My suggestion was still anchored to the default verbosity; yours isn't.

4 — fixed. The static local function keeps nameof doing the work, and both call sites now assert ParamName. TaskLoggingHelperReporter.Report's own default arm already threw with nameof(level), so the two throw sites agree.

5 — fixed. No line over 140 remains in ReleaseCommand.cs, and the primary constructor is inside the 120-character declaration limit. GitService.cs has none either.

6 — your call, and the reasoning holds: errors survive quiet, and a release's outcome is independently visible as a tag and a published release.

df14e81 — correct by the enum's own words: Trace is documented as "fine-grained diagnostic chatter", which is what a one-line JSON dump is. And RunHookAsync_LeavesArgsFileInPlace_AfterHookCompletes pins the claim that nothing is lost.

---

New: the act survives at minimal, the undoing of it does not

This is a consequence of commit 863f7e6, not a pre-existing problem — before it there was nothing to contradict.

The cheap half. ServerRelease.cs:227 reports the publish before OnPublishedAsync(), which is the one step that can still throw after DoPublishAsync returns (SetActionsStepOutput throws BuildFailedException on an unset GITHUB_OUTPUT, and UserFile.AppendAllLines can fail on I/O). If it throws, DisposeAsync rolls back and UndoPublishAsync deletes the release and the tag — after the log has already claimed the publish. Moving the notice below _published = true (line 236) closes the window at zero cost: rollbacks are cleared there, so nothing can undo the publish once the line is printed.

The design half. The rollback's own acts are invisible at minimal, except the one you promoted:

• GitHubServerAdapter.cs:188 — Info("Deleting the previously created release...")
• GitService.cs:260 — Info("Undoing last commit...")
• GitService.cs:293 — Notice("Force-pushed ...") ✅

Your argument for the force-push is that it "would leave the trail describing a remote state that was subsequently undone". That argument covers the release deletion and the commit reset verbatim: both change state that an earlier notice claimed. As it stands, a rolled-back release reads, at the default:

notice: Pushed 'main' to 'origin'.
notice: Published release 1.2.3 with 2 assets.
error:  ...
notice: Force-pushed 'main' to 'origin'.

Three notices describing a state that no longer exists, and a force-push that at minimal is indistinguishable from an ordinary one — its "Force pushing changes to..." narration is Info. Nothing says the release and the tag were deleted or the commits reset. Fix this →

Judgment call whether it is worth it — it only bites on the failure path, which is also the only time anyone reads the log closely. The first half I'd just take; the second is yours to weigh.

Nits

• HookRunner.cs:115 — Hook release/post-release: ran .buildvana/hooks/release/post-release.cs. repeats the context/event pair twice on one line. Harmless at Info; at Notice it is in everyone's default output. Hook release/post-release ran. carries the same fact.
• ServerRelease.cs still has four pre-existing lines past 140 (80, 125, 168, 173, all ThrowHelper calls). The whole-file clause arguably reaches them now that 863f7e6 works in the file — the same clause you applied to ReleaseCommand in 57322ee. Whether an 8-line addition counts as "working in" a file is your call; flagging it only for consistency between the two.

---

Nothing here blocks. Finding 1's cheap half is a two-line move; the rest is discretionary.
· branch dev/346-default-minimal-verbosity

[rdeago]

Both halves of the new finding taken, both nits taken, plus one change of @rdeago's that the first half made obvious. Five commits.

The publication is announced before it becomes irreversible — fixed

Right, and it would have shipped: OnPublishedAsync is the last step that can still fail, and its failure rolls the release back, deleting the release and the tag the notice had already claimed. The notice moves to the end of PublishAsync, past _rollbackActions.Clear(), so it prints only once nothing can undo the publication. Ordinary-path output is unchanged — the steps it now follows print nothing of their own. (24c402c)

The rollback's own acts are invisible at minimal — fixed

Agreed: the argument for the force push covers the release deletion and the commit reset verbatim. DeleteReleaseAsync now records the release deletion and the tag deletion separately, each right after its act, as the pushes do; UndoLastCommit records the reset. The paths that find no tag to delete stay at Info, having changed nothing. (7b05f70)

Wording of the reset line is Undid the last commit. rather than the branch-and-SHA form: @rdeago's call, and the clearer line.

The third sub-item is declined. The rollback's force push does not need renaming: at minimal it already reads Force-pushed 'main' to 'origin'., distinct from Pushed 'main' to 'origin'.. What Info hides is the Force pushing changes to... narration, not the outcome — and with the two lines above now printing around it, the rollback is unmistakable.

Nits

• HookRunner — taken, and extended to the line above, which repeated the path just as well. The pair now reads Running hook release/post-release... / Hook release/post-release ran.. The absent-hook line keeps its path: there, the path is the file one would create. (63dbee9)
• Line sweep — taken, for ServerRelease.cs and for GitHubServerAdapter.cs, which this round works in as well. Seven lines between them. GitHubServerRelease.cs's single long line is its CreateAsync declaration, wrapped by the commit below. (ce86eee)

Additional change: GITHUB_OUTPUT is read before the release exists

The first half's diagnosis pointed at more than the notice's position: SetActionsStepOutput read GITHUB_OUTPUT at the worst possible moment — after a successful publication, where an unset variable fails and rolls back a release that was correct. Moving the notice hides that; @rdeago asked for the cause to go too. GitHubServerRelease now requires the variable in its factory method, before the draft release is created — nothing exists to undo yet, and the message names the variable exactly as before — and remembers the path for the life of the release. SetActionsStepOutput takes the path from its caller and keeps only the Actions-specific part. docs/EnvironmentVariables.md states the new guarantee. (163f84f)

---

dotnet bv pack and ReSharper at warning severity are both clean. @claude, one more look, please.

[claude]

Claude finished @rdeago's task in 5m 38s —— View job

---

Re-review: PR #347 — the five new commits

Walked 24c402c..ce86eee against 57322ee. All four review items are fixed and verified; the unrequested GITHUB_OUTPUT change is a real fix and correctly placed. One gap it leaves behind, one discretionary observation, two nits. CI is green on ce86eee (build/test/pack + CodeQL); I could not build locally, so compile/analyzer status here is CI's.

• Read the five commits
• Verify 24c402c (notice moved past the point of no return)
• Verify 7b05f70 (rollback records)
• Verify 63dbee9 (hook wording)
• Review 163f84f (GITHUB_OUTPUT) — the only new logic
• Check ce86eee (line sweep)
• Post verdict

---

Verified

24c402c — the window is fully closed. PublishAsync now runs DoPublishAsync → OnRollback → OnPublishedAsync → _published = true → _rollbackActions.Clear() → Notice (ServerRelease.cs:230-244). Nothing after the notice can undo the publication, and nothing after PublishAsync returns either: ReleaseCommand.cs:350 is the last statement in the await using, and DisposeAsync returns at its first line when _published. The path is exercised — RecordingServerRelease derives from ServerRelease, so ReleaseCommandTests runs this exact sequence.

7b05f70 — correct, and it does not leak into the success path. UndoLastCommit has exactly one caller, the rollback lambda in EnsureReleaseCommit (ServerRelease.cs:101), so Undid the last commit. can only ever appear during a rollback. DeleteReleaseAsync's two notices sit right after their respective API calls, and the NotFoundException path still returns at Info, having changed nothing (GitHubServerAdapter.cs:201,215,221). Wording of the reset line is yours to pick; Undid the last commit. is unambiguous.

63dbee9 — good, and no loose end. relativePath is still live at HookRunner.cs:98,101, so nothing was orphaned by dropping it from the two messages, and the absent-hook line keeps the path that names the file one would create.

ce86eee — clean. No line over 140 remains in ServerRelease.cs, GitService.cs, GitHubServerAdapter.cs, GitHubServerRelease.cs, or HookRunner.cs, and CreateAsync's and the constructor's parameter lists are inside the 120-character declaration limit. The four ThrowHelper messages moving to their own line is the right call — they are the sole argument.

163f84f — the diagnosis was right and the fix is in the right place. EnvVarHelper.Require runs before createGitHubReleaseAsync() (GitHubServerRelease.cs:71-72), and ReleaseCommand calls CreateReleaseAsync() at line 103, ahead of every repository mutation — the version file, the changelog, the release commit, and the push all happen inside the await using. So "nothing exists to be undone yet" is literally true, not approximately. Require's { Length: > 0 } matches the old string.IsNullOrEmpty guard and still throws BuildFailedException; no test asserted the old message text. SetActionsStepOutput is left with only the Actions-specific line format, and its remark points at where the value comes from.

---

The GITHUB_OUTPUT fix has no changelog entry

bv shipped in 1.0.220, so this is a change to released behavior, and a user-visible one twice over: an unset variable used to fail the release after publishing it, rolling back a release that was correct, and the failure message itself changed (Cannot set Actions step output: GITHUB_OUTPUT not set. → Required environment variable GITHUB_OUTPUT is not set or empty.). docs/EnvironmentVariables.md records the new guarantee; CHANGELOG.md records nothing.

CHANGELOG.md:103's Bugs fixed in this release is full of this exact species — three entries are "bv release no longer publishes/tags the wrong thing", and one line here would read as a sibling of them. This PR gave changelog lines to smaller user-visible changes than this one. Fix this →

Observation: the two Actions requirements are now checked on either side of the build

GITHUB_TOKEN is required in GitHubServerAdapter's constructor (:50), which runs when ReleaseCommand.cs:49 resolves the adapter — before the verification pipeline. GITHUB_OUTPUT is now required at :103, after it. Both are pure environment reads, and the comment at ReleaseCommand.cs:59-63 states the principle for the block they'd share: "a release that cannot succeed is refused at once, instead of after a full clean, build, and test cycle whose result is then thrown away." As it stands, an unset GITHUB_OUTPUT still costs a full build before it is discovered.

I am not proposing the obvious move: the adapter constructor is wrong for this, because DotNetService and ChangelogService also take ServerAdapter, so bv build on Actions would start requiring a variable it never uses. Getting it before the build needs a server-independent pre-flight on ServerAdapter (a virtual no-op, overridden to validate), which is more machinery than the remaining exposure justifies. Flagging it so the asymmetry is a decision rather than an accident — the doc's "fails up front, before creating anything" is true as written either way.

Nits

• Undid the last commit. prints twice when the rollback has a post-release commit to walk back: ServerRelease.cs:98-102 loops 1 + _postReleaseCommits times, and the notice is per-commit. Accurate, but two identical lines read as a duplicate rather than as two undos. Only the dogfood/hook rollback path hits it.
• Deleted the previously created release. does not name the release, while its counterpart does (Published release X with N assets.). In the pre-publish rollback the reader has never been told a release existed — the draft creation is Info — so at minimal the line arrives out of nowhere. Deleted release {release.TagName}. pairs with the publish line and stands alone in the draft case. Note it has to come from release.TagName, not _version.CurrentStr: the version can move under NameReleaseCommit after the draft was created, so the adapter's own copy would name the wrong tag in exactly the case that needs naming. Fix this →

---

Nothing blocks. The changelog entry is the one I'd take before merge; the rest is yours to weigh.
· branch dev/346-default-minimal-verbosity

[rdeago]

The changelog entry and both nits taken; the observation left as a recorded decision. Three commits.

The GITHUB_OUTPUT fix has no changelog entry — fixed

Agreed, and it belongs exactly where you put it: an unset variable used to fail — and therefore roll back — a release that had already been published correctly, which makes it a sibling of the three bv release entries above it. The entry names the message change too. (9e77a49)

Observation: the two Actions requirements are checked on either side of the build — left as is

Your own objection decides it, and there is a second: after 163f84f an unset GITHUB_OUTPUT costs a wasted build rather than a rolled-back release, and on a runner the variable is always set — reaching the failure at all means running bv release with GITHUB_ACTIONS=true off a runner, which is a debugging scenario. A pre-flight virtual would also be one more member to place when ServerAdapter is eventually split into its platform and git-host halves. Recorded here so it is a decision, not an accident.

Nits

• Undid the last commit. printed twice — fixed, one level up. The duplication belongs to the loop, not to the primitive: the rollback walks back the release commit and everything committed on top of it as a single undoing, and only the caller knows what that set was. UndoLastCommit keeps its narration and reports no outcome; the loop records the act once, in the vocabulary the rest of the release log uses — Undid the release commit., or Undid the release commit and 2 post-release commits. (ec6f634)
• Deleted the previously created release. names nothing — fixed, but not from release.TagName. Your caveat points the right way and understates the problem: the draft's tag name is not sometimes stale, it is always stale. The draft is created at ReleaseCommand.cs:103, before the release commit exists, and that commit always moves the height and with it the version — so the tag the draft carries is never the one PublishReleaseAsync publishes under, and the local object is never refreshed either, the Edit result being discarded. Naming the post-publish deletion from it would print the wrong version; naming the draft deletion from it would print a version that never existed. The name now comes from the caller — Deleted release 1.2.3., pairing with the publish line — and the draft, having no tag of its own, is reported as the draft it is: Deleted the provisional draft release., matching the Creating a provisional draft release... that opened it. (e9477b3)

---

dotnet bv pack and ReSharper at warning severity are both clean. @claude, one more look, please.

[claude]

Claude finished @rdeago's task in 3m 55s —— View job

---

Re-review: PR #347 — the three new commits

Walked 9e77a49, ec6f634, e9477b3 against ce86eee. All three items are fixed, and both nits are fixed in ways that are better than what I proposed — the second one because my caveat was too weak, not merely different. Nothing new that blocks. CI is green on e9477b3 (build/test/pack + CodeQL); I could not build locally, so compile/analyzer status here is CI's.

• Read the three commits
• Verify 9e77a49 (changelog entry)
• Verify ec6f634 (undone commits recorded once)
• Verify e9477b3 (naming the deleted release)
• Post verdict

---

Verified

9e77a49 — correct in placement, scope, and quotation. The entry sits at CHANGELOG.md:109, inside ## Unreleased changes → ### Bugs fixed in this release, immediately after the three bv release siblings and before the URL-separator one — exactly the neighbourhood it belongs to. Both quoted messages check out against the code: EnvVarHelper.Require's Required environment variable {name} is not set or empty. is what is thrown now, and origin/main's GitHubServerAdapter.cs:102 really did read Cannot set Actions step output: GITHUB_OUTPUT not set. The entry describes the rollback consequence, which is the part a reader needs in order to recognize the bug they hit.

ec6f634 — the record is in the right place, and the primitive is honest again. UndoLastCommit has exactly one caller (ServerRelease.cs:101), so moving the record to the loop loses nothing, and the remark at GitService.cs:257-258 states why the method reports no outcome — which is the thing that would otherwise be re-added by someone six months from now. The switch reads _postReleaseCommits at rollback time, so the count is whatever AddPostReleaseCommit had reached; the notice sits after the loop and before the force push, so the rollback prints as reset, then push, matching the order of the acts. Invariant culture on the counted arm, consistent with the publish notice two hundred lines below.

Grammar covers all three cases: 0 → Undid the release commit., 1 → … and 1 post-release commit., n → … and 2 post-release commits. No test, doc, or workflow referenced the old string.

e9477b3 — you were right that my caveat understated it, and the fix follows from that. I said release.TagName "can have moved"; it always has. GitHubServerAdapter.cs:136 captures tag = _version.CurrentStr before the release commit exists, ReleaseCommand.cs:103 creates the draft there, and every later path through EnsureReleaseCommit/UpdateRepository ends in NameReleaseCommit, which refreshes the version from the commit that was just made. Release.Edit's result is discarded at GitHubServerAdapter.cs:186, so the local object never learns the published tag either. So release.TagName is wrong in both directions — a version that never existed for the draft, the pre-commit version for the published release. Taking the name from the caller is the only correct source, and UndoPublishAsync passes _version.CurrentStr, which is the same value PublishReleaseAsync published under (nothing between the two refreshes it: AddPostReleaseCommit deliberately does not).

The two notices land right after their own acts (:213 release, :229 tag), the NotFoundException path still returns at Info having changed nothing, and Deleted the provisional draft release. pairs with Creating a provisional draft release... at :138. A reader who sees the deletion without a creation still learns the right thing: that what was deleted never became a release.

Rollback ordering still holds after all this. The stack is constructor-delete-draft → undo-commits → undo-publish, popped in reverse, and _gitHubReleaseDeleted keeps the last one from deleting a release UndoPublishAsync already removed. A post-publish rollback now reads:

notice: Deleted release 1.2.3.
notice: Deleted tag 1.2.3.
notice: Undid the release commit.
notice: Force-pushed 'main' to 'origin'.

which is the complete undoing, in order, with no line claiming a state that outlives it.

---

Nit

The changelog's "where a failure costs nothing" is a shade stronger than what you decided. CHANGELOG.md:109 says the variable is now required "before the provisional draft release is created and before any change to the repository, where a failure costs nothing". Nothing has to be undone — that part is exact — but by your own note in this comment the failure still costs a full clean/build/test cycle, since CreateReleaseAsync runs after pipeline.RunThroughAsync(BuildStep.Test, …). "where nothing has to be undone" says what the fix actually bought without inviting the reader to expect an immediate failure. docs/EnvironmentVariables.md is already precise on this — "before creating anything" is true and does not overreach. Fix this →

Note, pre-existing and narrow — no action proposed

If client.Repository.Release.Edit at GitHubServerAdapter.cs:186 applies server-side but its response is lost (timeout, reset), DoPublishAsync throws before PublishAsync registers UndoPublishAsync, so the rollback that runs is the constructor's — DeleteReleaseAsync(_gitHubRelease, null). The release is deleted, the tag GitHub created at publication is not, and the log says Deleted the provisional draft release. for something that briefly was not a draft. The dangling tag is main's behavior, untouched by this branch; only the wording is new, and it is the right wording for every case anyone will actually meet. Recording it so the window is known, not asking for machinery to close it.

---

Nothing blocks; the nit is a clause. From my side this reads ready to merge.
· branch dev/346-default-minimal-verbosity

[rdeago]

The nit taken, the note recorded. One commit.

where a failure costs nothing — fixed

You are right, and the overreach is mine twice over: the note you are quoting is the one I wrote in the same comment where the entry claims the opposite. The check runs in GitHubServerRelease.CreateAsync, reached from ReleaseCommand.cs:103, and :101 is 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 having been created or changed at that point, and that is the whole of what the fix bought. The clause now says so; docs/EnvironmentVariables.md was already precise and needed nothing. (5664fc2)

The lost Edit response — recorded, not closed

Worth having written down, and the window is real: Release.Edit applies server-side, the response is lost, DoPublishAsync throws before PublishAsync registers UndoPublishAsync, and the constructor's rollback deletes a release that had stopped being a draft a moment earlier — leaving GitHub's tag behind and calling the thing a draft in the log.

Leaving it. The dangling tag is main's behavior and no line of this branch touches it; distinguishing the case would mean re-reading the release before deciding what to call it, i.e. a network round-trip on the rollback path, to get the wording right in a window measured in the milliseconds between GitHub committing the edit and failing to say so. The wording is correct for every case that is actually reachable, which is the standard the rest of these notices are held to. Recorded here so the next reader of that code finds the window described rather than having to rediscover it.

---

dotnet bv pack and ReSharper at warning severity are both clean.

[rdeago]

Codecov's patch check, not a review finding. Four commits, and one of them touches production code — flagging that here, since it lands after the ready-to-merge verdict.

What the red number was made of

codecov/patch was red at 60.55%; codecov/project passed, +0.08%. Of the 41 missed lines, 31 already existed on main, already uncovered: Info → Notice and the line-length rewraps are what made git call them new. One ThrowHelper.ThrowInvalidOperationException(…) wrapped across two lines is four old misses becoming eight; the GenerateReleaseNotes call is one becoming four. Ten lines were genuinely new, seven of them in the two Octokit files.

The public API count could never say "one" (77e9318)

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. was unreachable. The switch is now an expression over the two cases that exist.

The counted lines are tested (888654e, c91601c, ae10864)

What was genuinely untested is one thing throughout: every counted line except its plural case. The command's cases go through ReleaseHarness, which grows the knob they needed — how many of the three self-reference targets the repository has — and a Notices view to assert on. ServerRelease's go through the class directly: it is a contract for every adapter, and its one caller today never calls a method out of order, always has a commit to push, and adds at most one post-release commit. So the guards that keep a release rollbackable, and the undone-commit and asset counts, are asserted by driving it over the harness's repository.

Measured on the Release build the gate produces: ReleaseCommand.cs 146/158 → 157/157, ServerRelease.cs 118/162 → 132/162, every one of the 19 patch misses now hit and both partials resolved.

The Octokit files are left alone

GitHubServerAdapter and GitHubServerRelease contribute 18 of the missed lines and have never been covered on main either. [ExcludeFromCodeCoverage] on the methods that do nothing but call Octokit is defensible — that is behavior the environment owns — but it moves the badge, and class-wide would be too coarse: the adapter also parses the origin URL and composes file URLs. A decision of its own, not one for this PR's tail.

---

dotnet bv pack and ReSharper at warning severity are both clean.