diff --git a/CHANGELOG.md b/CHANGELOG.md index 843cca0..8a6cae8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes ## Unreleased +- `--output-file` on a path that already exists says so and suggests a new path, instead of showing an OS error; any other refusal keeps its cause and gets the same hint. Both say that no change was sent. - `-o` is short for `--output`, as in `voltage wallets list -o json`. ## 0.2.0 - 2026-10-22 diff --git a/src/output.rs b/src/output.rs index 95ee613..54b69de 100644 --- a/src/output.rs +++ b/src/output.rs @@ -188,6 +188,22 @@ pub struct Output { file: Option, } +/// Reserve `--output-file`. It is reserved before any request, so a refusal sent nothing; the +/// error says why the path was refused and what to pass instead. +fn reserve_output_file(path: &Path) -> Result { + new_private(path).map_err(|error| { + if path.symlink_metadata().is_ok() { + Error::usage(format!( + "{} already exists; --output-file writes only to a new file", + path.display() + )) + .with_hint("Pass a path that does not exist yet, or move the existing file. No change was sent.") + } else { + error.and_hint("Pass a path in a directory you can write to. No change was sent.") + } + }) +} + impl Output { /// `output_file` is reserved immediately as a new owner-only file so a secret never /// waits on a path that another process could create first. @@ -196,7 +212,7 @@ impl Output { show_secrets: bool, output_file: Option<&Path>, ) -> Result { - let file = output_file.map(new_private).transpose()?; + let file = output_file.map(reserve_output_file).transpose()?; Ok(Self { format, show_secrets, diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 9f68120..7238e31 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -2557,6 +2557,55 @@ async fn a_described_change_sends_nothing_records_nothing_and_omits_private_valu assert!(server.received_requests().await.unwrap().is_empty()); } +#[tokio::test] +async fn output_file_refusals_name_the_path_problem_and_send_nothing() { + let (server, dir) = fixture().await; + let existing = dir.path().join("result.json"); + std::fs::write(&existing, "kept").unwrap(); + let delete = ["wallets", "delete", WALLET, "--org", ORG, "--execute"]; + let refused = cli(dir.path(), &server) + .args(delete) + .arg("--output-file") + .arg(&existing) + .assert() + .code(2); + let error = &json_error(&refused)["error"]; + assert_eq!( + error["message"], + format!( + "{} already exists; --output-file writes only to a new file", + existing.display() + ) + ); + assert_eq!( + error["hint"], + "Pass a path that does not exist yet, or move the existing file. No change was sent." + ); + assert_eq!(std::fs::read_to_string(&existing).unwrap(), "kept"); + + // Any other failure keeps its cause and gets the same next step. + let missing_dir = dir.path().join("missing").join("result.json"); + let refused = cli(dir.path(), &server) + .args(delete) + .arg("--output-file") + .arg(&missing_dir) + .assert() + .code(2); + let error = &json_error(&refused)["error"]; + assert!( + error["message"].as_str().unwrap().starts_with(&format!( + "Cannot reserve output {}: ", + missing_dir.display() + )), + "{error}" + ); + assert_eq!( + error["hint"], + "Pass a path in a directory you can write to. No change was sent." + ); + assert!(server.received_requests().await.unwrap().is_empty()); +} + #[tokio::test] async fn changes_are_described_unless_execute_or_voltage_execute_asks_to_send_them() { let (server, dir) = fixture().await;