feat(command): let commands opt in to handler-driven --dry-run

[jpage-godaddy]

Summary

• --dry-run today short-circuits any Tier::Mutate/Destructive command generically, before the handler ever runs, always returning a fixed {"command": ..., "action": "dry-run: would execute"} envelope. No command can validate input or preview a real effect under --dry-run.
• Add CommandSpec::handles_dry_run(true) so a command can opt out of that short-circuit and run its handler as normal. The handler runs its real validation unconditionally, checks CommandContext::dry_run() to skip only the mutating step, and tags its preview result with CommandResult::with_dry_run() so middleware records a dry-run (not ok) audit/activity outcome and envelope.
  • The handler-supplied with_dry_run() tag is untrusted input: middleware only honors it when the invocation was actually --dry-run and the command opted into handles_dry_run (self.dry_run && meta.handles_dry_run && metadata.dry_run), so a handler bug can't mis-tag a real execution, or a non-opted-in command's execution, as a dry-run in the audit trail.
  • handles_dry_run only makes sense with a context-aware handler (RuntimeCommandSpec::new_with_context) — new/new_typed handlers never receive a CommandContext and so can never check dry_run(). RuntimeCommandSpec::new/new_typed now debug_assert! against this misuse.
• handles_dry_run does not change a command's AuthRequirement. If the dry-run branch needs a live call to build an accurate preview, Required is unaffected. If it doesn't, pair handles_dry_run with auth_optional() and check dry_run() before resolving a credential, so a preview never triggers an unnecessary (possibly interactive) auth flow. Documented in docs/design.md/docs/concepts.md.
• Behaviorally additive and backward compatible: any command that doesn't opt in keeps today's exact generic short-circuit behavior byte-for-byte (pinned by the existing middleware_dry_run_short_circuits_mutating_command and lazy_resolution_skips_auth_for_dry_run tests, both left unmodified). At the API level, this adds public fields to CommandMeta/CommandSpec (neither is #[non_exhaustive]), which is a struct-literal-breaking change for any external exhaustive construction — consistent with this crate's established pre-1.0 practice (e.g. feat: add stage-based feature flagging for modules, groups, and commands #43 added feature_flag to CommandSpec the same way) and its bump-minor-pre-major: true release policy.

Motivated by three gddy (godaddy/cli) tickets — DEVEX-890, DEVEX-889, GDDEVPLAT-81 — where pat add --dry-run and domain contacts init --dry-run couldn't validate input or preview a real effect. The gddy-side consumers of this API are in a separate, not-yet-merged branch pending this crate's release.

Test plan

• cargo fmt --all --check
• cargo clippy --all-targets -- -D warnings
• cargo test --all-targets

New tests: command_spec_handles_dry_run_flows_into_metadata, middleware_handler_driven_dry_run_runs_handler_and_tags_outcome, middleware_handler_driven_dry_run_without_with_dry_run_tag_is_ok, middleware_ignores_with_dry_run_tag_when_invocation_was_not_dry_run, middleware_ignores_with_dry_run_tag_when_command_did_not_opt_in, runtime_command_spec_new_panics_when_paired_with_handles_dry_run, runtime_command_spec_new_typed_panics_when_paired_with_handles_dry_run, cli_runtime_handles_dry_run_lets_handler_preview_instead_of_generic_short_circuit.

Manual verification

Setup:

# In the cli repo, temporarily override the engine dependency:
cd cli/rust
# Edit Cargo.toml → cli-engine = { features = ["pkce-auth"], path = "../../cli-engine" }
cargo build --release && cp target/release/gddy ~/.local/bin/gddy

Test WITHOUT the fix (baseline):

cd cli-engine && git checkout main
cd cli/rust && cargo build --release && cp target/release/gddy ~/.local/bin/gddy
gddy pat add --env ote "test" --token "literally anything" --dry-run
# Expected: {"action": "dry-run: would execute"} regardless of the garbage token

Test WITH the fix:

cd cli-engine && git checkout dry-run-improvements
cd cli/rust && cargo build --release && cp target/release/gddy ~/.local/bin/gddy
gddy pat add --env ote "test" --token "literally anything" --dry-run
# Expected: rejected with a real validation error, nothing stored

Cleanup:

# Revert cli/rust/Cargo.toml back to:
# cli-engine = { features = ["pkce-auth"], version = "<published-version>" }

[Copilot]

Pull request overview

This PR adds an opt-in pathway for mutating commands to run their handler under --dry-run (instead of always being generically short-circuited), enabling real validation + preview output while preserving existing byte-for-byte behavior for commands that do not opt in.

Changes:

• Introduces CommandSpec::handles_dry_run(true) and threads it through CommandMeta to bypass the generic dry-run short-circuit when opted in.
• Adds CommandContext::dry_run() and CommandResult::with_dry_run() so handlers can branch safely and middleware can tag audit/activity + envelopes as dry-run.
• Extends middleware and test coverage to validate handler-driven dry-run behavior and metadata propagation; updates docs to describe the contract and auth implications.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/foundation.rs	Adds tests covering metadata propagation and handler-driven dry-run rendering/audit behavior.
src/middleware.rs	Skips generic dry-run short-circuit when handles_dry_run is set; tags audit/activity/envelope based on handler-provided dry-run marker.
src/command.rs	Adds CommandContext::dry_run(), CommandResult::with_dry_run(), and CommandSpec::handles_dry_run.
docs/design.md	Documents the handler-driven dry-run contract and auth-resolution implications.
docs/concepts.md	Updates conceptual docs for risk tiers/dry-run behavior and the opt-in handler-driven flow.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/command.rs:797

• RuntimeCommandSpec::new only uses debug_assert! to prevent pairing handles_dry_run(true) with a handler shape that cannot observe CommandContext::dry_run(). In release builds this check is compiled out, so a mis-registered command can silently execute real side effects under --dry-run. Consider using a non-debug assert! (or equivalent) so the safety invariant holds in all builds.

        debug_assert!(
            !spec.handles_dry_run,
            "command {:?} sets handles_dry_run but RuntimeCommandSpec::new's handler \
             (CredentialResolver, args) has no CommandContext and can never check \
             CommandContext::dry_run(), so it would silently run its real side effects \

src/command.rs:876

• RuntimeCommandSpec::new_typed only uses debug_assert! to prevent pairing handles_dry_run(true) with a handler shape that cannot observe CommandContext::dry_run(). In release builds this check is compiled out, so a mis-registered command can silently execute real side effects under --dry-run. Consider using a non-debug assert! (or equivalent) so the safety invariant holds in all builds.

        debug_assert!(
            !spec.handles_dry_run,
            "command {:?} sets handles_dry_run but RuntimeCommandSpec::new_typed's handler \
             (CredentialResolver, args) has no CommandContext and can never check \
             CommandContext::dry_run(), so it would silently run its real side effects \

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

src/command.rs:797

• RuntimeCommandSpec::new uses debug_assert! to forbid handles_dry_run with a non-context handler. In release builds this check is compiled out, so a misconfigured command can silently execute real side effects under --dry-run (exactly the footgun the tests/docs call out). Prefer an unconditional assert! (or returning a Result from registration if you ever refactor the API) so the safety guarantee holds in release too.

        debug_assert!(
            !spec.handles_dry_run,
            "command {:?} sets handles_dry_run but RuntimeCommandSpec::new's handler \
             (CredentialResolver, args) has no CommandContext and can never check \
             CommandContext::dry_run(), so it would silently run its real side effects \

src/command.rs:876

• Same issue as RuntimeCommandSpec::new: debug_assert! is removed in release builds, so the crate won’t actually prevent pairing handles_dry_run(true) with new_typed, which can lead to real side effects running under --dry-run. Use an unconditional assert! here as well.

        debug_assert!(
            !spec.handles_dry_run,
            "command {:?} sets handles_dry_run but RuntimeCommandSpec::new_typed's handler \
             (CredentialResolver, args) has no CommandContext and can never check \
             CommandContext::dry_run(), so it would silently run its real side effects \

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/command.rs:797

• RuntimeCommandSpec::new only debug_assert!s against pairing handles_dry_run(true) with a handler that cannot observe --dry-run. In release builds this check is compiled out, so a misconfigured command can silently execute real side effects under --dry-run (exactly the footgun the docs warn about). Prefer a hard assert!/panic here so the invariant is enforced in all builds.

        debug_assert!(
            !spec.handles_dry_run,
            "command {:?} sets handles_dry_run but RuntimeCommandSpec::new's handler \
             (CredentialResolver, args) has no CommandContext and can never check \
             CommandContext::dry_run(), so it would silently run its real side effects \

src/command.rs:876

• Same issue as RuntimeCommandSpec::new: the debug_assert! in RuntimeCommandSpec::new_typed is compiled out in release builds, allowing handles_dry_run(true) to be used with a handler that cannot branch on --dry-run, potentially executing real side effects during a dry-run invocation. Use a hard assert!/panic to enforce this invariant in all builds.

        debug_assert!(
            !spec.handles_dry_run,
            "command {:?} sets handles_dry_run but RuntimeCommandSpec::new_typed's handler \
             (CredentialResolver, args) has no CommandContext and can never check \
             CommandContext::dry_run(), so it would silently run its real side effects \