From 4cf9940ddc47a0957e4c3e4f210f5e1422196f3b Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Mon, 19 Oct 2026 20:41:21 -0500 Subject: [PATCH] feat(cli)!: remove --dry-run, and describe every help example offline A change without --execute already printed the same description as --dry-run, offline and without credentials, so the flag only added a second way to preview. The remaining differences did not need a flag: VOLTAGE_EXECUTE=0 overrides an exported VOLTAGE_EXECUTE=1 for one command, --no-input and --json keep the pickers from asking, and exit code 6 is how a script recognizes a description. Reads are safe to run, so describing them offline had little use. One rule remains: reads run, and changes are described (exit 6) unless --execute asks to send them. -n and --dry-run are gone (they were never released). The description and its outcome: dry_run envelope are unchanged. The docs and help examples show a preview as the same command without --execute; the example test accepts a change without --execute only when the same command is also documented with --execute, so a forgotten --execute still fails. A new integration test walks the command lists that --help prints, collects each command's Examples section, and runs every example that sends a change without --execute. Each must exit 6 with outcome dry_run, create no file, and send no request. Help examples were only parsed before, so their amounts, units, and scope were never validated. The safety model leads with a table of reads, changes, and --execute. A test that previewed checkout reads is replaced by one that sends a real session read and checks its Origin header. --- CHANGELOG.md | 2 +- README.md | 4 +- docs/README.md | 4 +- docs/concepts/safety-model.md | 26 +++-- docs/development.md | 9 +- docs/getting-started.md | 4 +- docs/guides/login-and-profiles.md | 2 +- docs/guides/wallets.md | 4 +- docs/reference/environment.md | 2 +- src/api.rs | 17 +-- src/cli.rs | 59 +++++----- src/dry_run.rs | 6 +- src/output.rs | 2 +- src/output/human.rs | 2 +- src/pick.rs | 11 +- tests/cli_integration.rs | 184 +++++++++++++++++++----------- tests/support/examples.rs | 2 +- 17 files changed, 186 insertions(+), 154 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39f30cf..eec637f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes - Pick a missing organization, environment, or wallet from a list on a terminal. - `voltage context` shows the effective profile, scope, credential, API and auth URLs, and where each came from. -- `--dry-run` describes any API request offline; changes are only described unless `--execute` or `VOLTAGE_EXECUTE=1` asks to send them. +- Changes are only described (exit code 6) unless `--execute` or `VOLTAGE_EXECUTE=1` asks to send them; the description is built offline and sends nothing. - `--no-input` and `--quiet`, with interrupt-safe progress and the time waited so far on payment waits and device approval. - `voltage --version` shows the git build. - Every command that changes something has examples in `--help`, and every command links its guide. diff --git a/README.md b/README.md index 3bacfe9..1bdbf66 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ The invoice prints as a QR code as soon as it is ready. Leave out `--execute` to ```sh voltage payments list --profile prod --json | jq '.data.items[].status' voltage payments send --profile prod --wallet WALLET_ID \ - --currency btc --invoice BOLT11_INVOICE --max-fee 10 --fee-unit sats --dry-run + --currency btc --invoice BOLT11_INVOICE --max-fee 10 --fee-unit sats # exits 6: described, not sent VOLTAGE_EXECUTE=1 ./pay-invoices.sh ``` @@ -77,7 +77,7 @@ All codes are in the [exit code reference](docs/reference/exit-codes.md). | [Requests](docs/guides/requests.md) | Request bodies, filters, paging | | [Checkout](docs/guides/checkout.md) | Session and stream tokens, events | | [Scripting](docs/guides/scripting.md) | JSON, `--execute` in scripts, quiet output | -| [Safety model](docs/concepts/safety-model.md) | Describe-by-default, dry runs, payment IDs, Ctrl-C, redaction | +| [Safety model](docs/concepts/safety-model.md) | Describe-by-default, payment IDs, Ctrl-C, redaction | | [Output reference](docs/reference/output.md) | Tables, the JSON envelope, streams | | [Command reference](docs/commands.md) | Every command and its API route | diff --git a/docs/README.md b/docs/README.md index 6a62487..e8f3623 100644 --- a/docs/README.md +++ b/docs/README.md @@ -25,11 +25,11 @@ Start with the [README](../README.md) for install and a quick start, or follow [ ## Understand -- [Safety model](concepts/safety-model.md): `--execute`, dry runs, payment IDs, Ctrl-C, redaction +- [Safety model](concepts/safety-model.md): `--execute`, described changes, payment IDs, Ctrl-C, redaction ## For AI agents -Use `--json` and branch on exit codes. Describe a change with `--dry-run` before you send it with `--execute`. Never pass `--show-secrets`. For product concepts, see the [Voltage API docs](https://voltageapi.com/v1/docs). +Use `--json` and branch on exit codes. Run a change without `--execute` to describe it (exit code 6), then send it with `--execute`. Never pass `--show-secrets`. For product concepts, see the [Voltage API docs](https://voltageapi.com/v1/docs). ## For contributors diff --git a/docs/concepts/safety-model.md b/docs/concepts/safety-model.md index bed6953..66f6d35 100644 --- a/docs/concepts/safety-model.md +++ b/docs/concepts/safety-model.md @@ -4,25 +4,27 @@ ## Changes need `--execute` -Commands that change something (any method but GET) only describe their request unless you pass `--execute`, then exit with code 6. Reads run as usual. `--execute` is the only approval: nothing asks again before sending. A described change reports the same `resource_id` a real run would use, so chained commands see the same IDs in both modes. Scripts can set `VOLTAGE_EXECUTE=1` instead; see [scripting](../guides/scripting.md#send-changes-from-a-script). +Commands that change something (any method but GET) only describe their request unless you pass `--execute`. Reads run as usual. `--execute` is the only approval: nothing asks again before sending. -## Dry runs +| Command | What happens | Exit code | +|---|---|---| +| A read (`list`, `get`, `watch`) | Sends the request | 0 on success | +| A change without `--execute` | Describes the request and sends nothing | 6 | +| A change with `--execute` | Sends the request | 0 on success | -`-n, --dry-run` validates any API command and describes the request it would send, then stops: it reports the method, URL, query, `Origin` header, and body, plus a `run` object with what a real run would do next. That is the same plan a real run follows: - -- A `read` says whether it checks the wallet's environment and whether it then pages, waits for a payment, or waits for a checkout session. -- A `mutation` says whether it verifies the wallet's environment, the ID it would journal for recovery, whether a failed payment submission is reconciled, and what it waits for. -- An `event_stream` follows checkout events. - -A dry run authenticates nothing, makes no network request (including the wallet environment check a real mutation performs), prompts for nothing, writes no journal entry, and does not create `--output-file`. Body and metadata-filter values are replaced with `[OMITTED]` unless the field holds only identifiers, enums, or amounts; `--show-secrets` shows them. `--data -` still reads stdin, and a friendly create generates a fresh ID each run unless you pass `--id`. Passing local validation does not mean the API will accept the request. +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 voltage payments send --profile prod --wallet WALLET_ID --currency btc \ - --invoice BOLT11_INVOICE --max-fee 10 --fee-unit sats --dry-run -voltage wallets delete WALLET_ID --profile prod -n --json + --invoice BOLT11_INVOICE --max-fee 10 --fee-unit sats +voltage wallets delete WALLET_ID --profile prod ``` -The result is a normal envelope with `outcome: dry_run` and no `http_status`. +## What a described change shows + +The description reports the method, URL, query, `Origin` header, and body, plus a `run` object with what a real run would do next: whether it verifies the wallet's environment, the ID it would journal for recovery, whether a failed payment submission is reconciled, and what it waits for. It reports the same `resource_id` a real run would use, so chained commands see the same IDs in both modes. The result is a normal envelope with `outcome: dry_run` and no `http_status`. + +Describing a change authenticates nothing, makes no request for the change itself (including the wallet environment check a real run performs), writes no journal entry, and does not create `--output-file`. On a terminal, a missing organization, environment, or wallet can still open a [picker](../guides/login-and-profiles.md#pick-the-scope-interactively), which lists choices from the API; pass the scope, `--no-input`, or `--json` to stay offline. Body and metadata-filter values are replaced with `[OMITTED]` unless the field holds only identifiers, enums, or amounts; `--show-secrets` shows them. `--data -` still reads stdin, and a friendly create generates a fresh ID each run unless you pass `--id`. Passing local validation does not mean the API will accept the request. ## Payment IDs and the request journal diff --git a/docs/development.md b/docs/development.md index 55783bd..a26b7be 100644 --- a/docs/development.md +++ b/docs/development.md @@ -78,10 +78,11 @@ python3 scripts/check-coverage.py Integration tests start mock HTTP servers on port 0 and never contact a real service. They cover every operation in the contract, query encoding, complete JSON bodies, credential selection, describe-by-default changes, secret output, pagination, payment waits, uncertain submissions, -checkout tokens and streams, and the credential lifecycle. Every change the docs show with -`--execute` also runs as an offline `--dry-run`, with stub bodies for `--data` files, so -documented flags, units, scope, and request bodies keep validating. Live payment tests must -use an explicitly configured test-network wallet; routine tests never send a live payment. +checkout tokens and streams, and the credential lifecycle. Every change the docs or `--help` +examples show with `--execute` also runs offline without `--execute`, with stub bodies for +`--data` files, and must be described (exit code 6), so documented flags, units, scope, and +request bodies keep validating. Live payment tests must use an explicitly configured +test-network wallet; routine tests never send a live payment. `docs/reference/global-flags.md` is rendered from the command definitions; after changing a global flag, run `VOLTAGE_BLESS=1 cargo test` to regenerate it, since plain `cargo test` fails on a stale copy. Tests also require every `VOLTAGE_*` variable read in `src/` to appear in `docs/reference/environment.md`, and every exit code in `docs/reference/exit-codes.md`. diff --git a/docs/getting-started.md b/docs/getting-started.md index 4508df9..a62517c 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -31,12 +31,12 @@ voltage wallets list --profile test Use a wallet on a BTC line of credit. A wallet on a USD line of credit needs a quote for every payment, so step 4 would fail with [`quote_required`](guides/troubleshooting.md#quote_required). -If there is no such wallet, create one on a BTC line of credit from `credit-lines list`. Check the request with a dry run first, then send it with `--execute`: +If there is no such wallet, create one on a BTC line of credit from `credit-lines list`. Run it without `--execute` first to see the request, then send it with `--execute`: ```sh voltage credit-lines list --profile test voltage wallets create --profile test --name tutorial \ - --credit-line CREDIT_LINE_ID --network mutinynet --limit 100000 --dry-run + --credit-line CREDIT_LINE_ID --network mutinynet --limit 100000 voltage wallets create --profile test --name tutorial \ --credit-line CREDIT_LINE_ID --network mutinynet --limit 100000 --execute ``` diff --git a/docs/guides/login-and-profiles.md b/docs/guides/login-and-profiles.md index 40f1851..3366199 100644 --- a/docs/guides/login-and-profiles.md +++ b/docs/guides/login-and-profiles.md @@ -54,7 +54,7 @@ voltage payments receive --currency btc --kind bolt11 --amount 1000 --unit sats 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. +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`, or `--output json|ndjson`. In those cases, the command fails with the same error as before. ## Profiles diff --git a/docs/guides/wallets.md b/docs/guides/wallets.md index 0ce117e..232c641 100644 --- a/docs/guides/wallets.md +++ b/docs/guides/wallets.md @@ -36,10 +36,10 @@ voltage wallets policies get WALLET_ID --profile prod --json | jq .data > polici voltage wallets policies update WALLET_ID --profile prod --data @policies.json --execute ``` -Policies have no friendly flags: save the current policies, edit the file, and send the complete object with `--data`. Save only `.data`: `--json` and `--output-file` wrap the response in an envelope, which is not a policy body. Deleting a wallet cannot be undone, so check the request with a dry run first: +Policies have no friendly flags: save the current policies, edit the file, and send the complete object with `--data`. Save only `.data`: `--json` and `--output-file` wrap the response in an envelope, which is not a policy body. Deleting a wallet cannot be undone, so run it without `--execute` first to see the request: ```sh -voltage wallets delete WALLET_ID --profile prod --dry-run +voltage wallets delete WALLET_ID --profile prod voltage wallets delete WALLET_ID --profile prod --execute ``` diff --git a/docs/reference/environment.md b/docs/reference/environment.md index 66a19f3..46deb88 100644 --- a/docs/reference/environment.md +++ b/docs/reference/environment.md @@ -8,7 +8,7 @@ A flag beats a variable. A selected profile is a complete configuration, so `VOL | `VOLTAGE_ORGANIZATION_ID` | Organization UUID | `--org`, `--profile` | | `VOLTAGE_ENVIRONMENT_ID` | Environment UUID | `--env`, `--profile` | | `VOLTAGE_WALLET_ID` | Wallet UUID | `--wallet`, `--profile` | -| `VOLTAGE_EXECUTE` | `1` or `true` sends every change without `--execute`; `0`, `false`, or empty only describes changes. Any other value fails before a change. | `--execute`, `--dry-run` | +| `VOLTAGE_EXECUTE` | `1` or `true` sends every change without `--execute`; `0`, `false`, or empty only describes changes. Any other value fails before a change. | `--execute` | | `VOLTAGE_CHECKOUT_TOKEN` | A checkout session token for `checkout sessions` reads | `--token-file` | | `VOLTAGE_STREAM_TOKEN` | A checkout stream token for `checkout events watch` | `--token-file` | | `VOLTAGE_CONFIG_DIR` | The configuration directory, as an absolute path | `--config-dir` | diff --git a/src/api.rs b/src/api.rs index c181cc0..b770189 100644 --- a/src/api.rs +++ b/src/api.rs @@ -523,12 +523,6 @@ pub async fn execute( check_invoice_flags(invocation, &request)?; let run = Run::plan(invocation, &request, scope); match execution { - Execution::DryRun => { - global - .terminal() - .notice("Dry run: nothing was authenticated, confirmed, recorded, or sent."); - return out.write(dry_run::describe(invocation, global, &request, run)?, &[]); - } Execution::DescribeChange => { out.write(dry_run::describe(invocation, global, &request, run)?, &[])?; return Err(Error::new( @@ -592,8 +586,6 @@ pub async fn wallets( /// Whether an API command sends its request, and what decided it. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum Execution { - /// `--dry-run`: describe the command and succeed. - DryRun, /// A change with neither `--execute` nor `VOLTAGE_EXECUTE`: describe it, then fail, so /// a script that forgot `--execute` does not pass silently. DescribeChange, @@ -608,7 +600,6 @@ impl Execution { /// changes are described. pub fn resolve(invocation: &ApiInvocation, execute_variable: Option) -> Self { match invocation.requested { - Requested::DryRun => Self::DryRun, Requested::Execute => Self::Send, Requested::Unspecified if invocation.operation.method == Method::Get => Self::Send, Requested::Unspecified if execute_variable == Some(true) => Self::SendFromEnvironment, @@ -618,7 +609,7 @@ impl Execution { /// The command describes its request instead of sending it. pub fn describes(self) -> bool { - matches!(self, Self::DryRun | Self::DescribeChange) + self == Self::DescribeChange } } @@ -626,7 +617,7 @@ pub const OFFSET_PAGINATION_DEPRECATED: &str = "Offset pagination is deprecated by the API; omit --offset and --pagination to page by cursor."; /// What a validated API command does, decided once after local validation. `execute` -/// follows it and `--dry-run` reports it, so the two cannot disagree. +/// follows it and a described change reports it, so the two cannot disagree. #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize)] #[serde(tag = "kind", rename_all = "snake_case")] pub enum Run { @@ -1850,17 +1841,15 @@ mod tests { fn the_flag_decides_then_voltage_execute_then_reads_send_and_changes_are_described() { let resolve = |args: &[&str], variable| Execution::resolve(&invocation(args), variable); let change = ["wallets", "delete", WALLET]; - let dry_run = ["wallets", "delete", WALLET, "--dry-run"]; let execute = ["wallets", "delete", WALLET, "--execute"]; for variable in [None, Some(false), Some(true)] { - assert_eq!(resolve(&dry_run, variable), Execution::DryRun); assert_eq!(resolve(&execute, variable), Execution::Send); assert_eq!(resolve(&["wallets", "list"], variable), Execution::Send); } assert_eq!(resolve(&change, None), Execution::DescribeChange); assert_eq!(resolve(&change, Some(false)), Execution::DescribeChange); assert_eq!(resolve(&change, Some(true)), Execution::SendFromEnvironment); - assert!(Execution::DescribeChange.describes() && Execution::DryRun.describes()); + assert!(Execution::DescribeChange.describes()); assert!(!Execution::SendFromEnvironment.describes()); } diff --git a/src/cli.rs b/src/cli.rs index 8eb7f03..c18db40 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -362,24 +362,16 @@ struct InvoiceFlags { copy: bool, } -#[derive(Debug, Args)] -struct DryRunFlags { - /// Validate and show the request without authenticating, confirming, or sending it - #[arg(short = 'n', long, help_heading = "Safety")] - dry_run: bool, -} - #[derive(Debug, Args)] struct ExecuteFlags { /// Send this change. Without it or VOLTAGE_EXECUTE=1, the change is only described - #[arg(long, help_heading = "Safety", conflicts_with = "dry_run")] + #[arg(long, help_heading = "Safety")] execute: bool, } -/// What the command line asked for: `--dry-run`, `--execute`, or neither. +/// What the command line asked for: `--execute`, or nothing. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum Requested { - DryRun, Execute, Unspecified, } @@ -727,7 +719,6 @@ fn api_invocation( .collect(); let target = parse_if::(operation.target.is_some(), m)?; let query = parse::(m)?; - let dry_run = parse::(m)?; let execute = parse_if::(operation.method != Method::Get, m)?; let pagination = parse_if::(operation.has_parameter("limit"), m)?; let checkout = parse_if::(operation.auth.is_checkout(), m)?; @@ -755,9 +746,7 @@ fn api_invocation( qr: invoice.as_ref().is_some_and(|flags| flags.qr), copy: invoice.as_ref().is_some_and(|flags| flags.copy), body, - requested: if dry_run.dry_run { - Requested::DryRun - } else if execute.is_some_and(|flags| flags.execute) { + requested: if execute.is_some_and(|flags| flags.execute) { Requested::Execute } else { Requested::Unspecified @@ -934,7 +923,6 @@ fn endpoint(mut cmd: ClapCommand, operation: &Operation, about: String) -> ClapC cmd = cmd.arg(filter_arg(parameter)); } cmd = QueryFlags::augment_args(cmd); - cmd = DryRunFlags::augment_args(cmd); if operation.method != Method::Get { cmd = ExecuteFlags::augment_args(cmd); } @@ -1054,8 +1042,8 @@ fn endpoint_examples(id: OperationId, command_name: &str) -> Option<&'static str voltage wallets update WALLET_ID --profile prod --name operations --execute" } (OperationId::DeleteWallet, _) => { - " # Check the request, then delete the wallet - voltage wallets delete WALLET_ID --profile prod --dry-run + " # Show the request without --execute, then delete the wallet + voltage wallets delete WALLET_ID --profile prod voltage wallets delete WALLET_ID --profile prod --execute" } (OperationId::UpdateWalletPolicies, _) => { @@ -1076,8 +1064,8 @@ fn endpoint_examples(id: OperationId, command_name: &str) -> Option<&'static str voltage webhooks update WEBHOOK_ID --profile prod --event receive.completed --event send.failed --execute" } (OperationId::DeleteWebhook, _) => { - " # Check the request, then delete the webhook - voltage webhooks delete WEBHOOK_ID --profile prod --dry-run + " # Show the request without --execute, then delete the webhook + voltage webhooks delete WEBHOOK_ID --profile prod voltage webhooks delete WEBHOOK_ID --profile prod --execute" } (OperationId::GenerateWebhookKey, _) => { @@ -1415,22 +1403,18 @@ mod tests { assert_eq!(flags.kind, Some(ReceiveKind::Bolt11)); assert_eq!(flags.currency, Some(Currency::Btc)); assert_eq!(api(&["payments", "create", "--data", "-"]).alias, None); - assert_eq!( - api(&["payments", "send", "-n"]).requested, - Requested::DryRun - ); assert_eq!( api(&["payments", "send", "--execute"]).requested, Requested::Execute ); - assert!( - Invocation::try_parse_from(["voltage", "payments", "send", "-n", "--execute"]).is_err() - ); + assert_eq!(api(&["payments", "send"]).requested, Requested::Unspecified); + // A change without --execute is already the preview, so there is no --dry-run. + assert!(Invocation::try_parse_from(["voltage", "payments", "send", "--dry-run"]).is_err()); + assert!(Invocation::try_parse_from(["voltage", "payments", "send", "-n"]).is_err()); assert!(Invocation::try_parse_from(["voltage", "payments", "list", "--execute"]).is_err()); assert_eq!(api(&["payments", "send"]).command_name(), "payments send"); assert_eq!(api(&["payments", "list"]).command_name(), "payments list"); assert_eq!(api(&["payments", "list"]).requested, Requested::Unspecified); - assert!(Invocation::try_parse_from(["voltage", "login", "--dry-run"]).is_err()); } #[test] @@ -1973,6 +1957,20 @@ mod tests { from_docs >= MIN_DOC_EXAMPLES && examples.len() > from_docs, "{examples:?}" ); + // A change shown without --execute is a deliberate preview only when the same + // command is also documented with --execute; otherwise it forgot to send. + let words = |example: &str| example.split_whitespace().collect::>().join(" "); + let sent: std::collections::HashSet = examples + .iter() + .filter(|example| example.split_whitespace().any(|word| word == "--execute")) + .map(|example| { + example + .split_whitespace() + .filter(|word| *word != "--execute") + .collect::>() + .join(" ") + }) + .collect(); for example in &examples { // Placeholder IDs stand for real UUIDs; every other placeholder parses as typed. // Words split on whitespace with no shell quoting, so an example must not use a @@ -2001,8 +1999,11 @@ mod tests { .unwrap_or_else(|error| panic!("{example}: {error:?}")); if let Command::Api(api) = invocation.command { assert!( - api.operation.method == Method::Get || api.requested != Requested::Unspecified, - "a change without --execute or --dry-run only describes itself: {example}" + api.operation.method == Method::Get + || api.requested != Requested::Unspecified + || sent.contains(&words(example)), + "a change without --execute only describes itself; add --execute, or also \ + document the same command with --execute: {example}" ); } } diff --git a/src/dry_run.rs b/src/dry_run.rs index f3052f5..d8aa34d 100644 --- a/src/dry_run.rs +++ b/src/dry_run.rs @@ -1,4 +1,4 @@ -//! `--dry-run`: describe the request an API command would send, then stop. +//! A change without `--execute`: describe the request it would send, then stop. //! //! The description is built after local validation and before authentication, wallet //! preflight, confirmation, journaling, or any network request. It reports the `Run` that @@ -51,7 +51,7 @@ struct QueryPair<'a> { value: &'a str, } -/// The `data` of a dry-run envelope. +/// The `data` of a described change's envelope (`outcome: dry_run`). #[derive(Serialize)] struct DryRun<'a> { operation: String, @@ -102,7 +102,7 @@ pub fn describe( let mut notes = vec!["No request was sent. Local validation does not prove the API will accept it."]; if generated_id(invocation) { - notes.push("The id was generated for this dry run; a real run generates a new one unless you pass --id."); + notes.push("The id was generated for this description; a real run generates a new one unless you pass --id."); } if matches!(invocation.body, Some(BodySource::Raw(_))) { notes.push("The --data payload was read and validated; a real run reads it again."); diff --git a/src/output.rs b/src/output.rs index db0b029..1c94eca 100644 --- a/src/output.rs +++ b/src/output.rs @@ -86,7 +86,7 @@ pub enum Outcome { Ready, /// The payment settled. Completed, - /// `--dry-run` described a request without sending it. + /// A change without `--execute` was described without being sent. #[serde(rename = "dry_run")] DryRun, } diff --git a/src/output/human.rs b/src/output/human.rs index 88e3e9a..8acfd13 100644 --- a/src/output/human.rs +++ b/src/output/human.rs @@ -284,7 +284,7 @@ struct Line { } /// Nested objects flatten to dotted field names; amounts stay whole. A list of strings, such -/// as a dry run's notes, puts each item on its own line under the first. In a local result, a +/// as a described change's notes, puts each item on its own line under the first. In a local result, a /// `{value, source}` setting reads `name value (source)`. fn fields(out: &mut impl Write, map: &Map, local: bool) -> Result<()> { let mut lines = Vec::new(); diff --git a/src/pick.rs b/src/pick.rs index 8ecf964..48b0294 100644 --- a/src/pick.rs +++ b/src/pick.rs @@ -5,7 +5,7 @@ use crate::{ Result, api::{self, SubmissionState}, auth::{self, ApiCredential, Discovery}, - cli::{BodySource, Command, GlobalFlags, LocalCommand, ProfileCommand, Requested}, + cli::{BodySource, Command, GlobalFlags, LocalCommand, ProfileCommand}, config::{Credential, Scope, Settings}, output::OutputFormat, registry::{OperationId, ParameterLocation, ScopeParameter}, @@ -35,8 +35,6 @@ impl Needs { 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()); @@ -261,12 +259,7 @@ mod tests { needs_of(&["profiles", "create", "prod"]), Needs::new(true, true, false) ); - for args in [ - &["wallets", "list", "--dry-run"][..], - &["organizations", "list"], - &["context"], - &["price"], - ] { + for args in [&["organizations", "list"][..], &["context"], &["price"]] { assert_eq!(needs_of(args), Needs::default(), "{args:?}"); } } diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 4b4c0f0..ea4fc96 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -2208,10 +2208,11 @@ async fn interrupt_during_another_write_reports_an_uncertain_submission() { } #[tokio::test] -async fn dry_run_sends_nothing_records_nothing_and_omits_private_values() { +async fn a_described_change_sends_nothing_records_nothing_and_omits_private_values() { let (server, dir) = fixture().await; let output_file = dir.path().join("result.json"); let planned = cli(dir.path(), &server) + .env_remove("VOLTAGE_EXECUTE") .args([ "payments", "send", @@ -2233,12 +2234,11 @@ async fn dry_run_sends_nothing_records_nothing_and_omits_private_values() { "sats", "--metadata", "order=private-order", - "--dry-run", ]) .arg("--output-file") .arg(&output_file) .assert() - .success(); + .code(6); let envelope = json_stdout(&planned); assert_eq!(envelope["outcome"], "dry_run"); assert_eq!(envelope["http_status"], Value::Null); @@ -2274,26 +2274,13 @@ async fn dry_run_sends_nothing_records_nothing_and_omits_private_values() { // Wallet preflight reads are skipped when describing a delete. let delete = cli(dir.path(), &server) - .args([ - "wallets", "delete", WALLET, "--org", ORG, "--env", ENV, "-n", - ]) + .env_remove("VOLTAGE_EXECUTE") + .args(["wallets", "delete", WALLET, "--org", ORG, "--env", ENV]) .assert() - .success(); + .code(6); let data = json_stdout(&delete)["data"].clone(); assert_eq!(data["method"], "DELETE"); assert_eq!(data["run"]["verify_wallet_environment"], true); - let read = cli(dir.path(), &server) - .args(["wallets", "list", "--org", ORG, "-n"]) - .assert() - .success(); - assert_eq!( - json_stdout(&read)["data"]["run"], - json!({ - "kind": "read", - "check_wallet_environment": false, - "then": {"kind": "pages", "follow": false} - }) - ); assert!(server.received_requests().await.unwrap().is_empty()); } @@ -2322,12 +2309,12 @@ async fn changes_are_described_unless_execute_or_voltage_execute_asks_to_send_th .assert() .code(2); assert!(stderr(&unclear).contains("VOLTAGE_EXECUTE must be 1, true, 0, or false")); - // An explicit --dry-run describes and succeeds, even when the environment says to send. + // VOLTAGE_EXECUTE=0 describes one command even when a script exported it as 1. cli(dir.path(), &server) + .env("VOLTAGE_EXECUTE", "0") .args(delete) - .arg("--dry-run") .assert() - .success(); + .code(6); assert!(server.received_requests().await.unwrap().is_empty()); // Reads are unaffected by the default. @@ -2362,25 +2349,18 @@ async fn changes_are_described_unless_execute_or_voltage_execute_asks_to_send_th } #[tokio::test] -async fn dry_run_reads_raw_bodies_and_shows_them_only_with_show_secrets() { +async fn a_described_change_reads_raw_bodies_and_shows_them_only_with_show_secrets() { let (server, dir) = fixture().await; let body = json!({"id":RESOURCE,"wallet_id":WALLET,"currency":"btc","type":"bolt11","data":{"payment_request":"lnbc1raw"},"extension":{"note":"x"}}); let args = [ - "payments", - "create", - "--org", - ORG, - "--env", - ENV, - "--data", - "-", - "--dry-run", + "payments", "create", "--org", ORG, "--env", ENV, "--data", "-", ]; let hidden = cli(dir.path(), &server) + .env_remove("VOLTAGE_EXECUTE") .args(args) .write_stdin(body.to_string()) .assert() - .success(); + .code(6); let data = json_stdout(&hidden)["data"].clone(); assert_eq!(data["body"]["data"]["payment_request"], "[OMITTED]"); assert_eq!(data["body"]["extension"], "[OMITTED]"); @@ -2388,16 +2368,18 @@ async fn dry_run_reads_raw_bodies_and_shows_them_only_with_show_secrets() { "The --data payload was read and validated; a real run reads it again." ))); let shown = cli(dir.path(), &server) + .env_remove("VOLTAGE_EXECUTE") .args(args) .arg("--show-secrets") .write_stdin(body.to_string()) .assert() - .success(); + .code(6); let data = json_stdout(&shown)["data"].clone(); assert_eq!(data["body"], body); assert_eq!(data["values_omitted"], false); // Local validation still applies. cli(dir.path(), &server) + .env_remove("VOLTAGE_EXECUTE") .args(args) .write_stdin(json!({"wallet_id": WALLET}).to_string()) .assert() @@ -2407,20 +2389,18 @@ async fn dry_run_reads_raw_bodies_and_shows_them_only_with_show_secrets() { } #[tokio::test] -async fn dry_run_describes_checkout_requests_without_a_token() { +async fn checkout_session_reads_send_the_requested_origin() { let (server, dir) = fixture().await; - let watched = cli(dir.path(), &server) - .env_remove("VOLTAGE_STREAM_TOKEN") - .args(["checkout", "events", "watch", "-n"]) - .assert() - .success(); - let data = json_stdout(&watched)["data"].clone(); - assert_eq!(data["run"], json!({"kind": "event_stream"})); - assert_eq!(data["origin"], Value::Null); - // The Origin header decides whether a browser-bound session accepts the request. - let session = cli(dir.path(), &server) - .env_remove("VOLTAGE_CHECKOUT_TOKEN") + respond_once( + &server, + route("GET", format!("/checkout/sessions/{RESOURCE}")) + .and(header("origin", "https://shop.example.test")), + ok(json!({"id": RESOURCE, "status": "ready"})), + ) + .await; + cli(dir.path(), &server) + .env("VOLTAGE_CHECKOUT_TOKEN", CHECKOUT_KEY) .args([ "checkout", "sessions", @@ -2428,15 +2408,10 @@ async fn dry_run_describes_checkout_requests_without_a_token() { RESOURCE, "--origin", "https://shop.example.test", - "-n", ]) .assert() .success(); - assert_eq!( - json_stdout(&session)["data"]["origin"], - "https://shop.example.test" - ); - assert!(server.received_requests().await.unwrap().is_empty()); + server.verify().await; } #[tokio::test(flavor = "multi_thread")] @@ -3702,8 +3677,8 @@ async fn a_terminal_picks_a_missing_wallet_and_scripts_are_never_asked() { 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"] { + // JSON output and --no-input neither ask nor list. + for extra in ["--json", "--no-input"] { let mut args = send.to_vec(); args.push(extra); let run = on_a_terminal(dir.path(), &server, &args, &[]).await; @@ -4203,7 +4178,8 @@ mod examples { const EXAMPLE_INVOICE: &str = "lntbs10n1p4ft36app509h8j7cecenwaktcsrshu6cx4xsczarwfj4nl4tfjq6y7rz79vmqdqqcqzzsxqrrsssp5grs2zd2al99uw93vja4mc5wus9lshcjteuqwef2xkhlp9ppmkens9qxpqysgqmn4s544p8p0263cv83s3c3qjv69z3xxy3fh64ny2y766mfnqfxrrhjj8s7h0lrkxux23dfyzxfydvgjhrsw2p8z0nw54en5a9rvk4kgqxkh73g"; /// A documented placeholder as a value the CLI accepts: IDs become a fixed UUID, an invoice -/// a signet invoice, and an address a BIP 173 testnet address. Dry runs never send them. +/// a signet invoice, and an address a BIP 173 testnet address. Described changes never send +/// them. fn placeholder(word: &str) -> &str { match word { "BOLT11_INVOICE" => EXAMPLE_INVOICE, @@ -4215,9 +4191,9 @@ fn placeholder(word: &str) -> &str { } } -/// Request bodies for the files the documented `--data` examples name, just complete enough -/// for local validation. A new `@FILE` in the docs needs a body here. -fn data_stubs() -> [(&'static str, Value); 3] { +/// Request bodies for the files the documented and help `--data` examples name, just +/// complete enough for local validation. A new `@FILE` in an example needs a body here. +fn data_stubs() -> [(&'static str, Value); 8] { [ ( "payment.json", @@ -4230,6 +4206,17 @@ fn data_stubs() -> [(&'static str, Value); 3] { json!({"delivery_id": RESOURCE, "payload": {"type": "test", "detail": {"event": "created", "data": "Checking the endpoint"}}}), ), + ( + "movement.json", + json!({"id": RESOURCE, "wallet_id": WALLET}), + ), + ("sandbox.json", json!({})), + ( + "session-request.json", + json!({"wallet_id": WALLET, "id": RESOURCE}), + ), + ("stream.json", json!({})), + ("settings.json", json!({})), ] } @@ -4241,11 +4228,10 @@ fn file_names(dir: &Path) -> BTreeSet { .collect() } -/// Every change the docs show with `--execute` also passes as a dry run, offline: flags, -/// units, scope, and `--data` bodies still validate as documented, nothing reaches the API, -/// and no file is created. -#[tokio::test(flavor = "multi_thread")] -async fn documented_changes_pass_as_offline_dry_runs() { +/// Run each example that sends a change without `--execute` instead, and require that it is +/// described (exit 6) without a request reaching the API: flags, units, scope, and `--data` +/// bodies still validate as documented, and no file is created. +async fn assert_changes_describe_offline(examples: Vec, minimum: usize) { let (server, dir) = fixture().await; let mut settings = seeded_api_key(dir.path(), "key", ACCOUNT_KEY); for name in ["prod", "staging", "test"] { @@ -4266,11 +4252,11 @@ async fn documented_changes_pass_as_offline_dry_runs() { // `--data -` reads stdin, which the docs fill with `cat payment.json |`. let stdin = stubs[0].1.to_string(); let before = file_names(dir.path()); - let changes: Vec = examples::doc_examples() + let changes: Vec = examples .into_iter() .filter(|example| example.split_whitespace().any(|word| word == "--execute")) .collect(); - assert!(changes.len() >= 19, "{changes:?}"); + assert!(changes.len() >= minimum, "{changes:?}"); for example in &changes { let args = example .split_whitespace() @@ -4281,18 +4267,19 @@ async fn documented_changes_pass_as_offline_dry_runs() { let mut command = cli(dir.path(), &server); command .current_dir(dir.path()) + .env_remove("VOLTAGE_EXECUTE") .env("VOLTAGE_ORGANIZATION_ID", ORG) .env("VOLTAGE_ENVIRONMENT_ID", ENV) .env("VOLTAGE_WALLET_ID", WALLET) - .args(args) - .arg("--dry-run"); + .args(args); if example.contains("--data -") { command.write_stdin(stdin.clone()); } let result = command.assert(); let output = result.get_output(); - assert!( - output.status.success(), + assert_eq!( + output.status.code(), + Some(6), "{example}: {}", String::from_utf8_lossy(&output.stderr) ); @@ -4304,6 +4291,65 @@ async fn documented_changes_pass_as_offline_dry_runs() { assert!(server.received_requests().await.unwrap().is_empty()); } +/// The lines of the section of `help` under `heading`, up to the next blank line. +fn help_section<'a>(help: &'a str, heading: &str) -> Vec<&'a str> { + help.split_once(&format!("\n{heading}\n")) + .map(|(_, section)| { + section + .lines() + .take_while(|line| !line.trim().is_empty()) + .collect() + }) + .unwrap_or_default() +} + +/// Every example in each command's `--help`, found by walking the command lists the binary +/// prints, as a person or an agent reading help would. +fn help_examples(dir: &Path) -> Vec { + let mut examples = Vec::new(); + let mut pending = vec![Vec::::new()]; + while let Some(path) = pending.pop() { + let output = human_process_at(dir, "http://127.0.0.1:9") + .args(&path) + .arg("--help") + .output() + .unwrap(); + assert!(output.status.success(), "{path:?}"); + let help = String::from_utf8(output.stdout).unwrap(); + for line in help_section(&help, "Commands:") { + let name = line.split_whitespace().next().unwrap_or_default(); + if name != "help" { + pending.push(path.iter().cloned().chain([name.to_owned()]).collect()); + } + } + examples.extend( + help_section(&help, "Examples:") + .into_iter() + .map(str::trim) + .filter(|line| line.starts_with("voltage ")) + .map(|line| { + line.split(['<', '>', '|']) + .next() + .unwrap_or_default() + .trim() + .to_owned() + }), + ); + } + examples +} + +#[tokio::test(flavor = "multi_thread")] +async fn documented_changes_describe_themselves_offline() { + assert_changes_describe_offline(examples::doc_examples(), 19).await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn help_examples_describe_themselves_offline() { + let dir = private_tempdir(); + assert_changes_describe_offline(help_examples(dir.path()), 15).await; +} + // --------------------------------------------------------------------------------------- // Prices and conversions // --------------------------------------------------------------------------------------- diff --git a/tests/support/examples.rs b/tests/support/examples.rs index 66717cc..81acd39 100644 --- a/tests/support/examples.rs +++ b/tests/support/examples.rs @@ -1,5 +1,5 @@ // Documentation examples, shared by the unit test that parses them and the integration -// test that runs every documented change offline with `--dry-run`. Included with +// test that describes every documented change offline, without `--execute`. Included with // `include!`, so it names its own imports. use std::path::PathBuf;