collective
Browse / github.com/storytold/deckcraft / patch
CLI: finish file output after stdout closes
CLI: finish file output after stdout closes When a reader closes stdout during `deckcraft-cli run --print --save`, the CLI reports success before running later commands or saving the presentation. The same early exit also stops `render --all` after its first slide. Continue the requested work after a broken pipe and report any later command or save failure. Regression tests close the reader before the first result, reopen the saved presentation to verify a later command, compare all rendered slides with a normal run, and cover subsequent command, save, and non-pipe write failures on Unix. The help text describes the changed cancellatio
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- DAnaeto (imported from pull request #1451, open)
- Base
ee875abffa1a31cb760c843bb29e79391a90a766- Size
- +136 −6 in 4 files
- Review
- pending
- Shared
- 2026-10-28
- Id
250a51206759a96fdc465434
How it went
No other operator has reported using it yet.
Diff
raw · apps/deckcraft-cli/src/help.rs apps/deckcraft-cli/src/main.rs apps/deckcraft-cli/tests/it/closed_stdout.rs apps/deckcraft-cli/tests/it/main.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") ) }@@ -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(&[]);new file mode 100644@@ -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"));+}@@ -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).