From bae3be3988f1da707e3f9111e84a4f59c47ae6db Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 4 Jun 2026 13:20:53 -0700 Subject: [PATCH 1/5] feat: agent-first root discovery, curated help, and TTY-aware output Improves the bare-invocation and help experience for human users without sacrificing machine-readable output for agents. - Root discovery: new `CliConfig::with_root_next_actions` hook. On bare invocation, human output appends a "Suggested next actions" section; machine output emits a discovery envelope ({description, version} + next_actions). With no hook configured, behavior is unchanged long-help. - Curated root/group help: a root help template suppresses clap's duplicate command list and the global-options wall (still shown on leaf commands); group pages keep their subcommand list but drop the options wall. Categories, and the commands within them, are sorted. The engine-injected `auth` command is filed under an admin category (`CliConfig::with_admin_category`, default "Admin") so it stays discoverable; any uncategorized top-level command falls under a generic "Commands" section. - TTY-aware default output format: human on an interactive terminal, JSON otherwise (pipes, files, CI, most agents). Precedence: explicit --output/--json/--toon/--human > ${APP_ID}_OUTPUT env > TTY policy. Uses std::io::IsTerminal (no new dependency). Co-Authored-By: Claude Opus 4.8 --- src/cli.rs | 218 ++++++++++++++++++++++++++++--- src/cli/help.rs | 73 ++++++++--- src/flags.rs | 95 ++++++++++++-- src/lib.rs | 9 +- tests/consumer_cli.rs | 115 +++++++++++++++- tests/exhaustive_cli_contract.rs | 8 +- tests/exhaustive_public_api.rs | 2 +- tests/foundation.rs | 68 ++++++++-- 8 files changed, 528 insertions(+), 60 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 8154379..ee47b4f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -24,21 +24,22 @@ use crate::{ }, error::exit_code_for_error, flags::{ - GlobalFlags, derive_bool_flags, derive_value_flags, extract_command_path, - extract_output_format, extract_search_query, global_flags_from_matches, - has_true_schema_flag, register_global_flags, + GlobalFlags, default_output_format, derive_bool_flags, derive_value_flags, + extract_command_path, extract_output_format, extract_search_query, + global_flags_from_matches, has_true_schema_flag, register_global_flags, }, guide::guide_content, module::{Module, ModuleContext}, output::{ - HumanViewDef, SchemaRegistry, format_help_section, global_human_view_registry_snapshot, - global_schema_registry_snapshot, + HumanViewDef, NextAction, SchemaRegistry, format_help_section, + global_human_view_registry_snapshot, global_schema_registry_snapshot, }, search::{SearchDocument, SearchIndex}, }; use builtins::{guide_args, guide_command, help_args, help_command}; -pub use help::{ModuleHelpEntry, build_root_long}; +use help::{GROUP_HELP_TEMPLATE, ROOT_HELP_TEMPLATE}; +pub use help::{ModuleHelpEntry, build_root_long, render_next_actions_human}; /// Build metadata shown by the root `--version` flag. #[derive(Clone, Debug, Default, Eq, PartialEq)] @@ -105,6 +106,15 @@ pub type ResolveMeta = Arc CommandMeta + Send + Syn pub type OnShutdown = Arc; /// Hook that contributes extra root-scope `--search` documents. pub type ExtraSearchDocs = Arc Vec + Send + Sync>; +/// Hook that supplies the suggested next actions shown when the CLI is invoked +/// with no subcommand (bare root). The same actions drive a human "Next actions" +/// section and the JSON discovery envelope. +pub type RootNextActions = Arc Vec + Send + Sync>; + +/// Default name for the admin help category, under which the engine files the +/// built-in `auth` command when a consumer does not override it via +/// [`CliConfig::with_admin_category`]. +const DEFAULT_ADMIN_CATEGORY: &str = "Admin"; /// Declarative configuration for a CLI application. /// @@ -155,6 +165,13 @@ pub struct CliConfig { pub on_shutdown: Option, /// Optional root-scope search document provider. pub extra_search_docs: Option, + /// Optional provider for the bare-root suggested next actions. + pub root_next_actions: Option, + /// Name of the admin help category. The engine files its built-in `auth` + /// command under this heading; apps should use the same name for their own + /// admin modules (e.g. godaddy's `env`). When unset, defaults to `"Admin"`; + /// set it to match a consumer's own taxonomy (e.g. gdx's "Administration"). + pub admin_category: Option, } impl CliConfig { @@ -312,6 +329,22 @@ impl CliConfig { self.extra_search_docs = Some(extra_search_docs); self } + + /// Sets the provider for the bare-root suggested next actions. + #[must_use] + pub fn with_root_next_actions(mut self, root_next_actions: RootNextActions) -> Self { + self.root_next_actions = Some(root_next_actions); + self + } + + /// Sets the name of the admin help category. The engine files the built-in + /// `auth` command there; apps should use the same name for their own admin + /// modules (e.g. godaddy's `env`). Optional: defaults to `"Admin"`. + #[must_use] + pub fn with_admin_category(mut self, category: impl Into) -> Self { + self.admin_category = Some(category.into()); + self + } } impl std::fmt::Debug for CliConfig { @@ -339,6 +372,8 @@ impl std::fmt::Debug for CliConfig { .field("has_meta_resolver", &self.meta_resolver.is_some()) .field("has_on_shutdown", &self.on_shutdown.is_some()) .field("has_extra_search_docs", &self.extra_search_docs.is_some()) + .field("has_root_next_actions", &self.root_next_actions.is_some()) + .field("admin_category", &self.admin_category) .finish() } } @@ -380,6 +415,7 @@ pub struct Cli { meta_resolver: Option, on_shutdown: Option, extra_search_docs: Option, + root_next_actions: Option, init_state: Arc>>>, } @@ -437,6 +473,7 @@ impl std::fmt::Debug for Cli { .field("has_meta_resolver", &self.meta_resolver.is_some()) .field("has_on_shutdown", &self.on_shutdown.is_some()) .field("has_extra_search_docs", &self.extra_search_docs.is_some()) + .field("has_root_next_actions", &self.root_next_actions.is_some()) .finish() } } @@ -456,6 +493,7 @@ impl Cli { let meta_resolver = config.meta_resolver.clone(); let on_shutdown = config.on_shutdown.clone(); let extra_search_docs = config.extra_search_docs.clone(); + let root_next_actions = config.root_next_actions.clone(); let mut root = Command::new(config.name.clone()) .about(config.short.clone()) .disable_help_subcommand(true) @@ -477,7 +515,9 @@ impl Cli { .as_deref() .filter(|long| !long.is_empty()) .unwrap_or(config.short.as_str()); - root = root.long_about(build_root_long(intro, &[], false)); + root = root + .long_about(build_root_long(intro, &[], false)) + .help_template(ROOT_HELP_TEMPLATE); let mut middleware = Middleware::new(); middleware.app_id = config.app_id.clone(); @@ -505,6 +545,7 @@ impl Cli { meta_resolver, on_shutdown, extra_search_docs, + root_next_actions, init_state: Arc::new(Mutex::new(None)), }; for provider in auth_providers { @@ -528,9 +569,42 @@ impl Cli { for command in commands { cli.add_command(command); } + // Registered last so `auth` appends to its category after module-defined + // entries, preserving the consumer's category ordering. + cli.register_auth_help_entry(); cli } + /// Lists the auto-registered `auth` command under the admin help category so + /// it is never uncategorized once clap's auto subcommand list is suppressed. + /// Defaults to [`DEFAULT_ADMIN_CATEGORY`]; `admin_category` overrides it to + /// align with a consumer's own taxonomy. + fn register_auth_help_entry(&mut self) { + let category = self + .config + .admin_category + .clone() + .unwrap_or_else(|| DEFAULT_ADMIN_CATEGORY.to_owned()); + let already_listed = self.module_entries.iter().any(|entry| entry.name == "auth"); + let short = self + .root + .find_subcommand("auth") + .filter(|auth| !auth.is_hide_set()) + .map(|auth| { + auth.get_about() + .map(ToString::to_string) + .unwrap_or_default() + }); + if !already_listed && let Some(short) = short { + self.module_entries.push(ModuleHelpEntry { + category, + name: "auth".to_owned(), + short, + }); + } + self.refresh_root_long(); + } + /// Returns the shared middleware template. #[must_use] pub fn middleware(&self) -> &Middleware { @@ -759,7 +833,8 @@ impl Cli { } }; - let flags = global_flags_from_matches(&matches); + let default_format = default_output_format(&self.config.app_id); + let flags = global_flags_from_matches(&matches, &default_format); let command_timeout = match parse_command_timeout(&flags.timeout) { Ok(timeout) => timeout, Err(err) => { @@ -827,6 +902,23 @@ impl Cli { rendered: group.clone().render_long_help().to_string(), }); } + if command_path.is_empty() + && let Some(root_next_actions) = &self.root_next_actions + { + if let Err(err) = self.run_pre_run( + &mut middleware, + &command_path, + &crate::middleware::ValueMap::new(), + ) { + return self.finish_run(render_cli_error( + &middleware, + &err, + &self.config.app_id, + )); + } + let actions = root_next_actions(); + return self.finish_run(self.render_root(&middleware, actions)); + } return self.finish_run(CliRunOutput { exit_code: if command_path.is_empty() { 0 } else { 1 }, rendered: if command_path.is_empty() { @@ -930,7 +1022,8 @@ impl Cli { return None; } let scope = self.search_scope(args); - let output_format = extract_output_format(args); + let output_format = + extract_output_format(args, &default_output_format(&self.config.app_id)); Some(self.render_search(&query, &scope, &output_format)) } @@ -943,7 +1036,8 @@ impl Cli { let command_path = self.canonical_command_path(&extract_command_path(args, &bool_flags, &value_flags)); let schema = self.middleware.schema_registry.get_by_path(&command_path)?; - let output_format = extract_output_format(args); + let output_format = + extract_output_format(args, &default_output_format(&self.config.app_id)); Some(self.render_schema(schema, &output_format)) } @@ -1001,6 +1095,67 @@ impl Cli { } } + /// Renders the bare-root response. For human output, renders long help plus + /// a "Next actions" section so a human invoking the CLI with no arguments + /// gets readable guidance; for machine-readable output, emits a discovery + /// envelope (light metadata + next actions). The output format has already + /// resolved the TTY/env/flag policy, so this just branches on it. + fn render_root(&self, middleware: &Middleware, actions: Vec) -> CliRunOutput { + let format: crate::output::OutputFormat = match middleware.output_format.parse() { + Ok(format) => format, + Err(err) => { + return CliRunOutput { + exit_code: exit_code_for_error(&err), + rendered: err.to_string(), + }; + } + }; + if format == crate::output::OutputFormat::Human { + // Fold the suggested actions into the root long-about so they render + // alongside the other curated sections (before Usage) instead of + // dangling beneath clap's options dump. + let base_long = self + .root + .get_long_about() + .map(ToString::to_string) + .unwrap_or_default(); + let long = format!("{base_long}{}", render_next_actions_human(&actions)); + let rendered = self + .root + .clone() + .long_about(long) + .render_long_help() + .to_string(); + return CliRunOutput { + exit_code: 0, + rendered, + }; + } + let description = self + .config + .long + .as_deref() + .filter(|long| !long.is_empty()) + .unwrap_or(self.config.short.as_str()); + let data = serde_json::json!({ + "description": description, + "version": self.config.build.version, + }); + let envelope = crate::Envelope::success(data, self.config.app_id.clone()) + .with_next_actions(actions) + .prepare_for_render(&middleware.verbose); + match crate::output::render(format, &envelope) { + Ok(rendered) => CliRunOutput { + exit_code: 0, + rendered, + }, + Err(err) => CliRunOutput { + exit_code: exit_code_for_error(&err), + rendered: err.to_string(), + }, + } + } + fn search_documents(&self, scope: &str) -> Vec { let (scoped, mut prefix) = find_command_and_canonical_path_by_colon_path(&self.root, scope) .unwrap_or((&self.root, Vec::new())); @@ -1081,17 +1236,46 @@ impl Cli { } fn refresh_root_long(&mut self) { + // Module-categorized entries, plus any visible top-level command that is + // neither categorized nor an engine built-in, listed under a generic + // "Commands" section. This keeps every command discoverable once clap's + // auto subcommand list is suppressed by the root help template. + const BUILTINS: [&str; 4] = ["help", "guide", "tree", "completion"]; + let categorized: BTreeSet<&str> = self + .module_entries + .iter() + .map(|entry| entry.name.as_str()) + .collect(); + let mut generic: Vec = self + .root + .get_subcommands() + .filter(|command| !command.is_hide_set()) + .filter(|command| !BUILTINS.contains(&command.get_name())) + .filter(|command| !categorized.contains(command.get_name())) + .map(|command| ModuleHelpEntry { + category: "Commands".to_owned(), + name: command.get_name().to_owned(), + short: command + .get_about() + .map(ToString::to_string) + .unwrap_or_default(), + }) + .collect(); + generic.sort_by(|left, right| left.name.cmp(&right.name)); + + let mut entries = self.module_entries.clone(); + entries.extend(generic); + let has_guide = !self.guide_entries.is_empty() || has_subcommand(&self.root, "guide"); let intro = self .config .long .as_deref() .filter(|long| !long.is_empty()) .unwrap_or(self.config.short.as_str()); - self.root = self.root.clone().long_about(build_root_long( - intro, - &self.module_entries, - !self.guide_entries.is_empty() || has_subcommand(&self.root, "guide"), - )); + self.root = self + .root + .clone() + .long_about(build_root_long(intro, &entries, has_guide)); } fn ensure_auth_command(&mut self) { @@ -1716,7 +1900,9 @@ fn runtime_group_clap_command_with_schema_help( } fn group_clap_command_without_children(group: &GroupSpec) -> Command { - let mut command = Command::new(group.name.clone()).about(group.short.clone()); + let mut command = Command::new(group.name.clone()) + .about(group.short.clone()) + .help_template(GROUP_HELP_TEMPLATE); if let Some(long) = &group.long && !long.is_empty() { diff --git a/src/cli/help.rs b/src/cli/help.rs index 076cfc1..b71f508 100644 --- a/src/cli/help.rs +++ b/src/cli/help.rs @@ -1,5 +1,25 @@ use std::collections::BTreeMap; +use crate::output::NextAction; + +/// Help template for the root command. Renders the curated long-about (which +/// already lists every command grouped by category) and usage, but omits +/// clap's auto-generated subcommand list and the global options wall — those +/// are noise on the top-level navigation page. +pub const ROOT_HELP_TEMPLATE: &str = "\ +{before-help}{about-with-newline} +{usage-heading} {usage}{after-help}"; + +/// Help template for group (noun) commands. Keeps the child command list, which +/// is the point of a group page, but drops the global options wall. Leaf +/// commands keep clap's default template so their flags remain documented. +pub const GROUP_HELP_TEMPLATE: &str = "\ +{before-help}{about-with-newline} +{usage-heading} {usage} + +Commands: +{subcommands}{after-help}"; + /// One module row in root long help. #[derive(Clone, Debug, Eq, PartialEq)] pub struct ModuleHelpEntry { @@ -14,14 +34,12 @@ pub struct ModuleHelpEntry { /// Builds the root long help text from module categories and built-in command hints. #[must_use] pub fn build_root_long(intro: &str, entries: &[ModuleHelpEntry], has_guide: bool) -> String { - let mut categories = Vec::::new(); - let mut by_category = BTreeMap::>::new(); + // Group by category. `BTreeMap` keeps categories in sorted order so the + // rendered sections are deterministic regardless of registration order. + let mut by_category = BTreeMap::<&str, Vec<&ModuleHelpEntry>>::new(); for entry in entries { - if !by_category.contains_key(&entry.category) { - categories.push(entry.category.clone()); - } by_category - .entry(entry.category.clone()) + .entry(entry.category.as_str()) .or_default() .push(entry); } @@ -32,17 +50,16 @@ pub fn build_root_long(intro: &str, entries: &[ModuleHelpEntry], has_guide: bool .max() .unwrap_or_default(); let mut out = intro.to_owned(); - for category in categories { + for (category, category_entries) in &mut by_category { + category_entries.sort_by(|left, right| left.name.cmp(&right.name)); out.push_str(&format!("\n\n {category}:")); - if let Some(category_entries) = by_category.get(&category) { - for entry in category_entries { - out.push_str(&format!( - "\n {: String { + if actions.is_empty() { + return String::new(); + } + let max_width = actions + .iter() + .map(|action| action.command.len()) + .max() + .unwrap_or_default(); + let mut out = String::from("\n\n Suggested next actions:"); + for action in actions { + out.push_str(&format!( + "\n {: Command { ) } +/// Resolves the default output format when the user gave no explicit format. +/// +/// Precedence here is env-override first, then a TTY policy: an interactive +/// terminal gets human-friendly output, everything else (pipes, files, CI, +/// most agents) gets machine-readable JSON. Pure so it can be unit-tested +/// without a real terminal. +#[must_use] +pub fn resolve_default_output_format(env_override: Option<&str>, is_tty: bool) -> String { + match env_override { + Some(value) if !value.trim().is_empty() => value.trim().to_owned(), + _ if is_tty => "human".to_owned(), + _ => "json".to_owned(), + } +} + +/// Derives the per-application output-format override env var from an app id, +/// e.g. `godaddy` -> `GODADDY_OUTPUT`, `gdx` -> `GDX_OUTPUT`. +#[must_use] +pub fn output_env_var(app_id: &str) -> String { + let sanitized: String = app_id + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() { + c.to_ascii_uppercase() + } else { + '_' + } + }) + .collect(); + format!("{sanitized}_OUTPUT") +} + +/// Computes the default output format for `app_id`, consulting the +/// `${APP_ID}_OUTPUT` env override and whether stdout is an interactive +/// terminal. Used as the fallback when no explicit `--output`/`--json`/ +/// `--toon`/`--human` is given. +#[must_use] +pub fn default_output_format(app_id: &str) -> String { + let env = std::env::var(output_env_var(app_id)).ok(); + resolve_default_output_format(env.as_deref(), std::io::stdout().is_terminal()) +} + #[must_use] -/// Extracts framework-global flags from parsed `clap` matches. -pub fn global_flags_from_matches(matches: &ArgMatches) -> GlobalFlags { +/// Extracts framework-global flags from parsed `clap` matches, falling back to +/// `default_format` when the user gave no explicit output format. +pub fn global_flags_from_matches(matches: &ArgMatches, default_format: &str) -> GlobalFlags { let output_format = if matches.get_flag("toon") { "toon".to_owned() } else if matches.get_flag("human") { "human".to_owned() - } else { + } else if matches.get_flag("json") { + "json".to_owned() + } else if matches.value_source("output") == Some(clap::parser::ValueSource::CommandLine) { matches .get_one::("output") .cloned() - .unwrap_or_else(|| "json".to_owned()) + .unwrap_or_else(|| default_format.to_owned()) + } else { + default_format.to_owned() }; GlobalFlags { @@ -269,14 +317,16 @@ pub fn extract_search_query(args: &[impl AsRef]) -> String { /// Extracts output format from raw args. /// /// Recognizes `--output ` / `-o ` / `--output=`, -/// plus `--json`, `--toon`, and `--human` as shorthand for their respective formats. -pub fn extract_output_format(args: &[impl AsRef]) -> String { +/// plus `--json`, `--toon`, and `--human` as shorthand for their respective +/// formats. Falls back to `default_format` when none is present. +pub fn extract_output_format(args: &[impl AsRef], default_format: &str) -> String { for index in 0..args.len() { let arg = args[index].as_ref(); if arg == "--output" || arg == "-o" { - return args - .get(index + 1) - .map_or_else(|| "json".to_owned(), |value| value.as_ref().to_owned()); + return args.get(index + 1).map_or_else( + || default_format.to_owned(), + |value| value.as_ref().to_owned(), + ); } if let Some(value) = arg.strip_prefix("--output=") { return value.to_owned(); @@ -291,7 +341,7 @@ pub fn extract_output_format(args: &[impl AsRef]) -> String { return "human".to_owned(); } } - "json".to_owned() + default_format.to_owned() } #[must_use] @@ -419,3 +469,28 @@ fn arg_requires_value(arg: &Arg) -> bool { .is_some_and(|range| range.takes_values() && range.min_values() > 0), } } + +#[cfg(test)] +mod tests { + use super::{output_env_var, resolve_default_output_format}; + + #[test] + fn default_output_format_follows_tty_then_env_override() { + // TTY policy when no env override. + assert_eq!(resolve_default_output_format(None, true), "human"); + assert_eq!(resolve_default_output_format(None, false), "json"); + // Env override wins over the TTY policy in both directions. + assert_eq!(resolve_default_output_format(Some("json"), true), "json"); + assert_eq!(resolve_default_output_format(Some("human"), false), "human"); + // Blank/empty env override is ignored. + assert_eq!(resolve_default_output_format(Some(" "), false), "json"); + assert_eq!(resolve_default_output_format(Some(""), true), "human"); + } + + #[test] + fn output_env_var_is_derived_from_app_id() { + assert_eq!(output_env_var("godaddy"), "GODADDY_OUTPUT"); + assert_eq!(output_env_var("gdx"), "GDX_OUTPUT"); + assert_eq!(output_env_var("my-cli"), "MY_CLI_OUTPUT"); + } +} diff --git a/src/lib.rs b/src/lib.rs index e400257..65ce946 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -94,7 +94,8 @@ pub use auth::{ }; pub use cli::{ ApplyFlags, BuildInfo, Cli, CliConfig, CliRunOutput, ExtraSearchDocs, InitDeps, - ModuleHelpEntry, OnShutdown, PreRun, RegisterFlags, ResolveMeta, build_root_long, + ModuleHelpEntry, OnShutdown, PreRun, RegisterFlags, ResolveMeta, RootNextActions, + build_root_long, }; pub use command::{ CommandContext, CommandFuture, CommandHandler, CommandResult, CommandResultMetadata, @@ -106,9 +107,9 @@ pub use error::{ CliCoreError, DetailedError, ExitCoder, Result, exit_code_for_error, exit_code_for_exit_coder, }; pub use flags::{ - GlobalFlags, derive_bool_flags, derive_value_flags, extract_command_path, - extract_output_format, extract_search_query, global_flags_from_matches, has_true_schema_flag, - register_global_flags, + GlobalFlags, default_output_format, derive_bool_flags, derive_value_flags, + extract_command_path, extract_output_format, extract_search_query, global_flags_from_matches, + has_true_schema_flag, output_env_var, register_global_flags, resolve_default_output_format, }; pub use guide::{GuideEntry, parse_guides, parse_guides_from_markdown}; pub use middleware::{ diff --git a/tests/consumer_cli.rs b/tests/consumer_cli.rs index 9a71c27..057a087 100644 --- a/tests/consumer_cli.rs +++ b/tests/consumer_cli.rs @@ -1,7 +1,9 @@ +use std::sync::Arc; + use clap::Arg; use cli_engine::{ BuildInfo, Cli, CliConfig, CommandResult, CommandSpec, GroupSpec, HumanViewDef, Module, - RuntimeCommandSpec, RuntimeGroupSpec, TableColumn, Tier, + NextAction, RuntimeCommandSpec, RuntimeGroupSpec, TableColumn, Tier, }; use schemars::JsonSchema; use serde::Serialize; @@ -159,3 +161,114 @@ async fn consumer_style_cli_reports_invalid_args_and_output_separately() { assert_ne!(invalid_output.exit_code, 0); assert!(invalid_output.rendered.contains("invalid output format")); } + +fn consumer_cli_with_root_actions() -> Cli { + Cli::new( + CliConfig::new("my-cli", "Team CLI", "my-cli") + .with_build(BuildInfo::new("0.1.0")) + .with_module(platform_module()) + .with_root_next_actions(Arc::new(|| { + vec![ + NextAction::new("my-cli project list", "List projects"), + NextAction::new("my-cli tree", "Display the full command tree"), + ] + })), + ) +} + +#[tokio::test] +async fn bare_invocation_human_shows_help_with_next_actions() { + let cli = consumer_cli_with_root_actions(); + + // `--human` forces the interactive format (under `cargo test` stdout is not a + // TTY, so the default would otherwise resolve to json). + let bare = cli.run(["my-cli", "--human"]).await; + assert_eq!(bare.exit_code, 0); + // Long help is preserved (the command summary still lists `tree`). + assert!(bare.rendered.contains("tree"), "{}", bare.rendered); + // Cold-start guidance is appended. + assert!( + bare.rendered.contains("Suggested next actions:"), + "{}", + bare.rendered + ); + assert!( + bare.rendered.contains("my-cli project list"), + "{}", + bare.rendered + ); + // It is human text, not a JSON envelope. + assert!( + serde_json::from_str::(&bare.rendered).is_err(), + "{}", + bare.rendered + ); + // The root help template drops the global options wall. + assert!(!bare.rendered.contains("--fields"), "{}", bare.rendered); +} + +#[tokio::test] +async fn group_help_keeps_subcommands_but_drops_global_options() { + let cli = consumer_cli(); + + let group = cli.run(["my-cli", "project"]).await; + assert_eq!(group.exit_code, 0); + // Group page lists its child commands... + assert!(group.rendered.contains("Commands:"), "{}", group.rendered); + assert!(group.rendered.contains("list"), "{}", group.rendered); + assert!(group.rendered.contains("delete"), "{}", group.rendered); + // ...but not the global options wall. + assert!(!group.rendered.contains("--fields"), "{}", group.rendered); + + // Leaf commands keep their full flag set. + let leaf = cli.run(["my-cli", "project", "list", "--help"]).await; + assert!(leaf.rendered.contains("--fields"), "{}", leaf.rendered); +} + +#[tokio::test] +async fn bare_invocation_emits_discovery_envelope_for_explicit_json() { + let cli = consumer_cli_with_root_actions(); + + let bare = cli.run(["my-cli", "--output", "json"]).await; + assert_eq!(bare.exit_code, 0); + let envelope = serde_json::from_str::(&bare.rendered).expect("discovery json"); + assert_eq!(envelope["data"]["description"], "Team CLI"); + assert_eq!(envelope["data"]["version"], "0.1.0"); + let actions = envelope["next_actions"] + .as_array() + .expect("next_actions array"); + assert_eq!(actions.len(), 2); + assert_eq!(actions[0]["command"], "my-cli project list"); + // No env/auth/command_tree embedded — reachable via next_actions instead. + assert!(envelope["data"].get("command_tree").is_none()); + + // The `--json` shorthand is also an explicit machine-format request. + let shorthand = cli.run(["my-cli", "--json"]).await; + assert_eq!(shorthand.exit_code, 0); + let shorthand_envelope = + serde_json::from_str::(&shorthand.rendered).expect("shorthand discovery json"); + assert_eq!(shorthand_envelope["data"]["description"], "Team CLI"); + assert!(shorthand_envelope["next_actions"].is_array()); +} + +#[tokio::test] +async fn bare_invocation_without_hook_falls_back_to_long_help() { + let cli = consumer_cli(); + + let bare = cli.run(["my-cli"]).await; + assert_eq!(bare.exit_code, 0); + assert!( + !bare.rendered.contains("Suggested next actions:"), + "{}", + bare.rendered + ); + + // Even with explicit json, no hook means the unchanged long-help fallback. + let bare_json = cli.run(["my-cli", "--output", "json"]).await; + assert_eq!(bare_json.exit_code, 0); + assert!( + serde_json::from_str::(&bare_json.rendered).is_err(), + "{}", + bare_json.rendered + ); +} diff --git a/tests/exhaustive_cli_contract.rs b/tests/exhaustive_cli_contract.rs index 4f05db4..4b55929 100644 --- a/tests/exhaustive_cli_contract.rs +++ b/tests/exhaustive_cli_contract.rs @@ -34,7 +34,7 @@ fn global_bool_flags_accept_full_documented_bool_matrix() { let matches = parser().try_get_matches_from(args); assert!(matches.is_ok(), "true value {value:?} should parse"); let matches = matches.expect("checked ok"); - let flags = global_flags_from_matches(&matches); + let flags = global_flags_from_matches(&matches, "json"); assert!(flags.schema, "schema value {value:?}"); assert!(flags.dry_run, "dry-run value {value:?}"); } @@ -49,7 +49,7 @@ fn global_bool_flags_accept_full_documented_bool_matrix() { let matches = parser().try_get_matches_from(args); assert!(matches.is_ok(), "false value {value:?} should parse"); let matches = matches.expect("checked ok"); - let flags = global_flags_from_matches(&matches); + let flags = global_flags_from_matches(&matches, "json"); assert!(!flags.schema, "schema value {value:?}"); assert!(!flags.dry_run, "dry-run value {value:?}"); } @@ -61,8 +61,8 @@ fn global_optional_value_flags_have_missing_value_defaults() { .try_get_matches_from(["my-cli", "--verbose", "--debug"]) .expect("optional flags should accept omitted values"); assert_eq!(command_path_from_matches("my-cli", &matches), ""); - assert_eq!(global_flags_from_matches(&matches).verbose, "all"); - assert_eq!(global_flags_from_matches(&matches).debug, "*"); + assert_eq!(global_flags_from_matches(&matches, "json").verbose, "all"); + assert_eq!(global_flags_from_matches(&matches, "json").debug, "*"); } #[test] diff --git a/tests/exhaustive_public_api.rs b/tests/exhaustive_public_api.rs index 136676b..dada90e 100644 --- a/tests/exhaustive_public_api.rs +++ b/tests/exhaustive_public_api.rs @@ -144,7 +144,7 @@ fn parsed_global_flags_cover_defaults_short_aliases_and_optional_values() { "project:list" ); assert_eq!( - global_flags_from_matches(&matches), + global_flags_from_matches(&matches, "json"), GlobalFlags { output_format: "toon".to_owned(), verbose: "all".to_owned(), diff --git a/tests/foundation.rs b/tests/foundation.rs index 6cf935f..7091b68 100644 --- a/tests/foundation.rs +++ b/tests/foundation.rs @@ -3423,32 +3423,35 @@ fn raw_search_and_output_extraction_matches_legacy_bypass_helpers() { assert_eq!(extract_search_query(&["my-cli", "--search"]), ""); assert_eq!( - extract_output_format(&["my-cli", "-o", "json", "--search", "foo"]), + extract_output_format(&["my-cli", "-o", "json", "--search", "foo"], "json"), "json" ); assert_eq!( - extract_output_format(&["my-cli", "--output=human", "--search", "foo"]), + extract_output_format(&["my-cli", "--output=human", "--search", "foo"], "json"), "human" ); - assert_eq!(extract_output_format(&["my-cli", "--output"]), "json"); assert_eq!( - extract_output_format(&["my-cli", "--search", "foo"]), + extract_output_format(&["my-cli", "--output"], "json"), "json" ); assert_eq!( - extract_output_format(&["my-cli", "project", "list", "--json"]), + extract_output_format(&["my-cli", "--search", "foo"], "json"), "json" ); assert_eq!( - extract_output_format(&["my-cli", "project", "list", "--toon"]), + extract_output_format(&["my-cli", "project", "list", "--json"], "json"), + "json" + ); + assert_eq!( + extract_output_format(&["my-cli", "project", "list", "--toon"], "json"), "toon" ); assert_eq!( - extract_output_format(&["my-cli", "--toon", "project", "list"]), + extract_output_format(&["my-cli", "--toon", "project", "list"], "json"), "toon" ); assert_eq!( - extract_output_format(&["my-cli", "project", "list", "--human"]), + extract_output_format(&["my-cli", "project", "list", "--human"], "json"), "human" ); @@ -8751,6 +8754,55 @@ impl AuthProvider for FakeProvider { } } +#[tokio::test] +async fn auth_command_is_listed_under_configured_help_category() { + let module = Module::new("Workflows", |_context| { + RuntimeGroupSpec::new(GroupSpec::new("project", "Manage projects")) + }); + let cli = Cli::new( + CliConfig::new("my-cli", "Dev tooling", "my-cli") + .with_auth_provider(Arc::new(FakeProvider::new("primary", "me"))) + .with_default_auth_provider("primary") + .with_admin_category("Account") + .with_module(module), + ); + + // No discovery hook, so bare invocation renders the root long help. + let bare = cli.run(["my-cli"]).await; + // `auth` is folded into the curated category list (it would otherwise be + // visible only via clap's auto subcommand list, which the root template + // suppresses). + assert!(bare.rendered.contains("Account:"), "{}", bare.rendered); + assert!(bare.rendered.contains("auth"), "{}", bare.rendered); + // The root help template drops the global options wall. + assert!(!bare.rendered.contains("--fields"), "{}", bare.rendered); +} + +#[tokio::test] +async fn auth_command_uses_baked_in_default_category_without_override() { + let module = Module::new("Workflows", |_context| { + RuntimeGroupSpec::new(GroupSpec::new("project", "Manage projects")) + }); + let cli = Cli::new( + CliConfig::new("my-cli", "Dev tooling", "my-cli") + .with_auth_provider(Arc::new(FakeProvider::new("primary", "me"))) + .with_default_auth_provider("primary") + // No with_admin_category: auth gets the baked-in default category. + .with_module(module), + ); + + let bare = cli.run(["my-cli"]).await; + // Baked-in default category ("Admin"), not a generic "Commands" bucket. + assert!(bare.rendered.contains("Admin:"), "{}", bare.rendered); + assert!(bare.rendered.contains("auth"), "{}", bare.rendered); + // No generic "Commands:" heading (distinct from the built-in "Find Commands:"). + assert!( + !bare.rendered.contains("\n Commands:"), + "{}", + bare.rendered + ); +} + #[derive(Debug)] struct RecordingEnvProvider { envs: Arc>>, From f8868235333827c3ce3b840233a66731f45f756e Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 4 Jun 2026 13:58:56 -0700 Subject: [PATCH 2/5] fix: address Copilot review feedback - resolve_default_output_format: normalize the env override to lowercase and ignore blank/invalid values, so a miscased or stray ${APP_ID}_OUTPUT can't break all command output. - bare-root discovery: skip pre_run so the bare command stays discoverable even when pre_run would fail, matching the no-hook bare-root path. - auth categorization: invoke register_auth_help_entry from ensure_auth_command so auth is filed under the admin category even when a provider is registered after construction (not only during Cli::new). - tests: rename the resolver test to reflect env-then-TTY precedence; add coverage for case-insensitive/invalid env values and post-construction auth registration. Co-Authored-By: Claude Opus 4.8 --- src/cli.rs | 22 ++++++++-------------- src/flags.rs | 27 ++++++++++++++++++++------- tests/foundation.rs | 21 +++++++++++++++++++++ 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index ee47b4f..7b78f8a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -569,9 +569,6 @@ impl Cli { for command in commands { cli.add_command(command); } - // Registered last so `auth` appends to its category after module-defined - // entries, preserving the consumer's category ordering. - cli.register_auth_help_entry(); cli } @@ -905,17 +902,10 @@ impl Cli { if command_path.is_empty() && let Some(root_next_actions) = &self.root_next_actions { - if let Err(err) = self.run_pre_run( - &mut middleware, - &command_path, - &crate::middleware::ValueMap::new(), - ) { - return self.finish_run(render_cli_error( - &middleware, - &err, - &self.config.app_id, - )); - } + // Bare-root discovery is static (help text / metadata + action + // pointers) and must always be available as a cold-start entry + // point, so we skip `pre_run` here — matching the no-hook + // bare-root path below, which also renders help without it. let actions = root_next_actions(); return self.finish_run(self.render_root(&middleware, actions)); } @@ -1302,6 +1292,10 @@ impl Cli { } else { self.root.clone().subcommand(clap_group) }; + // Categorize `auth` wherever it is ensured (construction or a later + // `register_auth_provider`), so it never falls into the generic + // "Commands" bucket. Idempotent via the `already_listed` guard. + self.register_auth_help_entry(); } fn default_auth_provider(&self) -> String { diff --git a/src/flags.rs b/src/flags.rs index 81a3655..eab67a1 100644 --- a/src/flags.rs +++ b/src/flags.rs @@ -202,11 +202,16 @@ pub fn register_global_flags(command: Command) -> Command { /// without a real terminal. #[must_use] pub fn resolve_default_output_format(env_override: Option<&str>, is_tty: bool) -> String { - match env_override { - Some(value) if !value.trim().is_empty() => value.trim().to_owned(), - _ if is_tty => "human".to_owned(), - _ => "json".to_owned(), + if let Some(value) = env_override { + // Normalize case (env vars are commonly upper/mixed case) and ignore + // blank or unrecognized values, so a stray or miscased override can't + // break all command output — only a valid format is honored. + let normalized = value.trim().to_ascii_lowercase(); + if crate::output::is_valid_output_format(&normalized) { + return normalized; + } } + if is_tty { "human" } else { "json" }.to_owned() } /// Derives the per-application output-format override env var from an app id, @@ -475,16 +480,24 @@ mod tests { use super::{output_env_var, resolve_default_output_format}; #[test] - fn default_output_format_follows_tty_then_env_override() { + fn default_output_format_follows_env_override_then_tty() { // TTY policy when no env override. assert_eq!(resolve_default_output_format(None, true), "human"); assert_eq!(resolve_default_output_format(None, false), "json"); - // Env override wins over the TTY policy in both directions. + // A valid env override wins over the TTY policy in both directions. assert_eq!(resolve_default_output_format(Some("json"), true), "json"); assert_eq!(resolve_default_output_format(Some("human"), false), "human"); - // Blank/empty env override is ignored. + // Env override is case-insensitive (env vars are commonly upper-cased). + assert_eq!(resolve_default_output_format(Some("JSON"), true), "json"); + assert_eq!( + resolve_default_output_format(Some(" Human "), false), + "human" + ); + // Blank or unrecognized env overrides are ignored (fall back to TTY). assert_eq!(resolve_default_output_format(Some(" "), false), "json"); assert_eq!(resolve_default_output_format(Some(""), true), "human"); + assert_eq!(resolve_default_output_format(Some("yaml"), false), "json"); + assert_eq!(resolve_default_output_format(Some("yaml"), true), "human"); } #[test] diff --git a/tests/foundation.rs b/tests/foundation.rs index 7091b68..db0619b 100644 --- a/tests/foundation.rs +++ b/tests/foundation.rs @@ -8803,6 +8803,27 @@ async fn auth_command_uses_baked_in_default_category_without_override() { ); } +#[tokio::test] +async fn auth_registered_after_construction_is_categorized() { + let module = Module::new("Workflows", |_context| { + RuntimeGroupSpec::new(GroupSpec::new("project", "Manage projects")) + }); + // Built with no auth provider; one is added post-construction. + let mut cli = Cli::new(CliConfig::new("my-cli", "Dev tooling", "my-cli").with_module(module)); + cli.register_auth_provider(Arc::new(FakeProvider::new("primary", "me"))); + + let bare = cli.run(["my-cli"]).await; + // `auth` added by the later provider is still filed under the admin + // category, not the generic "Commands" bucket. + assert!(bare.rendered.contains("Admin:"), "{}", bare.rendered); + assert!(bare.rendered.contains("auth"), "{}", bare.rendered); + assert!( + !bare.rendered.contains("\n Commands:"), + "{}", + bare.rendered + ); +} + #[derive(Debug)] struct RecordingEnvProvider { envs: Arc>>, From 01f7dab5382e78120cd3f67ade7d598f48769c12 Mon Sep 17 00:00:00 2001 From: Jay Gowdy Date: Thu, 4 Jun 2026 15:17:15 -0700 Subject: [PATCH 3/5] fix: reject invalid --output on bare-root discovery path render_root parsed the output format via the infallible OutputFormat::from_str, which silently coerced an unrecognized explicit --output value (e.g. --output yaml) to JSON. Normal commands reject it via Middleware::render_envelope with CliCoreError::InvalidOutputFormat. Validate up front so the bare-root discovery path is consistent, and add a regression test. --- src/cli.rs | 24 +++++++++++++++--------- tests/consumer_cli.rs | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 7b78f8a..0cb23de 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1091,15 +1091,21 @@ impl Cli { /// envelope (light metadata + next actions). The output format has already /// resolved the TTY/env/flag policy, so this just branches on it. fn render_root(&self, middleware: &Middleware, actions: Vec) -> CliRunOutput { - let format: crate::output::OutputFormat = match middleware.output_format.parse() { - Ok(format) => format, - Err(err) => { - return CliRunOutput { - exit_code: exit_code_for_error(&err), - rendered: err.to_string(), - }; - } - }; + // Reject an invalid explicit `--output` here too, matching the normal + // command path (`Middleware::render_envelope`). `OutputFormat::from_str` + // is infallible and would otherwise silently coerce an unrecognized + // value (e.g. `--output yaml`) to JSON instead of reporting the error. + if !crate::output::is_valid_output_format(&middleware.output_format) { + let err = CliCoreError::InvalidOutputFormat(middleware.output_format.clone()); + return CliRunOutput { + exit_code: exit_code_for_error(&err), + rendered: err.to_string(), + }; + } + let format = middleware + .output_format + .parse() + .unwrap_or(crate::output::OutputFormat::Json); if format == crate::output::OutputFormat::Human { // Fold the suggested actions into the root long-about so they render // alongside the other curated sections (before Usage) instead of diff --git a/tests/consumer_cli.rs b/tests/consumer_cli.rs index 057a087..cb92ce5 100644 --- a/tests/consumer_cli.rs +++ b/tests/consumer_cli.rs @@ -251,6 +251,22 @@ async fn bare_invocation_emits_discovery_envelope_for_explicit_json() { assert!(shorthand_envelope["next_actions"].is_array()); } +#[tokio::test] +async fn bare_invocation_rejects_invalid_output_format() { + let cli = consumer_cli_with_root_actions(); + + // An unrecognized explicit `--output` must error on the bare-root discovery + // path just as it does for a normal command, rather than silently coercing + // to JSON. + let bare = cli.run(["my-cli", "--output", "yaml"]).await; + assert_ne!(bare.exit_code, 0, "{}", bare.rendered); + assert!( + bare.rendered.contains("invalid output format"), + "{}", + bare.rendered + ); +} + #[tokio::test] async fn bare_invocation_without_hook_falls_back_to_long_help() { let cli = consumer_cli(); From 494b027132fb5313dc7ab2a5a17986a76481984f Mon Sep 17 00:00:00 2001 From: Jay Gowdy Date: Thu, 4 Jun 2026 15:28:30 -0700 Subject: [PATCH 4/5] test: cover output-format default fall-through and explicit-override precedence Adds deterministic, TTY/env-independent coverage for the resolved-default wiring: global_flags_from_matches and extract_output_format must return the supplied default_format when no explicit format is given, while an explicit --output/--json/--toon/--human still wins. Uses a non-"json" default so the assertions distinguish the passed default from a hardcoded fallback, guarding the value_source == CommandLine gate against regression. --- tests/exhaustive_cli_contract.rs | 40 ++++++++++++++++++++++++++++++++ tests/foundation.rs | 18 ++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/tests/exhaustive_cli_contract.rs b/tests/exhaustive_cli_contract.rs index 4b55929..c393ed1 100644 --- a/tests/exhaustive_cli_contract.rs +++ b/tests/exhaustive_cli_contract.rs @@ -65,6 +65,46 @@ fn global_optional_value_flags_have_missing_value_defaults() { assert_eq!(global_flags_from_matches(&matches, "json").debug, "*"); } +#[test] +fn output_format_falls_back_to_default_when_not_given_explicitly() { + // With no explicit format flag, the resolved `default_format` must win. + // `--output` carries a clap `default_value("json")`, so this guards the + // `value_source == CommandLine` gate: a non-explicit default value must not + // be mistaken for a user-supplied `--output`, which would otherwise pin the + // format to "json" and defeat the TTY/env-aware default. A non-"json" + // default makes the distinction observable. + let matches = parser() + .try_get_matches_from(["my-cli"]) + .expect("bare invocation parses"); + assert_eq!( + global_flags_from_matches(&matches, "human").output_format, + "human" + ); + assert_eq!( + global_flags_from_matches(&matches, "toon").output_format, + "toon" + ); + + // An explicit `--output`/shorthand overrides the default in every case. + let explicit = parser() + .try_get_matches_from(["my-cli", "--output", "toon"]) + .expect("explicit output parses"); + assert_eq!( + global_flags_from_matches(&explicit, "human").output_format, + "toon" + ); + for (flag, expected) in [("--json", "json"), ("--toon", "toon"), ("--human", "human")] { + let matches = parser() + .try_get_matches_from(["my-cli", flag]) + .expect("shorthand flag parses"); + assert_eq!( + global_flags_from_matches(&matches, "human").output_format, + expected, + "{flag} shorthand should win over the default" + ); + } +} + #[test] fn global_flags_reject_invalid_bool_and_numeric_inputs() { for args in [ diff --git a/tests/foundation.rs b/tests/foundation.rs index db0619b..08f6e12 100644 --- a/tests/foundation.rs +++ b/tests/foundation.rs @@ -3455,6 +3455,24 @@ fn raw_search_and_output_extraction_matches_legacy_bypass_helpers() { "human" ); + // When no format token is present, the supplied default is returned (a + // non-"json" default makes this distinct from a hardcoded fallback). A bare + // `--output` with no value behaves the same way. + assert_eq!(extract_output_format(&["my-cli"], "human"), "human"); + assert_eq!( + extract_output_format(&["my-cli", "--search", "foo"], "toon"), + "toon" + ); + assert_eq!( + extract_output_format(&["my-cli", "--output"], "human"), + "human" + ); + // An explicit format still wins over the default. + assert_eq!( + extract_output_format(&["my-cli", "--human"], "json"), + "human" + ); + assert!(has_true_schema_flag(&["my-cli", "release", "--schema"])); assert!(has_true_schema_flag(&[ "my-cli", From a3fc7f94e6765886aaba806c57c285fde3d0eed1 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 4 Jun 2026 17:08:49 -0700 Subject: [PATCH 5/5] docs: document output-format precedence and ${APP_ID}_OUTPUT override Co-Authored-By: Claude Opus 4.8 --- README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/README.md b/README.md index 21e2180..7e514d4 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,27 @@ let module = Module::new("Platform Systems", register).with_guides_from_markdown For large guide directories, applications can use an embedding crate or a `build.rs` generated manifest that produces the same `(path, bytes)` pairs. +## Output formats + +Commands render as `json`, `toon`, or `human`. The default is **context-aware**: +an interactive terminal gets `human` output, while pipes, files, CI, and most +agents (anything where stdout is not a TTY) get `json`. The resolved format is +chosen by this precedence (highest first): + +1. An explicit flag on the invocation — `--output `, or the + `--json` / `--toon` / `--human` shorthands. +2. The `${APP_ID}_OUTPUT` environment variable (e.g. `GODADDY_OUTPUT=json`, + `GDX_OUTPUT=json`). The name is derived from the app id (upper-cased, + non-alphanumerics replaced with `_`). The value is case-insensitive; blank or + unrecognized values are ignored and fall through to the TTY policy. +3. The TTY policy: `human` on an interactive terminal, `json` otherwise. + +This keeps a human at a terminal from getting a wall of JSON while machine +callers still get JSON automatically. An agent running in a PTY (which reads as +interactive) can force machine output with either the env variable or an +explicit flag. Streaming commands always emit newline-delimited JSON regardless +of the resolved format. + ## Documentation - [Concepts](docs/concepts.md)