LC-001: stop commands after failed imports

apps/lightcraft-cli/src/main.rs

@@ -594,14 +594,7 @@ fn run(args: &[String]) -> Result<(), String> {
                     None => Headless::default(),
                 };
                 if !imports.is_empty() {
-                    let paths = expand_paths(&imports);
-                    let r = h.session.execute("library.import", &json!({"paths": paths})).map_err(|e| e.to_string())?;
-                    // what follows acts on the imported photos (already-known files are reported as duplicates)
-                    let mut ids: Vec<Value> = r["imported"].as_array().cloned().unwrap_or_default();
-                    ids.extend(r["duplicates"].as_array().into_iter().flatten().filter_map(|d| d.get("existing").filter(|v| v.is_u64()).cloned()));
-                    if let Some(first) = ids.first().cloned() {
-                        h.session.execute("library.select", &json!({"ids": ids, "active": first})).map_err(|e| e.to_string())?;
-                    }
+                    import_and_select(&mut h.session, &imports)?;
                 }
                 Box::new(h)
             }
@@ -626,6 +619,32 @@ fn run(args: &[String]) -> Result<(), String> {
     if failed > 0 { Err(format!("{failed} command(s) failed")) } else { Ok(()) }
 }
 
+fn import_and_select(session: &mut Session, imports: &[String]) -> Result<(), String> {
+    let selection = session.selection.clone();
+    let paths = expand_paths(imports);
+    let result = (|| {
+        let r = session.execute("library.import", &json!({"paths": paths})).map_err(|e| format!("import failed: {e}; no commands were run"))?;
+        let ids = import_targets(&r)?;
+        let first = ids.first().cloned().ok_or("import found no photos to select; no commands were run")?;
+        session.execute("library.select", &json!({"ids": ids, "active": first})).map_err(|e| e.to_string())?;
+        Ok::<(), String>(())
+    })();
+    if result.is_err() {
+        session.selection = selection;
+    }
+    result
+}
+
+fn import_targets(report: &Value) -> Result<Vec<Value>, String> {
+    if let Some(failed) = report["failed"].as_array().filter(|failed| !failed.is_empty()) {
+        return Err(format!("import failed for {} item(s): {}; no commands were run", failed.len(), json!(failed)));
+    }
+    let mut ids: Vec<Value> = report["imported"].as_array().cloned().unwrap_or_default();
+    ids.extend(report["duplicates"].as_array().into_iter().flatten().filter_map(|d| d.get("existing").filter(|v| v.is_u64()).cloned()));
+    ids.extend(report["restored"].as_array().into_iter().flatten().filter(|id| id.is_u64()).cloned());
+    if ids.is_empty() { Err("import found no photos to select; no commands were run".into()) } else { Ok(ids) }
+}
+
 fn take_value<'a>(args: &'a [String], i: &mut usize, flag: &str) -> Result<&'a str, String> {
     *i += 1;
     args.get(*i).map(String::as_str).ok_or_else(|| format!("{flag} needs a value"))
@@ -1028,3 +1047,48 @@ fn denoise_serve(args: &[String]) -> Result<(), String> {
 fn denoise_serve(_: &[String]) -> Result<(), String> {
     Err("denoise-serve: this build has no denoise support (build with the `denoise` feature)".into())
 }
+
+#[cfg(test)]
+mod import_target_tests {
+    use super::{import_and_select, import_targets, json};
+
+    #[test]
+    fn empty_reports_and_duplicates_without_ids_have_no_target() {
+        assert!(import_targets(&json!({})).is_err());
+        assert!(import_targets(&json!({"imported": [], "duplicates": [{"existing": null}]})).is_err());
+    }
+
+    #[test]
+    fn restored_only_and_mixed_reports_keep_target_order() {
+        assert_eq!(import_targets(&json!({"restored": [9]})).unwrap(), vec![json!(9)]);
+        assert_eq!(
+            import_targets(&json!({"imported": [1], "duplicates": [{"existing": null}, {"existing": 2}], "restored": [3]})).unwrap(),
+            vec![json!(1), json!(2), json!(3)]
+        );
+    }
+
+    #[test]
+    fn failed_items_reject_even_when_targets_exist() {
+        let error = import_targets(&json!({"imported": [1], "failed": [["broken.png", "invalid"]]})).unwrap_err();
+        assert!(error.contains("1") && error.contains("broken.png") && error.contains("no commands were run"), "{error}");
+    }
+
+    #[test]
+    fn failed_partial_import_restores_empty_selection() {
+        let dir = std::env::temp_dir().join(format!("lightcraft-import-selection-{}", std::process::id()));
+        std::fs::create_dir_all(&dir).unwrap();
+        let input = dir.join("valid.png");
+        let broken = dir.join("broken.png");
+        let img = lightcraft_raster::Rgba8::from_fn(2, 2, |_, _| [20, 40, 60, 255]);
+        let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &Default::default()).unwrap();
+        std::fs::write(&input, png).unwrap();
+        std::fs::write(&broken, b"not a PNG").unwrap();
+        let mut headless = lightcraft_mcp::Headless::default();
+        let session = &mut headless.session;
+        let selection = session.selection.clone();
+        assert!(selection.ids.is_empty() && selection.active.is_none());
+        assert!(import_and_select(session, &[input.to_string_lossy().into_owned(), broken.to_string_lossy().into_owned()]).is_err());
+        assert_eq!(session.selection, selection);
+        let _ = std::fs::remove_dir_all(dir);
+    }
+}

apps/lightcraft-cli/tests/it/cli.rs

@@ -513,6 +513,153 @@ fn run_cli(args: &[&str], stdin: Option<&str>) -> (bool, Vec<Value>, String) {
     (o.status.success(), lines, String::from_utf8_lossy(&o.stderr).to_string())
 }
 
+#[test]
+fn run_failed_import_never_edits_existing_selection() {
+    let dir = tmp("failed-import");
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    let lib = dir.join("library");
+    let missing = dir.join("missing.png");
+    let empty = dir.join("empty");
+    std::fs::create_dir(&empty).unwrap();
+    assert!(!missing.exists());
+    let lib = lib.to_str().unwrap();
+    let (ok, lines, err) = run_cli(&["--demo", "--library", lib, "library.state"], None);
+    assert!(ok, "{err}");
+    let id = lines[0]["result"]["selection"]["active"].as_u64().unwrap();
+    let (ok, _, err) = run_cli(&["--library", lib, "photo.rate", &format!("ids=[{id}]"), "rating=0"], None);
+    assert!(ok, "{err}");
+    for path in [&missing, &empty] {
+        let (ok, lines, err) = run_cli(&["--library", lib, "--import", path.to_str().unwrap(), "photo.rate", "rating=5"], None);
+        assert!(!ok, "import unexpectedly succeeded for {}: {lines:?}", path.display());
+        assert!(lines.is_empty(), "commands ran: {lines:?}");
+        assert!(err.contains("import") && err.contains("no commands were run"), "{err}");
+        if path == &missing {
+            assert!(err.contains("failed") && err.contains("missing.png"), "{err}");
+        }
+        let (ok, lines, err) = run_cli(&["--library", lib, "photo.inspect", &format!("id={id}")], None);
+        assert!(ok, "{err}");
+        assert_eq!(lines[0]["result"]["rating"], 0);
+    }
+}
+
+#[test]
+fn run_partial_import_stops_commands_and_import_targets_remain_usable() {
+    let dir = tmp("partial-import");
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    let input = dir.join("valid.png");
+    let broken = dir.join("broken.png");
+    gradient_png(&input);
+    std::fs::write(&broken, b"not a PNG").unwrap();
+    let mixed_lib = dir.join("mixed");
+    let mixed_lib = mixed_lib.to_str().unwrap();
+    let input = input.to_str().unwrap();
+    let (ok, lines, err) = run_cli(&["--demo", "--library", mixed_lib, "catalog.query"], None);
+    assert!(ok, "{err}");
+    let photos = lines[0]["result"]["photos"].as_array().unwrap();
+    let a = photos[0]["id"].as_u64().unwrap();
+    let b = photos[1]["id"].as_u64().unwrap();
+    let (ok, lines, err) = run_cli(
+        &[
+            "--library",
+            mixed_lib,
+            "library.select",
+            &format!("ids=[{a},{b}]"),
+            &format!("active={a}"),
+            "photo.rate",
+            &format!("ids=[{a}]"),
+            "rating=0",
+            "library.state",
+        ],
+        None,
+    );
+    assert!(ok, "{err}");
+    let selection = lines[2]["result"]["selection"].clone();
+    assert_eq!(selection["ids"], json!([a, b]));
+    assert_eq!(selection["active"], a);
+    let (ok, lines, err) =
+        run_cli(&["--library", mixed_lib, "--import", input, "--import", broken.to_str().unwrap(), "photo.rate", "rating=5"], None);
+    let (query_ok, after, query_err) =
+        run_cli(&["--library", mixed_lib, "library.state", "photo.inspect", &format!("id={a}"), "catalog.query"], None);
+    assert!(query_ok, "{query_err}");
+    assert_eq!(after[0]["result"]["selection"], selection, "selection changed after partial import");
+    assert!(!ok, "partial import unexpectedly succeeded: {lines:?}");
+    assert!(lines.is_empty() && err.contains("failed") && err.contains("no commands were run"), "{err}: {lines:?}");
+    assert_eq!(after[1]["result"]["rating"], 0);
+    assert!(after[2]["result"]["photos"].as_array().unwrap().iter().any(|p| p["fileName"] == "valid.png"));
+
+    let lib = dir.join("duplicates");
+    let lib = lib.to_str().unwrap();
+    let (ok, lines, err) = run_cli(&["--demo", "--library", lib, "library.state"], None);
+    assert!(ok, "{err}");
+    let previous = lines[0]["result"]["selection"]["active"].as_u64().unwrap();
+    let (ok, lines, err) = run_cli(&["--library", lib, "--import", input, "photo.rate", "rating=4", "library.state"], None);
+    assert!(ok, "{err}");
+    let id = lines[1]["result"]["selection"]["active"].as_u64().unwrap();
+    assert_ne!(id, previous);
+    let (ok, _, err) = run_cli(&["--library", lib, "library.select", &format!("ids=[{previous}]"), "photo.rate", "rating=0"], None);
+    assert!(ok, "{err}");
+    let (ok, _, err) = run_cli(&["--library", lib, "--import", input, "photo.rate", "rating=3"], None);
+    assert!(ok, "{err}");
+    let (ok, lines, err) = run_cli(&["--library", lib, "photo.inspect", &format!("id={id}"), "photo.inspect", &format!("id={previous}")], None);
+    assert!(ok, "{err}");
+    assert_eq!(lines[0]["result"]["rating"], 3);
+    assert_eq!(lines[1]["result"]["rating"], 0);
+}
+
+#[test]
+fn run_restored_import_is_selected() {
+    let dir = tmp("restored-import");
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    let input = dir.join("valid.png");
+    gradient_png(&input);
+    let lib = dir.join("library");
+    let (lib, input) = (lib.to_str().unwrap(), input.to_str().unwrap());
+    let (ok, lines, err) = run_cli(&["--library", lib, "--import", input, "library.state"], None);
+    assert!(ok, "{err}");
+    let id = lines[0]["result"]["selection"]["active"].as_u64().unwrap();
+    let (ok, _, err) = run_cli(&["--library", lib, "photo.delete"], None);
+    assert!(ok, "{err}");
+    let (ok, _, err) = run_cli(&["--library", lib, "--import", input, "photo.rate", "rating=4"], None);
+    assert!(ok, "{err}");
+    let (ok, lines, err) = run_cli(&["--library", lib, "catalog.query"], None);
+    assert!(ok, "{err}");
+    assert_eq!(lines[0]["result"]["photos"].as_array().unwrap().len(), 1);
+    assert_eq!(lines[0]["result"]["photos"][0]["id"], id);
+    assert_eq!(lines[0]["result"]["photos"][0]["rating"], 4);
+}
+
+#[test]
+fn run_mixed_new_and_restored_import_selects_both() {
+    let dir = tmp("mixed-restored-import");
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    let restored = dir.join("restored.png");
+    let new = dir.join("new.png");
+    gradient_png(&restored);
+    let image = lightcraft_raster::Rgba8::from_fn(120, 80, |x, y| [(x * 2) as u8, (y * 3) as u8, 200, 255]);
+    let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&image), &Default::default()).unwrap();
+    std::fs::write(&new, png).unwrap();
+    let lib = dir.join("library");
+    let (lib, restored, new) = (lib.to_str().unwrap(), restored.to_str().unwrap(), new.to_str().unwrap());
+    let (ok, lines, err) = run_cli(&["--library", lib, "--import", restored, "library.state"], None);
+    assert!(ok, "{err}");
+    let restored_id = lines[0]["result"]["selection"]["active"].as_u64().unwrap();
+    let (ok, _, err) = run_cli(&["--library", lib, "photo.delete"], None);
+    assert!(ok, "{err}");
+    let (ok, lines, err) = run_cli(&["--library", lib, "--import", new, "--import", restored, "photo.rate", "rating=4", "library.state"], None);
+    assert!(ok, "{err}");
+    assert_eq!(lines[1]["result"]["selection"]["ids"].as_array().unwrap().len(), 2);
+    let (ok, lines, err) = run_cli(&["--library", lib, "catalog.query"], None);
+    assert!(ok, "{err}");
+    let photos = lines[0]["result"]["photos"].as_array().unwrap();
+    assert_eq!(photos.len(), 2);
+    assert!(photos.iter().any(|p| p["id"] == restored_id && p["rating"] == 4));
+    assert!(photos.iter().any(|p| p["fileName"] == "new.png" && p["id"] != restored_id && p["rating"] == 4));
+}
+
 #[test]
 fn run_subcommand_chains_commands_and_persists_a_library() {
     let input = tmp("run-in.png");