fix(cli): completions exclude hidden/internal subcommands (v3.2.20)

CHANGELOG.md

@@ -1,5 +1,5 @@
 ---
-latest_version: 3.2.19
+latest_version: 3.2.20
 released: 2026-05-29
 ---
 
@@ -12,6 +12,13 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+## [3.2.20] — 2026-05-29 — completions: exclude hidden commands
+
+- fix(cli): shell completions no longer list hidden/internal/legacy subcommands
+  (avatar, daemon, session-init, …) — clap_complete's aot generators don't honor
+  `hide`, so completions are now generated from a recursively hidden-filtered
+  command tree.
+
 ## [3.2.19] — 2026-05-29 — shell completions
 
 - feat(cli): `onebrain completions <SHELL>` — hidden subcommand emitting a shell

Cargo.toml

@@ -4,7 +4,7 @@ default-members = ["crates/*"]
 resolver = "2"
 
 [workspace.package]
-version = "3.2.19"
+version = "3.2.20"
 edition = "2021"
 license = "AGPL-3.0-only"
 authors = ["OneBrain Contributors"]
@@ -24,7 +24,7 @@ serde_json  = { version = "1", features = ["preserve_order"] }
 thiserror   = "1"
 anyhow      = "1"
 chrono      = { version = "0.4", features = ["serde"] }
-clap        = { version = "4", features = ["derive"] }
+clap        = { version = "4", features = ["derive", "string"] }
 clap_complete = "4"
 
 # Filesystem (Slice 1 partial · used by count_unembedded)

crates/onebrain-cli/src/commands/completions.rs

@@ -7,14 +7,60 @@ use clap::CommandFactory;
 use clap::ValueEnum;
 use clap_complete::{generate, Shell};
 
+/// Rebuild `src` keeping only non-hidden subcommands, recursively. clap_complete's
+/// aot generators emit every subcommand (they don't honor `hide`), and clap has no
+/// remove-subcommand API — so we reconstruct the tree.
+///
+/// Only a CURATED set of command-level settings is copied — about, version, args,
+/// visible aliases, `disable_help_subcommand`, `propagate_version`. The auto
+/// `help`/`version` args are skipped so clap re-adds `help` without a duplicate-id
+/// panic. Any future command-level setting that affects completion output (e.g.
+/// another auto-injected subcommand toggle) MUST be added here too, or the rebuilt
+/// tree will silently diverge from the real CLI.
+fn visible_tree(src: &clap::Command) -> clap::Command {
+    let mut out = clap::Command::new(src.get_name().to_string());
+    if let Some(about) = src.get_about() {
+        out = out.about(about.clone());
+    }
+    if let Some(version) = src.get_version() {
+        out = out.version(version.to_string());
+    }
+    // Carry the `help` subcommand suppression (set on the root + resource groups);
+    // otherwise clap auto-injects a `help` subcommand that leaks as a candidate.
+    if src.is_disable_help_subcommand_set() {
+        out = out.disable_help_subcommand(true);
+    }
+    // Carry `--version` propagation so the rebuilt tree matches the real CLI.
+    if src.is_propagate_version_set() {
+        out = out.propagate_version(true);
+    }
+    for arg in src.get_arguments() {
+        let id = arg.get_id().as_str();
+        if id == "help" || id == "version" {
+            continue; // clap auto-adds help; avoid duplicate-definition panic
+        }
+        out = out.arg(arg.clone());
+    }
+    for alias in src.get_visible_aliases() {
+        out = out.visible_alias(alias.to_string());
+    }
+    for sub in src.get_subcommands().filter(|s| !s.is_hide_set()) {
+        out = out.subcommand(visible_tree(sub));
+    }
+    out
+}
+
 /// Generate the completion script for `shell` to stdout. Returns the process
 /// exit code (always 0 — clap already validated `shell` into a known variant).
 pub fn run(shell: Shell) -> i32 {
     // `CommandFactory::command()` reconstructs the clap `Command` that the
-    // derive macro built for `Cli`. `generate` needs `&mut Command` because it
-    // finalizes the command tree (propagating help/version) before walking it.
-    let mut cmd = Cli::command();
-    let bin = cmd.get_name().to_string();
+    // derive macro built for `Cli`. clap_complete's aot generators don't honor
+    // `#[command(hide = true)]`, so we regenerate from a tree with hidden
+    // subcommands recursively stripped (`visible_tree`). `bin` is taken from the
+    // original command's name (`onebrain`) so the script binds to the real binary.
+    let full = Cli::command();
+    let bin = full.get_name().to_string();
+    let mut cmd = visible_tree(&full);
     generate(shell, &mut cmd, bin, &mut std::io::stdout());
     0
 }
@@ -53,6 +99,61 @@ pub fn hint_line(detected: Option<Shell>) -> String {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use clap::Command;
+
+    fn sample() -> Command {
+        Command::new("app")
+            .subcommand(Command::new("visible-a").subcommand(Command::new("kid")))
+            .subcommand(Command::new("hidden-top").hide(true))
+            .subcommand(
+                Command::new("group")
+                    .subcommand(Command::new("vis-verb"))
+                    .subcommand(Command::new("hid-verb").hide(true)),
+            )
+    }
+
+    fn names(cmd: &Command) -> Vec<String> {
+        cmd.get_subcommands()
+            .map(|c| c.get_name().to_string())
+            .collect()
+    }
+
+    #[test]
+    fn visible_tree_drops_hidden_top_level() {
+        let t = visible_tree(&sample());
+        let n = names(&t);
+        assert!(n.contains(&"visible-a".to_string()));
+        assert!(n.contains(&"group".to_string()));
+        assert!(
+            !n.contains(&"hidden-top".to_string()),
+            "hidden top-level kept: {n:?}"
+        );
+    }
+
+    #[test]
+    fn visible_tree_drops_hidden_nested_verbs() {
+        let t = visible_tree(&sample());
+        let group = t
+            .get_subcommands()
+            .find(|c| c.get_name() == "group")
+            .unwrap();
+        let verbs = names(group);
+        assert!(verbs.contains(&"vis-verb".to_string()));
+        assert!(
+            !verbs.contains(&"hid-verb".to_string()),
+            "hidden nested verb kept: {verbs:?}"
+        );
+    }
+
+    #[test]
+    fn visible_tree_keeps_deep_visible_children() {
+        let t = visible_tree(&sample());
+        let a = t
+            .get_subcommands()
+            .find(|c| c.get_name() == "visible-a")
+            .unwrap();
+        assert!(names(a).contains(&"kid".to_string()));
+    }
 
     #[test]
     fn detects_zsh_from_login_path() {

crates/onebrain-cli/tests/completions.rs

@@ -51,3 +51,155 @@ fn init_yes_does_not_print_completions_hint() {
         .success()
         .stdout(predicate::str::contains("Shell completions").not());
 }
+
+fn completions_for(shell: &str) -> String {
+    String::from_utf8(
+        Command::cargo_bin("onebrain")
+            .unwrap()
+            .args(["completions", shell])
+            .output()
+            .unwrap()
+            .stdout,
+    )
+    .unwrap()
+}
+
+#[test]
+fn completions_exclude_hidden_top_level_commands() {
+    // These hidden command names never appear anywhere in the visible tree
+    // (not as candidates, not inside any help/description text), so a plain
+    // substring check is a sound and strict guard for them. `migrate` is NOT
+    // in this set: `plugin migrate` is a *visible* verb, so its name legitimately
+    // appears — the hidden top-level `migrate` alias is covered structurally by
+    // `completions_hidden_aliases_absent_from_top_level` below.
+    let out = completions_for("zsh");
+    for hidden in [
+        "avatar",
+        "daemon",
+        "bundle",
+        "serve",
+        "session-init",
+        "orphan-scan",
+    ] {
+        assert!(
+            !out.contains(hidden),
+            "hidden command `{hidden}` leaked into zsh completions"
+        );
+    }
+}
+
+#[test]
+fn completions_keep_visible_commands() {
+    let out = completions_for("zsh");
+    for visible in ["init", "update", "doctor", "qmd", "schedule"] {
+        assert!(
+            out.contains(visible),
+            "visible command `{visible}` missing from zsh completions"
+        );
+    }
+}
+
+/// Hardened replacement for the plan's weak `completions_hide_the_completions_command_itself`.
+///
+/// The original asserted `!out.contains("completions")`, which is bogus: the word
+/// "completions" appears throughout the generated script's own function names. The
+/// robust check is structural — extract the bash top-level candidate list (the
+/// `opts="..."` line in the root `onebrain)` case arm, which lists exactly the
+/// registered top-level subcommands) and assert that every hidden top-level name
+/// (including `completions` itself and the legacy aliases like `migrate`) is absent
+/// from it, while the visible ones are present. This pins the exact mechanism the
+/// fix targets: hidden subcommands must not be registered as completion candidates.
+#[test]
+fn completions_hidden_aliases_absent_from_top_level() {
+    let out = completions_for("bash");
+    // The root command's candidate list is the `opts="..."` line that enumerates
+    // the top-level subcommands. Several leaf contexts also emit `opts=`, so select
+    // the one that holds the known visible top-level commands rather than the first.
+    let opts_line = out
+        .lines()
+        .map(str::trim_start)
+        .filter(|l| l.starts_with("opts=\""))
+        .find(|l| l.contains(" init ") && l.contains(" doctor "))
+        .expect("bash script must contain a root opts= candidate list");
+
+    // Hidden top-level names — including the `migrate`/`session-init`/... legacy
+    // aliases and the hidden `completions` command — must NOT be candidates.
+    // `help` is included: the root sets `disable_help_subcommand`, so clap's
+    // auto-injected `help` subcommand must not leak as a candidate either
+    // (regression guard — `visible_tree` must carry `disable_help_subcommand`).
+    for hidden in [
+        "completions",
+        "help",
+        "avatar",
+        "daemon",
+        "bundle",
+        "serve",
+        "session-init",
+        "orphan-scan",
+        "migrate",
+        "vault-sync",
+        "run-skill",
+        "register-hooks",
+        "register-schedule",
+        "qmd-reindex",
+    ] {
+        // Match as a whole word in the space-separated candidate list.
+        let leaked = opts_line
+            .split_whitespace()
+            .any(|tok| tok.trim_matches('"') == hidden);
+        assert!(
+            !leaked,
+            "hidden top-level command `{hidden}` is a completion candidate: {opts_line}"
+        );
+    }
+
+    // Sanity: the visible top-level commands ARE candidates.
+    for visible in [
+        "init", "update", "doctor", "qmd", "schedule", "vault", "session",
+    ] {
+        let present = opts_line
+            .split_whitespace()
+            .any(|tok| tok.trim_matches('"') == visible);
+        assert!(
+            present,
+            "visible top-level command `{visible}` missing from candidate list: {opts_line}"
+        );
+    }
+}
+
+/// Real-tree proof that hidden NESTED verbs are filtered. The unit tests cover
+/// this on a synthetic tree; this asserts it against the actual `qmd` group in
+/// the generated bash script: `embed`/`status`/`reindex` are visible verbs while
+/// `setup`/`search` are `#[command(hide = true)]` (and `help` is suppressed via
+/// the group's `disable_help_subcommand`).
+#[test]
+fn completions_exclude_hidden_nested_verbs() {
+    let out = completions_for("bash");
+    // The qmd group's candidate list is the `opts="..."` line holding its visible
+    // verbs (and not the top-level list, which carries `init`/`doctor`).
+    let qmd_opts = out
+        .lines()
+        .map(str::trim_start)
+        .filter(|l| l.starts_with("opts=\""))
+        .find(|l| l.contains(" embed ") || l.ends_with(" embed\"") || l.contains(" embed reindex"))
+        .filter(|l| !(l.contains(" init ") && l.contains(" doctor ")))
+        .expect("bash script must contain a qmd group opts= candidate list");
+
+    let candidates: Vec<&str> = qmd_opts
+        .split_whitespace()
+        .map(|tok| tok.trim_start_matches("opts=\"").trim_matches('"'))
+        .collect();
+
+    for visible in ["embed", "status", "reindex"] {
+        assert!(
+            candidates.contains(&visible),
+            "visible qmd verb `{visible}` missing from candidate list: {qmd_opts}"
+        );
+    }
+    for hidden in ["setup", "search", "help"] {
+        assert!(
+            !candidates.contains(&hidden),
+            "hidden qmd verb `{hidden}` leaked into candidate list: {qmd_opts}"
+        );
+    }
+}