From 12f3f85afdf75224fe73dab1080ca8d2f6f01547 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 6 Aug 2026 10:28:02 -0700 Subject: [PATCH 1/5] fix(tree): show the completion built-in in `tree` output `build_tree_from_clap_with_path` special-cased "completion" out of the tree listing, a leftover from before the completion command existed. It ran fine via direct dispatch but never appeared under ` tree`, contradicting docs describing tree as the full command hierarchy. Also documents the completion built-in in concepts.md's Built-In Commands table, which omitted it entirely. --- cli-engine/docs/concepts.md | 1 + cli-engine/src/tree.rs | 2 +- cli-engine/tests/foundation.rs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cli-engine/docs/concepts.md b/cli-engine/docs/concepts.md index b7854ba..61cff5b 100644 --- a/cli-engine/docs/concepts.md +++ b/cli-engine/docs/concepts.md @@ -228,6 +228,7 @@ The framework registers built-in commands for common CLI behavior: | --- | --- | --- | | `help` | Always | Displays usage for root, groups, and commands. | | `tree` | Always | Displays the full command hierarchy. | +| `completion [shell] [--install]` | Always | Prints or installs a shell completion script generated from the command tree via `clap_complete`; see [Shell Completion](completion.md). | | `auth login` / `auth status` / `auth logout` | Auth providers are registered or a default provider is configured | Manages credentials. | | `guide [topic]` | Guides are registered | Lists and displays embedded guides. | | `flags list` / `flags info ` | Always, unless a consumer module already registers a top-level `flags` group (the built-in group yields to it) | Inspects declared feature flags and the active policy; see [Feature Flags & Stages](#feature-flags--stages). | diff --git a/cli-engine/src/tree.rs b/cli-engine/src/tree.rs index 75799a6..21fecb2 100644 --- a/cli-engine/src/tree.rs +++ b/cli-engine/src/tree.rs @@ -67,7 +67,7 @@ pub fn build_tree_from_clap(command: &Command) -> TreeNode { fn build_tree_from_clap_with_path(command: &Command, path: String) -> TreeNode { let children = command .get_subcommands() - .filter(|child| !child.is_hide_set() && child.get_name() != "completion") + .filter(|child| !child.is_hide_set()) .map(|child| { let child_path = format!("{path} {}", child.get_name()); build_tree_from_clap_with_path(child, child_path) diff --git a/cli-engine/tests/foundation.rs b/cli-engine/tests/foundation.rs index ac59126..0d5e4d2 100644 --- a/cli-engine/tests/foundation.rs +++ b/cli-engine/tests/foundation.rs @@ -10143,7 +10143,7 @@ fn tree_node_json_shape_and_human_rendering_match_source_contract() { .iter() .map(|child| child.name.as_str()) .collect::>(), - vec!["visible"] + vec!["visible", "completion"] ); } From 494e5c08dcdf976e245e83cf76d3eddd52275955 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 6 Aug 2026 10:39:36 -0700 Subject: [PATCH 2/5] fix(help): list completion under root help's Find Commands section completion was discoverable via `tree` and direct invocation but absent from the root --help page, which hardcodes its "Find Commands" hints (search/tree/guide) rather than deriving them from the command tree. --- cli-engine/src/cli/help.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cli-engine/src/cli/help.rs b/cli-engine/src/cli/help.rs index f7e1791..7feffc1 100644 --- a/cli-engine/src/cli/help.rs +++ b/cli-engine/src/cli/help.rs @@ -69,6 +69,7 @@ pub fn build_root_long(intro: &str, entries: &[ModuleHelpEntry], has_guide: bool "Search all commands and guides by keyword", ), ("tree", "Display full command tree"), + ("completion", "Generate or install shell completion scripts"), ]; if has_guide { find_commands.push(("guide", "Built-in guides for AI agents and developers")); From 05cf2bb7c50d0ada4fd9453aed116567525f8df9 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 6 Aug 2026 11:22:56 -0700 Subject: [PATCH 3/5] feat(completion): advertise known shells as completable values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit completion's own `shell` positional had no possible_values, so tab completion never suggested bash/zsh/fish/powershell/elvish. Attach PossibleValuesParser with ignore_case(true), matching parse_shell's case-insensitive matching and pwsh alias, so no parsing behavior changes — only completion candidates are added. guide's topic arg was investigated for the same treatment but skipped: attaching possible_values there would make clap reject an unknown topic at parse time with its own generic error, breaking the existing exact-match "unknown guide topic ... valid topics: ..." custom error test. That needs clap_complete's unstable dynamic completion engine to decouple candidates from enforcement, which is a separate, larger change. --- cli-engine/src/cli/builtins.rs | 20 +++++++++++++++-- cli-engine/tests/completion.rs | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/cli-engine/src/cli/builtins.rs b/cli-engine/src/cli/builtins.rs index 9c2ea18..2df7214 100644 --- a/cli-engine/src/cli/builtins.rs +++ b/cli-engine/src/cli/builtins.rs @@ -1,4 +1,4 @@ -use clap::{Arg, ArgAction, ArgMatches, Command}; +use clap::{Arg, ArgAction, ArgMatches, Command, builder::PossibleValuesParser}; use serde_json::Value; use crate::{ @@ -42,7 +42,23 @@ pub(crate) fn guide_args(matches: &ArgMatches) -> ValueMap { pub(crate) fn completion_command() -> Command { Command::new("completion") .about("Generate or install shell completion scripts") - .arg(Arg::new("shell").value_name("shell").num_args(0..=1)) + .arg( + Arg::new("shell") + .value_name("shell") + .num_args(0..=1) + // Matches `parse_shell`'s accepted names (case-insensitively, same as + // `parse_shell` itself) so ` completion ` and + // ` completion --install ` suggest real shells. + .ignore_case(true) + .value_parser(PossibleValuesParser::new([ + "bash", + "zsh", + "fish", + "powershell", + "pwsh", + "elvish", + ])), + ) .arg( Arg::new("install") .long("install") diff --git a/cli-engine/tests/completion.rs b/cli-engine/tests/completion.rs index 72efe8c..00ea8af 100644 --- a/cli-engine/tests/completion.rs +++ b/cli-engine/tests/completion.rs @@ -173,6 +173,45 @@ mod completion_integration { ); } + // ========================================================================= + // (a.1) The `shell` positional declares possible values so + // ` completion ` suggests real shell names; parsing still + // accepts them case-insensitively and via the `pwsh` alias, matching + // `parse_shell`. + // ========================================================================= + + #[tokio::test] + async fn completion_shell_arg_advertises_possible_values_in_generated_script() { + let cli = demo_cli(); + let out = cli.run(["demo", "completion", "bash"]).await; + assert_eq!(out.exit_code, 0, "bash: {}", out.rendered); + for shell in ["bash", "zsh", "fish", "powershell", "pwsh", "elvish"] { + assert!( + out.rendered.contains(shell), + "generated script should list {shell} as a completable value; got: {}", + out.rendered + ); + } + } + + #[tokio::test] + async fn completion_print_accepts_uppercase_shell_name() { + let cli = demo_cli(); + let out = cli.run(["demo", "completion", "BASH"]).await; + assert_eq!( + out.exit_code, 0, + "possible-values arg must stay case-insensitive: {}", + out.rendered + ); + } + + #[tokio::test] + async fn completion_print_accepts_pwsh_alias() { + let cli = demo_cli(); + let out = cli.run(["demo", "completion", "pwsh"]).await; + assert_eq!(out.exit_code, 0, "pwsh: {}", out.rendered); + } + // ========================================================================= // (b) Auto-detect: set $SHELL, call `completion` with no shell arg. // ========================================================================= From bfbab71bb22ab5aeed9cac81bd9042578cadef09 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 6 Aug 2026 11:42:13 -0700 Subject: [PATCH 4/5] feat(guide): complete real guide topic names, not just the command name Threads the registered guide names into the `guide` subcommand's `topic` arg as clap possible values, refreshed via a new sync_guide_topic_values() every time add_guides()/set_has_guide() run so config-time and later per-module guide contributions both stay in sync. ` guide ` now completes real topics. This intentionally changes how an unrecognized topic is rejected: clap itself now rejects it at parse time (exit code 2, clap's standard "invalid value ... possible values: ..." message) instead of guide::guide_content's custom "unknown guide topic ... valid topics: ..." message (exit code 1) ever running. Verified this matches the framework's existing exit-code convention for other bad-argument-value rejections (e.g. an unopted command's --limit), and that neither the old nor new message was ever JSON-enveloped under --output json, so this isn't a machine-readable-output regression. Updated the one test that asserted the old exact message. --- cli-engine/docs/concepts.md | 4 ++- cli-engine/src/cli.rs | 34 ++++++++++++++++++++++- cli-engine/tests/foundation.rs | 51 +++++++++++++++++++++++++++++++--- 3 files changed, 83 insertions(+), 6 deletions(-) diff --git a/cli-engine/docs/concepts.md b/cli-engine/docs/concepts.md index 61cff5b..f019db7 100644 --- a/cli-engine/docs/concepts.md +++ b/cli-engine/docs/concepts.md @@ -234,7 +234,9 @@ The framework registers built-in commands for common CLI behavior: | `flags list` / `flags info ` | Always, unless a consumer module already registers a top-level `flags` group (the built-in group yields to it) | Inspects declared feature flags and the active policy; see [Feature Flags & Stages](#feature-flags--stages). | `guide` accepts zero or one topic. Additional positional arguments are rejected before guide content -is rendered. +is rendered. The topic's registered names are declared as clap possible values, so +` guide ` completes real topics and an unrecognized topic is rejected by clap itself +(exit code 2) rather than reaching a command-specific error. Application `pre_run` hooks run for executable commands, including bare command groups that render group help, `help`, `tree`, `guide`, and auth commands. `init_deps` is narrower: it initializes diff --git a/cli-engine/src/cli.rs b/cli-engine/src/cli.rs index b058464..13d39af 100644 --- a/cli-engine/src/cli.rs +++ b/cli-engine/src/cli.rs @@ -13,7 +13,7 @@ mod completion; mod help; mod tree_render; -use clap::{Arg, ArgMatches, Command}; +use clap::{Arg, ArgMatches, Command, builder::PossibleValuesParser}; use crate::{ ActivityEmitter, Auditor, AuthProvider, Authorizer, CliCoreError, CommandMeta, CommandSpec, @@ -1342,6 +1342,7 @@ impl Cli { if has_guide && self.guide_entries.is_empty() && !has_subcommand(&self.root, "guide") { self.root = self.root.clone().subcommand(guide_command()); } + self.sync_guide_topic_values(); self.refresh_root_long(); self } @@ -1361,10 +1362,41 @@ impl Cli { if !self.guide_entries.is_empty() && !has_subcommand(&self.root, "guide") { self.root = self.root.clone().subcommand(guide_command()); } + self.sync_guide_topic_values(); self.refresh_root_long(); self } + /// Re-attaches the `guide` subcommand's `topic` arg possible values from + /// the current [`Self::guide_entries`], so ` guide ` completes + /// real topic names. Guides can arrive across several calls (config-time, + /// then per-module), so this re-syncs on every change rather than once. + /// + /// Declaring possible values makes clap itself reject an unrecognized + /// topic at parse time (exit code 2, clap's own "invalid value ... + /// possible values: ..." message) rather than reaching + /// [`crate::guide::guide_content`]'s own "unknown guide topic" error — + /// an intentional trade: clap's generic parse error in exchange for shell + /// completion, since clap has no stable way to advertise values without + /// also enforcing them. A no-op when there are no guides yet, leaving the + /// bare arg clap adds by default. + fn sync_guide_topic_values(&mut self) { + if self.guide_entries.is_empty() { + return; + } + let names = self + .guide_entries + .iter() + .map(|entry| entry.name.clone()) + .collect::>(); + if let Some(guide_cmd) = self.root.find_subcommand_mut("guide") { + let taken = std::mem::replace(guide_cmd, Command::new("guide")); + *guide_cmd = taken.mut_arg("topic", |arg| { + arg.value_parser(PossibleValuesParser::new(names)) + }); + } + } + /// Resolves busybox/git-style `argv[0]` dispatch before the normal pipeline. /// /// Returns [`Argv0Outcome::Proceed`] with the (possibly rewritten) argument diff --git a/cli-engine/tests/foundation.rs b/cli-engine/tests/foundation.rs index 0d5e4d2..6e87f25 100644 --- a/cli-engine/tests/foundation.rs +++ b/cli-engine/tests/foundation.rs @@ -2366,13 +2366,56 @@ async fn cli_runtime_guide_command_errors_with_valid_topics() { let output = cli.run(["my-cli", "guide", "missing"]).await; - assert_eq!(output.exit_code, 1); - assert_eq!( - output.rendered, - "unknown guide topic \"missing\" — valid topics: deploy" + // The `topic` arg declares its guide names as clap possible values (so + // `guide ` completes them), which makes clap itself reject an + // unrecognized topic at parse time — exit code 2, clap's own message — + // rather than reaching `guide::guide_content`'s custom error. + assert_eq!(output.exit_code, 2); + assert!( + output.rendered.contains("invalid value 'missing'") && output.rendered.contains("deploy"), + "{}", + output.rendered ); } +#[tokio::test] +async fn cli_runtime_guide_topics_are_completable_and_stay_in_sync_across_add_guides_calls() { + let mut cli = Cli::new(CliConfig { + name: "my-cli".to_owned(), + short: "Developer tooling".to_owned(), + ..CliConfig::default() + }); + cli.add_guides([GuideEntry { + name: "deploy".to_owned(), + summary: "Deploy safely".to_owned(), + content: "# Deploy\n".to_owned(), + }]); + // A second, later call (e.g. a module contributing its own guides) must + // also be reflected — not just the first guide-entries transition. + cli.add_guides([GuideEntry { + name: "rollback".to_owned(), + summary: "Roll back safely".to_owned(), + content: "# Rollback\n".to_owned(), + }]); + + let script = cli.run(["my-cli", "completion", "bash"]).await; + assert_eq!(script.exit_code, 0, "{}", script.rendered); + for topic in ["deploy", "rollback"] { + assert!( + script.rendered.contains(topic), + "generated script should list {topic} as a completable guide topic; got: {}", + script.rendered + ); + } + + // Both topics still resolve correctly through the normal dispatch path. + let rollback = cli + .run(["my-cli", "guide", "rollback", "--output", "json"]) + .await; + assert_eq!(rollback.exit_code, 0, "{}", rollback.rendered); + assert_eq!(rollback.rendered, "# Rollback\n"); +} + #[tokio::test] async fn cli_runtime_guide_command_rejects_extra_args_preserves_parser_maximum_one_arg() { let mut cli = Cli::new(CliConfig { From f067cd8b0d679e39a494d35e16fafca8b23bf658 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 6 Aug 2026 11:59:28 -0700 Subject: [PATCH 5/5] Remove wordy, unnecessary comments --- cli-engine/docs/concepts.md | 5 ----- cli-engine/src/cli.rs | 14 ++------------ cli-engine/src/cli/builtins.rs | 3 --- 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/cli-engine/docs/concepts.md b/cli-engine/docs/concepts.md index f019db7..9897cf2 100644 --- a/cli-engine/docs/concepts.md +++ b/cli-engine/docs/concepts.md @@ -233,11 +233,6 @@ The framework registers built-in commands for common CLI behavior: | `guide [topic]` | Guides are registered | Lists and displays embedded guides. | | `flags list` / `flags info ` | Always, unless a consumer module already registers a top-level `flags` group (the built-in group yields to it) | Inspects declared feature flags and the active policy; see [Feature Flags & Stages](#feature-flags--stages). | -`guide` accepts zero or one topic. Additional positional arguments are rejected before guide content -is rendered. The topic's registered names are declared as clap possible values, so -` guide ` completes real topics and an unrecognized topic is rejected by clap itself -(exit code 2) rather than reaching a command-specific error. - Application `pre_run` hooks run for executable commands, including bare command groups that render group help, `help`, `tree`, `guide`, and auth commands. `init_deps` is narrower: it initializes runtime dependencies for real command execution and auth provider loading, but search/schema diff --git a/cli-engine/src/cli.rs b/cli-engine/src/cli.rs index 13d39af..03c4e1c 100644 --- a/cli-engine/src/cli.rs +++ b/cli-engine/src/cli.rs @@ -1368,18 +1368,8 @@ impl Cli { } /// Re-attaches the `guide` subcommand's `topic` arg possible values from - /// the current [`Self::guide_entries`], so ` guide ` completes - /// real topic names. Guides can arrive across several calls (config-time, - /// then per-module), so this re-syncs on every change rather than once. - /// - /// Declaring possible values makes clap itself reject an unrecognized - /// topic at parse time (exit code 2, clap's own "invalid value ... - /// possible values: ..." message) rather than reaching - /// [`crate::guide::guide_content`]'s own "unknown guide topic" error — - /// an intentional trade: clap's generic parse error in exchange for shell - /// completion, since clap has no stable way to advertise values without - /// also enforcing them. A no-op when there are no guides yet, leaving the - /// bare arg clap adds by default. + /// the current [`Self::guide_entries`], so shell completion knows about + /// guide names, which are not all registered up front. fn sync_guide_topic_values(&mut self) { if self.guide_entries.is_empty() { return; diff --git a/cli-engine/src/cli/builtins.rs b/cli-engine/src/cli/builtins.rs index 2df7214..16a654a 100644 --- a/cli-engine/src/cli/builtins.rs +++ b/cli-engine/src/cli/builtins.rs @@ -46,9 +46,6 @@ pub(crate) fn completion_command() -> Command { Arg::new("shell") .value_name("shell") .num_args(0..=1) - // Matches `parse_shell`'s accepted names (case-insensitively, same as - // `parse_shell` itself) so ` completion ` and - // ` completion --install ` suggest real shells. .ignore_case(true) .value_parser(PossibleValuesParser::new([ "bash",