diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0e27034..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 @@ -258,11 +261,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) @@ -508,13 +514,76 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | + set -euo pipefail VERSION="${{ needs.build-test-pack.outputs.version }}" PRERELEASE="" [[ "$VERSION" == *-* ]] && PRERELEASE="--prerelease" - gh release create "v${VERSION}" packages/* \ - --title "v${VERSION}" \ - --generate-notes \ - $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 + # 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 [[ "$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" + echo + 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 + # 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/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/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 eed5a4d8..6fc30ad4 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -152,6 +152,32 @@ 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; +}); +``` + +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 +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`. @@ -341,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. diff --git a/docs/publishing.md b/docs/publishing.md index e90a29de..04a30e18 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: @@ -40,18 +42,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,12 +70,113 @@ 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, 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. Neither publish step treats the repeat +as an error, and neither of them can republish the version either. + +**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 +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 + +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 the fix's pull request into this branch, then: +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. + +**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 +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 these paths +do not cover; do not improvise a version edit. + +### 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 +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? - No for normal flow. - 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}" --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. 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. +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 - Package and symbol packages (`.snupkg`) are produced by `Build, Test, Pack` and published