From 9f29f8169191ac568ae151dd603b37d0d363f6f6 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Fri, 9 Oct 2026 13:16:28 -0500 Subject: [PATCH] feat(cli): pick a missing organization, environment, or wallet On a terminal, a command that lacks the organization, environment, or wallet it needs now asks for it instead of failing. A picker, filtered by typing part of a name or ID, lists each choice as "name UUID" from the same sources as organizations list, environments list, and wallets list, in the environment when one is selected. Afterwards a notice names the flags that select the same scope. API commands ask for the scope their path needs. Friendly payment creation asks for a wallet, and friendly wallet creation for an environment. environments list asks for an organization, and profiles create for an organization and environment. A positional target is still typed. Nothing is asked when stdin or stderr is not a terminal, with --no-input, with JSON or NDJSON output, or with --dry-run, so scripts see the same errors as before. Esc leaves the scope unchanged and the command reports what is missing; Ctrl-C exits with code 130. An API key cannot list organizations or environments, so with a key only a wallet is offered and a missing organization fails as before. Picking never sends a change, which still needs --execute. API names are scrubbed before the picker draws them. The picker resolves the credential once and exchanges at most one organization token. inquire draws it through crossterm, which it now runs with its event features. Pseudo-terminal tests cover a pick, Esc, Ctrl-C, the no-prompt flags, an empty wallet list, an API key without an organization or environment, and a login that picks an organization and then an environment. --- Cargo.lock | 77 +++++++ Cargo.toml | 3 + docs/guides/login-and-profiles.md | 13 ++ docs/quality.md | 7 +- src/api.rs | 27 ++- src/auth.rs | 28 ++- src/input.rs | 12 + src/lib.rs | 1 + src/pick.rs | 295 ++++++++++++++++++++++++ src/startup.rs | 4 +- src/terminal.rs | 96 +++++++- tests/cli_integration.rs | 370 ++++++++++++++++++++++++++++++ 12 files changed, 922 insertions(+), 11 deletions(-) create mode 100644 src/pick.rs diff --git a/Cargo.lock b/Cargo.lock index f29b3ee..7535b36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -486,6 +486,15 @@ version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6ef517f0926dd24a1582492c791b6a4818a4d94e789a334894aa15b0d12f55c" +[[package]] +name = "convert_case" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "633458d4ef8c78b72454de2d54fd6ab2e60f9e02be22f3c6104cdc8a4e0fceb9" +dependencies = [ + "unicode-segmentation", +] + [[package]] name = "core-foundation" version = "0.10.1" @@ -531,9 +540,13 @@ checksum = "d8b9f2e4c67f833b660cdb0a3523065869fb35570177239812ed4c905aeff87b" dependencies = [ "bitflags", "crossterm_winapi", + "derive_more", "document-features", + "mio", "parking_lot", "rustix", + "signal-hook", + "signal-hook-mio", "winapi", ] @@ -582,6 +595,28 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "092966b41edc516079bdf31ec78a2e0588d1d0c08f78b91d8307215928642b2b" +[[package]] +name = "derive_more" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d751e9e49156b02b44f9c1815bcb94b984cdcc4396ecc32521c739452808b134" +dependencies = [ + "derive_more-impl", +] + +[[package]] +name = "derive_more-impl" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "799a97264921d8623a957f6c3b9011f3b5492f557bbb7a5a19b7fa6d06ba8dcb" +dependencies = [ + "convert_case", + "proc-macro2", + "quote", + "rustc_version", + "syn 2.0.119", +] + [[package]] name = "difflib" version = "0.4.0" @@ -630,6 +665,12 @@ dependencies = [ "litrs", ] +[[package]] +name = "dyn-clone" +version = "1.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d0881ea181b1df73ff77ffaaf9c7544ecc11e82fba9b5f27b262a3c73a332555" + [[package]] name = "endi" version = "1.1.1" @@ -1158,6 +1199,19 @@ dependencies = [ "hybrid-array", ] +[[package]] +name = "inquire" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6654738b8024300cf062d04a1c13c10c8e2cea598ec1c47dc9b6641159429756" +dependencies = [ + "bitflags", + "crossterm", + "dyn-clone", + "unicode-segmentation", + "unicode-width", +] + [[package]] name = "ipnet" version = "2.12.2" @@ -1324,6 +1378,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4b18443e9c262bfe8fa82f51666e2642c53393f7e5c27b3e1aeab922cff5b9d8" dependencies = [ "libc", + "log", "wasi", "windows-sys 0.61.2", ] @@ -2042,6 +2097,27 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" +[[package]] +name = "signal-hook" +version = "0.3.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d881a16cf4426aa584979d30bd82cb33429027e42122b169753d6ef1085ed6e2" +dependencies = [ + "libc", + "signal-hook-registry", +] + +[[package]] +name = "signal-hook-mio" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b75a19a7a740b25bc7944bdee6172368f988763b744e3d4dfe753f6b4ece40cc" +dependencies = [ + "libc", + "mio", + "signal-hook", +] + [[package]] name = "signal-hook-registry" version = "1.4.8" @@ -2475,6 +2551,7 @@ dependencies = [ "futures-util", "hex", "httpdate", + "inquire", "keyring", "libc", "qrcode", diff --git a/Cargo.toml b/Cargo.toml index 1c38af2..3ea233b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,9 @@ comfy-table = { version = "=8.0.1", default-features = false } # Reads the terminal width on every platform. Without its `windows` feature, crossterm # refuses to compile on Windows. crossterm = { version = "=0.29.0", default-features = false, features = ["windows"] } +# Pickers for a missing organization, environment, or wallet. Its default filter matches a +# substring of a name or ID. It draws through crossterm, so the CLI keeps one terminal backend. +inquire = { version = "=0.9.4", default-features = false, features = ["crossterm"] } [target.'cfg(unix)'.dependencies] libc = "=0.2.190" diff --git a/docs/guides/login-and-profiles.md b/docs/guides/login-and-profiles.md index 8ba311d..fcd3c9f 100644 --- a/docs/guides/login-and-profiles.md +++ b/docs/guides/login-and-profiles.md @@ -43,6 +43,19 @@ Without a profile, flags override `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMEN Only endpoints that filter by environment receive `--env`. Organization-wide commands say so in their help, and wallet mutations verify a supplied environment against the wallet before changing anything. +## Pick the scope interactively + +On a terminal, a command that lacks the organization, environment, or wallet it needs asks for it instead of failing. The picker lists each choice as `name UUID`; type to filter, use the arrow keys, and press Enter. Afterwards, the CLI prints the flags that select the same scope, so you can pass them next time or save a profile. + +```sh +voltage wallets list # asks for the organization +voltage payments receive --currency btc --kind bolt11 --amount 1000 --unit sats --execute # asks for the wallet +``` + +The picker offers every wallet in the environment. For this example, pick a wallet on a BTC line of credit. A wallet on a USD line of credit needs a quote for every payment, so the API rejects the receive with `quote_required` unless you pass `--quote QUOTE_ID` from `voltage quotes create`. + +An API key cannot list organizations or environments, so with a key the CLI offers only wallets; pass `--org` and `--env` or use a profile. Esc leaves the scope unchanged, and the command then reports what is missing. Ctrl-C exits with code 130. Picking a scope never sends a change: changes still need `--execute`. The CLI never asks when stdin or stderr is not a terminal, when stdout is piped without `--output table` (as in `voltage wallets list | jq`), or with `--no-input`, `--json`, `--output json|ndjson`, or `--dry-run`. In those cases, the command fails with the same error as before. + ## Profiles A profile bundles an organization, an environment, and a credential, and ignores the scope variables above and `VOLTAGE_API_KEY`; flags still override its scope for one command. diff --git a/docs/quality.md b/docs/quality.md index d4e89fd..5b2bdf5 100644 --- a/docs/quality.md +++ b/docs/quality.md @@ -53,9 +53,10 @@ not precedent. Reviews must approve and document exceptions. `auth`. Keep settings, credential storage, and private files in `config`. Keep payment values and payment read views in `payment`. Keep secret text in `secret`. Keep result envelopes, redaction, and rendering, including the - table layout in `output::human`, in `output`. Keep prompts, progress, notices, - the terminal's width, and scrubbing of untrusted text bound for the terminal - in `terminal`. Keep exit-code categories and typed error detail in `error`. + table layout in `output::human`, in `output`. Keep prompts, pickers, progress, + notices, the terminal's width, and scrubbing of untrusted text bound for the + terminal in `terminal`. Keep which commands ask for missing scope, and in what + order, in `pick`. Keep exit-code categories and typed error detail in `error`. - Name each module for the operation that it performs. Do not give two modules the same capability name. For example, keep private-file mechanics in `config` and redaction in `output`. diff --git a/src/api.rs b/src/api.rs index b8ede67..ab8de38 100644 --- a/src/api.rs +++ b/src/api.rs @@ -19,7 +19,7 @@ use crate::{ input::{PaginationMode, Request}, output::{Envelope, Outcome, Output}, payment::{PaymentDirection, PaymentView, ReceiveKind, StatusText, WaitProgress, WaitTarget}, - registry::{AuthScheme, Method, Operation, OperationId}, + registry::{AuthScheme, Method, Operation, OperationId, operation}, secret::Secret, terminal::Terminal, }; @@ -127,14 +127,14 @@ struct Api { impl Api { fn new( global: &GlobalFlags, - invocation: &ApiInvocation, + origin: Option, authorization: Authorization, ) -> Result { Ok(Self { client: auth::client(global.timeout)?, base: auth::base_url(global.api_url.as_deref().unwrap_or(API_URL))?, authorization, - origin: invocation.origin.clone(), + origin, terminal: global.terminal(), }) } @@ -491,7 +491,7 @@ pub async fn execute( } let authorization = Authorization::resolve(invocation, global, settings, scope, submission).await?; - let api = Api::new(global, invocation, authorization)?; + let api = Api::new(global, invocation.origin.clone(), authorization)?; let steps = Steps { api: &api, invocation, @@ -512,6 +512,24 @@ pub async fn execute( } } +/// The organization's wallets, in the environment when the scope names one, for a person to +/// pick from. It reads with the account credential, as `wallets list` does, and builds the +/// request from the same registry operation. More than one `--env` is an error, as it is for +/// `wallets list`. +pub async fn wallets( + global: &GlobalFlags, + credential: ApiCredential, + scope: &Scope, +) -> Result { + let operation = operation(OperationId::GetAllOrganizationsWalletsAsUser); + let request = Request::scoped(operation, scope)?; + let api = Api::new(global, None, Authorization::Account(credential))?; + Ok(api + .send(operation.method, &request.path, &request.query, None) + .await? + .body) +} + /// Whether an API command sends its request, and what decided it. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum Execution { @@ -1314,7 +1332,6 @@ mod tests { use crate::{ cli::{Command, Invocation}, config::Config, - registry::operation, }; use reqwest::header::{HeaderMap, HeaderValue}; use serde_json::json; diff --git a/src/auth.rs b/src/auth.rs index 3b1545b..98275a8 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -682,7 +682,21 @@ pub async fn resolve_organization( flags: &GlobalFlags, submission: &SubmissionState, ) -> Result { - let login = match resolve(settings, scope, flags, submission).await? { + let credential = resolve(settings, scope, flags, submission).await?; + exchange(settings, scope, flags, credential).await +} + +/// The credential to send to the Voltage API for the scope's organization. An API key is sent +/// unchanged. A browser login's access token is account-wide and the API does not accept it, +/// so it is traded at the auth service (OAuth token exchange) for a short-lived token whose +/// audience is the `--org` organization. +pub async fn exchange( + settings: &Settings, + scope: &Scope, + flags: &GlobalFlags, + credential: Credential, +) -> Result { + let login = match credential { Credential::ApiKey(key) => return Ok(ApiCredential::ApiKey(key)), Credential::Login(login) => login, }; @@ -826,6 +840,18 @@ pub async fn discover( } } }; + list(settings, scope, flags, what, &token).await +} + +/// One discovery listing with a token already in hand: the login's access token for +/// organizations, or an organization token for environments. +pub async fn list( + settings: &Settings, + scope: &Scope, + flags: &GlobalFlags, + what: Discovery, + token: &Secret, +) -> Result { let name = settings.account_name(scope)?; let url = base_url(&settings.account(&name)?.auth_url)?; let path = match what { diff --git a/src/input.rs b/src/input.rs index 5cd69e2..91c5bb1 100644 --- a/src/input.rs +++ b/src/input.rs @@ -78,6 +78,18 @@ impl Request { }) } + /// A read that the scope alone fills: its path, and the scope's filters in its query. + /// No flag, `--query` override, or body applies. + pub fn scoped(operation: &Operation, scope: &Scope) -> Result { + Ok(Self { + path: path(operation, None, scope)?, + query: query(operation, &[], &[], scope)?, + body: None, + body_id: None, + resource_id: None, + }) + } + /// The receive kind a payment body names, if it is one the CLI knows. pub fn receive_kind(&self) -> Option { self.body diff --git a/src/lib.rs b/src/lib.rs index 0e72469..3c3a23a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,6 +14,7 @@ mod error; mod input; mod output; mod payment; +mod pick; mod price; pub mod registry; pub mod secret; diff --git a/src/pick.rs b/src/pick.rs new file mode 100644 index 0000000..8ecf964 --- /dev/null +++ b/src/pick.rs @@ -0,0 +1,295 @@ +//! Interactive scope: when a command lacks the organization, environment, or wallet it needs, +//! a person at a terminal picks one instead of copying a UUID. Scripts never see a prompt. + +use crate::{ + Result, + api::{self, SubmissionState}, + auth::{self, ApiCredential, Discovery}, + cli::{BodySource, Command, GlobalFlags, LocalCommand, ProfileCommand, Requested}, + config::{Credential, Scope, Settings}, + output::OutputFormat, + registry::{OperationId, ParameterLocation, ScopeParameter}, + terminal::{Choice, Terminal}, +}; +use serde_json::Value; +use uuid::Uuid; + +/// The scope a command cannot run without. +#[derive(Debug, Default, Eq, PartialEq)] +struct Needs { + org: bool, + env: bool, + wallet: bool, +} + +impl Needs { + /// An environment or wallet is listed within an organization, so either needs one. + fn new(org: bool, env: bool, wallet: bool) -> Self { + Self { + org: org || env || wallet, + env, + wallet, + } + } +} + +fn needs(command: &Command) -> Needs { + match command { + // A dry run promises no prompt and no network request. + Command::Api(api) if api.requested == Requested::DryRun => Needs::default(), + Command::Api(api) => { + let operation = api.operation; + let target = operation.target.map(|target| target.parameter_name()); + let scopes: Vec = operation + .parameters + .iter() + .filter(|parameter| parameter.location == ParameterLocation::Path) + .filter(|parameter| Some(parameter.name.as_str()) != target) + .filter_map(|parameter| parameter.scope()) + .collect(); + let friendly = matches!(api.body, Some(BodySource::Friendly(_))); + Needs::new( + scopes.contains(&ScopeParameter::Organization), + scopes.contains(&ScopeParameter::Environment) + || (friendly && operation.id == OperationId::CreateWallet), + scopes.contains(&ScopeParameter::Wallet) + || (friendly && operation.id == OperationId::CreatePayment), + ) + } + Command::Local(LocalCommand::Environments { .. }) => Needs::new(true, false, false), + Command::Local(LocalCommand::Profiles { + command: ProfileCommand::Create { .. }, + }) => Needs::new(true, true, false), + // Listed by name so that a new local command must decide what scope it needs. + Command::Local( + LocalCommand::Login(_) + | LocalCommand::Logout(_) + | LocalCommand::Auth { .. } + | LocalCommand::Organizations { .. } + | LocalCommand::Profiles { + command: + ProfileCommand::List | ProfileCommand::Get { .. } | ProfileCommand::Delete { .. }, + } + | LocalCommand::Context + | LocalCommand::Price(_) + | LocalCommand::Convert(_) + | LocalCommand::Completions { .. }, + ) => Needs::default(), + } +} + +/// Ask for each missing part of the scope the command needs, in the order organization, +/// environment, wallet. Without an interactive terminal, with `--no-input`, or for JSON +/// output, nothing is asked and the command reports what is missing as before. Esc, an empty +/// list, or a credential that cannot list choices also leaves the rest of the scope unset. +pub async fn fill( + command: &Command, + global: &GlobalFlags, + settings: &Settings, + scope: &mut Scope, + submission: &SubmissionState, +) -> Result<()> { + let needs = needs(command); + let missing = Needs { + org: needs.org && scope.org.is_none(), + env: needs.env && scope.envs.is_empty(), + wallet: needs.wallet && scope.wallet.is_none(), + }; + let terminal = global.terminal(); + if missing == Needs::default() + || global.output_format() != OutputFormat::Table + || !terminal.can_pick() + { + return Ok(()); + } + // Without a credential there is nothing to list. That, not the missing scope, is what the + // person must fix first, so report it. + let credential = auth::resolve(settings, scope, global, submission).await?; + let mut picked = Vec::new(); + let asked = ask(missing, credential, global, settings, scope, &mut picked).await; + if asked.is_ok() && !picked.is_empty() { + let profile = match command { + Command::Local(LocalCommand::Profiles { .. }) => "", + _ => " or save a profile with voltage profiles create", + }; + terminal.notice(format!("Next time, pass {}{profile}.", picked.join(" "))); + } + asked +} + +/// The pickers themselves, with one credential resolution and at most one organization token +/// exchange. An API key cannot list organizations or environments, so only its wallets are +/// offered. +async fn ask( + missing: Needs, + credential: Credential, + global: &GlobalFlags, + settings: &Settings, + scope: &mut Scope, + picked: &mut Vec, +) -> Result<()> { + let terminal = global.terminal(); + if missing.org { + let Credential::Login(login) = &credential else { + terminal.notice("An API key cannot list organizations, so there is nothing to pick."); + return Ok(()); + }; + let listed = auth::list( + settings, + scope, + global, + Discovery::Organizations, + &login.access_token, + ) + .await?; + let Some(org) = choose(terminal, "Organization:", "organizations", &listed).await? else { + return Ok(()); + }; + scope.org = Some(org); + picked.push(format!("--org {org}")); + } + if !missing.env && !missing.wallet { + return Ok(()); + } + let credential = auth::exchange(settings, scope, global, credential).await?; + if missing.env { + let ApiCredential::OrganizationToken(token) = &credential else { + terminal.notice("An API key cannot list environments, so there is nothing to pick."); + return Ok(()); + }; + let listed = auth::list(settings, scope, global, Discovery::Environments, token).await?; + let Some(env) = choose(terminal, "Environment:", "environments", &listed).await? else { + return Ok(()); + }; + scope.envs = vec![env]; + picked.push(format!("--env {env}")); + } + if missing.wallet { + let listed = api::wallets(global, credential, scope).await?; + let Some(wallet) = choose(terminal, "Wallet:", "wallets", &listed).await? else { + return Ok(()); + }; + scope.wallet = Some(wallet); + picked.push(format!("--wallet {wallet}")); + } + Ok(()) +} + +/// Pick from a listing, or say why there is nothing to pick. +async fn choose( + terminal: Terminal, + question: &'static str, + plural: &str, + listed: &Value, +) -> Result> { + let choices = choices(listed); + if choices.is_empty() { + terminal.notice(format!("No {plural} to choose from.")); + return Ok(None); + } + terminal.pick(question, choices).await +} + +/// The listed resources that carry a UUID `id`, sorted by name. +fn choices(listed: &Value) -> Vec { + let items = listed + .as_array() + .or_else(|| listed.get("items").and_then(Value::as_array)) + .map(Vec::as_slice) + .unwrap_or_default(); + let mut choices: Vec = items + .iter() + .filter_map(|item| { + let id = item.get("id")?.as_str()?.parse().ok()?; + let name = item.get("name").and_then(Value::as_str).unwrap_or("-"); + Some(Choice::new(id, name)) + }) + .collect(); + choices.sort_by_key(|choice| choice.name().to_lowercase()); + choices +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::cli::Invocation; + use serde_json::json; + + const ID: &str = "11111111-1111-4111-8111-111111111111"; + + fn needs_of(args: &[&str]) -> Needs { + let mut full = vec!["voltage"]; + full.extend_from_slice(args); + needs(&Invocation::try_parse_from(full).unwrap().command) + } + + #[test] + fn commands_ask_only_for_the_scope_they_cannot_run_without() { + assert_eq!( + needs_of(&["wallets", "list"]), + Needs::new(true, false, false) + ); + assert_eq!( + needs_of(&["payments", "list"]), + Needs::new(true, true, false) + ); + assert_eq!( + needs_of(&[ + "payments", + "send", + "--currency", + "btc", + "--invoice", + "lnbc1" + ]), + Needs::new(true, true, true) + ); + assert_eq!( + needs_of(&["wallets", "create", "--name", "a"]), + Needs::new(true, true, false) + ); + // The positional target is typed, not picked. + assert_eq!( + needs_of(&["wallets", "get", ID]), + Needs::new(true, false, false) + ); + assert_eq!( + needs_of(&["environments", "list"]), + Needs::new(true, false, false) + ); + assert_eq!( + needs_of(&["profiles", "create", "prod"]), + Needs::new(true, true, false) + ); + for args in [ + &["wallets", "list", "--dry-run"][..], + &["organizations", "list"], + &["context"], + &["price"], + ] { + assert_eq!(needs_of(args), Needs::default(), "{args:?}"); + } + } + + #[test] + fn choices_keep_identified_items_scrub_names_and_sort_by_name() { + let listed = json!([ + {"id": ID, "name": "zeta\u{1b}[2J"}, + {"id": "22222222-2222-4222-8222-222222222222", "name": "Alpha"}, + {"id": "33333333-3333-4333-8333-333333333333"}, + {"id": "not-a-uuid", "name": "skipped"}, + {"name": "no id"} + ]); + let labels: Vec = choices(&listed).iter().map(ToString::to_string).collect(); + assert_eq!( + labels, + [ + "- 33333333-3333-4333-8333-333333333333", + "Alpha 22222222-2222-4222-8222-222222222222", + &format!("zeta [2J {ID}"), + ] + ); + assert_eq!(choices(&json!({"items": [{"id": ID}]})).len(), 1); + assert!(choices(&json!({"unexpected": true})).is_empty()); + } +} diff --git a/src/startup.rs b/src/startup.rs index 222656a..689e36f 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -11,6 +11,7 @@ use crate::{ config::{self, Profile, Scope, Settings}, context, output::{Envelope, Output, OutputFormat, report_error, write_stdout}, + pick, price::{PRICE_URL, PriceService}, terminal::Terminal, }; @@ -129,7 +130,8 @@ async fn execute(invocation: Invocation, submission: &SubmissionState) -> Result return out.write(envelope, &[]); } let mut settings = Settings::open(config::directory(global.config_dir.clone())?)?; - let scope = settings.scope(global.scope_selection())?; + let mut scope = settings.scope(global.scope_selection())?; + pick::fill(&command, &global, &settings, &mut scope, submission).await?; match command { Command::Api(api) => { let execution = Execution::resolve(&api, config::execute_variable()?); diff --git a/src/terminal.rs b/src/terminal.rs index 4a2d78f..2ac22a0 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -2,11 +2,13 @@ //! errors are written by `output`. use crate::{Error, Result, secret::Secret}; +use inquire::{InquireError, Select}; use std::{ - fmt::Display, + fmt::{self, Display}, io::{ErrorKind, IsTerminal, Write}, }; use tokio::sync::oneshot; +use uuid::Uuid; /// What the process may show to and ask of the person at the terminal, from `--quiet` and /// `--no-input`. @@ -56,6 +58,24 @@ impl Terminal { eprintln!("{}", scrub_text(&message.to_string())); } + /// A picker also draws on stderr, so it needs both streams to be interactive. + pub fn can_pick(self) -> bool { + self.can_prompt() && std::io::stderr().is_terminal() + } + + /// Ask the person to pick one of a non-empty list of choices, filtering as they type. + /// `None` means they pressed Esc. Raw mode turns a typed Ctrl-C into a key, so the picker + /// reports Ctrl-C as an interruption. + pub async fn pick(self, question: &'static str, choices: Vec) -> Result> { + #[cfg(unix)] + let _modes = TerminalModes::save(); + let _screen = PickerScreen; + picked( + read_detached(move || Select::new(question, choices).with_page_size(12).prompt()) + .await?, + ) + } + /// Read a secret without echo. pub async fn read_hidden(self, prompt: &'static str) -> Result { // The detached read restores echo only when it returns, and Ctrl-C does not wait for @@ -72,6 +92,58 @@ impl Terminal { } } +/// What the picker's answer means: a choice, Esc as `None`, Ctrl-C as an interruption, and +/// any other failure, such as a terminal that cannot be read, as a transport error. +fn picked(answer: std::result::Result) -> Result> { + match answer { + Ok(choice) => Ok(Some(choice.id)), + Err(InquireError::OperationCanceled) => Ok(None), + Err(InquireError::OperationInterrupted) => Err(Error::interrupted( + "Interrupted before any resource change was submitted", + )), + Err(_) => Err(Error::transport( + "Could not read the selection from the terminal", + )), + } +} + +/// The picker turns on bracketed paste and hides the cursor, and undoes both only when its +/// read returns. A signal that ends the command first leaves that to this guard. +struct PickerScreen; + +impl Drop for PickerScreen { + fn drop(&mut self) { + let mut stderr = std::io::stderr(); + let _ = stderr.write_all(b"\x1b[?2004l\x1b[?25h"); + let _ = stderr.flush(); + } +} + +/// One resource a person can pick: its name, scrubbed because the API supplied it, and its ID. +pub struct Choice { + id: Uuid, + name: String, +} + +impl Choice { + pub fn new(id: Uuid, name: &str) -> Self { + Self { + id, + name: scrub_line(name), + } + } + + pub fn name(&self) -> &str { + &self.name + } +} + +impl Display for Choice { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} {}", self.name, self.id) + } +} + /// The terminal's width in columns: `COLUMNS` when set, else the controlling terminal's /// size while stdout is interactive. crossterm falls back to running `tput` when the size /// query fails. `None` means output is not width-limited. @@ -201,6 +273,28 @@ impl Drop for Progress { #[cfg(test)] mod tests { use super::*; + use crate::error::ErrorKind as Exit; + + #[test] + fn a_picker_answer_becomes_a_choice_nothing_an_interruption_or_a_transport_error() { + let id = Uuid::nil(); + assert_eq!(picked(Ok(Choice::new(id, "a"))).unwrap(), Some(id)); + assert_eq!(picked(Err(InquireError::OperationCanceled)).unwrap(), None); + let interrupted = picked(Err(InquireError::OperationInterrupted)).unwrap_err(); + assert_eq!(interrupted.kind, Exit::Interrupted); + // A terminal that goes away or is not one fails the read; no choice is assumed. + for failure in [ + InquireError::NotTTY, + InquireError::IO(std::io::Error::from(ErrorKind::BrokenPipe)), + ] { + let error = picked(Err(failure)).unwrap_err(); + assert_eq!(error.kind, Exit::Transport); + assert_eq!( + error.message, + "Could not read the selection from the terminal" + ); + } + } #[test] fn scrubbing_removes_controls_and_bidirectional_marks() { diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 500f8db..4a4d3df 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -2994,6 +2994,376 @@ async fn interrupting_checkout_stream_returns_130_without_leaking_its_token() { assert!(!String::from_utf8_lossy(&output.stderr).contains("private-stream-token")); } +// --------------------------------------------------------------------------------------- +// Interactive scope pickers +// --------------------------------------------------------------------------------------- + +/// What a command did on a pseudo-terminal. +#[cfg(unix)] +struct TerminalRun { + code: Option, + /// The terminal's local modes before the command started and after it exited. + modes: (libc::tcflag_t, libc::tcflag_t), + shown: String, + stdout: String, +} + +/// Run a command with stdin and stderr on a pseudo-terminal and stdout piped. For each step, +/// once the terminal shows its prompt after the previous step's, type its keys. +#[cfg(unix)] +async fn on_a_terminal( + dir: &Path, + server: &MockServer, + args: &[&str], + steps: &[(&'static str, &'static [u8])], +) -> TerminalRun { + use std::io::{Read, Write}; + use std::os::unix::process::CommandExt; + use std::process::Stdio; + let (mut controller, terminal) = open_pty(); + let observer = controller.try_clone().unwrap(); + let before = local_modes(&observer); + let mut command = human_process_at(dir, &server.uri()); + command + .env_remove("VOLTAGE_EXECUTE") + .args(args) + .stdin(Stdio::from(terminal.try_clone().unwrap())) + .stderr(Stdio::from(terminal)) + .stdout(Stdio::piped()); + // SAFETY: as in `interrupt_the_hidden_key_prompt`: setsid and ioctl are + // async-signal-safe, and the closure allocates nothing. + unsafe { + command.pre_exec(|| { + if libc::setsid() == -1 || libc::ioctl(0, libc::TIOCSCTTY as _, 0) == -1 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + }); + } + let child = command.spawn().unwrap(); + let mut prompts = steps + .iter() + .map(|&(prompt, _)| prompt.as_bytes()) + .collect::>(); + prompts.reverse(); + let (prompted, mut shown_prompt) = tokio::sync::mpsc::unbounded_channel(); + let drain = tokio::task::spawn_blocking(move || { + let (mut shown, mut seen) = (Vec::new(), 0); + let mut chunk = [0; 4096]; + while let Ok(read @ 1..) = controller.read(&mut chunk) { + shown.extend_from_slice(&chunk[..read]); + while let Some(prompt) = prompts.last() { + let Some(at) = shown[seen..] + .windows(prompt.len()) + .position(|w| w == *prompt) + else { + break; + }; + seen += at + prompt.len(); + prompts.pop(); + let _ = prompted.send(()); + } + } + String::from_utf8_lossy(&shown).into_owned() + }); + for &(_, keys) in steps { + tokio::time::timeout(Duration::from_secs(10), shown_prompt.recv()) + .await + .unwrap() + .unwrap(); + (&observer).write_all(keys).unwrap(); + } + let pid = child.id().to_string(); + let Ok(output) = tokio::time::timeout( + Duration::from_secs(10), + tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()), + ) + .await + else { + std::process::Command::new("kill") + .args(["-KILL", &pid]) + .status() + .unwrap(); + panic!("the command on the terminal did not exit"); + }; + let output = output.unwrap(); + let after = local_modes(&observer); + drop(command); + let shown = tokio::time::timeout(Duration::from_secs(5), drain) + .await + .unwrap() + .unwrap(); + TerminalRun { + code: output.status.code(), + modes: (before, after), + shown, + stdout: String::from_utf8_lossy(&output.stdout).into_owned(), + } +} + +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_terminal_picks_a_missing_wallet_and_scripts_are_never_asked() { + let (server, dir) = fixture().await; + Mock::given(method("GET")) + .and(path(wallets_path(ORG))) + .and(query_param("environment_id", ENV)) + .respond_with(ok(json!([ + {"id": RESOURCE, "name": "ops"}, + {"id": WALLET, "name": "Payroll"} + ]))) + .mount(&server) + .await; + let send = [ + "payments", + "send", + "--org", + ORG, + "--env", + ENV, + "--currency", + "btc", + "--invoice", + "lnbc1", + "--output", + "table", + ]; + let listings = || async { + server + .received_requests() + .await + .unwrap() + .iter() + .filter(|request| request.url.path() == wallets_path(ORG)) + .count() + }; + + // Typing filters the list, and Enter picks; the change is still only described. + let picked = on_a_terminal(dir.path(), &server, &send, &[("Wallet:", b"Pay\r")]).await; + assert_eq!(picked.code, Some(6), "{}", picked.shown); + assert!( + picked.stdout.contains("body.wallet_id"), + "{}", + picked.stdout + ); + assert!(picked.stdout.contains(WALLET), "{}", picked.stdout); + assert!( + picked + .shown + .contains(&format!("Next time, pass --wallet {WALLET}")) + ); + assert_eq!(picked.modes.0, picked.modes.1); + + // Esc leaves the scope unset, so the command fails as it did before pickers. + let escaped = on_a_terminal(dir.path(), &server, &send, &[("Wallet:", b"\x1b")]).await; + assert_eq!(escaped.code, Some(2), "{}", escaped.shown); + assert!( + escaped + .shown + .contains("--wallet is required for payment creation") + ); + assert_eq!(escaped.modes.0, escaped.modes.1); + + // A typed Ctrl-C reaches the picker as a key in raw mode and still exits with 130. + let interrupted = on_a_terminal(dir.path(), &server, &send, &[("Wallet:", b"\x03")]).await; + assert_eq!(interrupted.code, Some(130), "{}", interrupted.shown); + assert!( + interrupted + .shown + .contains("Interrupted before any resource change was submitted") + ); + assert_eq!(interrupted.modes.0, interrupted.modes.1); + assert_eq!(listings().await, 3); + + // JSON output, --no-input, and dry runs neither ask nor list. + for extra in ["--json", "--no-input", "--dry-run"] { + let mut args = send.to_vec(); + args.push(extra); + let run = on_a_terminal(dir.path(), &server, &args, &[]).await; + assert_eq!(run.code, Some(2), "{extra}: {}", run.shown); + assert!( + run.shown + .contains("--wallet is required for payment creation"), + "{extra}" + ); + assert!(!run.shown.contains("Wallet:"), "{extra}"); + } + assert_eq!(listings().await, 3); + + // An empty list says so instead of opening the picker. + Mock::given(method("GET")) + .and(path(wallets_path(ORG))) + .and(query_param("environment_id", RESOURCE)) + .respond_with(ok(json!([]))) + .mount(&server) + .await; + let mut args = send.to_vec(); + args[5] = RESOURCE; + let empty = on_a_terminal(dir.path(), &server, &args, &[]).await; + assert_eq!(empty.code, Some(2), "{}", empty.shown); + assert!( + empty.shown.contains("No wallets to choose from."), + "{}", + empty.shown + ); +} + +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn an_api_key_without_an_organization_keeps_the_original_error() { + let (server, dir) = fixture().await; + let run = on_a_terminal( + dir.path(), + &server, + &["wallets", "list", "--output", "table"], + &[], + ) + .await; + assert_eq!(run.code, Some(2), "{}", run.shown); + assert!( + run.shown.contains("--org is required for wallets list"), + "{}", + run.shown + ); + assert!(run.shown.contains("--profile NAME"), "{}", run.shown); + + // A payment needs a wallet, listed within an organization the key cannot list, so the + // person is told why nothing is offered. + let receive = [ + "payments", + "receive", + "--currency", + "btc", + "--kind", + "bolt11", + "--amount", + "1000", + "--output", + "table", + ]; + let run = on_a_terminal(dir.path(), &server, &receive, &[]).await; + assert_eq!(run.code, Some(2), "{}", run.shown); + assert!( + run.shown + .contains("An API key cannot list organizations, so there is nothing to pick."), + "{}", + run.shown + ); + assert!( + run.shown + .contains("--wallet is required for payment creation"), + "{}", + run.shown + ); + + // With an organization, the key still cannot list environments, and says so. + let mut args = receive.to_vec(); + args.extend(["--org", ORG]); + let run = on_a_terminal(dir.path(), &server, &args, &[]).await; + assert_eq!(run.code, Some(2), "{}", run.shown); + assert!( + run.shown + .contains("An API key cannot list environments, so there is nothing to pick."), + "{}", + run.shown + ); + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_missing_credential_is_reported_before_the_missing_scope() { + let (server, dir) = fixture().await; + let run = on_a_terminal( + dir.path(), + &server, + &[ + "payments", + "receive", + "--account", + "nobody", + "--currency", + "btc", + "--kind", + "bolt11", + "--amount", + "1000", + "--output", + "table", + ], + &[], + ) + .await; + assert_eq!(run.code, Some(3), "{}", run.shown); + assert!( + run.shown.contains("Unknown credential nobody"), + "{}", + run.shown + ); + assert!(!run.shown.contains("--wallet is required"), "{}", run.shown); + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_login_picks_an_organization_then_an_environment() { + let auth = MockServer::start().await; + let (api, dir) = fixture().await; + let mut settings = seeded_login(dir.path(), &auth, false); + let organizations = json!({"organizations": [{"id": ORG, "name": "Acme"}]}); + respond(&auth, route("GET", "/users/current"), ok(organizations), 2).await; + organization_exchange(&auth, LOGIN_ACCESS, ORG, "picker-access", 2).await; + let environments = route("GET", format!("/organizations/{ORG}/environments")) + .and(header("authorization", "Bearer picker-access")); + respond( + &auth, + environments, + ok(json!([{"id": ENV, "name": "prod"}])), + 2, + ) + .await; + + // A profile needs both. Saving one is what the hint would suggest, so it is left out. + let args = [ + "profiles", + "create", + "prod", + "--account", + LOGIN_NAME, + "--output", + "table", + ]; + let steps: &[(&str, &[u8])] = &[("Organization:", b"\r"), ("Environment:", b"\r")]; + let run = on_a_terminal(dir.path(), &api, &args, steps).await; + assert_eq!(run.code, Some(0), "{}", run.shown); + assert!( + run.shown + .contains(&format!("Next time, pass --org {ORG} --env {ENV}.")), + "{}", + run.shown + ); + settings.reload().unwrap(); + let profile = &settings.config.profiles["prod"]; + assert_eq!( + (profile.organization_id, profile.environment_id), + (uuid(ORG), uuid(ENV)) + ); + + // Listing environments needs only the organization, so nothing more is asked. + let args = [ + "environments", + "list", + "--account", + LOGIN_NAME, + "--output", + "table", + ]; + let run = on_a_terminal(dir.path(), &api, &args, &[("Organization:", b"\r")]).await; + assert_eq!(run.code, Some(0), "{}", run.shown); + assert!(!run.shown.contains("Environment:"), "{}", run.shown); + auth.verify().await; +} + // --------------------------------------------------------------------------------------- // Prices and conversions // ---------------------------------------------------------------------------------------