From 3938e267bb077fcd7543c8c6f5b76feda324bc66 Mon Sep 17 00:00:00 2001 From: NOWN-DEV <257771410+NOWN-DEV@users.noreply.github.com> Date: Tue, 13 Oct 2026 00:45:10 +0200 Subject: [PATCH] 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 --- apps/solidcraft-cli/src/main.rs | 26 ++++++- apps/solidcraft-cli/tests/run_strict.rs | 90 +++++++++++++++++++++++++ docs/book/src/guide/getting-started.md | 5 ++ 3 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 apps/solidcraft-cli/tests/run_strict.rs diff --git a/apps/solidcraft-cli/src/main.rs b/apps/solidcraft-cli/src/main.rs index a067afb9..fa993671 100644 --- a/apps/solidcraft-cli/src/main.rs +++ b/apps/solidcraft-cli/src/main.rs @@ -1,7 +1,7 @@ //! SolidCraft headless command line. //! //! ```text -//! solidcraft-cli run [--in design|part.step|mesh.3mf] [--out FILE]... [--save FILE] [--quiet] +//! solidcraft-cli run [--in design|part.step|mesh.3mf] [--out FILE]... [--save FILE] [--quiet] [--allow-errors] //! solidcraft-cli eval (alias: inspect) measurements as JSON //! solidcraft-cli bench-open [--parse-only] loading timings as JSON lines //! solidcraft-cli snapshot --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 [--in design.solidcraft|part.step] [--out FILE]... [--save FILE] [--quiet] [--results] + solidcraft-cli run [--in design.solidcraft|part.step] [--out FILE]... [--save FILE] [--quiet] [--results] [--allow-errors] solidcraft-cli run --in [--out FILE]... + (a timeline feature that fails fails the run and nothing is written; --allow-errors writes anyway) solidcraft-cli eval (alias: inspect; or --in FILE) solidcraft-cli bench-open [--parse-only] solidcraft-cli snapshot --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 = 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)?; diff --git a/apps/solidcraft-cli/tests/run_strict.rs b/apps/solidcraft-cli/tests/run_strict.rs new file mode 100644 index 00000000..f7629946 --- /dev/null +++ b/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); +} diff --git a/docs/book/src/guide/getting-started.md b/docs/book/src/guide/getting-started.md index 0c1cc7c5..9deccc8d 100644 --- a/docs/book/src/guide/getting-started.md +++ b/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.