Merge pull request #1451

CLI: finish file output after stdout closes

apps/deckcraft-cli/src/help.rs

@@ -180,7 +180,10 @@ A presentation is edited by running commands on it:
   deckcraft-cli run --cmd 'slide.new={{\"title\":\"Q3 results\"}}' --save q3.deckcraft
 
 `deckcraft-cli help SUBCOMMAND`, or `deckcraft-cli SUBCOMMAND --help`, explains one subcommand;
-`deckcraft-cli help run` explains how to write a command.",
+`deckcraft-cli help run` explains how to write a command.
+
+If an output reader closes early, run, render, render-transition and convert continue; their exit
+status reflects the work, not output delivery. Stop the process to cancel them.",
         list.join("\n")
     )
 }

apps/deckcraft-cli/src/main.rs

@@ -7,23 +7,38 @@ mod automation_args;
 mod help;
 
 use std::process::ExitCode;
+use std::sync::atomic::{AtomicBool, Ordering};
 
 use deckcraft_engine::Session;
 use serde_json::{Value, json};
 
+static STDOUT_CLOSED: AtomicBool = AtomicBool::new(false);
+
 macro_rules! outln {
     ($($arg:tt)*) => {{
-        use std::io::Write as _;
-        if let Err(e) = writeln!(std::io::stdout(), $($arg)*) {
+        if !STDOUT_CLOSED.load(Ordering::Relaxed) && let Err(e) = stdout_line(format_args!($($arg)*)) {
             if e.kind() == std::io::ErrorKind::BrokenPipe {
-                std::process::exit(0);
+                STDOUT_CLOSED.store(true, Ordering::Relaxed);
+            } else {
+                eprintln!("deckcraft-cli: can't write to stdout: {e}");
+                std::process::exit(1);
             }
-            eprintln!("deckcraft-cli: can't write to stdout: {e}");
-            std::process::exit(1);
         }
     }};
 }
 
+fn stdout_line(args: std::fmt::Arguments<'_>) -> std::io::Result<()> {
+    use std::io::Write as _;
+    #[cfg(unix)]
+    let mut stdout = {
+        use std::os::fd::AsFd as _;
+        std::fs::File::from(std::io::stdout().as_fd().try_clone_to_owned()?)
+    };
+    #[cfg(not(unix))]
+    let mut stdout = std::io::stdout();
+    stdout.write_all(format!("{args}\n").as_bytes())
+}
+
 fn main() -> ExitCode {
     let args: Vec<String> = std::env::args().skip(1).collect();
     let rest = args.get(1..).unwrap_or(&[]);

apps/deckcraft-cli/tests/it/closed_stdout.rs

@@ -0,0 +1,110 @@
+use std::fs::{self, File};
+use std::os::fd::OwnedFd;
+use std::os::unix::net::UnixStream;
+use std::path::{Path, PathBuf};
+use std::process::{Command, Output, Stdio};
+use std::time::{SystemTime, UNIX_EPOCH};
+
+struct Scratch(PathBuf);
+
+impl Scratch {
+    fn new(name: &str) -> Self {
+        let nonce = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_nanos();
+        let path = std::env::temp_dir().join(format!("deckcraft-closed-stdout-{name}-{}-{nonce}", std::process::id()));
+        fs::create_dir(&path).unwrap();
+        Self(path)
+    }
+}
+
+impl Drop for Scratch {
+    fn drop(&mut self) {
+        let _ = fs::remove_dir_all(&self.0);
+    }
+}
+
+fn closed_stdout() -> Stdio {
+    let (writer, reader) = UnixStream::pair().unwrap();
+    drop(reader);
+    Stdio::from(OwnedFd::from(writer))
+}
+
+fn run(dir: &Path, args: &[&str], stdout: Stdio) -> Output {
+    Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).current_dir(dir).args(args).stdout(stdout).stderr(Stdio::piped()).output().unwrap()
+}
+
+#[test]
+fn broken_pipe_does_not_skip_later_commands_or_save() {
+    let dir = Scratch::new("save");
+    let out = run(
+        &dir.0,
+        &[
+            "run",
+            "--cmd",
+            r#"slide.new={"layout":"titleOnly","title":"First"}"#,
+            "--cmd",
+            r#"slide.new={"layout":"titleOnly","title":"Later"}"#,
+            "--print",
+            "--save",
+            "out.deckcraft",
+        ],
+        closed_stdout(),
+    );
+    assert!(out.status.success(), "exit {:?}: {}", out.status, String::from_utf8_lossy(&out.stderr));
+    let saved = dir.0.join("out.deckcraft");
+    assert!(saved.is_file(), "requested save was skipped");
+    let info = Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).arg("info").arg(saved).output().unwrap();
+    assert!(info.status.success(), "{}", String::from_utf8_lossy(&info.stderr));
+    let data: serde_json::Value = serde_json::from_slice(&info.stdout).unwrap();
+    let slides = data["slides"].as_array().unwrap();
+    assert!(slides.iter().any(|slide| slide["title"] == "Later"), "later command did not persist: {data}");
+}
+
+#[test]
+fn broken_pipe_does_not_hide_a_later_command_failure() {
+    let dir = Scratch::new("command-failure");
+    let out = run(
+        &dir.0,
+        &["run", "--cmd", r#"slide.new={"title":"First"}"#, "--cmd", r#"slide.go={"index":999}"#, "--print", "--save", "out.deckcraft"],
+        closed_stdout(),
+    );
+    assert!(!out.status.success(), "a later command failure returned success");
+    assert!(!dir.0.join("out.deckcraft").exists(), "failed run unexpectedly saved");
+    assert!(String::from_utf8_lossy(&out.stderr).contains("no slide 999"), "missing command failure diagnostic");
+}
+
+#[test]
+fn broken_pipe_does_not_hide_a_save_failure() {
+    let dir = Scratch::new("save-failure");
+    fs::create_dir(dir.0.join("out.deckcraft")).unwrap();
+    let out = run(&dir.0, &["run", "--cmd", r#"slide.new={"title":"First"}"#, "--print", "--save", "out.deckcraft"], closed_stdout());
+    assert!(!out.status.success(), "a failed save returned success");
+    assert!(!out.stderr.is_empty(), "missing save failure diagnostic");
+}
+
+#[test]
+fn broken_pipe_does_not_skip_later_rendered_slides() {
+    let dir = Scratch::new("render-all");
+    let baseline = run(&dir.0, &["render", "--sample", "--all", "--scale", "0.1", "normal"], Stdio::piped());
+    assert!(baseline.status.success(), "{}", String::from_utf8_lossy(&baseline.stderr));
+    let closed = run(&dir.0, &["render", "--sample", "--all", "--scale", "0.1", "closed"], closed_stdout());
+    assert!(closed.status.success(), "{}", String::from_utf8_lossy(&closed.stderr));
+    let names = |folder: &str| {
+        let mut files: Vec<_> = fs::read_dir(dir.0.join(folder)).unwrap().map(|entry| entry.unwrap().file_name()).collect();
+        files.sort();
+        files
+    };
+    let expected = names("normal");
+    assert!(expected.len() > 1, "sample must have multiple slides");
+    assert_eq!(names("closed"), expected, "rendering stopped after stdout closed");
+}
+
+#[test]
+fn other_stdout_write_errors_fail() {
+    let dir = Scratch::new("write-failure");
+    let path = dir.0.join("readonly");
+    fs::write(&path, b"").unwrap();
+    let readonly = File::open(path).unwrap();
+    let out = run(&dir.0, &["run", "--cmd", r#"slide.new={"title":"First"}"#, "--print"], Stdio::from(readonly));
+    assert!(!out.status.success(), "a non-broken stdout error returned success");
+    assert!(String::from_utf8_lossy(&out.stderr).contains("can't write to stdout"));
+}

apps/deckcraft-cli/tests/it/main.rs

@@ -1,4 +1,6 @@
 mod cli;
+#[cfg(unix)]
+mod closed_stdout;
 mod convert;
 mod damaged;
 // Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md).