feat: add --dry-run to release create and release delete

[NickJosevski]

Refs #63

Adds a --dry-run flag that does everything a command normally does — gathers input, runs the read-only API calls, resolves what it can — then prints what would happen instead of mutating Octopus.

Mechanism, and why

The obvious implementation is a persistent flag on the root command. That is the one design I deliberately avoided: a persistent --dry-run is accepted by every command, including the ones that have not implemented it, so a caller would be told nothing happened while the mutation went through. A flag that lies is worse than no flag.

So this PR uses opt-in per command, backed by a client-level guard:

1. --dry-run is declared locally by the commands that genuinely implement it (dryrun.AddFlag). Anywhere else it is an unknown flag error:

   $ octopus release list --dry-run
   unknown flag: --dry-run
   

2. The API client refuses to mutate while a dry run is in progress. NewCmdRoot's PersistentPreRun sees that the command being executed set --dry-run and calls ClientFactory.SetDryRun(true), which wraps the HTTP transport in dryrun.GuardRoundTripper. Anything other than GET/HEAD/OPTIONS is refused before it leaves the process:

   dry run blocked a POST request to /api/Spaces-1/releases/create/v1;
   this command does not fully support --dry-run, please raise an issue
   

   This is the safety net, not the mechanism. It means a bug in a dry-run code path (or a half-finished implementation added later) fails loudly instead of quietly mutating.

Covered vs not

Command	Status
release create	Implemented
release delete	Implemented
Everything else	Rejects --dry-run with unknown flag

release create and release delete are the two the issue calls out as highest value, and release create already computes a lot (channel, package versions, release version) before it submits, so the preview is genuinely informative.

account create (also mentioned in the issue) is not covered here — it goes through the same executor path and would be a small follow-up, but I did not want to grow this PR further before the mechanism is agreed.

Behaviour notes

• release create --dry-run in automation mode does extra read-only work it would not normally do: it resolves the channel, loads the deployment process template, resolves package versions against the feeds and channel rules, and works out the release version. This is what makes it useful in CI.
• If no --channel is given, the server picks the channel by applying channel rules, and the package versions and release version follow from that choice. Rather than guess (and risk showing a plan that does not match reality) the preview says (determined by the Octopus Server).
• release delete --dry-run skips the "Confirm delete of N release(s)" prompt — there is nothing to confirm — and prints the plan instead.
• -f json emits a machine-readable plan with "DryRun": true as the first field, so a CI consumer cannot mistake a plan for a result.

Sample output

octopus release create --project "Fire Project" --channel "Fire Project Default Channel" --package pterm:9.9 --release-notes "Some notes" --dry-run

DRY RUN: no changes will be made in Octopus.

Would create a release with:
Space          Default Space
Project        Fire Project
Channel        Fire Project Default Channel
Version        27.9.33
Release Notes  Some notes

Packages:
PACKAGE  VERSION  STEP NAME/PACKAGE REFERENCE
pterm    9.9      Install/pterm-on-install

DRY RUN: no release was created.

octopus release create --project "Fire Project" --dry-run (no channel, so the server would decide):

DRY RUN: no changes will be made in Octopus.

Would create a release with:
Space          Default Space
Project        Fire Project
Channel        (determined by the Octopus Server)
Version        (determined by the Octopus Server)
Release Notes  (none)

DRY RUN: no release was created.

octopus release delete --project "Fire Project" --version 2.0 --version 2.1 --no-prompt --dry-run

DRY RUN: no changes will be made in Octopus.

Would delete 2 release(s) from project Fire Project:
  2.1
  2.0

DRY RUN: no releases were deleted.

-f json:

{"DryRun":true,"Space":"Default Space","Project":"Fire Project","Channel":"","Version":"","IgnoreExisting":false,"IgnoreChannelRules":false}

Tests

New tests:

• pkg/dryrun/dryrun_test.go — the guard blocks POST/PUT/PATCH/DELETE and lets GET/HEAD/OPTIONS through; IsEnabled is false for commands that do not declare the flag; and an end-to-end assertion that an unsupported command (release list) rejects --dry-run.
• pkg/apiclient/client_factory_test.go — SetDryRun(true) installs the guard on the real client: a POST is refused and never reaches the transport, a GET still goes through.
• pkg/cmd/release/create/create_test.go — TestReleaseCreate_DryRun, three cases (no channel, resolved channel with packages, JSON output). No POST /releases/create/v1 is expected; the mock HTTP server has nothing queued to answer an unexpected request, so a stray mutating call fails the test.
• pkg/cmd/release/delete/delete_test.go — automation and interactive dry runs. No DELETE requests are expected, and the interactive case asserts the confirmation prompt is not asked.

Results, from a clean worktree:

$ go build ./...
(ok)

$ go test ./pkg/...
65 packages ok, 0 failures

Also ran go vet ./pkg/... — the only findings are four pre-existing unreachable code warnings in files this PR does not touch.

Refactors carried along

• packages.BuildPackageVersionOverrides extracted from AskPackageOverrideLoop so the dry-run path resolves --package-version / --package exactly the way the interactive path does, rather than reimplementing it.
• resolveVersioningStrategy extracted from create.AskQuestions for the same reason.

Open questions / options

I'd like a decision on the surface before filling in more commands.

Option (a) — opt-in per command (what this PR does)

A shared helper each command adds explicitly, starting with the highest-value commands.

• For: safe by construction — --dry-run is only ever accepted where it means something. No risk of a command claiming to support it when it doesn't. Output quality is high because each command knows what it would have done. Incremental: ship two commands now, add more as they're needed.
• Against: coverage is inconsistent until filled in. A CI author has to know which commands support it, and octopus account create --dry-run (from the issue) fails today. Discoverability is only through per-command --help.

Option (b) — global persistent flag with the client guard as enforcement

--dry-run on the root command; the client refuses non-GET requests; any command that hasn't implemented a preview fails with a clear "does not support dry run" error rather than lying.

• For: consistent surface, one thing to document, works everywhere immediately. Safe by construction in a different way — the failure mode is a loud error, never a silent mutation.
• Against: much bigger behavioural change. Output quality varies wildly: a command that has implemented dry run prints a useful plan, one that hasn't prints a stack of half-finished output and then a transport error, which reads like a bug. The error also surfaces wrapped by the go-octopusdeploy SDK, so it's ugly. And "the flag exists everywhere but only really works in four places" is arguably its own kind of dishonesty.

Recommendation

(a) for the surface, with (b)'s guard as the safety net — which is what's implemented here. The guard is already wired in, so moving to (b) later is a small change: declare the flag persistently in NewCmdRoot and decide what an unimplemented command should print. Nothing in this PR forecloses that.

How this prevents the "silently ignored flag" failure mode

Three independent layers, in order of when they fire:

1. Cobra rejects --dry-run on a command that doesn't declare it (unknown flag, exit 1) — the caller finds out immediately, at parse time.
2. The transport guard refuses mutating requests during a dry run, so an implementation bug cannot mutate silently.
3. Machine-readable output carries "DryRun": true, so a CI step parsing JSON cannot confuse a plan for a result.

Smaller things I'd like an opinion on

• release delete --dry-run skips the confirmation prompt. I think re-asking "Confirm delete of 2 release(s)" and then not deleting is more confusing than helpful, but it is a deviation from "perform every step except permanent actions".
• release create --dry-run still prints the Automation Command: line in interactive mode. That command is deliberately the real one, without --dry-run. Reasonable, or confusing?
• -f basic currently gets the same human-readable preview as table. The non-dry-run basic output is just the release version, which doesn't exist yet in a dry run. Happy to change if there's a convention I've missed.
• Should --dry-run imply anything about exit codes? Right now a dry run exits 0 if the plan could be built. If a CI system wants "would this have failed", validation errors from the server (channel rules, duplicate version) are not surfaced — the server only evaluates those at create time.

🤖 Generated with Claude Code

[NickJosevski]

Dry run fails for version-controlled (CaC) projects when --git-ref is omitted.

gitReferenceKey stays "" here when neither --git-ref nor --git-commit is given. DeploymentProcesses.Get tolerates that (the SDK falls back to gitPersistenceSettings.DefaultBranch()), but determineReleaseVersion → resolveVersioningStrategy → Deployments.GetDeploymentSettings(project, "") does not: for a CaC project the DeploymentSettings link is git-templated, and the SDK expands it with no gitRef parameter (its doc says "If the project is version controlled you'll need to specify a gitRef"), producing a malformed path/404. So release create -p CacProject -c SomeChannel --dry-run errors, while the real (non-dry-run) create succeeds because the server defaults to the default branch. It also means the process template and the deployment settings could come from different refs.

Suggested fix: mirror the SDK — when the project is version-controlled and gitReferenceKey is empty, default it to project.PersistenceSettings.(projects.GitPersistenceSettings).DefaultBranch() before using it.

[NickJosevski]

Actioned in b30d1fe, with a regression test added in 1b4fc87.

buildReleasePreview now pins gitReferenceKey to project.PersistenceSettings.(projects.GitPersistenceSettings).DefaultBranch() when the project is version-controlled and neither --git-ref nor --git-commit was given, exactly as you suggested. Pinning it at that one point (rather than only before the settings call) also keeps the process template and the deployment settings on the same ref, which was the second half of the problem.

I confirmed the diagnosis rather than taking it on trust: with the pin removed, the new test fails with

expected: "/api/Spaces-1/projects/Projects-87/main/deploymentsettings"
actual  : "/api/Spaces-1/projects/Projects-87//deploymentsettings"

so the malformed path is real, and DeploymentProcesses.Get did resolve fine without it — the asymmetry you described.

Test: TestReleaseCreate_DryRun/"dry run for a config-as-code project without --git-ref reads everything from the default branch". It drives the whole command against the mock server and asserts the exact request sequence, so the main segment in both the template and the settings URLs is load-bearing.

Not covered: a version-controlled project whose PersistenceSettings doesn't type-assert to GitPersistenceSettings. That still falls through with an empty ref rather than erroring; I left it alone because it shouldn't be reachable.

[NickJosevski]

Pre-existing nil-pointer panic made more visible by this PR (line 811, just below this hunk).

This condition enters the donor branch when DonorPackageStepID != nil even if DonorPackage == nil, and line 811 then dereferences versioningStrategy.DonorPackage.PackageReference unconditionally — a nil-pointer panic in interactive mode.

Your new determineReleaseVersion explicitly acknowledges that state is real ("a donor step with no package reference; nothing to read a version from") and handles it safely. Since AskQuestions is being refactored here anyway, worth guarding the interactive path the same way: if versioningStrategy.DonorPackage != nil { donor branch } else if versioningStrategy.DonorPackageStepID != nil { skip/ask }.

[NickJosevski]

Actioned in cef406d, test in e03b697.

The condition now branches on DonorPackage first, and a bare DonorPackageStepID falls into an explicit no-op branch with a comment, so the version is left blank for the server to assign — the same conclusion determineReleaseVersion reaches on the automation path. I kept the branch rather than folding it into the Template case so the three states stay visible in the code.

Verified the panic was real before fixing it: with the old DonorPackageStepID != nil || DonorPackage != nil condition restored, the new test dies with

panic: runtime error: invalid memory address or nil pointer dereference
  create.AskQuestions ... create.go:847

Test: TestReleaseCreate_AskQuestions_RegularProject/"a donor step with no donor package leaves the version to the server rather than panicking". It asserts no version question is asked and options.Version stays empty.

One judgement call worth flagging: leaving the version blank means the server assigns it, which is the best available behaviour here but is silent. If you'd rather the interactive path told the user ("this project versions from a step with no package reference; the server will assign the version"), say so and I'll add the line — I didn't want to invent user-facing copy in a fix commit.

[NickJosevski]

Interactive dry run echoes an automation command that mutates (line 313).

In interactive mode the dry-run check happens after the Automation Command: line is printed, and that echoed command deliberately omits --dry-run. You flagged this as an open question — my vote: it's a footgun. The natural reading of "I ran a rehearsal, here's the equivalent automation command" is that the echoed command reproduces the rehearsal; pasting it into CI creates a real release. Either append --dry-run when the flag is set, or annotate the line (e.g. Automation Command (add --dry-run to rehearse): ...) so the asymmetry is explicit.

[NickJosevski]

Actioned in 2b97415, test in e03b697.

Agreed it's a footgun — an echoed command that quietly upgrades a rehearsal into a real create is worse than no echo at all. Took the first of your two options: resolvedFlags.DryRun.Value is now carried from the run's own flag and DryRun is passed to GenerateAutomationCmd, so the echoed line ends ... --dry-run --no-prompt and reproduces what just ran. The annotation option would have left the copy-paste hazard in place and relied on the reader noticing the parenthetical.

Test: TestReleaseCreate_DryRun_Interactive in create_test.go — a new interactive test through the root command (there wasn't one for release create; the existing interactive tests call AskQuestions directly and never reach the echo). It supplies everything on the command line so the Q&A asks nothing, then asserts the exact stdout, including the automation command line and the preview that follows it.

One cosmetic residual I noticed while asserting that output: because the echo ends with a single newline and dryrun.Header doesn't lead with one, the automation command and DRY RUN: no changes will be made in Octopus. end up on adjacent lines with no blank between them. Readable, but tighter than the rest of the dry-run output. Happy to add a separating newline if you want it; I left the formatting alone rather than changing shared output.

[NickJosevski]

--ignore-channel-rules --dry-run shows a plan filtered by the rules the real create would ignore.

BuildPackageVersionBaselineForChannel always applies the channel's version rules, and options.IgnoreChannelRules is never consulted on this path. With --ignore-channel-rules, the server resolves/validates without the rules, so the previewed package versions (and a donor-derived Version) can differ from the release actually created — e.g. a rule pinning 1.x makes the preview show 1.9 (or unknown if nothing matches) while the real create picks 2.0. The interactive Q&A has the same behavior, so this may be acceptable — but a dry run is specifically a promise about what would happen, so consider skipping the rule filter (or noting it in the output) when IgnoreChannelRules is set.

[NickJosevski]

Partly actioned in 1b90bed, and I want a decision from you on the rest.

The finding holds: BuildPackageVersionBaselineForChannel injects the channel's VersionRange/Tag/VersionTagRegex into every feed query and options.IgnoreChannelRules is never consulted on this path, so the previewed versions (and a donor-derived Version) are rule-filtered while the create the user is rehearsing is told to ignore those rules.

What I did: took the "note it in the output" half. The table now prints a dimmed caveat under the package list when IgnoreChannelRules is set, saying the versions were resolved with the channel's rules applied and the server may pick differently. JSON already carries IgnoreChannelRules: true, so a machine consumer can see the same thing without new copy.

What I deliberately did not do: skip the rule filter. The mechanical change is one line —

baseline, err := packages.BuildPackageVersionBaseline(octopus, deploymentProcessTemplate.Packages, nil)

(the callback is nil-checked, so passing nil gives "latest in feed, no rules"). I didn't take it because it only improves the preview if the server, under ignoreChannelRules, also selects versions without the rules rather than merely skipping rule validation. If it's validation-only, that change would make the preview wrong in the common case, having previously been right.

This is the same question as the --ignore-channel-rules blind spot you raised on #695's package diagnosis, and I'd rather both paths make the same assumption. The difference in stakes: there, a wrong assumption mislabels an error message; here it changes the plan the user is trusting.

Open question: does ignoreChannelRules on the create-release API skip rule-constrained package selection, or only rule validation? I can't answer it from this repo and I haven't tested it against a server. If it skips selection too, I'll make the one-line change here and drop the caveat note; if it's validation-only, the current note is the right answer and I'll leave it.

[NickJosevski]

release delete --dry-run -f json emits ANSI-colored human text, not JSON.

--output-format is a persistent root flag, so it's accepted here, and printDeletePlan ignores it — a CI consumer that parses -f json output gets DRY RUN: ... prose (with color codes) on stdout. The PR description's safety argument ("a CI step parsing JSON cannot confuse a plan for a result, because the plan carries DryRun: true") only holds for release create. The non-dry-run delete has no JSON output either, so this is consistent — but the dry-run feature is aimed squarely at CI, so a small DeletePlan{DryRun: true, Project, Versions} JSON branch mirroring printReleasePreview would close the gap.

[NickJosevski]

Actioned in 98b908b.

Added the DeletePlan{DryRun, Project, Versions} branch you described. printDeletePlan now takes the output format, reads it from the persistent root flag, and marshals JSON when -f json was asked for, keeping DryRun as the first field so it mirrors ReleasePreview and a CI consumer cannot read a plan as a result. The human branch is unchanged apart from the empty case (your other comment).

Test: TestReleaseDelete/"noprompt: dry run with json output emits a machine readable plan flagged as a dry run" asserts the exact payload {"DryRun":true,"Project":"Fire Project","Versions":["2.0"]} — exact-match, so colour codes or prose leaking back in would fail it.

Agreed on your framing that the non-dry-run delete still has no JSON output; I left that alone as out of scope for this PR. It does mean release delete -f json without --dry-run still prints prose, so the two differ in shape — worth an issue if we want -f json to be uniform across the command.

[NickJosevski]

Dry run prints nothing at all when no versions match.

When none of the requested versions exist (octopus release delete -p X -v 9.9 --no-prompt --dry-run), the len(releasesToDelete) == 0 return just above exits 0 with zero output — the caller can't tell "the dry run ran and would delete nothing" from "the flag was ignored". The live command shares the silent no-op, but for a rehearsal an explicit line matters more; consider printing the dry-run header plus "Would delete 0 release(s) — no releases matched" before that early return when flags.DryRun.Value is set.

[NickJosevski]

Actioned in 98b908b.

The dry-run branch now sits before the len(releasesToDelete) == 0 early return, so a rehearsal that matched nothing prints

DRY RUN: no changes will be made in Octopus.

Would delete 0 release(s) from project Fire Project: no releases matched.

DRY RUN: no releases were deleted.

and in JSON {"DryRun":true,"Project":"...","Versions":[]}. The interactive confirmation guard became len(releasesToDelete) > 0 && !flags.DryRun.Value, which keeps the non-dry-run behaviour identical: zero matches still skips the prompt and exits 0 silently, because the old early return and the surviving len == 0 check land in the same place.

Test: TestReleaseDelete/"noprompt: dry run says so when nothing matches" asserts that exact output.

Left the live command's silent no-op alone, per your read that it matters less there.

[NickJosevski]

Custom Field rows print in nondeterministic order.

Iterating preview.CustomFields (a map) shuffles the rows run-to-run for a release with two or more custom fields — noisy for humans diffing two dry runs, and untestable with an exact-match assertion like the ones in create_test.go. Sort the keys before appending rows. (Same pattern exists at line 291 for the automation command, but this one is new.)

[NickJosevski]

Actioned in df350e6, test in 04f2b12.

Added a small sortedKeys helper and used it for both the preview rows and the automation command line you pointed at (line 291) — same bug, same fix, and the automation command is worth stabilising for the same diff-two-runs reason.

Test: TestReleaseCreate_DryRun/"dry run prints custom fields in a stable order" supplies three fields in non-sorted order and asserts the exact table, which is only possible now that the order is deterministic — your point about untestability was the reason there was no such assertion before.

[NickJosevski]

SetDryRun(false) is a silent no-op — the signature promises a toggle the implementation doesn't deliver.

Once armed, c.dryRun short-circuits every later call, so a future caller writing SetDryRun(false) (say, a test harness or a multi-command flow re-using the factory) leaves the guard installed and every mutation blocked, with nothing to say why. Since the guard is deliberately one-way for the process lifetime, make the API say so: either rename to something like EnableDryRunGuard() with no parameter, or document on the interface that disabling is not supported.

[NickJosevski]

Actioned in 4028685.

Took the rename rather than the doc-only option: SetDryRun(enabled bool) is now EnableDryRunGuard() with no parameter, on both the ClientFactory interface and *Client, plus the stub factory and the one caller in root.go. The interface comment now says outright that the guard is one-way for the lifetime of the process. With no parameter there is no false to silently swallow, so the failure mode you described stops being expressible.

Test: TestClientFactory_EnableDryRunGuard_RefusesMutatingRequests (renamed with it) still covers the arming path.

[NickJosevski]

recordingRoundTripper is defined twice in this PR — identically here and in pkg/apiclient/client_factory_test.go. Worth hoisting into test/testutil (next to MockHttpServer) so the next dry-run-covered command's tests don't make a third copy.

[NickJosevski]

Actioned in fbd2077.

Hoisted it to testutil.RecordingRoundTripper in test/testutil/testutil.go, next to MockHttpServer as you suggested, and deleted both local copies (pkg/dryrun/dryrun_test.go and pkg/apiclient/client_factory_test.go now use the shared one). Gave it a doc comment saying what it's for, since the point is the next dry-run-covered command finds it instead of writing a third.

Both existing tests still pass unchanged otherwise: TestGuardRoundTripper and TestClientFactory_EnableDryRunGuard_RefusesMutatingRequests.

[NickJosevski]

Preserve --package-version in the preview when no channel is supplied.

DefaultPackageVersion is not copied into ReleasePreview, and the no-channel branch returns before applying it to a baseline. As a result, release create -p Example --package-version 9.9.42 --no-prompt --dry-run produces exactly the same plan as omitting the flag, in both table and JSON output, even though the real request sends PackageVersion: "9.9.42". I reproduced this with a focused test of buildReleasePreview and both output formats.

Please carry the default package version as a separate preview field and display it even when the channel/package selection is deferred to the server. Add no-channel tests with --package-version, including a case combined with a per-package override.

[NickJosevski]

Actioned in 2044a2a.

Confirmed the reproduction: DefaultPackageVersion was never copied into ReleasePreview, and the no-channel branch returns before BuildPackageVersionOverrides would have applied it, so the flag left no trace in either output format while the real request still sends PackageVersion.

ReleasePreview now carries DefaultPackageVersion as its own field (placed after Version, json:",omitempty"), populated straight from options.DefaultPackageVersion before either early return. The table prints a Default Package Version row whenever it's set — including when a channel was supplied and the resolved Packages table already reflects it. Slightly redundant in that case, but it states what was asked for rather than only what it resolved to, and the alternative (show it only when deferred) makes the output shape depend on something the reader can't see.

Tests, all in TestReleaseCreate_DryRun:

• "dry run without a channel still reports the default package version" — table output, --package-version alone.
• "dry run without a channel reports the default package version alongside per-package overrides" — table, combined with --package pterm:1.2, which also shows the existing Package overrides: block.
• "dry run json output without a channel carries the default package version and overrides" — exact JSON, {"DryRun":true,...,"DefaultPackageVersion":"9.9.42","PackageOverrides":["pterm:1.2"],...}.