From 05ef3fee5958e1a78d65fdc86480ab6d5725cc20 Mon Sep 17 00:00:00 2001 From: Dennis Wuitz Date: Fri, 29 May 2026 15:48:40 +0200 Subject: [PATCH] fix(cli): dynamic shell completion registering against gradient binary with API-driven resource names --- cli/Cargo.lock | 12 ++ cli/Cargo.toml | 4 +- cli/src/commands/base.rs | 24 ++- cli/src/commands/cache.rs | 5 + cli/src/commands/completion.rs | 292 +++++++++++++++++++++++++++++++ cli/src/commands/mod.rs | 1 + cli/src/commands/organization.rs | 18 +- cli/src/commands/project.rs | 3 + cli/src/commands/worker.rs | 3 + cli/src/config.rs | 8 + cli/src/main.rs | 9 +- cli/tests/completion.rs | 47 +++++ docs/src/tests.md | 8 + docs/src/usage/cli.md | 7 + 14 files changed, 427 insertions(+), 14 deletions(-) create mode 100644 cli/src/commands/completion.rs create mode 100644 cli/tests/completion.rs diff --git a/cli/Cargo.lock b/cli/Cargo.lock index 90c74f54..8f6592e4 100644 --- a/cli/Cargo.lock +++ b/cli/Cargo.lock @@ -304,6 +304,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e0a7a9bfdb35811f9e59832f0f05975114d2251b415fb534108e6f34060fd772" dependencies = [ "clap", + "clap_lex", + "is_executable", + "shlex", ] [[package]] @@ -1043,6 +1046,15 @@ version = "2.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d98f6fed1fde3f8c21bc40a1abb88dd75e67924f9cffc3ef95607bad8017f8e2" +[[package]] +name = "is_executable" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baabb8b4867b26294d818bf3f651a454b6901431711abb96e296245888d6e8c4" +dependencies = [ + "windows-sys 0.60.2", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.2" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index d78e287e..300f16d8 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -16,10 +16,10 @@ members = [".", "connector"] [dependencies] blake3 = { version = "1", default-features = false, features = ["std"] } -clap = { version = "4.6", features = ["derive"] } +clap = { version = "4.6", features = ["derive", "unstable-ext"] } password-auth = { version = "1.0", features = ["argon2"] } rpassword = "7.4" -clap_complete = "4" +clap_complete = { version = "4", features = ["unstable-dynamic"] } strum = "0.28" strum_macros = "0.28" serde = { version = "1.0", features = ["derive"] } diff --git a/cli/src/commands/base.rs b/cli/src/commands/base.rs index 074b2117..0adf1109 100644 --- a/cli/src/commands/base.rs +++ b/cli/src/commands/base.rs @@ -9,16 +9,16 @@ use crate::config::*; use crate::input::*; use crate::output::{ExitKind, Output, to_exit_kind}; use clap::{CommandFactory, Parser, Subcommand}; -use clap_complete::{Shell, generate}; +use clap_complete::{CompleteEnv, Shell}; use connector::auth::{ CliDevicePollRequest, CliPollOutcome, MakeLoginRequest, MakeUserRequest, }; -use std::io; +use std::io::{self, Write}; use std::process::Command; use std::time::Duration; #[derive(Parser, Debug)] -#[command(name = "Gradient", display_name = "Gradient", bin_name = "gradient", author = "Wavelens", version, about, long_about = None)] +#[command(name = "gradient", display_name = "Gradient", bin_name = "gradient", author = "Wavelens", version, about, long_about = None)] #[command(arg_required_else_help = true, subcommand_required = true)] struct Cli { /// Emit machine-readable JSON envelopes; disables interactive prompts. @@ -128,15 +128,27 @@ enum MainCommands { Hash, } +/// Intercept dynamic completion requests (`COMPLETE= gradient …`) and exit. +/// Must run before the tokio runtime starts: completers build their own runtime. +pub fn complete_env() { + CompleteEnv::with_factory(Cli::command).complete(); +} + pub async fn run_cli() -> std::io::Result<()> { let cli = Cli::parse(); let out = Output::new(cli.json); match cli.cmd { MainCommands::Completion { shell } => { - let mut app = Cli::command(); - let bin_name = app.get_name().to_string(); - generate(shell, &mut app, bin_name, &mut io::stdout()); + let exe = std::env::current_exe() + .unwrap_or_else(|e| out.err(ExitKind::Api, format!("cannot locate binary: {e}"))); + let output = Command::new(&exe) + .env("COMPLETE", shell.to_string()) + .output() + .unwrap_or_else(|e| { + out.err(ExitKind::Api, format!("failed to generate completions: {e}")) + }); + io::stdout().write_all(&output.stdout).ok(); } MainCommands::Config { key, value } => { set_get_value_from_string(key, value, false) diff --git a/cli/src/commands/cache.rs b/cli/src/commands/cache.rs index ca651c45..942b84ad 100644 --- a/cli/src/commands/cache.rs +++ b/cli/src/commands/cache.rs @@ -5,9 +5,11 @@ */ use crate::commands::cache_nar; +use crate::commands::completion; use crate::input::{client_from_config, handle_input}; use crate::output::{ExitKind, Output, to_exit_kind}; use clap::Subcommand; +use clap_complete::engine::ArgValueCompleter; use connector::caches::MakeCacheRequest; use std::fs; @@ -25,6 +27,7 @@ pub enum Commands { }, List, Edit { + #[arg(add = ArgValueCompleter::new(completion::complete_caches))] name: String, #[arg(short, long)] display_name: Option, @@ -34,9 +37,11 @@ pub enum Commands { priority: Option, }, Delete { + #[arg(add = ArgValueCompleter::new(completion::complete_caches))] name: String, }, Show { + #[arg(add = ArgValueCompleter::new(completion::complete_caches))] name: String, }, /// Build a netrc entry locally for a cache and install it into a netrc file. diff --git a/cli/src/commands/completion.rs b/cli/src/commands/completion.rs new file mode 100644 index 00000000..2b29d27a --- /dev/null +++ b/cli/src/commands/completion.rs @@ -0,0 +1,292 @@ +/* + * SPDX-FileCopyrightText: 2026 Wavelens GmbH + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +use crate::config::{ConfigKey, load_config_quiet}; +use clap_complete::CompletionCandidate; +use connector::Client; +use std::ffi::OsStr; +use std::future::Future; +use std::time::Duration; + +const COMPLETION_TIMEOUT: Duration = Duration::from_secs(2); + +pub fn complete_orgs(current: &OsStr) -> Vec { + run(current, |client, prefix| async move { + org_names(&client, &prefix).await + }) +} + +pub fn complete_projects(current: &OsStr) -> Vec { + run(current, |client, prefix| async move { + let Some(org) = selected_org() else { + return Vec::new(); + }; + project_names(&client, &org, &prefix).await + }) +} + +pub fn complete_workers(current: &OsStr) -> Vec { + run(current, |client, prefix| async move { + let Some(org) = selected_org() else { + return Vec::new(); + }; + worker_ids(&client, &org, &prefix).await + }) +} + +pub fn complete_caches(current: &OsStr) -> Vec { + run(current, |client, prefix| async move { + cache_names(&client, &prefix).await + }) +} + +pub fn complete_subscribed_caches(current: &OsStr) -> Vec { + run(current, |client, prefix| async move { + let Some(org) = selected_org() else { + return Vec::new(); + }; + subscribed_cache_names(&client, &org, &prefix).await + }) +} + +pub fn complete_org_users(current: &OsStr) -> Vec { + run(current, |client, prefix| async move { + let Some(org) = selected_org() else { + return Vec::new(); + }; + org_user_names(&client, &org, &prefix).await + }) +} + +async fn org_names(client: &Client, prefix: &str) -> Vec { + match client.orgs().list().await { + Ok(res) => matching(res.items.into_iter().map(|i| i.name), prefix), + Err(_) => Vec::new(), + } +} + +async fn project_names(client: &Client, org: &str, prefix: &str) -> Vec { + match client.projects().list(org).await { + Ok(res) => matching(res.items.into_iter().map(|i| i.name), prefix), + Err(_) => Vec::new(), + } +} + +async fn worker_ids(client: &Client, org: &str, prefix: &str) -> Vec { + match client.workers().list(org).await { + Ok(workers) => matching(workers.into_iter().map(|w| w.worker_id), prefix), + Err(_) => Vec::new(), + } +} + +async fn cache_names(client: &Client, prefix: &str) -> Vec { + match client.caches().list().await { + Ok(caches) => matching(caches.into_iter().map(|c| c.name), prefix), + Err(_) => Vec::new(), + } +} + +async fn subscribed_cache_names(client: &Client, org: &str, prefix: &str) -> Vec { + match client.orgs().subscriptions(org).await { + Ok(caches) => matching(caches.into_iter().map(|c| c.name), prefix), + Err(_) => Vec::new(), + } +} + +async fn org_user_names(client: &Client, org: &str, prefix: &str) -> Vec { + match client.orgs().users(org).await { + Ok(users) => matching(users.into_iter().map(|u| u.name), prefix), + Err(_) => Vec::new(), + } +} + +fn matching(values: impl Iterator, prefix: &str) -> Vec { + values.filter(|v| v.starts_with(prefix)).collect() +} + +fn selected_org() -> Option { + load_config_quiet() + .get(&ConfigKey::SelectedOrganization) + .and_then(|v| v.clone()) + .filter(|s| !s.is_empty()) +} + +fn client_quiet() -> Option { + let cfg = load_config_quiet(); + let server = cfg + .get(&ConfigKey::Server) + .and_then(|v| v.clone()) + .filter(|s| !s.is_empty())?; + let token = cfg + .get(&ConfigKey::AuthToken) + .and_then(|v| v.clone()) + .filter(|t| !t.is_empty()); + let mut builder = Client::builder() + .base_url(server) + .timeout(COMPLETION_TIMEOUT); + if let Some(token) = token { + builder = builder.token(token); + } + builder.build().ok() +} + +fn run(current: &OsStr, core: F) -> Vec +where + F: FnOnce(Client, String) -> Fut, + Fut: Future>, +{ + let Some(prefix) = current.to_str().map(str::to_owned) else { + return Vec::new(); + }; + let Ok(rt) = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + else { + return Vec::new(); + }; + rt.block_on(async move { + let Some(client) = client_quiet() else { + return Vec::new(); + }; + tokio::time::timeout(COMPLETION_TIMEOUT, core(client, prefix)) + .await + .unwrap_or_default() + }) + .into_iter() + .map(CompletionCandidate::new) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use connector::Client; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + fn client(base_url: &str) -> Client { + Client::builder() + .base_url(base_url) + .token("test-token") + .build() + .unwrap() + } + + async fn mount_json(server: &MockServer, endpoint: &str, body: serde_json::Value) { + Mock::given(method("GET")) + .and(path(endpoint)) + .respond_with(ResponseTemplate::new(200).set_body_json(body)) + .mount(server) + .await; + } + + #[tokio::test] + async fn cache_names_returns_names_and_respects_prefix() { + let server = MockServer::start().await; + mount_json( + &server, + "/api/v1/caches", + serde_json::json!({ + "error": false, + "message": [ + {"id": "1", "name": "alpha"}, + {"id": "2", "name": "beta"}, + {"id": "3", "name": "alfa"} + ] + }), + ) + .await; + let client = client(&server.uri()); + + let all = cache_names(&client, "").await; + assert_eq!(all, vec!["alpha", "beta", "alfa"]); + + let filtered = cache_names(&client, "al").await; + assert_eq!(filtered, vec!["alpha", "alfa"]); + } + + #[tokio::test] + async fn org_names_reads_paginated_items() { + let server = MockServer::start().await; + mount_json( + &server, + "/api/v1/orgs", + serde_json::json!({ + "error": false, + "message": { + "items": [ + {"id": "1", "name": "acme"}, + {"id": "2", "name": "acorn"} + ], + "total": 2, "page": 1, "per_page": 50 + } + }), + ) + .await; + let client = client(&server.uri()); + + assert_eq!(org_names(&client, "ac").await, vec!["acme", "acorn"]); + assert_eq!(org_names(&client, "aco").await, vec!["acorn"]); + } + + #[tokio::test] + async fn project_names_uses_selected_org() { + let server = MockServer::start().await; + mount_json( + &server, + "/api/v1/projects/acme", + serde_json::json!({ + "error": false, + "message": { + "items": [{"id": "1", "name": "web"}], + "total": 1, "page": 1, "per_page": 50 + } + }), + ) + .await; + let client = client(&server.uri()); + + assert_eq!(project_names(&client, "acme", "").await, vec!["web"]); + } + + #[tokio::test] + async fn worker_ids_returns_ids() { + let server = MockServer::start().await; + mount_json( + &server, + "/api/v1/orgs/acme/workers", + serde_json::json!({ + "error": false, + "message": [ + { + "worker_id": "builder-1", "display_name": "Builder One", + "registered_at": "now", "active": true, "url": null, + "created_by": null, "enable_fetch": true, "enable_eval": true, + "enable_build": true, "live": null + } + ] + }), + ) + .await; + let client = client(&server.uri()); + + assert_eq!(worker_ids(&client, "acme", "build").await, vec!["builder-1"]); + assert!(worker_ids(&client, "acme", "zzz").await.is_empty()); + } + + #[tokio::test] + async fn errors_yield_no_candidates() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/api/v1/caches")) + .respond_with(ResponseTemplate::new(500)) + .mount(&server) + .await; + let client = client(&server.uri()); + + assert!(cache_names(&client, "").await.is_empty()); + } +} diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 6c37fbb4..dd9657eb 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -9,6 +9,7 @@ pub mod base; pub mod build; pub mod cache; pub mod cache_nar; +pub mod completion; pub mod download; pub mod generate; pub mod organization; diff --git a/cli/src/commands/organization.rs b/cli/src/commands/organization.rs index 5472ab6e..b6175e2c 100644 --- a/cli/src/commands/organization.rs +++ b/cli/src/commands/organization.rs @@ -4,10 +4,12 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +use crate::commands::completion; use crate::config::*; use crate::input::{client_from_config, handle_input}; use crate::output::{ExitKind, Output, to_exit_kind}; use clap::Subcommand; +use clap_complete::engine::ArgValueCompleter; use connector::orgs::{ AddUserRequest, MakeOrganizationRequest, PatchOrganizationRequest, RemoveUserRequest, }; @@ -15,6 +17,7 @@ use connector::orgs::{ #[derive(Subcommand, Debug)] pub enum Commands { Select { + #[arg(add = ArgValueCompleter::new(completion::complete_orgs))] organization: String, }, Create { @@ -54,7 +57,10 @@ pub enum Commands { pub enum UserCommands { List, Add { user: String, role: Option }, - Remove { user: String }, + Remove { + #[arg(add = ArgValueCompleter::new(completion::complete_org_users))] + user: String, + }, } #[derive(Subcommand, Debug)] @@ -66,8 +72,14 @@ pub enum SshCommands { #[derive(Subcommand, Debug)] pub enum CacheCommands { List, - Add { cache: String }, - Remove { cache: String }, + Add { + #[arg(add = ArgValueCompleter::new(completion::complete_caches))] + cache: String, + }, + Remove { + #[arg(add = ArgValueCompleter::new(completion::complete_subscribed_caches))] + cache: String, + }, } pub async fn handle(cmd: Commands, out: Output) { diff --git a/cli/src/commands/project.rs b/cli/src/commands/project.rs index c0162b87..fe6cffa1 100644 --- a/cli/src/commands/project.rs +++ b/cli/src/commands/project.rs @@ -4,10 +4,12 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +use crate::commands::completion; use crate::config::*; use crate::input::{client_from_config, handle_input}; use crate::output::{ExitKind, Output, to_exit_kind}; use clap::Subcommand; +use clap_complete::engine::ArgValueCompleter; use colored::*; use connector::projects::{MakeProjectRequest, PatchProjectRequest}; use futures::StreamExt; @@ -16,6 +18,7 @@ use futures::pin_mut; #[derive(Subcommand, Debug)] pub enum Commands { Select { + #[arg(add = ArgValueCompleter::new(completion::complete_projects))] project: String, }, Show, diff --git a/cli/src/commands/worker.rs b/cli/src/commands/worker.rs index a0885130..0ce67f01 100644 --- a/cli/src/commands/worker.rs +++ b/cli/src/commands/worker.rs @@ -4,10 +4,12 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +use crate::commands::completion; use crate::config::*; use crate::input::client_from_config; use crate::output::{ExitKind, Output, to_exit_kind}; use clap::Subcommand; +use clap_complete::engine::ArgValueCompleter; use connector::workers::MakeWorkerRequest; #[derive(Subcommand, Debug)] @@ -32,6 +34,7 @@ pub enum Commands { /// Unregister a worker from the selected organization Delete { /// Worker ID to unregister + #[arg(add = ArgValueCompleter::new(completion::complete_workers))] worker_id: String, }, } diff --git a/cli/src/config.rs b/cli/src/config.rs index 5104cc1a..eb9b1ee5 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -60,6 +60,14 @@ pub fn load_config() -> HashMap> { } } +pub fn load_config_quiet() -> HashMap> { + let config_file = get_config_file(); + fs::read_to_string(&config_file) + .ok() + .and_then(|contents| toml::from_str(&contents).ok()) + .unwrap_or_else(|| ConfigKey::iter().map(|key| (key, None)).collect()) +} + pub fn save_config(config: &HashMap>) { let config_file = get_config_file(); let config_dir = config_file diff --git a/cli/src/main.rs b/cli/src/main.rs index 0a384cc9..8b973fac 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -11,7 +11,10 @@ pub mod output; use commands::base; -#[tokio::main] -pub async fn main() -> std::io::Result<()> { - base::run_cli().await +fn main() -> std::io::Result<()> { + base::complete_env(); + tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build()? + .block_on(base::run_cli()) } diff --git a/cli/tests/completion.rs b/cli/tests/completion.rs new file mode 100644 index 00000000..24f50247 --- /dev/null +++ b/cli/tests/completion.rs @@ -0,0 +1,47 @@ +/* + * SPDX-FileCopyrightText: 2026 Wavelens GmbH + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +use assert_cmd::Command; + +// Regression for the broken completion bin name: the generated script must +// register against the real `gradient` binary, never the `Gradient` app name. +#[test] +fn completion_bash_registers_lowercase_binary() { + let output = Command::cargo_bin("gradient") + .unwrap() + .args(["completion", "bash"]) + .output() + .unwrap(); + + assert!(output.status.success()); + let script = String::from_utf8(output.stdout).unwrap(); + assert!( + script.contains("_clap_complete_gradient"), + "completion function should be _clap_complete_gradient:\n{script}" + ); + assert!( + script.contains("-F _clap_complete_gradient gradient"), + "completion must be registered against the gradient binary:\n{script}" + ); + assert!( + !script.contains("Gradient"), + "completion script must not reference the capitalised app name:\n{script}" + ); +} + +#[test] +fn completion_zsh_registers_lowercase_binary() { + let output = Command::cargo_bin("gradient") + .unwrap() + .args(["completion", "zsh"]) + .output() + .unwrap(); + + assert!(output.status.success()); + let script = String::from_utf8(output.stdout).unwrap(); + assert!(!script.contains("Gradient"), "zsh script: {script}"); + assert!(script.contains("gradient"), "zsh script: {script}"); +} diff --git a/docs/src/tests.md b/docs/src/tests.md index fe22e6cc..482a91ac 100644 --- a/docs/src/tests.md +++ b/docs/src/tests.md @@ -2516,6 +2516,14 @@ loading degrades silently when `/etc/ssl/certs` is missing. CLI integration tests in `cli/tests/`: - `download_attr.rs` - `gradient download '#attr' --json` writes the right files; `--json` without args returns a structured missing-argument envelope and exits 2. +- `completion.rs` - regression for the broken completion bin name: `gradient completion {bash,zsh}` must emit a script that registers against the real `gradient` binary (`-F _clap_complete_gradient gradient`) and never the capitalised `Gradient` app name, which silently disabled `gradient `. + +Dynamic completer unit tests live in `cli/src/commands/completion.rs` (`#[cfg(test)]`, +wiremock-backed). They drive each completer core against a mock server and assert it +returns the resource names and honours the partial prefix +(`cache_names`, `org_names` reading paginated `items`, `project_names`/`worker_ids` +scoped to the selected org), and that a non-2xx response yields no candidates so the +shell never errors. Run a single connector test file with `cargo test -p connector --test `; CI runs the full suite. diff --git a/docs/src/usage/cli.md b/docs/src/usage/cli.md index e5cb0d75..15d73629 100644 --- a/docs/src/usage/cli.md +++ b/docs/src/usage/cli.md @@ -184,6 +184,13 @@ gradient completion fish >> ~/.config/fish/completions/gradient.fish gradient generate ``` +Completions are dynamic: besides subcommands and flags, TAB also completes +existing resource names (organizations, projects, workers, and caches) by +querying the server. The resource lookups require a configured server and a +valid login; when offline or logged out they yield nothing instead of erroring. +Project and worker name completion uses the currently selected organization +(`gradient organization select `). + ## Global Options ```