fix: fail solidcraft-cli run when a timeline feature fails (#402)

A parameter change can break a feature without failing the command that
made it; parameters.change reports it only in its `errors`. `run` then
exited 0 and exported the bodies as they were before the failing feature:
a shrunk sheet-metal tray whose unfold failed came out folded, without
the holes and cut-outs made after the unfold.

`run` now checks the evaluated timeline before writing anything. Any
feature with an error fails the run with status 1, lists the failing
features on stderr and writes none of the --out and --save files, as a
failing command already does. --allow-errors writes them anyway. `eval`
is unchanged and still reports the errors in its timeline.

Co-authored-by: Claude Opus 5.5 <[email protected]>

apps/solidcraft-cli/src/main.rs

@@ -1,7 +1,7 @@
 //! SolidCraft headless command line.
 //!
 //! ```text
-//! solidcraft-cli run <script.json|design.solidcraft|part.step|mesh.3mf> [--in design|part.step|mesh.3mf] [--out FILE]... [--save FILE] [--quiet]
+//! solidcraft-cli run <script.json|design.solidcraft|part.step|mesh.3mf> [--in design|part.step|mesh.3mf] [--out FILE]... [--save FILE] [--quiet] [--allow-errors]
 //! solidcraft-cli eval <script.json|design.solidcraft|part.step> (alias: inspect) measurements as JSON
 //! solidcraft-cli bench-open <design.solidcraft | part.step> [--parse-only] loading timings as JSON lines
 //! solidcraft-cli snapshot <script|design> --out shot.png [--width W] [--height H] [--view iso|front|top|…]
@@ -32,8 +32,9 @@ use solidcraft_engine::Session;
 use solidcraft_engine::render::{StandardView, render_png};
 
 const USAGE: &str = "usage:
-  solidcraft-cli run <script.json|design.solidcraft|part.step> [--in design.solidcraft|part.step] [--out FILE]... [--save FILE] [--quiet] [--results]
+  solidcraft-cli run <script.json|design.solidcraft|part.step> [--in design.solidcraft|part.step] [--out FILE]... [--save FILE] [--quiet] [--results] [--allow-errors]
   solidcraft-cli run --in <design.solidcraft|part.step> [--out FILE]...
+      (a timeline feature that fails fails the run and nothing is written; --allow-errors writes anyway)
   solidcraft-cli eval <script.json|design.solidcraft|part.step>     (alias: inspect; or --in FILE)
   solidcraft-cli bench-open <design.solidcraft | part.step> [--parse-only]
   solidcraft-cli snapshot <script|design> --out shot.png [--width W] [--height H] [--view iso|front|back|top|bottom|left|right]
@@ -147,7 +148,7 @@ fn positional(args: &[String]) -> Option<&String> {
     let mut i = 0;
     while let Some(a) = args.get(i) {
         if a.starts_with("--") {
-            i += if matches!(a.as_str(), "--quiet" | "--results") { 1 } else { 2 };
+            i += if matches!(a.as_str(), "--quiet" | "--results" | "--allow-errors") { 1 } else { 2 };
             continue;
         }
         return Some(a);
@@ -182,6 +183,12 @@ fn cmd_run(args: &[String]) -> Result<(), String> {
         }
         None
     };
+    // A feature that fails leaves the bodies as they were before it (a parameter change can
+    // break a feature without failing the command that made it): writing them would hand out
+    // the wrong part with success, so fail the run unless asked to write it anyway.
+    if !args.iter().any(|a| a == "--allow-errors") {
+        timeline_ok(&s)?;
+    }
     for out in flags(args, "--out") {
         s.execute("file.export", &json!({"path": out})).map_err(|e| e.to_string())?;
     }
@@ -197,6 +204,19 @@ fn cmd_run(args: &[String]) -> Result<(), String> {
     Ok(())
 }
 
+/// Every failing timeline feature, one per line, as an error; Ok when none fails.
+fn timeline_ok(s: &Session) -> Result<(), String> {
+    let failed: Vec<String> = s.model.results.iter().filter_map(|r| r.error.as_ref().map(|e| format!("  {}: {e}", r.name))).collect();
+    if failed.is_empty() {
+        return Ok(());
+    }
+    Err(format!(
+        "{} timeline feature(s) failed, so nothing was written (--allow-errors writes the bodies anyway):\n{}",
+        failed.len(),
+        failed.join("\n")
+    ))
+}
+
 fn cmd_eval(args: &[String]) -> Result<(), String> {
     let path = flag(args, "--in").or_else(|| positional(args).map(String::as_str)).ok_or_else(|| USAGE.to_string())?;
     let mut s = load(path)?;

apps/solidcraft-cli/tests/run_strict.rs

@@ -0,0 +1,90 @@
+//! A timeline feature that fails fails `run`, and nothing is written: the bodies lack what the
+//! feature should have made, and handing them out with exit 0 hides the failure.
+
+use std::path::PathBuf;
+use std::process::Command;
+use std::sync::atomic::{AtomicUsize, Ordering};
+
+use serde_json::{Value, json};
+
+static NEXT: AtomicUsize = AtomicUsize::new(0);
+
+struct Scratch(PathBuf);
+
+impl Scratch {
+    fn new() -> Self {
+        let dir = std::env::temp_dir().join(format!("solidcraft-run-strict-{}-{}", std::process::id(), NEXT.fetch_add(1, Ordering::Relaxed)));
+        std::fs::create_dir(&dir).unwrap();
+        Self(dir)
+    }
+
+    /// A filleted box whose fillet a later parameter change makes impossible: every command
+    /// succeeds, the timeline ends with a failing feature.
+    fn broken_script(&self) -> PathBuf {
+        let path = self.0.join("broken.json");
+        let script = json!({"commands": [
+            {"command": "parameters.add", "params": {"name": "r", "expression": "2 mm"}},
+            {"command": "solid.box", "params": {"length": 20, "width": 20, "height": 10}},
+            {"command": "solid.fillet", "params": {"edges": [[10, 0, 10]], "radius": "r"}},
+            {"command": "parameters.change", "params": {"name": "r", "expression": "50 mm"}},
+        ]});
+        std::fs::write(&path, script.to_string()).unwrap();
+        path
+    }
+}
+
+impl Drop for Scratch {
+    fn drop(&mut self) {
+        let _ = std::fs::remove_dir_all(&self.0);
+    }
+}
+
+fn cli() -> Command {
+    Command::new(env!("CARGO_BIN_EXE_solidcraft-cli"))
+}
+
+#[test]
+fn a_failing_feature_fails_the_run_and_writes_nothing() {
+    let dir = Scratch::new();
+    let script = dir.broken_script();
+    let (step, saved) = (dir.0.join("part.step"), dir.0.join("part.solidcraft"));
+    for extra in [&[][..], &["--results"][..], &["--quiet"][..]] {
+        let output = cli().arg("run").arg(&script).args(extra).arg("--out").arg(&step).arg("--save").arg(&saved).output().unwrap();
+        assert_eq!(output.status.code(), Some(1), "{extra:?}");
+        let stderr = String::from_utf8_lossy(&output.stderr);
+        assert!(stderr.contains("Fillet1") && stderr.contains("nothing was written"), "{stderr}");
+        assert!(output.stdout.is_empty(), "{extra:?}");
+        assert!(!step.exists() && !saved.exists(), "{extra:?}");
+    }
+}
+
+#[test]
+fn allow_errors_writes_anyway_and_a_saved_design_keeps_failing() {
+    let dir = Scratch::new();
+    let (step, saved) = (dir.0.join("part.step"), dir.0.join("part.solidcraft"));
+    let output = cli().arg("run").arg(dir.broken_script()).arg("--allow-errors").arg("--out").arg(&step).arg("--save").arg(&saved).output().unwrap();
+    assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr));
+    let measured: Value = serde_json::from_slice(&output.stdout).unwrap();
+    assert_eq!(measured["total"]["volume_mm3"], json!(4000.0), "the box without its fillet");
+    assert!(std::fs::metadata(&step).unwrap().len() > 0);
+    // The saved design still holds the failing feature: running it fails too, `eval` reports it.
+    let again = dir.0.join("again.step");
+    let output = cli().arg("run").arg(&saved).arg("--out").arg(&again).output().unwrap();
+    assert_eq!(output.status.code(), Some(1));
+    assert!(!again.exists());
+    let output = cli().arg("eval").arg(&saved).output().unwrap();
+    assert!(output.status.success());
+    let report: Value = serde_json::from_slice(&output.stdout).unwrap();
+    assert!(report["timeline"].as_array().unwrap().iter().any(|f| f["error"].is_string()), "{}", report["timeline"]);
+}
+
+#[test]
+fn a_clean_timeline_still_writes() {
+    let dir = Scratch::new();
+    let script = dir.0.join("box.json");
+    std::fs::write(&script, json!({"commands": [{"command": "solid.box", "params": {"length": 2, "width": 3, "height": 4}}]}).to_string()).unwrap();
+    let step = dir.0.join("box.step");
+    let output = cli().arg("run").arg(&script).arg("--out").arg(&step).arg("--quiet").output().unwrap();
+    assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr));
+    assert!(std::fs::metadata(&step).unwrap().len() > 0);
+}

docs/book/src/guide/getting-started.md

@@ -104,6 +104,11 @@ A script is a list of commands:
 
 ![The bracket example, rendered by solidcraft-cli snapshot](../generated/bracket.png)
 
+A command that fails stops the script. A feature can also fail later, when a parameter change
+rebuilds it; `run` then exits with status 1, lists the failing features and writes none of
+its `--out` and `--save` files, because the bodies lack what those features should have made.
+`--allow-errors` writes them anyway.
+
 Units are millimetres and degrees unless an expression says otherwise (`"1 in"`, `"30 deg"`),
 and Z is up.