CLI: a reader that closes stdout early is not a crash (#167)

The CLI printed with println!/eprintln!, which panic when the stream is
gone, so `exec --list | head` (or any output larger than the pipe buffer)
ended in a broken-pipe panic with exit status 101.

Output now goes through say!/note!, which never panic. Once stdout fails,
later output is dropped but the command still finishes its work (a
`run ... --save` sequence isn't cut short); a closed pipe is quiet and
exits 0, any other stdout error is reported and makes the exit status 1.
The MCP server ends quietly when its client closes stdout.

Test: a_closed_stdout_is_not_a_crash (info --json, exec --list, a saving
run sequence and mcp with stdout closed before they write).

Co-Authored-By: Claude Opus 5.5 <[email protected]>

apps/effectcraft-cli/src/main.rs

@@ -36,10 +36,54 @@
 
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 
+use std::io::Write;
+
 use effectcraft_automation::tools::{self, Reply};
 use effectcraft_automation::{Backend, McpServer};
 use serde_json::{Value, json};
 
+/// `println!` that never panics: see [`write_line`].
+macro_rules! say {
+    ($($t:tt)*) => {
+        write_line(format_args!($($t)*))
+    };
+}
+
+/// `eprintln!` that never panics: a diagnostic that can't be written is dropped.
+macro_rules! note {
+    ($($t:tt)*) => {{
+        let _ = writeln!(std::io::stderr(), $($t)*);
+    }};
+}
+
+/// Why stdout stopped taking output, once it has.
+static STDOUT_FAILED: std::sync::OnceLock<std::io::ErrorKind> = std::sync::OnceLock::new();
+
+/// Write a line to stdout. Once that fails, later output is dropped but the command still
+/// finishes its work (a `run … --save` sequence isn't cut short). A reader that closes stdout
+/// early (`info --json | head`) has read what it wanted; any other failure is reported and makes
+/// the exit status 1 ([`stdout_broken`]).
+fn write_line(text: std::fmt::Arguments) {
+    if STDOUT_FAILED.get().is_some() {
+        return;
+    }
+    let r = {
+        let mut out = std::io::stdout().lock();
+        out.write_fmt(text).and_then(|()| out.write_all(b"\n"))
+    };
+    if let Err(e) = r {
+        let _ = STDOUT_FAILED.set(e.kind());
+        if e.kind() != std::io::ErrorKind::BrokenPipe {
+            note!("effectcraft-cli: cannot write to stdout: {e}");
+        }
+    }
+}
+
+/// Whether stdout failed for a reason other than its reader closing it early.
+fn stdout_broken() -> bool {
+    STDOUT_FAILED.get().is_some_and(|k| *k != std::io::ErrorKind::BrokenPipe)
+}
+
 const USAGE: &str = "usage: effectcraft-cli <info|commands|exec|run|props|get|set|render-frame|render|script|mcp> [args] [--json]
   info                                     project + engine summary
   commands [--filter TEXT] [--enabled]     list engine commands
@@ -173,11 +217,11 @@ fn value_arg(s: &str) -> Value {
 fn main() {
     let raw: Vec<String> = std::env::args().skip(1).collect();
     if matches!(raw.first().map(String::as_str), Some("--version" | "-V" | "version")) {
-        println!("effectcraft-cli {}", env!("CARGO_PKG_VERSION"));
+        say!("effectcraft-cli {}", env!("CARGO_PKG_VERSION"));
         return;
     }
     if raw.is_empty() || raw.iter().any(|a| a == "-h" || a == "--help") {
-        println!("{USAGE}");
+        say!("{USAGE}");
         std::process::exit(if raw.is_empty() { 2 } else { 0 });
     }
     let mut args = match Args::parse(raw) {
@@ -190,20 +234,21 @@ fn main() {
     }
     let cmd = args.pos.remove(0);
     match run(&cmd, &args, json_out) {
+        Ok(()) if stdout_broken() => std::process::exit(1),
         Ok(()) => {}
         Err(Failure::Usage(e)) => fail_usage(&e),
         Err(Failure::Error(e)) => {
             if json_out {
-                println!("{}", json!({"error": e}));
+                say!("{}", json!({"error": e}));
             }
-            eprintln!("effectcraft-cli {cmd}: {e}");
+            note!("effectcraft-cli {cmd}: {e}");
             std::process::exit(1);
         }
     }
 }
 
 fn fail_usage(e: &str) -> ! {
-    eprintln!("effectcraft-cli: {e}\n{USAGE}");
+    note!("effectcraft-cli: {e}\n{USAGE}");
     std::process::exit(2)
 }
 
@@ -292,13 +337,13 @@ fn script_cmd(args: &Args, json_out: bool) -> Result<(), Failure> {
         emit(&o, true);
     } else {
         if let Some(out) = r["output"].as_str().filter(|o| !o.is_empty()) {
-            println!("{out}");
+            say!("{out}");
         }
         if !r["result"].is_null() && r["error"].is_null() {
-            println!("{}", r["result"]);
+            say!("{}", r["result"]);
         }
         if let Some(p) = saved {
-            eprintln!("saved {p}");
+            note!("saved {p}");
         }
     }
     if let Some(e) = r.get("error").filter(|e| !e.is_null()) {
@@ -319,9 +364,9 @@ fn script_cmd(args: &Args, json_out: bool) -> Result<(), Failure> {
 /// Print a result: compact JSON with `--json`, else pretty JSON.
 fn emit(v: &Value, json_out: bool) {
     if json_out {
-        println!("{v}");
+        say!("{v}");
     } else {
-        println!("{}", serde_json::to_string_pretty(v).unwrap_or_default());
+        say!("{}", serde_json::to_string_pretty(v).unwrap_or_default());
     }
 }
 
@@ -344,19 +389,19 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> {
             if json_out {
                 emit(&info, true);
             } else {
-                println!(
+                say!(
                     "EffectCraft {} ({}), {} commands, {} effects",
                     info["version"].as_str().unwrap_or(""),
                     info["mode"].as_str().unwrap_or(""),
                     info["commands"],
                     info["effects"]
                 );
-                println!("project: {}", p["path"].as_str().unwrap_or("(unsaved)"));
+                say!("project: {}", p["path"].as_str().unwrap_or("(unsaved)"));
                 for i in p["items"].as_array().into_iter().flatten() {
-                    println!("  item {:>3}  {:<12} {}", i["id"], i["type"].as_str().unwrap_or(""), i["name"].as_str().unwrap_or(""));
+                    say!("  item {:>3}  {:<12} {}", i["id"], i["type"].as_str().unwrap_or(""), i["name"].as_str().unwrap_or(""));
                 }
                 if let Some(c) = comp {
-                    println!(
+                    say!(
                         "active comp {} \"{}\" {}x{} @ {} fps, {} s",
                         c["id"],
                         c["name"].as_str().unwrap_or(""),
@@ -366,7 +411,7 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> {
                         c["duration"]
                     );
                     for l in c["layers"].as_array().into_iter().flatten() {
-                        println!("  #{:<3} id {:<4} {:<10} {}", l["index"], l["id"], l["type"].as_str().unwrap_or(""), l["name"].as_str().unwrap_or(""));
+                        say!("  #{:<3} id {:<4} {:<10} {}", l["index"], l["id"], l["type"].as_str().unwrap_or(""), l["name"].as_str().unwrap_or(""));
                     }
                 }
             }
@@ -422,7 +467,7 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> {
             if json_out {
                 emit(&v, true);
             } else {
-                println!("layer {} \"{}\" ({}) at {} s", v["id"], v["name"].as_str().unwrap_or(""), v["type"].as_str().unwrap_or(""), v["time"]);
+                say!("layer {} \"{}\" ({}) at {} s", v["id"], v["name"].as_str().unwrap_or(""), v["type"].as_str().unwrap_or(""), v["time"]);
                 for p in v["properties"].as_array().into_iter().flatten() {
                     let mut extra = String::new();
                     if let Some(k) = p.get("keys") {
@@ -431,7 +476,7 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> {
                     if let Some(e) = p.get("expression").and_then(Value::as_str) {
                         extra += &format!("  expr: {e}");
                     }
-                    println!("  {:<44} {:<8} {}{extra}", p["path"].as_str().unwrap_or(""), p["type"].as_str().unwrap_or(""), p["value"]);
+                    say!("  {:<44} {:<8} {}{extra}", p["path"].as_str().unwrap_or(""), p["type"].as_str().unwrap_or(""), p["value"]);
                 }
             }
         }
@@ -481,12 +526,16 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> {
             if json_out {
                 emit(&info, true);
             } else {
-                eprintln!("rendered {}x{} at {:.3}s in {ms:.1} ms -> {out}", f.width, f.height, f.time);
+                note!("rendered {}x{} at {:.3}s in {ms:.1} ms -> {out}", f.width, f.height, f.time);
             }
         }
         "mcp" => {
             let b = backend(args, false)?;
-            McpServer::new(b).serve_stdio().map_err(|e| e.to_string())?;
+            // A client that closes its end of stdout has ended the session.
+            match McpServer::new(b).serve_stdio() {
+                Err(e) if e.kind() != std::io::ErrorKind::BrokenPipe => return Err(e.to_string().into()),
+                _ => {}
+            }
         }
         other => return usage_err(&format!("unknown subcommand `{other}`")),
     }
@@ -577,7 +626,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> {
             let _ = s.execute("renderQueue.setRender", json!({"index": k + 1, "render": false}));
         }
         let r = s.execute("renderQueue.add", p).map_err(err)?;
-        eprintln!(
+        note!(
             "{} → {} ({}×{}, {} frames, {})",
             r["compName"].as_str().unwrap_or("?"),
             r["outputPath"].as_str().unwrap_or("?"),
@@ -593,7 +642,8 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> {
         std::thread::sleep(std::time::Duration::from_millis(100));
         if tty && let Some(p) = s.render_progress() {
             let left = p.remaining.map(|r| format!(", ~{r:.1}s left")).unwrap_or_default();
-            eprint!(
+            let _ = write!(
+                std::io::stderr(),
                 "
   [{}/{}] frame {}/{}  {:.1}s{left}\x1b[K",
                 (p.items_done + 1).min(p.items_total),
@@ -607,7 +657,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> {
     }
     s.poll_render();
     if tty {
-        eprintln!();
+        note!();
     }
     let mut results = vec![];
     let mut failed = vec![];
@@ -615,7 +665,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> {
         let name = s.project.item(it.comp).map(|i| i.name.clone()).unwrap_or_else(|| "?".into());
         match &it.status {
             RenderStatus::Done => {
-                eprintln!("done: {name} → {} in {:.2}s", it.last_output.as_deref().unwrap_or("?"), it.render_time.unwrap_or(0.0));
+                note!("done: {name} → {} in {:.2}s", it.last_output.as_deref().unwrap_or("?"), it.render_time.unwrap_or(0.0));
                 results.push(json!({"comp": name, "output": it.last_output, "seconds": it.render_time}));
             }
             RenderStatus::Failed(e) => failed.push(format!("{name}: {e}")),
@@ -640,7 +690,7 @@ fn list_commands_cmd(args: &Args, json_out: bool) -> Result<(), Failure> {
         emit(&v, true);
     } else {
         for c in v.as_array().into_iter().flatten() {
-            println!(
+            say!(
                 "{:<36} {:<36} {:<16} {}",
                 c["id"].as_str().unwrap_or(""),
                 c["label"].as_str().unwrap_or(""),
@@ -708,7 +758,7 @@ fn bench_ops(args: &Args) -> Result<(), Failure> {
     m.extend(effectcraft_ui_egui::bench::ui_ops(|| {
         let mut s = effectcraft_host::session();
         if let Err(e) = s.execute("file.open", json!({"path": path})) {
-            eprintln!("bench --ops: {e}");
+            note!("bench --ops: {e}");
         }
         s
     }));
@@ -716,9 +766,9 @@ fn bench_ops(args: &Args) -> Result<(), Failure> {
     if args.flag("--json") {
         emit(&json!({"spec": format!("{spec:?}"), "measures": m.iter().map(perf::Measure::json).collect::<Vec<_>>()}), true);
     } else {
-        println!("bench --ops: {} comps, {} layers in Main, {} footage items", spec.comps, spec.main_layers, spec.footage);
+        say!("bench --ops: {} comps, {} layers in Main, {} footage items", spec.comps, spec.main_layers, spec.footage);
         for x in &m {
-            println!("  {:<22} {:>10.2} ms  {}", x.name, x.ms, x.note);
+            say!("  {:<22} {:>10.2} ms  {}", x.name, x.ms, x.note);
         }
     }
     Ok(())
@@ -788,7 +838,7 @@ fn bench(s: &Session, cid: ItemId, t: Tick, opts: RenderOpts, n: usize) {
             }
         }
     }
-    eprintln!(
+    note!(
         "bench (no cache): {n} runs at scale {}: wall min {:.2} ms, median {:.2} ms; CPU (all threads) min {:.2} ms",
         opts.scale,
         min(&totals),
@@ -796,9 +846,9 @@ fn bench(s: &Session, cid: ItemId, t: Tick, opts: RenderOpts, n: usize) {
         min(&cpus)
     );
     for (d, name, p, c, fx) in per_layer {
-        eprintln!("  {:indent$}{name:<28} process {:8.2} ms  composite {:8.2} ms  (min)", "", min(&p), min(&c), indent = d * 2);
+        note!("  {:indent$}{name:<28} process {:8.2} ms  composite {:8.2} ms  (min)", "", min(&p), min(&c), indent = d * 2);
         for (id, v) in fx {
-            eprintln!("  {:indent$}  fx {id:<30} {:8.2} ms", "", min(&v), indent = d * 2);
+            note!("  {:indent$}  fx {id:<30} {:8.2} ms", "", min(&v), indent = d * 2);
         }
     }
 }
@@ -819,7 +869,7 @@ fn bench_play(s: &Session, cid: ItemId, t: Tick, opts: RenderOpts, n: usize) {
         let mut v: Vec<f64> = runs.iter().map(|r| r.0).collect();
         let mean = v.iter().sum::<f64>() / n as f64;
         let cpu = runs.iter().map(|r| r.1).sum::<f64>() / n as f64;
-        eprintln!(
+        note!(
             "bench-play ({label}): {n} frames at scale {}: wall mean {mean:.2} ms, median {:.2} ms, min {:.2} ms; CPU mean {cpu:.2} ms",
             opts.scale,
             median(&mut v),
@@ -827,7 +877,7 @@ fn bench_play(s: &Session, cid: ItemId, t: Tick, opts: RenderOpts, n: usize) {
         );
     }
     let st = cache.stats();
-    eprintln!("  layer cache: {} hits, {} misses, {} entries, {:.1} MB", st.hits, st.misses, st.entries, st.bytes as f64 / 1e6);
+    note!("  layer cache: {} hits, {} misses, {} entries, {:.1} MB", st.hits, st.misses, st.entries, st.bytes as f64 / 1e6);
 }
 
 /// Median wall time (ms) of `n` runs of `f` after one warm-up run.
@@ -1008,10 +1058,21 @@ fn bench_gpu(s: &Session, args: &Args) -> Result<(), Failure> {
         }
     };
     let project = &project;
-    eprintln!("GPU: {} — median of {n} runs (ms/frame)", effectcraft_render::Accelerator::name(&gpu));
-    eprintln!(
+    note!("GPU: {} — median of {n} runs (ms/frame)", effectcraft_render::Accelerator::name(&gpu));
+    note!(
         "{:<28} {:>5} {:>10} {:>9} {:>9} {:>9} {:>9} {:>9} {:>8} {:>9} {:>9} {:>11}",
-        "comp", "res", "size", "cpu cold", "gpu cold", "cpu warm", "gpu warm", "gpu view", "speedup", "auto", "gpu≠cpu", "up/dn MB"
+        "comp",
+        "res",
+        "size",
+        "cpu cold",
+        "gpu cold",
+        "cpu warm",
+        "gpu warm",
+        "gpu view",
+        "speedup",
+        "auto",
+        "gpu≠cpu",
+        "up/dn MB"
     );
     for cid in comps {
         let Some(comp) = project.comp(cid) else { continue };
@@ -1077,7 +1138,7 @@ fn bench_gpu(s: &Session, args: &Args) -> Result<(), Failure> {
                 .count();
             let pct = 100.0 * off as f64 / a.data.len().max(1) as f64;
             let speedup = cpu_warm / gpu_warm.max(1e-9);
-            eprintln!(
+            note!(
                 "{name:<28} {label:>5} {size:>10} {cpu_cold:>9.2} {gpu_cold:>9.2} {cpu_warm:>9.2} {gpu_warm:>9.2} {view:>9.2} {speedup:>7.2}x {auto:>9.2} {pct:>8.3}% {traffic:>11}"
             );
         }

apps/effectcraft-cli/tests/cli.rs

@@ -130,6 +130,32 @@ fn errors_are_json() {
     assert_eq!(out.status.code(), Some(2));
 }
 
+/// A reader that closes stdout before the CLI writes (`| head`) is not a crash: the command
+/// still does its work and exits 0, without a panic (#167).
+#[test]
+fn a_closed_stdout_is_not_a_crash() {
+    let proj = tmp("closed-stdout.ecproj");
+    let p = proj.to_str().unwrap();
+    for args in [&["info", "--json"][..], &["exec", "--list"], &["run", "comp.new", r#"{"name":"Piped"}"#, "project.summary", "--empty", "--save-as", p]] {
+        let mut child = bin().args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap();
+        drop(child.stdout.take());
+        let out = child.wait_with_output().unwrap();
+        let err = String::from_utf8_lossy(&out.stderr);
+        assert_eq!(out.status.code(), Some(0), "{args:?}: {err}");
+        assert!(!err.contains("panicked"), "{args:?}: {err}");
+    }
+    assert!(std::fs::read_to_string(&proj).unwrap().contains("Piped"), "the sequence ran to the end and saved");
+    // The MCP server ends quietly when its client closes stdout.
+    let mut child = bin().arg("mcp").stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap();
+    drop(child.stdout.take());
+    {
+        let mut stdin = child.stdin.take().unwrap();
+        let _ = writeln!(stdin, "{}", json!({"jsonrpc": "2.0", "id": 1, "method": "ping"}));
+    }
+    let out = child.wait_with_output().unwrap();
+    assert_eq!(out.status.code(), Some(0), "{}", String::from_utf8_lossy(&out.stderr));
+}
+
 #[test]
 fn mcp_over_stdio() {
     let mut child = bin().args(["mcp", "--demo"]).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::null()).spawn().unwrap();