From db469f5b9c67de3530f6446dbf00b2f2b5e0cc7c Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 07:27:58 -0400 Subject: [PATCH 01/13] docs: retire CHANGELOG.md in favour of the generated release notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file arrived as a side effect of a feature PR (fde24c4, the hidden-option work) rather than as a release-process decision, and nothing consumes it: no workflow, no script, no packaging step, and no document links to it. Meanwhile CI already publishes what it was trying to be. The release job runs `gh release create "v${VERSION}" --generate-notes`, so every published version gets notes generated from the merged pull requests and anchored to the version a consumer installs. The file could never be anchored that way — its own header said so, because Nerdbank.GitVersioning assigns the version at pack time — and a review lens correctly flagged that a breaking change pinned to "the commit closing issue It was also the single point of conflict between concurrent pull requests, which is how the question came up: it was the only conflicting file when #85 merged and again when #80 rebased, with three more PRs open behind them. The one piece of guidance that lived only here is migrated: the recipe for restoring the pre-policy exit codes, with its test-suite and MCP consequences, now sits in docs/configuration-reference.md beside the table it talks about. The rest was already covered by the topic pages — docs/commands.md:172 for the additive `isHidden` export fields, docs/testing-toolkit.md for CommandExecution.ExitCode. docs/publishing.md now states where release notes come from and what that asks of a PR description, so the next person does not recreate the file. --- CHANGELOG.md | 214 -------------------------------- docs/configuration-reference.md | 20 +++ docs/publishing.md | 12 ++ 3 files changed, 32 insertions(+), 214 deletions(-) delete mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md deleted file mode 100644 index bde3f8fb..00000000 --- a/CHANGELOG.md +++ /dev/null @@ -1,214 +0,0 @@ -# Changelog - -Notable consumer-facing changes to the Repl packages. Versions are assigned automatically by -Nerdbank.GitVersioning at pack time; this file groups changes by theme instead of by release. - -## Unreleased - -### Added — execution outcomes and exit-code policy - -- Every run now ends in a structured `ReplExecutionOutcome` whose `ReplExecutionOutcomeKind` - distinguishes `Success`, `Help`, `UsageError`, `BindingError`, `HandlerError`, `HandlerExitCode`, - `HandlerException`, `Cancelled`, `Interrupted`, and `FrameworkError`. The kind is mapped to an - integer by the new `ReplOptions.ExitCodes` (`ExitCodeOptions`) table, then passed to the optional - `ExitCodes.Resolver` hook whose return value is the final exit code. An explicit `Results.Exit(n)` - keeps its code verbatim (`HandlerExitCode`) but is still visible to the resolver. See - `docs/execution-pipeline.md` (stage 12) and `docs/configuration-reference.md`. -- `ExitCodes.Cancelled` (`int?`) turns a cancellation through the caller's own token into an exit - code instead of letting `OperationCanceledException` escape `RunAsync`. It is unset by default, - which preserves the existing throwing behaviour; setting a `Resolver` also opts in to observing - cancellation, and the code the resolver is then handed is `130` (`128 + SIGINT`), not the - framework-error code — an aborted run stays distinguishable from a broken one. -- `ExitCodes.Interrupted` (`int?`) maps a process signal turned into a cooperative shutdown by a - process-signal handler; the core pipeline never produces this kind. Unset, the conventional - `128 + signal` code the handler supplies is used, falling back to `130` when it supplies none. -- `ReplExecutionOutcome.Scope` (`ReplExitCodeScope`) tells a resolver whether it is computing the - process exit code (`Process`, once per run) or one interactive command's shell-integration - command-end mark (`ShellIntegrationMark`, only when a mark actually carries a code — so never with - shell integration off, for a protocol-passthrough command, or for an abandoned prompt cycle). -- A resolver that throws does not escape the run: the table-mapped code is used and one diagnostic - line is written to the session's error stream. Interactive sessions survive a faulty resolver, and - a resolver failure on a failed command never replaces the original exception. -- `ReplExecutionContext.Result` exposes the handler's return value to middleware registered with - `app.Use(...)`: readable and replaceable after `await next()`, settable by a short-circuiting - middleware. `ReplNext` and the `Use` signature are unchanged. - -### Changed — breaking: framework exit codes - -These land together in **PR #85**, closing issue #81 — a consumer whose pipeline started seeing -exit `2` can search for either. (Package versions come from Nerdbank.GitVersioning at pack time, so -this file cannot name the build; the PR and issue numbers are the durable anchors.) - -- Framework refusals now exit `2` instead of `1`: unknown command, ambiguous prefix, invalid global - or command option, option collision, context validation failure, unknown `--output` format, - ambient-command misuse in one-shot mode (`exit` while disabled, `..`, `complete` without - `--target`), help that cannot be rendered (`UsageError`), and arguments that cannot be bound, - converted, or resolved from context/services (`BindingError`). Handler - failures (`Results.Error`/`Validation`/`NotFound`, exceptions) still exit `1`, help and success - still exit `0`. Set `ExitCodes.UsageError`/`BindingError` back to `1` to restore the old numbers. - The interactive loop reports the same resolved codes in shell-integration `D;` marks, - including the mark for a command whose dispatch threw, which previously always reported `1`. -- The `RunAsync` overloads that receive an already-built service provider now observe an - already-cancelled caller `CancellationToken` before touching that provider: - `ReplApp.RunAsync(args, IServiceProvider, …)` before starting hosted services, and - `ReplApp.RunAsync(args, IReplHost, IServiceProvider, …)` before building the session overlay from - it. A cancelled token throws `OperationCanceledException` (or returns `ExitCodes.Cancelled` when - mapped, and `130` when only a `Resolver` is set). Previously only `CoreReplApp.RunAsync` performed - any such check. The guarantee is per-overload and scoped to the caller's provider, not blanket: - session setup and terminal overrides run before the check on the `IReplHost` overload, and the - overloads that build the shared provider themselves (`Run(args)`, - `RunAsync(args, options, …)`) construct it before the check is reached further down the chain. -- A handler that raises `OperationCanceledException` without the caller having asked for cancellation - is now a `HandlerException`: the message is rendered and the run exits `1`, where it previously - either propagated silently or, with `ExitCodes.Cancelled` mapped, returned the cancellation code - with no diagnostic at all. Only the caller's own token yields `Cancelled`. The interactive loop's - Ctrl+C semantics are unchanged. -- Interactive `help` / `?` is now classified `Help` rather than a generic success, so an application - that maps `ExitCodes.Help` separately sees its own code in the command-end mark. An ambient command - that *failed* is still a `UsageError`, whatever it would have reported on success. -- Hosted-service start and stop failures in `ReplApp.RunAsync` now go through the exit-code policy as - `FrameworkError` instead of returning a hard-coded `1`. The code is resolved once, after the whole - lifecycle, so a failed shutdown outranks the command's own outcome and a resolver is handed exactly - one outcome per run. An already-cancelled caller token also follows `ExitCodes.Cancelled` on that - overload, without starting hosted services. -- A binding failure and a handler exception both carry the rendered refusal in - `ReplExecutionOutcome.Result` alongside the `Exception`, as routing refusals do, so a resolver can - map on the framework's own diagnostic for a thrown failure and not only for a refused invocation. -- Every framework refusal and failure is now reported through one guarded path, so an - application-supplied `IOutputTransformer` that throws can no longer escape the pipeline from any of - them. Six refusal sites (ambiguous prefix, option collision, option parse error, context - deep-link, context validation, global option diagnostics) sat outside any exception handler and - ended the run with no classified outcome and no exit code; they are all guarded now, and each still - reports `UsageError` whether the diagnostic was rendered, refused for an unknown format, or written - unformatted because the transformer failed. -- An output transformer that throws while the framework is reporting a failure no longer escapes the - run. Reporting a failure re-invokes the requested transformer, so one that fails consistently used - to throw a second time from inside the catch block handling its first failure, leaving the run with - no classified outcome and no exit code. The message now degrades to an unformatted line on stderr - and the run keeps its `HandlerException` classification. A cancellation raised while that fallback - runs is excluded and propagates to the cancellation policy, so `ExitCodes.Cancelled` still governs a - run that was asked to stop. -- A hosted-service failure carries its exception in the outcome, and a startup stopped by the - caller's own token is a `Cancelled` outcome rather than a `FrameworkError`: it prints no startup - error and, with no cancellation policy configured, propagates the `OperationCanceledException` like - every other path. A shutdown that fails still outranks everything the run produced, including any - exception the pipeline was propagating — but that exception is no longer discarded: the outcome - then carries an `AggregateException` of the stop failure and the suppressed cause, in that order, - and both are reported. -- Framework diagnostics now go to **stderr** instead of stdout: the hosted-lifecycle failures - (`Error: Failed to start/stop hosted service …`, which also name the wrapped cause) and the - `Error: unknown output format '…'` refusal. The lifecycle writes are best-effort. A framework error on stdout corrupts the - machine-readable payload of a headless run, and a torn-down transport could previously turn a - reportable shutdown failure into an escaping write with no exit code at all. A test asserting these - lines on a merged stdout capture needs to read stderr. -- An unknown `--output` format is a `UsageError` on every path, including while a failure was being - reported, for an `EnterInteractive` payload — the interactive loop is then not entered — for a - hosted protocol-passthrough refusal, and for a bare non-interactive invocation, which used to print - help and exit `Help` without reporting the format at all. A bare invocation with a *valid* format - still prints the human help: `--output` selects a format for a command result, and a bare - invocation produces none. The same now holds for a scoped-context invocation that does not enter - interactive mode (`contact --no-interactive --output:bogus`), its sibling path. -- A malformed global option is refused in the interactive loop as it is in a one-shot run. The loop - parses globals per command and checked them on no path at all, so `hello --help --result:page-size` - rendered help and reported the help code in its shell-integration mark. A diagnostic the caller never saw cannot stand as the run's - outcome. When the usage error displaces a - failure that was already being reported, `ReplExecutionOutcome.Exception` now carries that original - failure, so a caller-chosen output format cannot erase why the run ended. - -### Compatibility notes — exit codes - -- MCP tool calls (nested sub-invocations) always use the built-in exit-code defaults and ignore - `ExitCodes.Resolver`; they only test for non-zero, so `IsError` is unaffected by the policy. The - agent-visible failure text now reads "exit code 2" for usage and binding refusals. -- `Repl.Testing`'s per-command timeout still surfaces as `TimeoutException` when the app under test - maps `ExitCodes.Cancelled`: the handle observes the run's own outcome instead of relying on the - exception escaping. `RunCommandAsync` documents that exception. -- A handler-thrown `InvalidOperationException` is still rendered as a validation message, but it - is classified `HandlerException` (not `BindingError`); only exceptions raised while binding - arguments are `BindingError`. -- A handler that returns a bare `int` (or any scalar) is unchanged: the value is rendered as data - and the run is a `Success`. The documentation previously implied otherwise; `Results.Exit(n)` - remains the only return-value route to an explicit exit code. -- `Repl.Testing`'s `CommandExecution.ExitCode` follows the configured policy, so application test - suites asserting `1` for unknown commands or invalid options need to expect `2` (or configure - `ExitCodes`). -- An `IReplResult` whose `Kind` is not `text` or `success` is a `HandlerError` (exit `1`), including - a kind the framework does not recognize. An unclassifiable result never reports success to a - pipeline; use `Results.Exit(n)` to choose a code deliberately. -- `ReplExecutionOutcomeKind.Interrupted` and `ExitCodes.Interrupted` are produced by automatic - process-signal handling, described under *Added — standalone process signals* below. They are not - reachable any other way: an application cannot supply an outcome to the table from outside the - framework, so the kind only appears for a signal the framework itself claimed. -- Exit codes are not range-checked. Keep them within `0`-`255`: POSIX `wait` exposes only the low - eight bits to the parent process. - -### Added — standalone process signals - -- `ReplRunOptions.ProcessSignalHandling` and `ProcessSignalHandlingMode` let internally configured standalone `Run`/`RunAsync` calls opt into or out of cooperative process-signal handling. The nullable option inherits the active profile default: CLI and default-interactive profiles use `Automatic`; an unprofiled `ReplApp.Create()` and `UseEmbeddedConsoleProfile()` use `None`, preserving caller-owned shutdown unless a process-owning profile is selected. -- In automatic mode, the first Ctrl+C console event—or Ctrl+Break on Windows—cancels all overlapping standalone runs in one process-wide ownership epoch and reports a successful or cancelled run as `ReplExecutionOutcomeKind.Interrupted`, which resolves through `ExitCodes.Interrupted` and defaults to exit code `130`. On supported Unix platforms, SIGTERM behaves the same way with `143`. A run that already produced a refusal or a failure keeps reporting it. A subsequent signal uses the operating-system default, and stderr diagnostics identify both steps. Explicit non-zero handler exit codes remain authoritative. - -### Changed — process signal ownership - -- Apps that select `UseCliProfile()` or `UseDefaultInteractive()` now take process signal ownership by - default. Two observable changes follow for an existing consumer. Selecting - `ProcessSignalHandlingMode.None` restores the previous behavior: - - **Exit codes.** A run interrupted by Ctrl+C, Ctrl+Break on Windows, or SIGTERM on Unix now resolves - to `130` or `143` where it previously produced whatever the operating-system default termination - yielded. The interruption goes through the exit-code policy, so `ExitCodes.Interrupted` overrides - those defaults and `ExitCodes.Resolver` observes it like any other outcome. A wrapper script or CI step that treats any non-zero code as a failure will start seeing - these on interruption. An explicit non-zero handler exit code still takes precedence. - - **Handler token identity.** One-shot handlers now receive a run-scoped token linked to the caller - token instead of the caller token itself, and Repl disposes it when the run ends. No token Repl - creates may outlive its run. A handler that stored one and used it afterwards — for detached or - background work — sees `ObjectDisposedException` from `Register` or `WaitHandle`, and, worse, - nothing at all from `IsCancellationRequested`, which keeps reporting `false`. Handlers that only - await work within the run are unaffected. Apps with no profile, `UseEmbeddedConsoleProfile()`, and - the external `IServiceProvider`/`IHost`/`IReplHost` overloads keep passing the caller token through - unchanged. - -### Operational notes — process signals - -- Exit codes `130` (`128 + SIGINT(2)`) and `143` (`128 + SIGTERM(15)`) follow the widely adopted Unix/Bash convention; they are not universal .NET or Windows exit-code guarantees. SIGTERM bridging is Unix-only. -- Automatic handling has no built-in grace-period timeout. A supervisor can send a second signal to force termination. The process callbacks are installed lazily once and remain inert outside automatic runs so runtime callback snapshots cannot race handler teardown. -- For one-shot handlers, automatic mode injects a linked, run-scoped token, while external host/provider overloads pass the caller token through unchanged. Interactive commands receive a separate command-scoped linked token so Ctrl+C can cancel only the active command. Handlers must not retain any Repl-created token beyond its scope. An explicit `Automatic` request on an external overload is ignored with a diagnostic on the active error channel. -- Android, browser, iOS (including Mac Catalyst), and tvOS do not install the unsupported process-signal bridge; `Automatic` emits a diagnostic and their platform host must provide cancellation. Consumer cancellation-callback failures are also diagnosed without replacing an established `130`/`143` exit policy. - -### Added — option visibility - -- `.Hidden(bool isHidden = true)` on the option builder (`WithOption(name, option => option.Hidden())`) - hides an option's canonical token, aliases, description, default, and value candidates from help, - generated documentation, interactive/shell completion, and MCP tool schemas. The option remains a - fully parsable, invocable part of the command line — hiding is a discovery filter, not access - control. Available for direct command-handler parameters, options-group properties, manually - registered global options (`ParsingOptions.GlobalOption(name).Hidden()`), and typed global options. -- `.HiddenAlias(alias, isHidden = true)` and `[ReplOption(HiddenAliases = [...])]` mark specific - legacy/deprecated token spellings as parser-only: the canonical token and any current aliases stay - discoverable, while the hidden alias keeps binding from the CLI/REPL for backward compatibility. -- `doc export` (and `docs `) reports `isHidden` / `isAutomationHidden` per option so an - app author can inventory what a given command hides. Aggregate documentation (no target path) and - MCP's `tools/list` always omit hidden options entirely — see `docs/commands.md` for the full - visibility matrix. -- A hidden option must remain omittable for every provider that can build a discovery surface. - Hiding a required options-group property fails immediately at `Map` time. Hiding a required direct - handler parameter defers that check to the first time discovery runs against a real service - provider (aggregate documentation build or MCP startup), since a DI/synthesized-progress fallback - is only knowable once one exists — see the "Provider-aware requiredness" section of - `docs/commands.md`. - -### Changed — breaking - -- `WithOption(name, configure)` is now the only fluent entry point for configuring an existing - option's metadata (visibility included). This lands within the same change that introduces it — - no previously published `Option(...)` API is removed by this release. - -### Compatibility notes - -- `doc export --json` (and other structured documentation exports) now unconditionally include the - `isHidden` and `isAutomationHidden` fields on every option. A consumer validating that output - against a closed schema (`additionalProperties: false`) will need to allow these two additive - fields. -- The historical six-parameter `ParsingOptions.AddGlobalOptionCore` descriptor is preserved as a - distinct overload (not folded into a defaulted parameter) so an already-compiled `Repl.Defaults` - binary continues to work against a newer `Repl.Core`. The reverse is not guaranteed: this release's - `Repl.Defaults` calls APIs that only exist in this release's `Repl.Core`, so upgrading only one of - the two packages independently is not supported — upgrade them together. diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index eed5a4d8..c515d48e 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -152,6 +152,26 @@ of a top-level run; nested MCP sub-invocations always use the defaults and skip Codes should stay within `0`-`255` — POSIX `wait` exposes only the low eight bits to the parent process. Repl passes a configured code through unchanged rather than clamping it. +### Restoring the pre-policy codes + +Before this table existed, every framework refusal and every handler failure alike exited `1`. If an +application or its test suite depends on that, set the two refusal codes back: + +```csharp +app.Options(options => +{ + options.ExitCodes.UsageError = 1; + options.ExitCodes.BindingError = 1; +}); +``` + +Two consequences are worth knowing even if you keep the defaults. A test suite asserting `1` for an +unknown command or an invalid option needs to expect `2`, including through +`Repl.Testing`'s `CommandExecution.ExitCode`, which follows the configured policy. And an MCP tool +call reports the same numbers in its agent-visible failure text, so a refusal now reads +"exit code 2"; `IsError` is unaffected, since a nested sub-invocation only tests for non-zero and +always uses the built-in defaults. + ## AmbientCommandOptions Accessed via `ReplOptions.AmbientCommands`. diff --git a/docs/publishing.md b/docs/publishing.md index e90a29de..9a3f585a 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -69,6 +69,18 @@ NuGet publication. - On push to `main`/`release/*`, CI creates a GitHub Release with tag `v`. - Only create tags manually for exceptional/manual workflows. +## Release notes + +There is no changelog file to maintain. CI creates a GitHub Release per published version with +`gh release create "v${VERSION}" --generate-notes`, so the notes are generated from the pull requests +merged since the previous tag and are anchored to the version a consumer actually installs — which a +hand-written file cannot be, since Nerdbank.GitVersioning assigns the version at pack time. + +That puts the burden on pull request titles and descriptions: they *are* the release notes. A change +with a consumer-visible contract — a new default, a behavioural break, a migration step — belongs in +the PR description and, when it is durable guidance rather than a one-time note, in the topic page +under `docs/` that owns the feature. + ## NuGet publish status - Package and symbol packages (`.snupkg`) are produced by `Build, Test, Pack` and published From e1ec18bc0cb91fb2d57b655a4583a52442d3f54d Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 09:16:23 -0400 Subject: [PATCH 02/13] fix(ci): tag the commit that built the packages, and correct the notes guidance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings on this PR, all against prose I wrote here. Two are corrections to claims; the first turned out to be a latent CI defect that the claim exposed. - `gh release create` ran without `--target`, and gh creates a missing tag from the latest state of the default branch. The release job also triggers on `release/**`, so a release-branch publish could tag `main` while attaching packages built from the branch — which made the new claim that notes are anchored to the installed version false, and was wrong regardless of the claim. It now passes `--target "${GITHUB_SHA}"`. An existing tag is unaffected; this only governs the automatic creation path. - The guidance said a PR description *is* the release notes. `--generate-notes` publishes pull request titles, authors and links, not PR bodies, so a migration step written only in a description is reachable through the link but absent from the notes. docs/publishing.md now says which is which, and points durable guidance at the topic pages instead. - I claimed the rest of the deleted file was already covered by those topic pages. Two consumer constraints were not, and both are now written down: the coordinated Repl.Core/Repl.Defaults upgrade, with the one-directional compatibility that makes it a constraint (docs/architecture.md), and the unconditional isHidden / isAutomationHidden export fields that a closed schema has to allow (docs/commands.md). - The restore recipe promised more than it delivers. It restores the *code* for paths that were already refusals, but not a path whose classification changed — and one did: `tool --output:bogus` used to print help and exit `0`, is now a UsageError, and the recipe makes it `1`. docs/configuration-reference.md says so, and names the trade if that single path matters more than the distinction. --- .github/workflows/ci.yml | 4 ++++ docs/architecture.md | 9 +++++++++ docs/commands.md | 2 ++ docs/configuration-reference.md | 6 ++++++ docs/publishing.md | 22 ++++++++++++++-------- 5 files changed, 35 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0e27034..af1309fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -511,8 +511,12 @@ jobs: VERSION="${{ needs.build-test-pack.outputs.version }}" PRERELEASE="" [[ "$VERSION" == *-* ]] && PRERELEASE="--prerelease" + # --target is what makes the tag point at the commit that built these packages. Without + # it, gh creates a missing tag from the latest state of the default branch, so a + # release/** publish would tag main instead of the release branch it ran on. gh release create "v${VERSION}" packages/* \ --title "v${VERSION}" \ + --target "${GITHUB_SHA}" \ --generate-notes \ $PRERELEASE diff --git a/docs/architecture.md b/docs/architecture.md index e7873a29..bf9a0694 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -36,6 +36,15 @@ - `Repl.ShellCompletionTestHost` - Test host process for validating shell completion scripts. +## Upgrading the packages together + +`Repl.Defaults` calls into `Repl.Core` at a version-matched surface, so the two must be upgraded +together. A previously compiled `Repl.Defaults` keeps working against a newer `Repl.Core` — historical +descriptors are preserved as distinct overloads rather than folded into defaulted parameters — but the +reverse is not guaranteed: a release's `Repl.Defaults` may call APIs that only exist in the same +release's `Repl.Core`. Upgrading one and pinning the other is unsupported. The `Repl` meta-package +takes both at matched versions, which is the reason to prefer it. + ## Quality gates - Strict build rules from `src/Directory.Build.props`: diff --git a/docs/commands.md b/docs/commands.md index c72b8629..fe03c54e 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -171,6 +171,8 @@ Almost always a target-name mistake. `WithOption` takes the **CLR** handler para If the option is genuinely hidden and you want to confirm what the app thinks, export the command explicitly: `doc export --json` includes hidden options with `"isHidden": true`. The aggregate export omits them, so target the command. +Every option in a structured export carries `isHidden` and `isAutomationHidden` unconditionally. A consumer validating that output against a closed schema (`additionalProperties: false`) has to allow both fields. + Note there is no built-in signal for *use* of a hidden option. If the point is retiring a deprecated switch, record that in the handler yourself — otherwise nothing will tell you when it has become safe to remove. ### Accessing global options outside handlers diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index c515d48e..41d7d0f7 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -165,6 +165,12 @@ app.Options(options => }); ``` +This restores the *code* for paths that were already refusals. It cannot restore a path whose +**classification** changed, and one did: a bare invocation with an unknown format +(`tool --output:bogus`) used to print help and exit the `Help` code — `0` by default — and is now a +`UsageError`. The recipe above makes it `1`, not the former `0`. Set `ExitCodes.UsageError = 0` only +if that single path matters more to you than telling a refusal from a success everywhere else. + Two consequences are worth knowing even if you keep the defaults. A test suite asserting `1` for an unknown command or an invalid option needs to expect `2`, including through `Repl.Testing`'s `CommandExecution.ExitCode`, which follows the configured policy. And an MCP tool diff --git a/docs/publishing.md b/docs/publishing.md index 9a3f585a..5a2f29a4 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -72,14 +72,20 @@ NuGet publication. ## Release notes There is no changelog file to maintain. CI creates a GitHub Release per published version with -`gh release create "v${VERSION}" --generate-notes`, so the notes are generated from the pull requests -merged since the previous tag and are anchored to the version a consumer actually installs — which a -hand-written file cannot be, since Nerdbank.GitVersioning assigns the version at pack time. - -That puts the burden on pull request titles and descriptions: they *are* the release notes. A change -with a consumer-visible contract — a new default, a behavioural break, a migration step — belongs in -the PR description and, when it is durable guidance rather than a one-time note, in the topic page -under `docs/` that owns the feature. +`gh release create "v${VERSION}" --target "${GITHUB_SHA}" --generate-notes`, so the notes are +generated from the pull requests merged since the previous tag and the tag points at the commit whose +packages are attached — an anchoring a hand-written file cannot have, since Nerdbank.GitVersioning +assigns the version at pack time. + +Know what that publishes, and what it does not. `--generate-notes` emits **pull request titles**, +authors and links; it does **not** copy a PR description into the release body. So a PR title is +consumer-facing prose, and a migration step written only in a PR description is reachable through the +link but is not part of the notes. + +Durable guidance therefore belongs in the topic page under `docs/` that owns the feature — a new +default, a behavioural break and its restore recipe, a constraint on upgrading packages together. +The PR description is where you explain the change to a reviewer; `docs/` is where a consumer finds +it six months later. ## NuGet publish status From 7403d9b466db7680698c449b9a8d99560744b05c Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 14:51:36 -0400 Subject: [PATCH 03/13] docs: carry PR #80's migration notes over before the file goes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto the merged #80, which had added three CHANGELOG sections after this branch was written. Taking the deletion is right — nothing reads the file and the release job generates versioned notes — but deleting it unaudited would have lost what only it recorded, which is the mistake the review caught on this PR the first time. #80 documented its rule well: docs/configuration-reference.md already covers the run-scoped token, the mode table, the epoch machine and the platform matrix. What lived only in the changelog was the *migration consequence* for an existing app, so that moves next to the rule it belongs to: - taking signal ownership by default under UseCliProfile/UseDefaultInteractive, and that ProcessSignalHandlingMode.None restores the previous behaviour; - interruption now resolving to 130/143 where OS-default termination used to apply, and what that does to a script treating any non-zero code as failure; - the handler-token change and how it fails — ObjectDisposedException from Register or WaitHandle, and IsCancellationRequested silently continuing to report false, which the existing "must not retain it" sentence did not say. That last one is the reason this audit was worth doing rather than assumed: a rule tells a reader what to do, and only the consequence tells them what breaks if they did not. --- docs/configuration-reference.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index 41d7d0f7..6fc30ad4 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -367,4 +367,11 @@ If a handler completes normally with its own non-zero exit code, that code takes - SIGTERM bridging uses .NET's POSIX signal API and is enabled only on supported non-Windows platforms. SIGTERM does not participate in the interactive console-key priority rule. Repl does not install a direct POSIX SIGQUIT registration. Windows `taskkill`, console-window close, and service-control shutdown do not acquire equivalent SIGTERM semantics from this option; a Windows host must translate its lifecycle events into the caller cancellation token. - Android, browser, iOS (including Mac Catalyst), and tvOS do not support the required console/POSIX registrations. `Automatic` emits a diagnostic and installs no process-signal bridge there; the platform host must provide cancellation. .NET identifies Mac Catalyst as part of its iOS-like mobile family and compiles the platform-not-supported POSIX signal registration there. - In `Automatic` mode, a one-shot handler receives a run-scoped token linked to the caller token and the process-signal cancellation source. An interactive command receives a command-scoped token linked to that run token so Ctrl+C can cancel only the active command. Repl disposes each linked token when its scope ends; handlers may use it for awaited work but must not retain it for detached work. + +#### What changes for an existing application + +Selecting `UseCliProfile()` or `UseDefaultInteractive()` takes process signal ownership by default, which is a change in two observable ways. `ProcessSignalHandlingMode.None` restores the previous behaviour for either. + +- **Exit codes.** A run interrupted by Ctrl+C, Ctrl+Break on Windows, or SIGTERM on Unix resolves to `130` or `143`, where it previously produced whatever operating-system default termination yielded. A wrapper script or CI step that treats any non-zero code as a failure will start seeing these on interruption. +- **Handler token identity — the one that fails quietly.** A one-shot handler receives a run-scoped token rather than the caller's own, and Repl disposes it when the run ends. A handler that stored one and used it afterwards, for detached or background work, gets `ObjectDisposedException` from `Register` or `WaitHandle` — and, worse, nothing at all from `IsCancellationRequested`, which keeps reporting `false`. Handlers that only await work within the run are unaffected, as are apps with no profile, `UseEmbeddedConsoleProfile()`, and the external `IServiceProvider` / `IHost` / `IReplHost` overloads, which pass the caller's token through unchanged. - In `None` mode and external-host overloads, Repl does not create the standalone signal-linked token. A one-shot handler receives the caller token unchanged. An interactive command still receives its separate command-scoped linked token, so its identity and lifetime differ from the caller token even though host-shutdown cancellation flows through it. From 9e710c8198d27ed4a8ca78c2881ab7dce63ba7e0 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 15:12:30 -0400 Subject: [PATCH 04/13] docs: audit publishing.md against what CI actually does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This page is what AGENTS.md points at for release guidance, and #88 made it the authority on release notes, so its claims were checked line by line against .github/workflows/ci.yml and version.json. Three were wrong and one large fact was missing. Corrected: - The stated prerelease pattern was `0.11.0-dev.{height}`; version.json says `0.12.0-dev.{height}`. The page no longer quotes the value at all — nbgv rewrites it at every release, so quoting it guarantees it goes stale — and points at the field instead. Examples that read "for example" are left as examples. - The page contradicted itself about release branch names: `release/` with `release/0.10.0` in one section, "can be release/0.9 or release/1.0" in another. version.json's release.branchName is `release/{version}` and all five existing branches carry three-part versions, so the second form was simply wrong. - publicReleaseRefSpec was described as covering main and release/*; it also covers `v.` tags. The page now also records that CI passes PublicRelease=true only for pushes to main and release/**, which is why a pull-request build produces .g packages that are never published. Added, because it is the most likely way a future release goes wrong: a release branch has no `{height}`, so every commit on it computes the same version. The two publish steps then disagree — `dotnet nuget push` runs with --skip-duplicate and tolerates it, while `gh release create` does not, so a second push to a release branch fails that step and, because Publish to NuGet is gated on `if: success()`, publishes nothing. A follow-up commit on a release branch therefore turns CI red with nothing wrong in the code. The page now says so and gives the way through. Also recorded that the release job downloads the packages artifact rather than rebuilding, so a release ships exactly what CI tested. --- docs/publishing.md | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/docs/publishing.md b/docs/publishing.md index 5a2f29a4..c382e7f2 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -40,18 +40,23 @@ NuGet publication. - `main`: prerelease flow (`-dev.*`). - `release/*`: release branch flow (same packaging pipeline, intended for release stabilization). -- Branch name format can be `release/` (for example: `release/0.9` or `release/1.0`). -- Release branches should be created from `main` with `nbgv prepare-release`. +- Branch names come from `version.json`'s `release.branchName`, currently `release/{version}`, so + they carry the full three-part version: `release/0.11.0`, not `release/0.11`. +- Release branches should be created from `main` with `nbgv prepare-release` rather than by hand. ## Versioning (`version.json`) - Versioning is handled by Nerdbank.GitVersioning. - `version.json` is **never updated automatically** by CI. -- Current prerelease pattern is defined in `version.json`: - - `"version": "0.11.0-dev.{height}"` +- The prerelease pattern lives in `version.json`'s `version` field — read it there rather than + from this page, since `nbgv prepare-release` rewrites it at every release. On `main` it is a + `-dev.{height}` pattern; on a release branch the prerelease tag is removed. - `nbgv prepare-release` updates the `version` field when cutting a release. Manual edits should be reserved for changing the version line or policy outside the normal release flow. -- `publicReleaseRefSpec` already includes `main` and `release/*`, so package/release versions generated by CI do not include the `.g` suffix. +- `publicReleaseRefSpec` covers `main`, `release/*` and `v.` tags, so versions built + from those refs carry no `.g` suffix. CI additionally passes `-p:PublicRelease=true` for + pushes to `main` and `release/**` only — so a pull-request build deliberately produces + `.g`-suffixed packages, which are diagnostic artifacts and are never published. ### When should I modify it? @@ -63,6 +68,32 @@ NuGet publication. (for example `0.9.0-rc.{height}` instead of `0.9.0`). - If you do not change `version.json` on `release/*`, that branch keeps the same version pattern inherited from `main`. +## One publish per version, and what that means on a release branch + +`nbgv prepare-release` removes the prerelease tag on the release branch, so its `version` has no +`{height}`: **every commit on `release/0.11.0` computes the same `0.11.0`**. That is the intended +behaviour for a stable line, but it has a consequence worth knowing before you push twice. + +The two publish steps disagree about repetition: + +- `dotnet nuget push` runs with `--skip-duplicate`, so re-pushing an already-published version is + tolerated and reported, not fatal. +- `gh release create` has no such tolerance. The second push to a release branch tries to create a + release whose tag already exists, that step fails, and because `Publish to NuGet` is gated on + `if: success()` the publication is skipped for that run. + +So a follow-up commit on a release branch — a documentation fix, a cherry-picked hotfix — turns CI +red and publishes nothing, without anything being wrong with the code. Two ways through it: + +- **Preferred:** cut a new version. Bump the release branch's `version` (for example to `0.11.1`) + so the run produces a version that has never been released. +- **If the commit genuinely must not change the version** (say a workflow-only change on the release + branch), expect the release job to fail on the duplicate and treat it as such; nothing was + published, and nothing was lost. + +The release job does not rebuild: it downloads the `packages` artifact produced by +`Build, Test, Pack`, so a release publishes exactly the packages CI tested. + ## Do I need to create a Git tag manually? - No for normal flow. From 13812e91b82f463d1904af860f08de90ee7a5b69 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 15:19:34 -0400 Subject: [PATCH 05/13] fix(ci): make GitHub Release creation idempotent, loudly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A release branch's version has no {height}, so every commit on it computes the same number and `gh release create` failed on the second push — taking Publish to NuGet with it, since that step is gated on `if: success()`. A follow-up commit on a release branch therefore turned CI red with nothing wrong in the code. The step now checks whether the release exists and re-attaches the packages with `gh release upload --clobber` instead of recreating it, so the run stays green. Deliberately not silent, and this is the part worth arguing. A published NuGet version is immutable: if that follow-up commit changed shipped code, the packages are skipped and the fix reaches nobody under that version. Simply turning the build green would trade a false failure for a false success, which is worse. So the repeat path emits a workflow warning and a job-summary note saying the packages were re-attached, that NuGet will skip them, and that a change consumers should receive needs a version bump. The existing tag is left alone on that path. Moving a published tag changes what an already-released version points at, which is a decision to take deliberately rather than a thing CI should do by itself. Both branches were exercised offline with gh stubbed, and `set -euo pipefail` was checked against the `[[ ... ]] && VAR=` line it now precedes — bash exempts the left operand of an AND-list, so a stable version still reaches the create path. YAML validated. docs/publishing.md is rewritten to describe this rather than the failure it replaced: repeating is fine, repeating does not ship anything, and a change that consumers must receive needs a new version. --- .github/workflows/ci.yml | 35 +++++++++++++++++++++++++++-------- docs/publishing.md | 35 +++++++++++++++++++---------------- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index af1309fc..99f3e1b0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -508,17 +508,36 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | + set -euo pipefail VERSION="${{ needs.build-test-pack.outputs.version }}" PRERELEASE="" [[ "$VERSION" == *-* ]] && PRERELEASE="--prerelease" - # --target is what makes the tag point at the commit that built these packages. Without - # it, gh creates a missing tag from the latest state of the default branch, so a - # release/** publish would tag main instead of the release branch it ran on. - gh release create "v${VERSION}" packages/* \ - --title "v${VERSION}" \ - --target "${GITHUB_SHA}" \ - --generate-notes \ - $PRERELEASE + + # A release branch's version has no {height}, so every commit on it computes the same + # number. Creating the release twice is therefore an ordinary occurrence rather than an + # error, and failing here would skip Publish to NuGet for a run that had nothing wrong + # with it. Re-attach the packages instead — but say loudly that nothing new can ship + # under a version NuGet already has, because a green build must not imply otherwise. + if gh release view "v${VERSION}" >/dev/null 2>&1; then + echo "::warning title=Release v${VERSION} already exists::Packages were re-attached, but NuGet will skip them: a published version is immutable. If this commit changed shipped code, bump the version on the release branch and push again." + { + echo "### Release \`v${VERSION}\` already existed" + echo + echo "Assets re-attached to the existing release. **NuGet will skip these packages** —" + echo "a published version is immutable, so no code change in this commit can ship as" + echo "\`${VERSION}\`. Bump the version on the release branch if it needs to." + } >> "$GITHUB_STEP_SUMMARY" + gh release upload "v${VERSION}" packages/* --clobber + else + # --target is what makes the tag point at the commit that built these packages. Without + # it, gh creates a missing tag from the latest state of the default branch, so a + # release/** publish would tag main instead of the release branch it ran on. + gh release create "v${VERSION}" packages/* \ + --title "v${VERSION}" \ + --target "${GITHUB_SHA}" \ + --generate-notes \ + $PRERELEASE + fi - name: Publish to NuGet if: success() diff --git a/docs/publishing.md b/docs/publishing.md index c382e7f2..ca6f3de5 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -72,24 +72,27 @@ NuGet publication. `nbgv prepare-release` removes the prerelease tag on the release branch, so its `version` has no `{height}`: **every commit on `release/0.11.0` computes the same `0.11.0`**. That is the intended -behaviour for a stable line, but it has a consequence worth knowing before you push twice. - -The two publish steps disagree about repetition: +behaviour for a stable line, and pushing to such a branch more than once is an ordinary thing to do — +a documentation fix, a workflow tweak, a cherry-picked change. Both publish steps tolerate the repeat: - `dotnet nuget push` runs with `--skip-duplicate`, so re-pushing an already-published version is - tolerated and reported, not fatal. -- `gh release create` has no such tolerance. The second push to a release branch tries to create a - release whose tag already exists, that step fails, and because `Publish to NuGet` is gated on - `if: success()` the publication is skipped for that run. - -So a follow-up commit on a release branch — a documentation fix, a cherry-picked hotfix — turns CI -red and publishes nothing, without anything being wrong with the code. Two ways through it: - -- **Preferred:** cut a new version. Bump the release branch's `version` (for example to `0.11.1`) - so the run produces a version that has never been released. -- **If the commit genuinely must not change the version** (say a workflow-only change on the release - branch), expect the release job to fail on the duplicate and treat it as such; nothing was - published, and nothing was lost. + reported and skipped rather than fatal. +- The release job checks whether `v` already exists. If it does, it re-attaches the packages + with `gh release upload --clobber` instead of trying to create the release again, so the run stays + green and `Publish to NuGet` still executes. + +**What repeating does not do is ship anything new.** A published NuGet version is immutable, so if +that follow-up commit changed shipped code, the new packages are skipped and the fix does not reach +consumers under that version. The release job says so loudly — a workflow warning and a job-summary +note — precisely because a green build must not be read as "the change shipped". + +So: a commit on a release branch that changes what consumers get needs a new version. Bump the +release branch's `version` (for example to `0.11.1`) and push again. A commit that changes nothing +consumers receive can be pushed as-is and will simply re-attach identical packages. + +The existing tag is left where it is on the repeat path. Moving a published tag changes what an +already-released version points at, which is a deliberate decision rather than something CI should +do on its own. The release job does not rebuild: it downloads the `packages` artifact produced by `Build, Test, Pack`, so a release publishes exactly the packages CI tested. From 24f3564678d668e019b7e62da3d9f511c0f3ca78 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 15:26:09 -0400 Subject: [PATCH 06/13] fix(ci): pack the outputs the tests ran against, and ground the servicing flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings on the publishing audit, both against claims I wrote. - "A release publishes exactly the packages CI tested" did not follow from the workflow: Test ran with --no-build, Pack did not, so dotnet pack was free to re-run Build and a regenerated assembly could have reached a package the tests never exercised. Rather than weaken the claim, Pack now uses --no-build too, so Test and Pack share one Build's outputs and the guarantee holds. Verified locally: pack --no-build produces all 10 packages and 9 symbol packages with no errors, and the pack step runs on every pull request, so CI exercises this immediately — unlike the release job. - The servicing guidance told a maintainer to bump `version` on the release branch, which the review read as bypassing the required prepare-release flow. Checking Nerdbank.GitVersioning's own versioning-workflow documentation, neither reading was right: its "service a released version" flow works *on* the release branch, and prepare-release from main cuts a new line rather than a patch. It also documents prepare-release being run on a release branch to move its stability stage. So the page now follows that flow, and explains why this repository needs one extra step inside it: release.branchName is `release/{version}` and prepare-release strips the prerelease tag, leaving no {height}, so `nbgv get-version` on a serviced commit answers with the version already published. The bump is that gap, not an improvisation — and the resulting branch/version mismatch is cosmetic, since publicReleaseRefSpec matches any `release/*`. --- .github/workflows/ci.yml | 7 +++++-- docs/publishing.md | 31 +++++++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99f3e1b0..a584780a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -258,11 +258,14 @@ jobs: id: pack shell: pwsh run: | + # --no-build, like Test above: packing the same outputs the tests ran against is what lets + # docs/publishing.md promise a release ships exactly what CI exercised. Without it pack is + # free to re-run Build, and a regenerated assembly could reach a package untested. if ($env:PUBLIC_RELEASE -eq 'true') { - dotnet pack src/Repl.slnx -c Release --no-restore -p:PublicRelease=true -p:WarnOnPackingNonPackableProject=false -o '${{ runner.temp }}/packages' + dotnet pack src/Repl.slnx -c Release --no-build --no-restore -p:PublicRelease=true -p:WarnOnPackingNonPackableProject=false -o '${{ runner.temp }}/packages' } else { - dotnet pack src/Repl.slnx -c Release --no-restore -p:WarnOnPackingNonPackableProject=false -o '${{ runner.temp }}/packages' + dotnet pack src/Repl.slnx -c Release --no-build --no-restore -p:WarnOnPackingNonPackableProject=false -o '${{ runner.temp }}/packages' } - name: Package readiness report (non-blocking) diff --git a/docs/publishing.md b/docs/publishing.md index ca6f3de5..f828a577 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -86,16 +86,39 @@ that follow-up commit changed shipped code, the new packages are skipped and the consumers under that version. The release job says so loudly — a workflow warning and a job-summary note — precisely because a green build must not be read as "the change shipped". -So: a commit on a release branch that changes what consumers get needs a new version. Bump the -release branch's `version` (for example to `0.11.1`) and push again. A commit that changes nothing -consumers receive can be pushed as-is and will simply re-attach identical packages. +So: a commit on a release branch that changes what consumers get needs a new version, and a commit +that changes nothing they receive can be pushed as-is — it re-attaches identical packages. + +### Servicing a released version + +Nerdbank.GitVersioning's own workflow for a fix to an already-released line works **on the release +branch**, not through `nbgv prepare-release` from `main` — that command cuts a *new* line, which is +what the *Release preparation* section above covers: + +```powershell +git switch release/0.11.0 +# Merge or cherry-pick the fix and commit it. +nbgv get-version # what version will this commit build as? +``` + +Run `nbgv get-version` before pushing, because this repository's configuration makes the answer +`0.11.0` again: `release.branchName` is `release/{version}`, so the branch carries a full three-part +version, and `prepare-release` removed the prerelease tag, leaving no `{height}` to advance. A fix +therefore needs its `version` field bumped on that branch — to `0.11.1` — before it can ship. The +branch name then no longer matches its version, which is cosmetic: `publicReleaseRefSpec` matches +`^refs/heads/release/.*$`, so packaging and release creation are unaffected. + +`nbgv prepare-release` can also be run *on* a release branch to move its stability stage, for example +from a prerelease tag to stable. Consult Nerdbank.GitVersioning's versioning-workflow documentation +before doing anything here that these two paths do not cover; do not improvise a version edit. The existing tag is left where it is on the repeat path. Moving a published tag changes what an already-released version points at, which is a deliberate decision rather than something CI should do on its own. The release job does not rebuild: it downloads the `packages` artifact produced by -`Build, Test, Pack`, so a release publishes exactly the packages CI tested. +`Build, Test, Pack`. Within that job, `Test` and `Pack` both run with `--no-build` against the +outputs of a single `Build`, so a release publishes exactly the assemblies the tests ran against. ## Do I need to create a Git tag manually? From e461472fdab3ff2b08c5322a3712d31749ef0539 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 15:28:45 -0400 Subject: [PATCH 07/13] fix(ci): leave an existing release untouched instead of re-uploading to it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A review finding against my own idempotence change from earlier in this branch, and it is right: I conflated "idempotent" with "re-upload" when it means "no-op". Replacing a published release's assets while NuGet skips the same immutable version would leave a direct GitHub download and a NuGet install of one version number carrying different binaries, with the tag describing neither. The repeat path now does nothing at all: no upload, no tag move, warning and job summary only. The run still stays green so Publish to NuGet is not skipped for a run with nothing wrong in it, and the tag keeps pointing at the commit that produced the published packages — the only commit it can honestly describe. Dropping the upload also removes the failure mode entirely, since the path no longer mutates the release at all. One correction for the record: the finding attributed to `gh release upload --help` a statement that it deletes existing assets before uploading and that a failed upload loses the originals. That text is not in the help output (gh 2.73.0), which says only "Overwrite existing assets of the same name". The finding's substance did not depend on it and stands on its own. docs/publishing.md updated to describe the no-op and why immutability makes it the right behaviour. --- .github/workflows/ci.yml | 17 ++++++++++------- docs/publishing.md | 25 ++++++++++++++----------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a584780a..28914b0a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -519,18 +519,21 @@ jobs: # A release branch's version has no {height}, so every commit on it computes the same # number. Creating the release twice is therefore an ordinary occurrence rather than an # error, and failing here would skip Publish to NuGet for a run that had nothing wrong - # with it. Re-attach the packages instead — but say loudly that nothing new can ship - # under a version NuGet already has, because a green build must not imply otherwise. + # with it. So this path does nothing at all — deliberately, and loudly. + # + # Nothing, rather than re-uploading: a published version is immutable on NuGet, so + # replacing the release's assets would leave a direct GitHub download and a NuGet install + # of the same version carrying different binaries, with the tag describing neither. + # Whatever a repeat run built cannot ship under this version; that needs a version bump. if gh release view "v${VERSION}" >/dev/null 2>&1; then - echo "::warning title=Release v${VERSION} already exists::Packages were re-attached, but NuGet will skip them: a published version is immutable. If this commit changed shipped code, bump the version on the release branch and push again." + echo "::warning title=Release v${VERSION} already exists::Left untouched. A published version is immutable, so nothing this run built can ship as ${VERSION} — on NuGet or on the release. If this commit changed shipped code, bump the version on the release branch and push again." { - echo "### Release \`v${VERSION}\` already existed" + echo "### Release \`v${VERSION}\` already exists — left untouched" echo - echo "Assets re-attached to the existing release. **NuGet will skip these packages** —" - echo "a published version is immutable, so no code change in this commit can ship as" + echo "Its assets and tag are unchanged, and **nothing this run built was published**." + echo "A released version is immutable, so no code change in this commit can ship as" echo "\`${VERSION}\`. Bump the version on the release branch if it needs to." } >> "$GITHUB_STEP_SUMMARY" - gh release upload "v${VERSION}" packages/* --clobber else # --target is what makes the tag point at the commit that built these packages. Without # it, gh creates a missing tag from the latest state of the default branch, so a diff --git a/docs/publishing.md b/docs/publishing.md index f828a577..59c1a9a1 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -77,14 +77,17 @@ a documentation fix, a workflow tweak, a cherry-picked change. Both publish step - `dotnet nuget push` runs with `--skip-duplicate`, so re-pushing an already-published version is reported and skipped rather than fatal. -- The release job checks whether `v` already exists. If it does, it re-attaches the packages - with `gh release upload --clobber` instead of trying to create the release again, so the run stays - green and `Publish to NuGet` still executes. - -**What repeating does not do is ship anything new.** A published NuGet version is immutable, so if -that follow-up commit changed shipped code, the new packages are skipped and the fix does not reach -consumers under that version. The release job says so loudly — a workflow warning and a job-summary -note — precisely because a green build must not be read as "the change shipped". +- The release job checks whether `v` already exists. If it does, it leaves that release + entirely alone — assets and tag — instead of trying to create it again, so the run stays green and + `Publish to NuGet` still executes. It does not re-upload either: a released version is immutable on + NuGet, so replacing the release's assets would leave a direct GitHub download and a NuGet install of + the same version carrying different binaries, with the tag describing neither. + +**What repeating does not do is publish anything.** Neither NuGet nor the release accepts a second +version's worth of packages under a number already released, so if that follow-up commit changed +shipped code, the fix reaches nobody under that version. The release job says so loudly — a workflow +warning and a job-summary note — precisely because a green build must not be read as "the change +shipped". So: a commit on a release branch that changes what consumers get needs a new version, and a commit that changes nothing they receive can be pushed as-is — it re-attaches identical packages. @@ -112,9 +115,9 @@ branch name then no longer matches its version, which is cosmetic: `publicReleas from a prerelease tag to stable. Consult Nerdbank.GitVersioning's versioning-workflow documentation before doing anything here that these two paths do not cover; do not improvise a version edit. -The existing tag is left where it is on the repeat path. Moving a published tag changes what an -already-released version points at, which is a deliberate decision rather than something CI should -do on its own. +That includes the tag: it keeps pointing at the commit that produced the published packages, which +is the only commit it can honestly describe. Moving a published tag is a deliberate decision rather +than something CI should do on its own. The release job does not rebuild: it downloads the `packages` artifact produced by `Build, Test, Pack`. Within that job, `Test` and `Pack` both run with `--no-build` against the From 5fadcfd2bdb43d89b60fe32d9f9c608695ecb1f6 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 16:12:13 -0400 Subject: [PATCH 08/13] fix(ci): stop the repeat-path warning claiming more than it can MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Of four findings this round, one held. The warning I added said "nothing this run built was published", which is false in one case: Publish to NuGet still runs on the repeat path, and --skip-duplicate leaves published versions alone but uploads a package NuGet does not yet have. That is deliberate — it is how a push whose NuGet upload failed part-way recovers — so the message is corrected rather than the behaviour. Skipping NuGet on an existing release, as the finding proposed, would trade a misleading sentence for a pipeline that cannot recover from a transient push failure without manual intervention. docs/publishing.md now states the same thing, including the consequence worth knowing: such a recovery uploads from the current commit while the release's assets and tag stay on the one that created them. Also made the AGENTS.md distinction explicit rather than implied, since it was read the other way twice: running `nbgv prepare-release` on a release branch to move its stability stage is that tool's documented behaviour, and AGENTS.md's rule concerns not cutting a release from a feature or pull-request branch. Cutting a new line still starts from `main`. The other three findings are answered on the PR rather than applied. Two rest on premises that do not hold — one cites `gh release create --help` for a statement about draft/upload/publish mechanics that the help does not contain, and one describes a notes-baseline selection that this repository's own v0.11.0 notes contradict, having compared against v0.10.0 rather than a newer prerelease. --- .github/workflows/ci.yml | 12 ++++++++---- docs/publishing.md | 13 +++++++++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 28914b0a..0782c906 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -526,13 +526,17 @@ jobs: # of the same version carrying different binaries, with the tag describing neither. # Whatever a repeat run built cannot ship under this version; that needs a version bump. if gh release view "v${VERSION}" >/dev/null 2>&1; then - echo "::warning title=Release v${VERSION} already exists::Left untouched. A published version is immutable, so nothing this run built can ship as ${VERSION} — on NuGet or on the release. If this commit changed shipped code, bump the version on the release branch and push again." + echo "::warning title=Release v${VERSION} already exists::Its assets and tag are left untouched. NuGet still runs and will skip every version it already has — but it accepts one it is missing, which is how a partial push recovers. If this commit changed shipped code, bump the version on the release branch: the release's own assets will not be updated." { echo "### Release \`v${VERSION}\` already exists — left untouched" echo - echo "Its assets and tag are unchanged, and **nothing this run built was published**." - echo "A released version is immutable, so no code change in this commit can ship as" - echo "\`${VERSION}\`. Bump the version on the release branch if it needs to." + echo "Its assets and tag are unchanged. \`Publish to NuGet\` still runs: it skips every" + echo "version already published, which is the normal outcome here, and uploads one that" + echo "is missing, which is how a partially failed push recovers." + echo + echo "So a code change in this commit cannot reach consumers as \`${VERSION}\` through the" + echo "release assets, and reaches NuGet only for a package NuGet does not yet have." + echo "Bump the version on the release branch if the change is meant to ship." } >> "$GITHUB_STEP_SUMMARY" else # --target is what makes the tag point at the commit that built these packages. Without diff --git a/docs/publishing.md b/docs/publishing.md index 59c1a9a1..e493d778 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -82,6 +82,10 @@ a documentation fix, a workflow tweak, a cherry-picked change. Both publish step `Publish to NuGet` still executes. It does not re-upload either: a released version is immutable on NuGet, so replacing the release's assets would leave a direct GitHub download and a NuGet install of the same version carrying different binaries, with the tag describing neither. +- `Publish to NuGet` running on that path is deliberate: it is how a push whose NuGet upload failed + part-way recovers, since `--skip-duplicate` leaves what is already published alone and uploads only + what is missing. The consequence to keep in mind is that such a recovery uploads from the *current* + commit, while the release's assets and tag stay on the one that created them. **What repeating does not do is publish anything.** Neither NuGet nor the release accepts a second version's worth of packages under a number already released, so if that follow-up commit changed @@ -112,8 +116,13 @@ branch name then no longer matches its version, which is cosmetic: `publicReleas `^refs/heads/release/.*$`, so packaging and release creation are unaffected. `nbgv prepare-release` can also be run *on* a release branch to move its stability stage, for example -from a prerelease tag to stable. Consult Nerdbank.GitVersioning's versioning-workflow documentation -before doing anything here that these two paths do not cover; do not improvise a version edit. +from a prerelease tag to stable — that is Nerdbank.GitVersioning's documented behaviour, not an +exception invented here. `AGENTS.md`'s rule that release preparation runs from `main` is about not +cutting a release from a feature or pull-request branch; a release branch is neither, and cutting a +new line still starts from `main` as the *Release preparation* section describes. + +Consult Nerdbank.GitVersioning's versioning-workflow documentation before doing anything here that +these paths do not cover; do not improvise a version edit. That includes the tag: it keeps pointing at the commit that produced the published packages, which is the only commit it can honestly describe. Moving a published tag is a deliberate decision rather From a7af04a669bcba4538be4c777000db402fc3b5a7 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 16:15:20 -0400 Subject: [PATCH 09/13] docs: rewrite the release-repeat section instead of patching it a sixth time Five rounds of edits to the same paragraphs produced exactly the drift this page's subject is about, so reading it end to end was worth more than another patch. Three defects, all mine: - A direct contradiction. "What repeating does not do is publish anything. Neither NuGet nor the release accepts a second version's worth of packages" sat three lines below a bullet stating that NuGet does upload a package it is missing. The previous commit corrected the workflow warning and left the prose asserting the absolute it had just stopped claiming. - A sentence from the --clobber design that no longer exists: "it re-attaches identical packages". Nothing re-attaches anything; the release is left untouched. - An orphaned paragraph. "That includes the tag..." lost its antecedent when the servicing section was inserted between it and the repeat-path discussion. The region is now written once, as four claims that do not overlap: the release is left entirely alone including its tag; NuGet still runs and publishes precisely what it lacks, which is how a partial push recovers; therefore a change meant for consumers needs a new version; and what a release contains. The servicing flow and the build/pack guarantee follow as their own subsections rather than trailing paragraphs. Verified the remaining claims still match the workflow warning they describe. --- docs/publishing.md | 52 ++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/docs/publishing.md b/docs/publishing.md index e493d778..d0af5854 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -73,28 +73,28 @@ NuGet publication. `nbgv prepare-release` removes the prerelease tag on the release branch, so its `version` has no `{height}`: **every commit on `release/0.11.0` computes the same `0.11.0`**. That is the intended behaviour for a stable line, and pushing to such a branch more than once is an ordinary thing to do — -a documentation fix, a workflow tweak, a cherry-picked change. Both publish steps tolerate the repeat: - -- `dotnet nuget push` runs with `--skip-duplicate`, so re-pushing an already-published version is - reported and skipped rather than fatal. -- The release job checks whether `v` already exists. If it does, it leaves that release - entirely alone — assets and tag — instead of trying to create it again, so the run stays green and - `Publish to NuGet` still executes. It does not re-upload either: a released version is immutable on - NuGet, so replacing the release's assets would leave a direct GitHub download and a NuGet install of - the same version carrying different binaries, with the tag describing neither. -- `Publish to NuGet` running on that path is deliberate: it is how a push whose NuGet upload failed - part-way recovers, since `--skip-duplicate` leaves what is already published alone and uploads only - what is missing. The consequence to keep in mind is that such a recovery uploads from the *current* - commit, while the release's assets and tag stay on the one that created them. - -**What repeating does not do is publish anything.** Neither NuGet nor the release accepts a second -version's worth of packages under a number already released, so if that follow-up commit changed -shipped code, the fix reaches nobody under that version. The release job says so loudly — a workflow -warning and a job-summary note — precisely because a green build must not be read as "the change -shipped". - -So: a commit on a release branch that changes what consumers get needs a new version, and a commit -that changes nothing they receive can be pushed as-is — it re-attaches identical packages. +a documentation fix, a workflow tweak, a cherry-picked change. Neither publish step treats the repeat +as an error, and neither of them can republish the version either. + +**The GitHub Release is left entirely alone.** The job checks whether `v` already exists and, +if it does, touches nothing: not the assets, not the tag. It does not re-upload, because a released +version is immutable on NuGet — replacing the assets would leave a direct GitHub download and a NuGet +install of one version number carrying different binaries. The tag likewise keeps pointing at the +commit that produced the published packages, the only commit it can honestly describe. Moving a +published tag is a deliberate decision, not something CI should do on its own. + +**NuGet still runs, and that is deliberate.** `dotnet nuget push --skip-duplicate` leaves every +version it already has alone, which is the normal outcome here. But it *does* upload a package NuGet +is missing, and that is the only way a push whose upload failed part-way recovers without manual +intervention. Two things follow. A repeat run is not guaranteed to publish nothing at all — it +publishes precisely what NuGet lacks. And such a recovery uploads from the *current* commit, while +the release's assets and tag stay on the one that created them. + +**So a change meant for consumers needs a new version.** Under a number already released, the release +assets will not be updated and NuGet will accept only what it does not yet have — which is never the +changed build of a package it already holds. The job says so through a workflow warning and a +job-summary note, precisely because a green build must not be read as "the change shipped". A commit +that changes nothing consumers receive can be pushed as it is. ### Servicing a released version @@ -121,12 +121,10 @@ exception invented here. `AGENTS.md`'s rule that release preparation runs from ` cutting a release from a feature or pull-request branch; a release branch is neither, and cutting a new line still starts from `main` as the *Release preparation* section describes. -Consult Nerdbank.GitVersioning's versioning-workflow documentation before doing anything here that -these paths do not cover; do not improvise a version edit. +Consult Nerdbank.GitVersioning's versioning-workflow documentation before doing anything these paths +do not cover; do not improvise a version edit. -That includes the tag: it keeps pointing at the commit that produced the published packages, which -is the only commit it can honestly describe. Moving a published tag is a deliberate decision rather -than something CI should do on its own. +### What a release contains The release job does not rebuild: it downloads the `packages` artifact produced by `Build, Test, Pack`. Within that job, `Test` and `Pack` both run with `--no-build` against the From e15d4c10b77a24f031cd3276a5dc89d5fb67d1a9 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 16:17:01 -0400 Subject: [PATCH 10/13] docs: scope the prepare-release rule so the two sections cannot be read as conflicting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading the whole page rather than the section I had been editing turned up a tension the review was probably reaching for without naming it: the opening said "run `nbgv prepare-release` from a clean `main` only", four paragraphs before a servicing section that describes running it on a release branch. The qualifier "for stable release preparation" made the two technically consistent, but "only" reads as global, and a page whose job is to keep a release from going wrong should not depend on a reader parsing that carefully. The opening now says what it governs — cutting a release line starts from `main` — names the two invocations that operate on an existing release branch, and points at the section describing them. Same rule, no longer misreadable. --- docs/publishing.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/publishing.md b/docs/publishing.md index d0af5854..6415aaac 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -13,8 +13,10 @@ git status --short nbgv prepare-release ``` -For stable release preparation, run `nbgv prepare-release` from a clean `main` only. Do -not run it from a feature branch or a pull request branch. +Cutting a release line starts from a clean `main`, never from a feature branch or a pull +request branch. That is what this section covers. Two other `nbgv prepare-release` +invocations operate on an existing release branch instead — promoting its stability stage, +and the servicing flow — and are described under *Servicing a released version* below. The command uses the `release` settings in `version.json` to: From c7a2250ab2dbb48530936b1ccbcae917dcb54c1b Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 16:46:36 -0400 Subject: [PATCH 11/13] fix(ci): fail on an unpublished draft release instead of skipping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gh release create` with assets is not a single API call. Its manual documents that it creates the release as a draft, uploads the assets, then publishes it, so a run interrupted mid-upload leaves an unpublished draft behind — and `gh release view` finds it, because its lookup queries drafts alongside published releases. The existence check read that as a finished release, took the no-op path, and let `Publish to NuGet` push packages under a version with no release anyone could see. The check now tells three states apart through a single `--json isDraft` call. A draft stops the job with an error annotation and a job summary, publishing nothing. A published release is still left untouched, assets and tag. A lookup that fails for any other reason reads as missing and falls through to `gh release create`, which then fails loudly rather than skipping anything. Detection only, deliberately: publishing the draft and deleting it are both decisions about what has already reached consumers, and this job is the one part of the workflow that pull-request CI never exercises, so untested recovery logic here would be worse than a stop. --- .github/workflows/ci.yml | 35 ++++++++++++++++++++++++++++++++++- docs/publishing.md | 16 ++++++++++++---- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0782c906..7070efad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -516,6 +516,39 @@ jobs: PRERELEASE="" [[ "$VERSION" == *-* ]] && PRERELEASE="--prerelease" + # `gh release create` with assets is not one API call: gh's manual (Immutable Releases) + # states it creates the release as a draft, uploads the assets, then publishes it. A run + # interrupted mid-upload leaves an unpublished draft, which `gh release view` still finds + # because its lookup queries drafts too. Reading that as an existing release would skip + # the creation below and let Publish to NuGet run, putting packages on NuGet under a + # version no one can see a release for — so the three states are told apart here. A + # lookup that fails for any other reason reads as `missing` and falls through to + # `gh release create`, which then fails loudly rather than skipping anything. + RELEASE_STATE=missing + if IS_DRAFT="$(gh release view "v${VERSION}" --json isDraft --jq '.isDraft' 2>/dev/null)"; then + RELEASE_STATE=published + if [[ "$IS_DRAFT" == "true" ]]; then + RELEASE_STATE=draft + fi + fi + + if [[ "$RELEASE_STATE" == draft ]]; then + # Detection, not repair: publishing the draft and deleting it are both decisions about + # what has already reached consumers, and this job is the one part of the workflow that + # pull-request CI never runs, so untested recovery logic here is worse than stopping. + echo "::error title=Release v${VERSION} exists as an unpublished draft::A previous run created it and did not finish. Nothing was published by this run. Inspect the draft, then either publish it with the packages it is missing or delete it and re-run." + { + echo "### Release \`v${VERSION}\` exists as an unpublished draft" + echo + echo "A previous run created the release and did not finish uploading its assets." + echo "**Nothing was published by this run** — not the release, not NuGet." + echo + echo "Inspect the draft, then either publish it with the packages it is missing or" + echo "delete it and re-run this workflow." + } >> "$GITHUB_STEP_SUMMARY" + exit 1 + fi + # A release branch's version has no {height}, so every commit on it computes the same # number. Creating the release twice is therefore an ordinary occurrence rather than an # error, and failing here would skip Publish to NuGet for a run that had nothing wrong @@ -525,7 +558,7 @@ jobs: # replacing the release's assets would leave a direct GitHub download and a NuGet install # of the same version carrying different binaries, with the tag describing neither. # Whatever a repeat run built cannot ship under this version; that needs a version bump. - if gh release view "v${VERSION}" >/dev/null 2>&1; then + if [[ "$RELEASE_STATE" == published ]]; then echo "::warning title=Release v${VERSION} already exists::Its assets and tag are left untouched. NuGet still runs and will skip every version it already has — but it accepts one it is missing, which is how a partial push recovers. If this commit changed shipped code, bump the version on the release branch: the release's own assets will not be updated." { echo "### Release \`v${VERSION}\` already exists — left untouched" diff --git a/docs/publishing.md b/docs/publishing.md index 6415aaac..6404542e 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -78,13 +78,21 @@ behaviour for a stable line, and pushing to such a branch more than once is an o a documentation fix, a workflow tweak, a cherry-picked change. Neither publish step treats the repeat as an error, and neither of them can republish the version either. -**The GitHub Release is left entirely alone.** The job checks whether `v` already exists and, -if it does, touches nothing: not the assets, not the tag. It does not re-upload, because a released -version is immutable on NuGet — replacing the assets would leave a direct GitHub download and a NuGet -install of one version number carrying different binaries. The tag likewise keeps pointing at the +**A published release is left entirely alone.** The job checks whether `v` has already been +published and, if it has, touches nothing: not the assets, not the tag. It does not re-upload, +because a released version is immutable on NuGet — replacing the assets would leave a direct GitHub +download and a NuGet install of one version number carrying different binaries. The tag likewise keeps pointing at the commit that produced the published packages, the only commit it can honestly describe. Moving a published tag is a deliberate decision, not something CI should do on its own. +**An unfinished draft stops the run instead.** `gh release create` attaches assets by creating the +release as a draft, uploading the packages, then publishing it, so a run interrupted mid-upload +leaves a draft behind — and the existence check finds it, because `gh release view` looks up drafts +as well as published releases. Taking that for a finished release would skip creation and let NuGet +publish under a version with no release anyone can see, so the job fails instead and publishes +nothing. Inspect the draft and either publish it with the packages it is missing or delete it and +re-run: both are decisions about what has already reached consumers, and CI should not guess at them. + **NuGet still runs, and that is deliberate.** `dotnet nuget push --skip-duplicate` leaves every version it already has alone, which is the normal outcome here. But it *does* upload a package NuGet is missing, and that is the only way a push whose upload failed part-way recovers without manual From c45fac392ac204fa426dabc113c0020798ec3a5d Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 16:54:30 -0400 Subject: [PATCH 12/13] ci: retry the nushell asset download MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Shell Completion Smoke (Real Shells)` failed with `curl: (22) The requested URL returned error: 500` and no retry line in the log. Exit 22 is curl's own, so the failure was not the `gh_api` helper — that one retries three times and returns 1. It was the asset download, the single curl in the step with no retry, and the one whose host actually returned the 500. curl counts HTTP 500 among its transient errors, so plain `--retry` covers this without `--retry-all-errors`. Confirmed against a local server that always answers 500: without the flag curl makes one request and exits 22, with `--retry 3` it makes four and then exits 22 — the same signature the failed job logged. --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7070efad..dd18576a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,7 +140,10 @@ jobs: tmp_dir="$(mktemp -d)" trap 'rm -rf "$tmp_dir"' EXIT - curl -fsSL "$nu_url" -o "$tmp_dir/nu.tar.gz" + # The release lookup above retries; this download did not, and it is the one that + # fails — a 500 from the asset host took the job down with curl's own exit 22 and no + # retry line in the log. curl treats HTTP 500 as transient, so `--retry` covers it. + curl -fsSL --retry 3 --retry-delay 2 "$nu_url" -o "$tmp_dir/nu.tar.gz" tar -xzf "$tmp_dir/nu.tar.gz" -C "$tmp_dir" nu_binary="$(find "$tmp_dir" -type f -name nu | head -n 1)" if [[ -z "$nu_binary" ]]; then From 522737d08b0eb0baa896280233fbb62c54a6b90d Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 10 Sep 2026 17:00:36 -0400 Subject: [PATCH 13/13] docs: route a servicing fix through a pull request, for the notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The servicing flow said "merge or cherry-pick the fix and commit it", and the release-notes section said `--generate-notes` builds the body from merged pull requests. Both are true, and together they describe a patch release that says nothing: a cherry-pick reaches the branch outside a pull request, so it contributes no entry, and this branch is the one that deleted CHANGELOG.md. Measured rather than assumed. `POST /releases/generate-notes` for the twelve commits between v0.12.0-dev.45 and this branch head — real commits, no merged pull request, since #88 is still open — returns a body of one line, the compare link, with no `What's Changed` heading at all. The same call across a range containing #80 returns its title, author and link. So the servicing section now asks for a pull request targeting the release branch and says why, with editing the release body by hand as the fallback when a fix lands as a direct commit; the workflow passes `--generate-notes` unconditionally and cannot supply notes for that path. The release-notes section states the empty case alongside what it already said about PR descriptions. --- docs/publishing.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/publishing.md b/docs/publishing.md index 6404542e..04a30e18 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -114,7 +114,7 @@ what the *Release preparation* section above covers: ```powershell git switch release/0.11.0 -# Merge or cherry-pick the fix and commit it. +# Merge the fix's pull request into this branch, then: nbgv get-version # what version will this commit build as? ``` @@ -125,6 +125,17 @@ therefore needs its `version` field bumped on that branch — to `0.11.1` — be branch name then no longer matches its version, which is cosmetic: `publicReleaseRefSpec` matches `^refs/heads/release/.*$`, so packaging and release creation are unaffected. +**Bring the fix in through a pull request targeting the release branch**, rather than cherry-picking +straight onto it. The reason is the release notes: `--generate-notes` builds the body from the pull +requests merged since the previous tag, so a commit that reached the branch outside a pull request +contributes nothing to it. Measured against this repository, a range of twelve real commits with no +merged pull request generates no `What's Changed` section at all — the body is a single compare link. +There is no changelog file to fall back on, so that is a patch release that does not say what it +fixed. A pull request targeting `release/0.11.0` costs nothing extra and puts its title in the notes. + +If a fix does land as a direct commit anyway, edit the release body by hand afterwards: the workflow +passes `--generate-notes` unconditionally and has no way to supply notes for that path. + `nbgv prepare-release` can also be run *on* a release branch to move its stability stage, for example from a prerelease tag to stable — that is Nerdbank.GitVersioning's documented behaviour, not an exception invented here. `AGENTS.md`'s rule that release preparation runs from `main` is about not @@ -157,7 +168,9 @@ assigns the version at pack time. Know what that publishes, and what it does not. `--generate-notes` emits **pull request titles**, authors and links; it does **not** copy a PR description into the release body. So a PR title is consumer-facing prose, and a migration step written only in a PR description is reachable through the -link but is not part of the notes. +link but is not part of the notes. And when the range contains no merged pull request at all, the +body is only the compare link — no `What's Changed` heading, no commit list. That is why a servicing +fix belongs in a pull request targeting its release branch; see *Servicing a released version*. Durable guidance therefore belongs in the topic page under `docs/` that owns the feature — a new default, a behavioural break and its restore recipe, a constraint on upgrading packages together.