From d942aa317fd805d1db953677bd89f1368bf3eeb3 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Fri, 31 Jul 2026 18:03:50 -0700 Subject: [PATCH 1/3] feat(command)!: add context-aware typed constructors and ArgGroup support Adds RuntimeCommandSpec::new_typed_with_context and new_typed_streaming so commands needing CommandContext (command path, middleware, --dry-run) or NDJSON streaming can still get eager, compiler-checked typed args instead of falling back to context.typed_args::(). Also adds CommandSpec::arg_groups/ with_arg_group, with CommandSpec::from_args capturing clap_derive's struct-level #[group(...)] attribute, replacing required_unless_present_any/ conflicts_with chains with a single declarative relation. Closes the two gaps found while surveying gddy's 67 commands for a typed-args migration (DEVEX-954): new_typed only covered 7 of them, and the engine had no ArgGroup support at all. Co-Authored-By: Claude Sonnet 5 --- AGENTS.md | 2 + docs/design.md | 38 +++++ src/command.rs | 205 +++++++++++++++++++++++++-- tests/derive_bridge.rs | 312 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 541 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e241449..07a58aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -230,6 +230,8 @@ Command checklist: - Commands, groups, and modules default to `Stage::Ga` (visible everywhere). Add `.with_feature_flag(key, Stage::Experimental)` (or `Stage::Beta`) only when a command needs extra scrutiny before it reaches a public/external consumer CLI; promoting to GA later is a one-line removal or bump, not a rewrite. - Prefer returning structured JSON values from handlers; let cli-engine render JSON, human, and TOON formats. - Prefer `CommandSpec::from_args::()` + `RuntimeCommandSpec::new_typed` when the command has many flags, needs clap validation attributes, or when porting existing derive-based commands. Use the builder path for simple commands with one or two flags. +- Most commands need more than `new_typed`'s `(CredentialResolver, T)` shape — if the handler needs the command path, middleware, or `--dry-run` via `CommandContext`, use `RuntimeCommandSpec::new_typed_with_context` (handler: `Fn(CommandContext, T) -> Fut`) instead of `new_with_context` + `context.typed_args::()`; for streaming commands, use `RuntimeCommandSpec::new_typed_streaming` (handler: `Fn(CommandContext, T, StreamSender) -> Fut`). Both eagerly parse `T` before the handler runs, same guarantee `new_typed` gives the credential-only case. +- Use `CommandSpec::with_arg_group(ArgGroup::new(...).args([...]).required(true))` for "at least one of" or mutually-exclusive relationships between args, instead of a `required_unless_present_any`/`conflicts_with` chain. With `from_args::()`, express the same thing declaratively via a struct-level `#[group(required = true, multiple = true)]` (or `multiple = false` for mutually exclusive) on the derive struct — but not on a struct that also has a `#[command(flatten)]` field; `clap_derive` empties that struct's implicit group's members in that case, so the constraint silently does nothing. ## Output And Schemas diff --git a/docs/design.md b/docs/design.md index 9da2fd3..6715a4f 100644 --- a/docs/design.md +++ b/docs/design.md @@ -218,6 +218,8 @@ on-demand deserialization. The builder and derive paths are equivalent at runtime and can be mixed within a module. +Most real commands need more than `new_typed`'s `(CredentialResolver, T)` shape — they need the command path, middleware snapshot, or `--dry-run` via `CommandContext`. `RuntimeCommandSpec::new_typed_with_context` combines that with eager typed-arg deserialization: the engine parses `T` before calling the handler, so a `Fn(CommandContext, T) -> Fut` handler never needs `context.typed_args::()` itself. `RuntimeCommandSpec::new_typed_streaming` does the same for streaming commands (`Fn(CommandContext, T, StreamSender) -> Fut`). Prefer these two over `new_with_context`/`new_streaming` + `context.typed_args()` whenever a command's args are already a `#[derive(clap::Args)]` struct — they give the same compile-time guarantee `new_typed` gives the credential-only case. + Command metadata should be explicit: - `with_system` sets backend attribution for output and errors. @@ -226,9 +228,45 @@ Command metadata should be explicit: - `with_tier` and `mutates` mark risk and dry-run behavior. - `with_json_schema::()` publishes JSON Schema for output. - `with_arg` adds typed `clap::Arg` values. +- `with_arg_group` adds a `clap::ArgGroup` expressing "at least one of" or mutually-exclusive +relationships between args, without a `required_unless_present_any`/`conflicts_with` chain. - `handles_dry_run` opts a mutating command out of the generic `--dry-run` short-circuit so its handler runs instead (see [Dry-Run](#dry-run) below). +### Argument Relations + +`CommandSpec::with_arg_group(ArgGroup)` registers a `clap::ArgGroup` alongside a command's `Arg`s, for example an "at least one of" constraint: + +```rust +use clap::{Arg, ArgGroup}; + +CommandSpec::new("update", "Update an application") + .with_arg(Arg::new("label").long("label")) + .with_arg(Arg::new("description").long("description")) + .with_arg(Arg::new("status").long("status")) + .with_arg_group( + ArgGroup::new("update-fields") + .args(["label", "description", "status"]) + .required(true) + .multiple(true), + ); +``` + +`CommandSpec::from_args::()` captures the same relationship automatically from a derive struct's struct-level `#[group(...)]` attribute — every `#[derive(clap::Args)]` struct registers one implicit group over its own fields (`multiple(true)`, `required(false)` by default, so a plain struct with no `#[group(...)]` attribute is a no-op); `#[group(required = true, multiple = true)]` customizes it into an "at least one of" constraint, and `multiple = false` into a mutually-exclusive one: + +```rust +#[derive(clap::Args)] +#[group(required = true, multiple = true)] +struct UpdateArgs { + #[arg(long)] label: Option, + #[arg(long)] description: Option, + #[arg(long)] status: Option, +} +``` + +**Flatten caveat.** `clap_derive` empties a struct's own implicit group's member list when the struct also has a `#[command(flatten)]` field, so `#[group(required = true)]` on a struct mixing flattened and literal fields silently enforces nothing. `CommandSpec::from_args` debug-asserts against shipping a required-but-empty group, but — like the `handles_dry_run` debug_asserts above — +treat that as a development-time safety net, not the actual guarantee: release builds won't catch it. + ## Global Flags Framework global flags populate middleware and apply consistently to every command: diff --git a/src/command.rs b/src/command.rs index 9b60139..5292e4c 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1,6 +1,6 @@ use std::{collections::BTreeMap, future::Future, pin::Pin, sync::Arc}; -use clap::{Arg, ArgAction, ArgMatches, Command}; +use clap::{Arg, ArgAction, ArgGroup, ArgMatches, Command}; use schemars::JsonSchema; use serde_json::{Number, Value}; use tokio::sync::mpsc; @@ -323,22 +323,30 @@ pub struct CommandSpec { /// its preview result with [`CommandResult::with_dry_run`]. /// /// **Requires a context-aware handler.** Only handlers built with - /// [`RuntimeCommandSpec::new_with_context`] (or - /// [`new_streaming`](RuntimeCommandSpec::new_streaming)) receive a - /// [`CommandContext`] and can call [`CommandContext::dry_run`]. A handler - /// built with [`RuntimeCommandSpec::new`]/[`new_typed`](RuntimeCommandSpec::new_typed) + /// [`RuntimeCommandSpec::new_with_context`], + /// [`new_streaming`](RuntimeCommandSpec::new_streaming), + /// [`new_typed_with_context`](RuntimeCommandSpec::new_typed_with_context), + /// or [`new_typed_streaming`](RuntimeCommandSpec::new_typed_streaming) + /// receive a [`CommandContext`] and can call [`CommandContext::dry_run`]. + /// A handler built with [`RuntimeCommandSpec::new`]/[`new_typed`](RuntimeCommandSpec::new_typed) /// only receives `(CredentialResolver, args)` — it has no way to observe /// `--dry-run` at all, so opting it into `handles_dry_run` would silently /// execute the handler's real side effects under `--dry-run` instead of /// skipping them. `RuntimeCommandSpec::new`/`new_typed` debug-assert /// against this misuse; release builds do not, so treat the assert as a /// development-time safety net, not the actual guarantee — only pair this - /// field with `new_with_context`. + /// field with one of the four context-aware constructors above. pub handles_dry_run: bool, /// Provider-specific auth metadata. pub auth_metadata: BTreeMap, /// Command-specific `clap` arguments. pub args: Vec, + /// Argument relations (mutually-exclusive or "at least one of" groups). + /// + /// Set with [`with_arg_group`](CommandSpec::with_arg_group), or captured + /// automatically by [`from_args`](CommandSpec::from_args) from a + /// `#[derive(clap::Args)]` struct's `#[group(...)]` attribute. + pub arg_groups: Vec, /// Optional output schema published through `--schema` and help. pub output_schema: Option, /// Inline human-output table columns assigned directly to this command. @@ -384,9 +392,18 @@ impl CommandSpec { /// /// Extracts the argument definitions from the derive type and populates the /// spec's args list. The command name and help text are still required since - /// `Args` types do not carry those. + /// `Args` types do not carry those. Also captures any `ArgGroup`s the derive + /// macro registers (via a struct-level `#[group(...)]` attribute) into + /// [`arg_groups`](CommandSpec::arg_groups). + /// + /// **Flatten caveat**: `clap_derive` empties a struct's own implicit group's + /// member list when the struct also has a `#[command(flatten)]` field, so a + /// `#[group(required = true)]` on such a struct silently enforces nothing. + /// This is debug-asserted against below; treat it as a development-time + /// safety net, not the actual guarantee. #[must_use] pub fn from_args(name: impl Into, short: impl Into) -> Self { + let name = name.into(); let placeholder = Command::new("__placeholder"); let augmented = T::augment_args(placeholder); let args: Vec = augmented @@ -394,10 +411,20 @@ impl CommandSpec { .filter(|a| !matches!(a.get_id().as_str(), "help" | "version")) .cloned() .collect(); + let arg_groups: Vec = augmented.get_groups().cloned().collect(); + debug_assert!( + arg_groups + .iter() + .all(|group| !group.is_required_set() || group.get_args().count() > 0), + "command {name:?} has a required ArgGroup with no member args — likely the \ + clap_derive flatten+group interaction emptying the implicit group's \ + member list; the constraint will not be enforced" + ); Self { - name: name.into(), + name, short: short.into(), args, + arg_groups, ..Self::default() } } @@ -563,6 +590,21 @@ impl CommandSpec { self.with_arg(flag) } + /// Adds an argument relation (an `ArgGroup`) to this command, e.g. to + /// express "at least one of" or mutually-exclusive relationships between + /// arguments added with [`with_arg`](CommandSpec::with_arg)/[`with_flag`](CommandSpec::with_flag). + /// + /// The group's `ArgGroup::args([...])` ids must reference args already (or + /// later) added to this spec, matching `clap`'s own requirement that + /// referenced arg ids exist on the built `Command`. This replaces + /// hand-rolled `required_unless_present_any`/`conflicts_with` chains with a + /// single declarative relation. + #[must_use] + pub fn with_arg_group(mut self, group: ArgGroup) -> Self { + self.arg_groups.push(group); + self + } + /// Registers a compact framework schema from an [`OutputSchema`] type. #[must_use] pub fn with_output_schema(mut self) -> Self { @@ -594,9 +636,12 @@ impl CommandSpec { /// See [`handles_dry_run`](CommandSpec::handles_dry_run) (the field) for /// the contract a handler must follow once it opts in — in particular, /// **only use this with a context-aware handler** - /// ([`RuntimeCommandSpec::new_with_context`]); a `new`/`new_typed` - /// handler can't observe `--dry-run` and would execute its real side - /// effects under it regardless of this flag. + /// ([`RuntimeCommandSpec::new_with_context`], + /// [`new_streaming`](RuntimeCommandSpec::new_streaming), + /// [`new_typed_with_context`](RuntimeCommandSpec::new_typed_with_context), or + /// [`new_typed_streaming`](RuntimeCommandSpec::new_typed_streaming)); a + /// `new`/`new_typed` handler can't observe `--dry-run` and would execute + /// its real side effects under it regardless of this flag. #[must_use] pub fn handles_dry_run(mut self, handles: bool) -> Self { self.handles_dry_run = handles; @@ -658,6 +703,9 @@ impl CommandSpec { for (index, arg) in self.args.iter().enumerate() { command = command.arg(arg.clone().display_order(index)); } + for group in &self.arg_groups { + command = command.group(group.clone()); + } command } } @@ -827,7 +875,8 @@ impl RuntimeCommandSpec { "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 \ - under --dry-run; use RuntimeCommandSpec::new_with_context instead", + under --dry-run; use RuntimeCommandSpec::new_with_context (or \ + new_typed_with_context to keep typed args) instead", spec.name ); Self { @@ -887,8 +936,9 @@ impl RuntimeCommandSpec { /// type safety from argument definition through handler consumption. /// /// If the handler also needs the command path, middleware, or user-supplied - /// args, use [`RuntimeCommandSpec::new_with_context`] with - /// [`CommandContext::typed_args`] instead. + /// args, use [`RuntimeCommandSpec::new_typed_with_context`] (or + /// [`RuntimeCommandSpec::new_with_context`] with + /// [`CommandContext::typed_args`]) instead. /// /// This handler shape has no [`CommandContext`], so it can never call /// [`CommandContext::dry_run`] — do not pair this with @@ -906,7 +956,8 @@ impl RuntimeCommandSpec { "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 \ - under --dry-run; use RuntimeCommandSpec::new_with_context instead", + under --dry-run; use RuntimeCommandSpec::new_with_context (or \ + new_typed_with_context to keep typed args) instead", spec.name ); let handler = Arc::new(handler); @@ -926,6 +977,91 @@ impl RuntimeCommandSpec { streaming_handler: None, } } + + /// Creates a runtime command with full context and typed argument + /// deserialization. + /// + /// Combines [`new_with_context`](RuntimeCommandSpec::new_with_context)'s + /// access to [`CommandContext`] (command path, middleware snapshot, + /// user-supplied args, [`CommandContext::dry_run`]) with + /// [`new_typed`](RuntimeCommandSpec::new_typed)'s automatic + /// deserialization: the engine parses `T` from the raw matches before + /// invoking the handler, so the handler never needs to call + /// [`CommandContext::typed_args`] itself. + /// + /// Use this instead of `new_with_context` + `context.typed_args::()` + /// when a command needs full context and wants eager, guaranteed-parsed + /// typed args rather than parsing on demand. Because the handler receives + /// a [`CommandContext`], this is a valid pairing with + /// [`CommandSpec::handles_dry_run`]. + /// + /// # Errors + /// + /// The returned handler surfaces a `CliCoreError::Message` if `T` fails to + /// deserialize from the parsed matches (this should not happen for args + /// generated by `CommandSpec::from_args::()`, since `clap` already + /// validated them during parsing). + #[must_use] + pub fn new_typed_with_context(spec: CommandSpec, handler: F) -> Self + where + T: clap::FromArgMatches + Send + 'static, + F: Fn(CommandContext, T) -> Fut + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Output: Into + Send + 'static, + { + let handler = Arc::new(handler); + Self { + spec, + handler: Arc::new(move |context| { + let parsed = T::from_arg_matches(context.raw_matches.as_ref()); + let handler = handler.clone(); + Box::pin(async move { + let args = parsed.map_err(|e| { + crate::CliCoreError::Message(format!("argument parse error: {e}")) + })?; + handler(context, args).await.map(Into::into) + }) + }), + streaming_handler: None, + } + } + + /// Creates a streaming command with full context and typed argument + /// deserialization. + /// + /// Combines [`new_streaming`](RuntimeCommandSpec::new_streaming)'s NDJSON + /// event emission with [`new_typed`](RuntimeCommandSpec::new_typed)'s + /// automatic deserialization: the engine parses `T` from the raw matches + /// before invoking the handler. + /// + /// # Errors + /// + /// The returned handler surfaces a `CliCoreError::Message` if `T` fails to + /// deserialize from the parsed matches. + #[must_use] + pub fn new_typed_streaming(spec: CommandSpec, handler: F) -> Self + where + T: clap::FromArgMatches + Send + 'static, + F: Fn(CommandContext, T, StreamSender) -> Fut + Send + Sync + 'static, + Fut: Future> + Send + 'static, + { + let handler = Arc::new(handler); + let streaming: StreamingCommandHandler = Arc::new(move |context, sender| { + let parsed = T::from_arg_matches(context.raw_matches.as_ref()); + let handler = handler.clone(); + Box::pin(async move { + let args = parsed.map_err(|e| { + crate::CliCoreError::Message(format!("argument parse error: {e}")) + })?; + handler(context, args, sender).await + }) + }); + Self { + spec, + streaming_handler: Some(streaming), + handler: Arc::new(|_context| Box::pin(async { Ok(CommandResult::new(Value::Null)) })), + } + } } /// Executable command group with runtime children. @@ -1202,4 +1338,43 @@ mod tests { assert!(group.feature_flag.is_none()); } + + #[test] + fn command_spec_with_arg_group_registers_group_on_clap_command() { + let spec = CommandSpec::new("update", "Update a thing") + .with_arg(Arg::new("a").long("a")) + .with_arg(Arg::new("b").long("b")) + .with_arg_group(ArgGroup::new("ab").args(["a", "b"]).required(true)); + + assert!( + spec.clap_command() + .try_get_matches_from(["update"]) + .is_err(), + "neither `a` nor `b` present should fail the required group" + ); + assert!( + spec.clap_command() + .try_get_matches_from(["update", "--a", "x"]) + .is_ok() + ); + } + + #[test] + fn command_spec_from_args_preserves_derive_arg_group() { + #[derive(clap::Args)] + #[group(required = true, multiple = false)] + struct ExclusiveArgs { + #[arg(long)] + one: bool, + #[arg(long)] + two: bool, + } + + let spec = CommandSpec::from_args::("bump", "Bump one thing"); + + assert_eq!(spec.arg_groups.len(), 1); + let group = &spec.arg_groups[0]; + assert!(group.is_required_set()); + assert_eq!(group.get_args().count(), 2); + } } diff --git a/tests/derive_bridge.rs b/tests/derive_bridge.rs index 856df3c..f5078c4 100644 --- a/tests/derive_bridge.rs +++ b/tests/derive_bridge.rs @@ -1,6 +1,6 @@ use cli_engine::{ BuildInfo, Cli, CliConfig, CommandContext, CommandResult, CommandSpec, CredentialResolver, - GroupSpec, Module, RuntimeCommandSpec, RuntimeGroupSpec, + GroupSpec, Module, RuntimeCommandSpec, RuntimeGroupSpec, StreamSender, }; use serde_json::{Value, json}; @@ -334,3 +334,313 @@ async fn human_shorthand_flag_produces_human_output() { result.rendered ); } + +// --- new_typed_with_context --- + +#[derive(Debug, Clone, clap::Args)] +struct WhoamiArgs { + #[arg(long)] + tag: String, +} + +fn typed_with_context_cli() -> Cli { + let whoami_command = RuntimeCommandSpec::new_typed_with_context::( + CommandSpec::from_args::("whoami", "Show tag and command path").no_auth(true), + async |context: CommandContext, args: WhoamiArgs| { + Ok(CommandResult::new(json!({ + "tag": args.tag, + "command_path": context.command_path, + }))) + }, + ); + + let module = Module::new("Ident", move |_context| { + RuntimeGroupSpec::new(GroupSpec::new("ident", "Identity commands")) + .with_command(whoami_command.clone()) + }); + + Cli::new( + CliConfig::new("typed-ctx-test", "Typed Context Test CLI", "typed-ctx-test") + .with_build(BuildInfo::new("0.1.0")) + .with_module(module), + ) +} + +#[tokio::test] +async fn new_typed_with_context_exposes_parsed_args_and_command_path() { + let cli = typed_with_context_cli(); + + let result = cli + .run([ + "typed-ctx-test", + "ident", + "whoami", + "--tag", + "hello", + "--output", + "json", + ]) + .await; + assert_eq!(result.exit_code, 0, "output: {}", result.rendered); + let json: Value = serde_json::from_str(&result.rendered).expect("valid json"); + assert_eq!(json["data"]["tag"], "hello"); + assert_eq!(json["data"]["command_path"], "ident:whoami"); +} + +#[tokio::test] +async fn new_typed_with_context_reports_missing_required_arg() { + let cli = typed_with_context_cli(); + + let result = cli.run(["typed-ctx-test", "ident", "whoami"]).await; + assert_ne!(result.exit_code, 0); + assert!(result.rendered.contains("required"), "{}", result.rendered); +} + +// --- new_typed_streaming --- + +#[derive(Debug, Clone, clap::Args)] +struct CountArgs { + #[arg(long)] + label: String, + + #[arg(long, default_value = "2")] + count: u32, +} + +fn typed_streaming_cli() -> Cli { + let count_command = RuntimeCommandSpec::new_typed_streaming::( + CommandSpec::from_args::("count", "Stream counted events").no_auth(true), + async |_context: CommandContext, args: CountArgs, sender: StreamSender| { + // Successful NDJSON events are written straight to real stdout by + // the writer task (see tests/streaming.rs), so this test can't + // capture them via `result.rendered`. Prove the typed args were + // parsed correctly by failing loudly — into `rendered` — if they + // don't match what was passed on the command line. + if args.label != "tick" || args.count != 3 { + return Err(cli_engine::CliCoreError::message(format!( + "unexpected parsed args: label={:?} count={}", + args.label, args.count + ))); + } + for index in 0..args.count { + sender + .send(json!({ "label": args.label, "index": index })) + .await; + } + Ok(()) + }, + ); + + let module = Module::new("Stream", move |_context| { + RuntimeGroupSpec::new(GroupSpec::new("stream", "Streaming commands")) + .with_command(count_command.clone()) + }); + + Cli::new( + CliConfig::new( + "typed-stream-test", + "Typed Streaming Test CLI", + "typed-stream-test", + ) + .with_build(BuildInfo::new("0.1.0")) + .with_module(module), + ) +} + +#[tokio::test] +async fn new_typed_streaming_parses_args_before_invoking_handler() { + let cli = typed_streaming_cli(); + + let result = cli + .run([ + "typed-stream-test", + "stream", + "count", + "--label", + "tick", + "--count", + "3", + ]) + .await; + assert_eq!(result.exit_code, 0, "output: {}", result.rendered); + assert!( + result.rendered.is_empty(), + "successful stream should produce no rendered envelope, got: {:?}", + result.rendered + ); +} + +#[tokio::test] +async fn new_typed_streaming_reports_missing_required_arg() { + let cli = typed_streaming_cli(); + + let result = cli + .run(["typed-stream-test", "stream", "count", "--count", "3"]) + .await; + assert_ne!(result.exit_code, 0); + assert!(result.rendered.contains("required"), "{}", result.rendered); +} + +// --- ArgGroup passthrough via from_args --- + +#[derive(Debug, Clone, clap::Args)] +#[group(required = true, multiple = true)] +struct UpdateArgs { + #[arg(long)] + label: Option, + + #[arg(long)] + description: Option, + + #[arg(long)] + status: Option, +} + +fn update_cli() -> Cli { + let update_command = RuntimeCommandSpec::new_typed::( + CommandSpec::from_args::("update", "Update at least one field").no_auth(true), + async |_credential: CredentialResolver, args: UpdateArgs| { + Ok(CommandResult::new(json!({ + "label": args.label, + "description": args.description, + "status": args.status, + }))) + }, + ); + + let module = Module::new("Update", move |_context| { + RuntimeGroupSpec::new(GroupSpec::new("thing", "Thing commands")) + .with_command(update_command.clone()) + }); + + Cli::new( + CliConfig::new("group-test", "Group Test CLI", "group-test") + .with_build(BuildInfo::new("0.1.0")) + .with_module(module), + ) +} + +#[tokio::test] +async fn arg_group_from_args_rejects_when_none_present() { + let cli = update_cli(); + + let result = cli.run(["group-test", "thing", "update"]).await; + assert_ne!(result.exit_code, 0); +} + +#[tokio::test] +async fn arg_group_from_args_accepts_exactly_one() { + let cli = update_cli(); + + let result = cli + .run([ + "group-test", + "thing", + "update", + "--label", + "new-label", + "--output", + "json", + ]) + .await; + assert_eq!(result.exit_code, 0, "output: {}", result.rendered); + let json: Value = serde_json::from_str(&result.rendered).expect("valid json"); + assert_eq!(json["data"]["label"], "new-label"); + assert!(json["data"]["description"].is_null()); +} + +#[tokio::test] +async fn arg_group_from_args_accepts_multiple_when_group_allows_multiple() { + let cli = update_cli(); + + let result = cli + .run([ + "group-test", + "thing", + "update", + "--label", + "new-label", + "--status", + "ACTIVE", + "--output", + "json", + ]) + .await; + assert_eq!(result.exit_code, 0, "output: {}", result.rendered); + let json: Value = serde_json::from_str(&result.rendered).expect("valid json"); + assert_eq!(json["data"]["label"], "new-label"); + assert_eq!(json["data"]["status"], "ACTIVE"); +} + +#[derive(Debug, Clone, clap::Args)] +#[group(required = true, multiple = false)] +struct VersionArgs { + #[arg(long)] + major: bool, + + #[arg(long)] + minor: bool, +} + +fn version_cli() -> Cli { + let bump_command = RuntimeCommandSpec::new_typed::( + CommandSpec::from_args::("bump", "Bump exactly one version component") + .no_auth(true), + async |_credential: CredentialResolver, args: VersionArgs| { + Ok(CommandResult::new( + json!({ "major": args.major, "minor": args.minor }), + )) + }, + ); + + let module = Module::new("Version", move |_context| { + RuntimeGroupSpec::new(GroupSpec::new("version", "Version commands")) + .with_command(bump_command.clone()) + }); + + Cli::new( + CliConfig::new( + "exclusive-group-test", + "Exclusive Group Test CLI", + "exclusive-group-test", + ) + .with_build(BuildInfo::new("0.1.0")) + .with_module(module), + ) +} + +#[tokio::test] +async fn arg_group_from_args_rejects_mutually_exclusive_flags_together() { + let cli = version_cli(); + + let result = cli + .run([ + "exclusive-group-test", + "version", + "bump", + "--major", + "--minor", + ]) + .await; + assert_ne!(result.exit_code, 0); +} + +#[tokio::test] +async fn arg_group_from_args_accepts_one_of_mutually_exclusive_flags() { + let cli = version_cli(); + + let result = cli + .run([ + "exclusive-group-test", + "version", + "bump", + "--major", + "--output", + "json", + ]) + .await; + assert_eq!(result.exit_code, 0, "output: {}", result.rendered); + let json: Value = serde_json::from_str(&result.rendered).expect("valid json"); + assert_eq!(json["data"]["major"], true); + assert_eq!(json["data"]["minor"], false); +} From c1f436c9417e8c83ecfc6d1a101a58a81c596a1c Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Fri, 31 Jul 2026 18:25:03 -0700 Subject: [PATCH 2/3] feat(command)!: mark CommandSpec and GroupSpec non_exhaustive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both structs are builder-only in practice — surveyed every consumer (gdx: 140+ command/group sites, gddy: 67+) and found zero struct-literal construction or destructuring of either type. Adding #[non_exhaustive] now, bundled into this release's existing breaking bump, means future field additions (like arg_groups in this same PR) won't need one of their own. Co-Authored-By: Claude Sonnet 5 --- src/command.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/command.rs b/src/command.rs index 5292e4c..62e313c 100644 --- a/src/command.rs +++ b/src/command.rs @@ -283,7 +283,13 @@ impl CommandContext { /// /// `CommandSpec` intentionally keeps command metadata next to the command's /// handler. This is the primary copy/paste surface for teams adding commands. +/// +/// Construct with [`CommandSpec::new`] or [`CommandSpec::from_args`], then +/// configure with the `with_*` builder methods — never as a struct literal. +/// `#[non_exhaustive]` enforces this so the engine can add fields (as it did +/// for [`arg_groups`](CommandSpec::arg_groups)) without a breaking release. #[derive(Clone, Debug, Default)] +#[non_exhaustive] pub struct CommandSpec { /// Leaf command name. pub name: String, @@ -714,7 +720,12 @@ impl CommandSpec { /// /// Groups are noun-based containers. They do not run business logic directly; /// when invoked bare, the CLI renders group help. +/// +/// Construct with [`GroupSpec::new`], then configure with the `with_*` builder +/// methods — never as a struct literal. `#[non_exhaustive]` enforces this so +/// the engine can add fields later without a breaking release. #[derive(Clone, Debug, Default)] +#[non_exhaustive] pub struct GroupSpec { /// Group command name. pub name: String, From 37dace4ebd10d5fe8b051f0470f496c505a0cfb7 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Fri, 31 Jul 2026 19:08:44 -0700 Subject: [PATCH 3/3] feat(command)!: mark CommandResult, RuntimeCommandSpec, RuntimeGroupSpec, Module, and NextAction non_exhaustive Same rationale as the earlier CommandSpec/GroupSpec change in this PR: audited every public struct with pub fields across gdx, gddy, and this crate's own tests/examples (which compile as separate crates and are subject to the same restriction as any external consumer) for literal construction or exhaustive destructuring. These five had zero hits anywhere, so marking them non_exhaustive now closes off future field additions needing a breaking release of their own, at zero real cost. Excluded from this batch: CliConfig, BuildInfo, GuideEntry, and ActivityEvent, which do have real literal-construction sites in tests/foundation.rs (CliConfig's own doc comment explicitly documents this as intentional). Also excluded: Credential, OutputField, and NextActionParam, which have real literal construction in gdx/gddy with no non-literal alternative available yet. Co-Authored-By: Claude Sonnet 5 --- src/command.rs | 16 ++++++++++++++++ src/module.rs | 5 +++++ src/output/envelope.rs | 5 +++++ 3 files changed, 26 insertions(+) diff --git a/src/command.rs b/src/command.rs index 62e313c..fafdd55 100644 --- a/src/command.rs +++ b/src/command.rs @@ -42,7 +42,12 @@ pub type StreamingCommandHandler = /// Command handlers should return renderable data and keep output metadata on /// [`CommandSpec`]. The metadata field is reserved for future command-result /// extensions that are not known when the command is registered. +/// +/// Construct with [`CommandResult::new`], then chain `with_*` methods — +/// never as a struct literal. `#[non_exhaustive]` enforces this so the engine +/// can add fields without a breaking release. #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct CommandResult { /// JSON data rendered by the configured output formatter. pub data: Value, @@ -842,7 +847,13 @@ impl GroupSpec { /// /// Use [`RuntimeCommandSpec::new_streaming`] for commands that emit incremental /// NDJSON progress events (e.g. long-running deployments with `--follow`). +/// +/// Construct with one of the `new*` constructors — never as a struct literal. +/// Literal construction would bypass the `handles_dry_run`/handler-shape +/// misuse checks those constructors debug-assert. `#[non_exhaustive]` also +/// means the engine can add fields without a breaking release. #[derive(Clone)] +#[non_exhaustive] pub struct RuntimeCommandSpec { /// Declarative command metadata. pub spec: CommandSpec, @@ -1076,7 +1087,12 @@ impl RuntimeCommandSpec { } /// Executable command group with runtime children. +/// +/// Construct with [`RuntimeGroupSpec::new`], then chain `with_*` methods — +/// never as a struct literal. `#[non_exhaustive]` enforces this so the engine +/// can add fields without a breaking release. #[derive(Clone, Debug, Default)] +#[non_exhaustive] pub struct RuntimeGroupSpec { /// Declarative group metadata. pub group: GroupSpec, diff --git a/src/module.rs b/src/module.rs index 9afe272..2d971ea 100644 --- a/src/module.rs +++ b/src/module.rs @@ -37,7 +37,12 @@ pub trait CommandModule: Send + Sync + std::fmt::Debug + 'static { /// A module usually maps to a product, platform, resource family, or team /// ownership boundary. It contributes one top-level group plus optional guides /// and human output views. +/// +/// Construct with [`Module::new`], then chain `with_*` methods — never as a +/// struct literal. `#[non_exhaustive]` enforces this so the engine can add +/// fields without a breaking release. #[derive(Clone)] +#[non_exhaustive] pub struct Module { /// Root help category. pub category: String, diff --git a/src/output/envelope.rs b/src/output/envelope.rs index 66334ee..5c3bd68 100644 --- a/src/output/envelope.rs +++ b/src/output/envelope.rs @@ -32,7 +32,12 @@ pub struct Envelope { } /// A suggested follow-up command the caller can run next. +/// +/// Construct with [`NextAction::new`], then chain `with_*` methods — never as +/// a struct literal. `#[non_exhaustive]` enforces this so the engine can add +/// fields without a breaking release. #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[non_exhaustive] pub struct NextAction { /// Executable command template, e.g. `"application info --name "`. /// A param's placeholder is its key wrapped in angle brackets (``);