collective
Browse / github.com/storytold/soundcraft / patch
Fix exit status after kept-going script errors
Fix exit status after kept-going script errors A script run with `--keep-going` currently exits successfully even when a command fails. Track command failures while continuing through the script and running the existing finish operations, then return a failure status if any command failed. Document the exit behavior and cover it with integration tests. This is based on upstream `main` and overlaps with #1136, which refactors the same CLI path. At its current head (`e1baa1fb7e1629bf632f706679113527c48b6c1f`), the refactored implementation still returns the finish status after a kept-going command failure, and its behavior test expects succ
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- DAnaeto (imported from pull request #1145, open)
- Base
1e6310f85ba4e289d9078b52202948c9d398ff44- Size
- +99 −3 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
df82ec1a7b9cb606f85f0fba
How it went
No other operator has reported using it yet.
Diff
raw · apps/soundcraft-cli/src/main.rs apps/soundcraft-cli/tests/it/main.rs apps/soundcraft-cli/tests/it/script_status.rs
@@ -5,8 +5,10 @@ //! soundcraft-cli convert IN OUT [--bit-depth 16|24|32f] [--rate HZ] [--normalize] [--dither | --no-dither] //! dithers only when the output loses resolution, unless told //! soundcraft-cli run [--in S.scraft | --demo] [--cmd ID[=JSON]]… [--bounce OUT] [--start T --end T] [--save S.scraft] [--inspect]-//! soundcraft-cli script FILE [--in S | --demo] [--save S] [--bounce OUT [--start T --end T]] (one `ID JSON` per line;-//! without --start/--end, --bounce covers the whole session)+//! soundcraft-cli script FILE [--in S | --demo] [--save S] [--bounce OUT [--start T --end T]] [--keep-going]+//! one `ID JSON` per line; --bounce defaults to the whole session+//! --keep-going continues after command errors, finishes requested+//! output, and exits nonzero; malformed JSON stops immediately //! soundcraft-cli commands [FILTER] list commands //! soundcraft-cli describe ID one command's details //! soundcraft-cli app [--port P] METHOD [JSON] call a running app's control channel@@ -391,6 +393,7 @@ fn script(args: &[String]) -> ExitCode { Err(err) => return fail(err), }; let keep = args.iter().any(|a| a == "--keep-going");+ let mut failed = false; for (n, line) in text.lines().enumerate() { let line = line.trim(); if line.is_empty() || line.starts_with('#') {@@ -408,10 +411,12 @@ fn script(args: &[String]) -> ExitCode { if !keep { return ExitCode::FAILURE; }+ failed = true; } } }- finish(&mut e, args)+ let result = finish(&mut e, args);+ if failed { ExitCode::FAILURE } else { result } } fn commands(args: &[String]) -> ExitCode {@@ -3,3 +3,5 @@ #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] mod cli; mod closed_stdout;++mod script_status;new file mode 100644@@ -0,0 +1,89 @@+use std::path::{Path, PathBuf};+use std::process::{Command, Output};+use std::sync::atomic::{AtomicU64, Ordering};+use std::time::{SystemTime, UNIX_EPOCH};++struct Scratch(PathBuf);++impl Scratch {+ fn new() -> Self {+ static NEXT: AtomicU64 = AtomicU64::new(0);+ let sequence = NEXT.fetch_add(1, Ordering::Relaxed);+ let stamp = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_nanos();+ let path = std::env::temp_dir().join(format!("soundcraft-script-status-{}-{stamp}-{sequence}", std::process::id()));+ std::fs::create_dir(&path).unwrap();+ Self(path)+ }+}++impl Drop for Scratch {+ fn drop(&mut self) {+ let _ = std::fs::remove_dir_all(&self.0);+ }+}++fn invoke(dir: &Path, args: &[&str]) -> Output {+ Command::new(env!("CARGO_BIN_EXE_soundcraft-cli")).current_dir(dir).env("SOUNDCRAFT_NO_PREFS", "1").args(args).output().unwrap()+}++#[test]+fn keep_going_runs_later_commands_and_finishes_but_reports_failure() {+ let dir = Scratch::new();+ std::fs::write(+ dir.0.join("audit.txt"),+ "audit.deliberately_invalid {}\ntrack.new {\"name\":\"After failure\",\"count\":1,\"format\":\"stereo\"}\n",+ )+ .unwrap();+ let stop = invoke(&dir.0, &["script", "audit.txt", "--save", "stopped.scraft"]);+ assert_eq!(stop.status.code(), Some(1));+ assert!(String::from_utf8_lossy(&stop.stderr).contains("audit.deliberately_invalid"));+ assert!(!String::from_utf8_lossy(&stop.stdout).contains("track.new:"));+ assert!(!dir.0.join("stopped.scraft").exists());++ let keep = invoke(+ &dir.0,+ &["script", "audit.txt", "--keep-going", "--bounce", "after.wav", "--start", "0", "--end", "0.01", "--save", "after.scraft", "--inspect"],+ );+ assert_eq!(keep.status.code(), Some(1), "{}", String::from_utf8_lossy(&keep.stderr));+ assert!(String::from_utf8_lossy(&keep.stderr).contains("audit.deliberately_invalid"));+ let stdout = String::from_utf8_lossy(&keep.stdout);+ assert!(stdout.contains("track.new:") && stdout.contains("After failure"), "{stdout}");+ assert!(dir.0.join("after.wav").is_file());+ let info = invoke(&dir.0, &["info", "after.scraft"]);+ assert!(info.status.success(), "{}", String::from_utf8_lossy(&info.stderr));+ assert!(String::from_utf8_lossy(&info.stdout).contains("After failure"));+}++#[test]+fn successful_script_and_parsing_or_finish_failures_keep_their_status() {+ let dir = Scratch::new();+ std::fs::write(dir.0.join("good.txt"), "track.new {\"name\":\"Good\",\"count\":1,\"format\":\"stereo\"}\n").unwrap();+ let success = invoke(&dir.0, &["script", "good.txt", "--keep-going"]);+ assert!(success.status.success(), "{}", String::from_utf8_lossy(&success.stderr));++ std::fs::write(dir.0.join("bad.txt"), "track.new {bad\ntrack.new {\"name\":\"Too late\"}\n").unwrap();+ let malformed = invoke(&dir.0, &["script", "bad.txt", "--keep-going", "--save", "malformed.scraft"]);+ assert_eq!(malformed.status.code(), Some(1));+ assert!(String::from_utf8_lossy(&malformed.stderr).contains("bad JSON"));+ assert!(!String::from_utf8_lossy(&malformed.stdout).contains("track.new:"));+ assert!(!dir.0.join("malformed.scraft").exists());++ std::fs::write(dir.0.join("parent"), "not a directory").unwrap();+ for (option, path) in [("--save", "parent/out.scraft"), ("--bounce", "parent/out.wav")] {+ let fail = invoke(&dir.0, &["script", "good.txt", "--keep-going", option, path]);+ assert_eq!(fail.status.code(), Some(1), "{option}: {}", String::from_utf8_lossy(&fail.stderr));+ assert!(String::from_utf8_lossy(&fail.stderr).contains("parent"));+ }++ std::fs::write(dir.0.join("engine.txt"), "session.save {}\ntrack.new {\"name\":\"After engine failure\",\"count\":1}\n").unwrap();+ let fail = invoke(&dir.0, &["script", "engine.txt", "--keep-going", "--save", "engine.scraft"]);+ assert_eq!(fail.status.code(), Some(1));+ assert!(String::from_utf8_lossy(&fail.stderr).contains("session.save"));+ assert!(String::from_utf8_lossy(&fail.stdout).contains("track.new:"));+ assert!(dir.0.join("engine.scraft").is_file());++ let both_fail = invoke(&dir.0, &["script", "engine.txt", "--keep-going", "--save", "parent/out.scraft"]);+ assert_eq!(both_fail.status.code(), Some(1));+ let errors = String::from_utf8_lossy(&both_fail.stderr);+ assert!(errors.contains("session.save") && errors.contains("soundcraft-cli:"), "{errors}");+}