From b17d0cade031af86f86ceaeb5635b4247308ab91 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:49:21 -0500 Subject: [PATCH] feat(cli): say what a described change would do on a terminal A change without --execute now starts, on a terminal with table output, with "! Not sent: this would ..." and the target and values the description shows, and ends with "Send it: add --execute, or set VOLTAGE_EXECUTE=1" instead of an error line. The full description and exit code 6 are unchanged, and redirected stderr keeps the error line. --- CHANGELOG.md | 1 + docs/concepts/safety-model.md | 2 + src/api.rs | 7 +- src/dry_run.rs | 182 +++++++++++++++++++++++++++++++++- src/output.rs | 32 +++++- src/style.rs | 4 + tests/cli_integration.rs | 72 ++++++++++++++ 7 files changed, 294 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df1b26f..c5aeb42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes - Color on a terminal: help, `error:`, `hint:`, and `details:` labels. `--color auto|always|never` (or `--no-color`) decides, else `NO_COLOR`, `CLICOLOR_FORCE`, and `CLICOLOR`. JSON is never colored. - An animated spinner on interactive stderr, with the time waited after two seconds; `VOLTAGE_SPINNER=0` keeps it still. A payment wait on a terminal reads as a timeline: one line per status change and a closing `✓` or `✗` line. - `voltage login` on a terminal shows the verification URL as a clickable link, the code in bold, a countdown to the code's expiry, and a closing `✓ Logged in as EMAIL` line. +- A change described without `--execute` starts, on a terminal, with what it would do and the values it would use (`! Not sent: this would send a payment`) and ends with how to send it, instead of an `error:` line. Exit code 6 is unchanged. ## 0.2.0 - 2026-10-22 diff --git a/docs/concepts/safety-model.md b/docs/concepts/safety-model.md index 4b6174a..8b1f1cb 100644 --- a/docs/concepts/safety-model.md +++ b/docs/concepts/safety-model.md @@ -12,6 +12,8 @@ Commands that change something (any method but GET) only describe their request | A change without `--execute` | Describes the request and sends nothing | 6 | | A change with `--execute` | Sends the request | 0 on success | +On a terminal with table output, the description starts with what the change would do and the values it would use, such as `! Not sent: this would send a payment` followed by its wallet and amount, and ends with `› Send it: add --execute, or set VOLTAGE_EXECUTE=1` instead of an `error:` line. The summary shows only the values the description shows. Redirected stderr keeps the `error: Described this change without sending it` line. + Exit code 6 means "described, not sent", so a script that forgot `--execute` fails instead of looking as if it sent money. To check a change on purpose, run it without `--execute` and treat exit code 6 as the expected result. Scripts can set `VOLTAGE_EXECUTE=1` instead of passing `--execute`, and `VOLTAGE_EXECUTE=0` describes one command again; see [scripting](../guides/scripting.md#send-changes-from-a-script). ```sh diff --git a/src/api.rs b/src/api.rs index 5f7bf88..4628999 100644 --- a/src/api.rs +++ b/src/api.rs @@ -600,7 +600,12 @@ pub async fn execute( let run = Run::plan(invocation, &request, scope); match execution { Execution::DescribeChange => { - out.write(dry_run::describe(invocation, global, &request, run)?, &[])?; + let described = dry_run::describe(invocation, global, &request, run)?; + let terminal = global.terminal(); + if out.is_human() && terminal.decorates() { + dry_run::announce(terminal, invocation, &described); + } + out.write(described, &[])?; return Err(Error::new( ErrorKind::NotExecuted, format!( diff --git a/src/dry_run.rs b/src/dry_run.rs index d8aa34d..08c7cab 100644 --- a/src/dry_run.rs +++ b/src/dry_run.rs @@ -12,11 +12,14 @@ use crate::{ cli::{ApiInvocation, BodySource, GlobalFlags}, config::API_URL, input::{PaginationMode, Request}, - output::{Envelope, Outcome}, + output::{self, Envelope, Outcome}, + payment::PaymentDirection, registry::{AuthScheme, Method}, + style::{Line, Role}, + terminal::Terminal, }; use serde::Serialize; -use serde_json::Value; +use serde_json::{Map, Value}; /// Body fields whose values are identifiers, enums, or amounts. Everything else, including /// invoices, addresses, descriptions, names, URLs, and metadata, is omitted by default. @@ -132,6 +135,84 @@ pub fn describe( )) } +/// On a terminal, say in words what the described change would do and the values it would +/// use, before the description's full details. It shows only what the description shows. +pub fn announce(terminal: Terminal, invocation: &ApiInvocation, described: &Envelope) { + let caution = Line::default() + .paint(Role::Caution, "!") + .text(" Not sent: this would ") + .styled(Role::Emphasis, &action(invocation)); + terminal.notice_line(caution); + for line in output::field_lines(&facts(invocation, described)) { + terminal.notice_line(Line::default().text(&format!(" {line}"))); + } + terminal.notice_line(Line::default()); +} + +/// The change in words: the payment direction for `payments send` and `receive`, else the +/// operation's summary, such as "delete a wallet". +fn action(invocation: &ApiInvocation) -> String { + match invocation.alias { + Some(PaymentDirection::Send) => "send a payment".into(), + Some(PaymentDirection::Receive) => "receive a payment".into(), + None => { + let summary = invocation.operation.description.trim_end_matches('.'); + let summary = summary.strip_suffix(" as a user").unwrap_or(summary); + let mut chars = summary.chars(); + chars.next().map_or_else(String::new, |first| { + first.to_lowercase().chain(chars).collect() + }) + } + } +} + +/// The target resource and the body values the description shows. Omitted values and an +/// `id` generated only for the description are left out, and a send's `data` fields read as +/// the payment's own. +fn facts(invocation: &ApiInvocation, described: &Envelope) -> Value { + let mut facts = Map::new(); + // The request path's target, which a raw `--data` body's own `id` does not change. + if let (Some(target), Some(id)) = (invocation.operation.target, invocation.resource_id) { + facts.insert( + target.parameter_name().into(), + Value::String(id.to_string()), + ); + } + let Some(Value::Object(body)) = described.data.get("body").and_then(shown) else { + return Value::Object(facts); + }; + for (key, value) in body { + match value { + _ if key == "id" && generated_id(invocation) => {} + Value::Object(data) if key == SEND_DATA => { + for (key, value) in data { + facts.entry(key).or_insert(value); + } + } + value => { + facts.entry(key).or_insert(value); + } + } + } + Value::Object(facts) +} + +/// `value` without its omitted parts, or `None` when nothing is left. +fn shown(value: &Value) -> Option { + match value { + Value::String(text) if text == OMITTED => None, + Value::Object(map) => { + let kept = map + .iter() + .filter_map(|(key, value)| Some((key.clone(), shown(value)?))) + .collect::>(); + (!kept.is_empty()).then_some(Value::Object(kept)) + } + Value::Null => None, + other => Some(other.clone()), + } +} + /// A friendly create built its own `id` because `--id` was not given. fn generated_id(invocation: &ApiInvocation) -> bool { match &invocation.body { @@ -185,7 +266,104 @@ fn contains_omission(value: &Value) -> bool { #[cfg(test)] mod tests { use super::*; + use crate::cli::{Command, Invocation}; use serde_json::json; + use uuid::Uuid; + + const WALLET: &str = "33333333-3333-4333-8333-333333333333"; + + fn api(args: &[&str]) -> ApiInvocation { + let mut full = vec!["voltage"]; + full.extend_from_slice(args); + match Invocation::try_parse_from(full).unwrap().command { + Command::Api(api) => *api, + Command::Local(local) => panic!("expected an API command, got {local:?}"), + } + } + + fn described(body: Value, resource_id: Option) -> Envelope { + Envelope::new(None, json!({"body": body}), resource_id, Outcome::DryRun) + } + + #[test] + fn a_change_reads_as_its_payment_direction_or_its_summary() { + let send = api(&["payments", "send", "--wallet", WALLET, "--invoice", "lnbc1"]); + assert_eq!(action(&send), "send a payment"); + let receive = api(&[ + "payments", + "receive", + "--kind", + "bolt11", + "--currency", + "btc", + ]); + assert_eq!(action(&receive), "receive a payment"); + assert_eq!( + action(&api(&["wallets", "delete", WALLET])), + "delete a wallet" + ); + // A trailing period and "as a user" are dropped from the API's summary. + assert_eq!( + action(&api(&["quotes", "create", "--wallet", WALLET])), + "request a currency conversion quote" + ); + assert_eq!( + action(&api(&[ + "wallets", "policies", "update", WALLET, "--data", "@p.json" + ])), + "update a wallet's policies" + ); + } + + #[test] + fn facts_name_the_target_and_the_shown_values_only() { + let delete = api(&["wallets", "delete", WALLET]); + let wallet = Uuid::parse_str(WALLET).unwrap(); + assert_eq!( + facts(&delete, &described(Value::Null, Some(wallet))), + json!({"wallet_id": WALLET}) + ); + // A raw body's `id` can differ from the argument; the target is the one the path uses. + let other = "22222222-2222-4222-8222-222222222222"; + let update = api(&["wallets", "update", WALLET, "--data", "@b.json"]); + let body = json!({"id": other, "name": "x"}); + let other_id = Uuid::parse_str(other).unwrap(); + assert_eq!( + facts(&update, &described(body, Some(other_id)))["wallet_id"], + json!(WALLET) + ); + // A send's `data` fields are its own; omitted values and the generated ID are left out. + let send = api(&["payments", "send", "--wallet", WALLET, "--invoice", "lnbc1"]); + let body = json!({ + "id": "generated", "wallet_id": WALLET, "type": "bolt11", "currency": "btc", + "data": {"payment_request": OMITTED, "amount": {"currency": "btc", "amount": 1000}}, + "metadata": OMITTED, "note": null + }); + assert_eq!( + facts(&send, &described(body, None)), + json!({"wallet_id": WALLET, "type": "bolt11", "currency": "btc", + "amount": {"currency": "btc", "amount": 1000}}) + ); + // An ID the person gave stays. + let chosen = api(&[ + "payments", + "send", + "--wallet", + WALLET, + "--invoice", + "lnbc1", + "--id", + "44444444-4444-4444-8444-444444444444", + ]); + assert_eq!( + facts( + &chosen, + &described(json!({"id": "chosen", "data": OMITTED}), None) + ), + json!({"id": "chosen"}) + ); + assert_eq!(facts(&send, &described(json!(OMITTED), None)), json!({})); + } #[test] fn private_body_values_are_omitted_and_identifiers_and_amounts_kept() { diff --git a/src/output.rs b/src/output.rs index 02ab39d..28f3917 100644 --- a/src/output.rs +++ b/src/output.rs @@ -7,9 +7,9 @@ mod human; use crate::{ Error, Result, - config::new_private, - error::ErrorDetail, - style::{self, ColorChoice, Role}, + config::{EXECUTE_VARIABLE, new_private}, + error::{ErrorDetail, ErrorKind}, + style::{self, ColorChoice, Glyphs, Line, Role}, terminal::{self, scrub_line}, }; use clap::ValueEnum; @@ -333,11 +333,25 @@ pub fn report_error(error: &Error, format: OutputFormat, color: ColorChoice) { redact(&mut body, &[]); if format.is_machine_readable() { eprintln!("{body}"); + } else if error.kind == ErrorKind::NotExecuted && std::io::stderr().is_terminal() { + // The description above says it was not sent; what is left is how to send it. + let step = Glyphs::detect(|name| std::env::var(name).ok()).step; + style::write_stderr(color, &format!("{}\n", send_it(step).as_str())); } else { style::write_stderr(color, &human_error(error, &body["error"]["detail"])); } } +/// On a terminal, the line after a described change. +fn send_it(step: &'static str) -> Line { + Line::default() + .paint(Role::Hint, step) + .text(" Send it: add ") + .styled(Role::Emphasis, "--execute") + .text(", or set ") + .styled(Role::Emphasis, &format!("{EXECUTE_VARIABLE}=1")) +} + /// The human error report with painted labels, which the stream strips when color is off. fn human_error(error: &Error, detail: &Value) -> String { // Messages can interpolate untrusted input; keep control characters off the terminal. @@ -364,6 +378,18 @@ fn human_error(error: &Error, detail: &Value) -> String { report } +/// A JSON object as the aligned, scrubbed `field value` lines human output uses. +pub fn field_lines(value: &Value) -> Vec { + let mut lines = Vec::new(); + match human::detail(&mut lines, value) { + Ok(()) => String::from_utf8_lossy(&lines) + .lines() + .map(str::to_owned) + .collect(), + Err(_) => Vec::new(), + } +} + /// The part of an error's detail a person needs. A client error (4xx) shows the API's error /// without the reason `message` already gives; `--json` keeps the whole response. fn human_detail(detail: &Value, message: &str) -> Option { diff --git a/src/style.rs b/src/style.rs index e268abf..4b17d06 100644 --- a/src/style.rs +++ b/src/style.rs @@ -105,6 +105,8 @@ pub enum Role { Success, /// The spinner and in-flight statuses such as `sending` Pending, + /// `!`: something was deliberately not done + Caution, /// A URL a person opens Link, /// A value a person reads or types, such as a device code @@ -119,6 +121,7 @@ impl Role { Self::Muted => Style::new().dimmed(), Self::Success => AnsiColor::Green.on_default(), Self::Pending => AnsiColor::Yellow.on_default(), + Self::Caution => AnsiColor::Yellow.on_default().bold(), Self::Link => AnsiColor::Blue.on_default().underline(), Self::Emphasis => Style::new().bold(), } @@ -283,6 +286,7 @@ mod tests { Role::Muted, Role::Success, Role::Pending, + Role::Caution, Role::Link, Role::Emphasis, ] { diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 6cc9531..8e719b4 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -5727,6 +5727,78 @@ async fn a_submitted_payment_on_a_terminal_reads_as_a_timeline() { server.verify().await; } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_described_change_on_a_terminal_says_what_it_would_do() { + let (server, dir) = fixture().await; + let args = [ + "wallets", "delete", WALLET, "--org", ORG, "--output", "table", + ]; + let run = on_a_terminal(dir.path(), &server, &args, &[]).await; + assert_eq!(run.code, Some(6), "{}", run.shown); + let shown = strip_styles(&run.shown); + let summary = shown + .find("! Not sent: this would delete a wallet\r\n") + .expect(&shown); + assert!( + shown[summary..].starts_with(&format!( + "! Not sent: this would delete a wallet\r\n wallet_id {WALLET}\r\n\r\n" + )), + "{shown}" + ); + // The details still follow on stdout, and the error line becomes how to send it. + assert!(run.stdout.contains("method"), "{}", run.stdout); + assert!( + shown.ends_with("› Send it: add --execute, or set VOLTAGE_EXECUTE=1\r\n"), + "{shown:?}" + ); + assert!(!shown.contains("error:"), "{shown}"); + assert!( + run.shown.contains("\u{1b}[1m\u{1b}[33m!"), + "{:?}", + run.shown + ); + assert!(server.received_requests().await.unwrap().is_empty()); + + // Redirected stderr keeps the error line, and nothing else changes. + let piped = Command::from_std(human_process_at(dir.path(), &server.uri())) + .env_remove("VOLTAGE_EXECUTE") + .args(args) + .assert() + .code(6); + let error = stderr(&piped); + assert!(error.starts_with("error: Described this change"), "{error}"); + assert!(!error.contains("Not sent"), "{error}"); + + // On a terminal, JSON output gets neither the summary nor the line, only the JSON error. + let json = on_a_terminal( + dir.path(), + &server, + &["wallets", "delete", WALLET, "--org", ORG, "--json"], + &[], + ) + .await; + assert_eq!(json.code, Some(6), "{}", json.shown); + assert!(json.shown.contains("\"exit_code\":6"), "{}", json.shown); + assert!( + !json.shown.contains("Not sent") && !json.shown.contains("Send it"), + "{}", + json.shown + ); + // `--quiet` hides the summary but keeps how to send it. + let quiet = on_a_terminal( + dir.path(), + &server, + &[&args[..], &["--quiet"]].concat(), + &[], + ) + .await; + assert_eq!(quiet.code, Some(6), "{}", quiet.shown); + let shown = strip_styles(&quiet.shown); + assert!(!shown.contains("Not sent"), "{shown}"); + assert!(shown.contains("› Send it: add --execute"), "{shown}"); +} + #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn a_failed_payment_on_a_terminal_ends_with_a_cross_before_the_error() {