From d55b776ce8a9a37a1f724cd90d9ad32395148f14 Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Fri, 29 May 2026 21:41:05 +0700 Subject: [PATCH 1/6] =?UTF-8?q?feat(cli):=20visible=5Ftree=20=E2=80=94=20r?= =?UTF-8?q?ecursive=20hidden-subcommand=20filter=20for=20completions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Cargo.toml | 2 +- .../onebrain-cli/src/commands/completions.rs | 84 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index ffe5078..fb85fd7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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) diff --git a/crates/onebrain-cli/src/commands/completions.rs b/crates/onebrain-cli/src/commands/completions.rs index 7d96a46..748f73c 100644 --- a/crates/onebrain-cli/src/commands/completions.rs +++ b/crates/onebrain-cli/src/commands/completions.rs @@ -7,6 +7,35 @@ 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 the config that affects +/// completion output is copied (about/version/args/visible aliases); the auto +/// `help`/`version` args are skipped so clap re-adds `help` without a duplicate-id panic. +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()); + } + 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 { @@ -53,6 +82,61 @@ pub fn hint_line(detected: Option) -> 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 { + 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() { From 1c9011c04004744258f131ac390372c62f85cfb1 Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Fri, 29 May 2026 21:43:27 +0700 Subject: [PATCH 2/6] fix(cli): generate completions from hidden-filtered command tree --- crates/onebrain-cli/src/commands/completions.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/onebrain-cli/src/commands/completions.rs b/crates/onebrain-cli/src/commands/completions.rs index 748f73c..03c9453 100644 --- a/crates/onebrain-cli/src/commands/completions.rs +++ b/crates/onebrain-cli/src/commands/completions.rs @@ -40,10 +40,13 @@ fn visible_tree(src: &clap::Command) -> clap::Command { /// 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 } From 59b9e5f22cc6350b7fecbaad158507da1f272a3e Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Fri, 29 May 2026 21:45:36 +0700 Subject: [PATCH 3/6] test(cli): completions exclude hidden, keep visible commands --- crates/onebrain-cli/tests/completions.rs | 109 +++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/crates/onebrain-cli/tests/completions.rs b/crates/onebrain-cli/tests/completions.rs index 24551af..06b91f0 100644 --- a/crates/onebrain-cli/tests/completions.rs +++ b/crates/onebrain-cli/tests/completions.rs @@ -51,3 +51,112 @@ 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. + for hidden in [ + "completions", + "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}" + ); + } +} From f95b75821563d68c3f380b3876e071eed910793d Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Fri, 29 May 2026 21:47:08 +0700 Subject: [PATCH 4/6] =?UTF-8?q?chore(cli):=20bump=20v3.2.20=20=E2=80=94=20?= =?UTF-8?q?completions=20hidden-command=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 9 ++++++++- Cargo.toml | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 932e50f..b7fc815 100644 --- a/CHANGELOG.md +++ b/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 ` — hidden subcommand emitting a shell diff --git a/Cargo.toml b/Cargo.toml index fb85fd7..8375675 100644 --- a/Cargo.toml +++ b/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"] From 5f54fa21843127a54c0f0592bc7562c8b4fb2ca5 Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Fri, 29 May 2026 21:56:55 +0700 Subject: [PATCH 5/6] fix(cli): carry disable_help_subcommand/propagate_version in visible_tree --- .../onebrain-cli/src/commands/completions.rs | 20 +++++++-- crates/onebrain-cli/tests/completions.rs | 43 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/crates/onebrain-cli/src/commands/completions.rs b/crates/onebrain-cli/src/commands/completions.rs index 03c9453..2633133 100644 --- a/crates/onebrain-cli/src/commands/completions.rs +++ b/crates/onebrain-cli/src/commands/completions.rs @@ -9,9 +9,14 @@ 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 the config that affects -/// completion output is copied (about/version/args/visible aliases); the auto -/// `help`/`version` args are skipped so clap re-adds `help` without a duplicate-id panic. +/// 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() { @@ -20,6 +25,15 @@ fn visible_tree(src: &clap::Command) -> clap::Command { 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" { diff --git a/crates/onebrain-cli/tests/completions.rs b/crates/onebrain-cli/tests/completions.rs index 06b91f0..16b00de 100644 --- a/crates/onebrain-cli/tests/completions.rs +++ b/crates/onebrain-cli/tests/completions.rs @@ -124,8 +124,12 @@ fn completions_hidden_aliases_absent_from_top_level() { // 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", @@ -160,3 +164,42 @@ fn completions_hidden_aliases_absent_from_top_level() { ); } } + +/// 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}" + ); + } +} From 21b0c05e962eb6d75dbba28bb39f51e16a556048 Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Fri, 29 May 2026 21:57:56 +0700 Subject: [PATCH 6/6] style(cli): cargo fmt visible_tree --- Cargo.lock | 8 ++++---- crates/onebrain-cli/tests/completions.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 64cf745..5391c13 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -997,7 +997,7 @@ checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" [[package]] name = "onebrain-cache" -version = "3.2.19" +version = "3.2.20" dependencies = [ "chrono", "onebrain-core", @@ -1010,7 +1010,7 @@ dependencies = [ [[package]] name = "onebrain-cli" -version = "3.2.19" +version = "3.2.20" dependencies = [ "anyhow", "assert_cmd", @@ -1039,7 +1039,7 @@ dependencies = [ [[package]] name = "onebrain-core" -version = "3.2.19" +version = "3.2.20" dependencies = [ "chrono", "indexmap", @@ -1055,7 +1055,7 @@ dependencies = [ [[package]] name = "onebrain-fs" -version = "3.2.19" +version = "3.2.20" dependencies = [ "chrono", "dirs", diff --git a/crates/onebrain-cli/tests/completions.rs b/crates/onebrain-cli/tests/completions.rs index 16b00de..2458f3b 100644 --- a/crates/onebrain-cli/tests/completions.rs +++ b/crates/onebrain-cli/tests/completions.rs @@ -154,7 +154,9 @@ fn completions_hidden_aliases_absent_from_top_level() { } // Sanity: the visible top-level commands ARE candidates. - for visible in ["init", "update", "doctor", "qmd", "schedule", "vault", "session"] { + for visible in [ + "init", "update", "doctor", "qmd", "schedule", "vault", "session", + ] { let present = opts_line .split_whitespace() .any(|tok| tok.trim_matches('"') == visible); @@ -179,9 +181,7 @@ fn completions_exclude_hidden_nested_verbs() { .lines() .map(str::trim_start) .filter(|l| l.starts_with("opts=\"")) - .find(|l| { - l.contains(" embed ") || l.ends_with(" embed\"") || l.contains(" embed reindex") - }) + .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");