Address review: Batch stops, rename targets, Shape Blur memory, sampler releases

- A direct Batch in a script or a CLI command list stops it when it is cancelled,
  cannot write its error log, or ran out of script steps; per-file errors alone
  still continue (CodeRabbit, outside-diff findings on automate_cmds.rs and
  photocraft-cli document.rs).
- Recorded Save steps count against the script's step budget, and a Batch whose
  files failed with the budget spent carries scriptLimitError, so Browse reports
  the failure instead of ok: true (yoriswind).
- actions.rename rewrites a named Batch, nested Play or Conditional call only if
  it resolved to the renamed action before the rename; a Batch with explicit
  steps is left alone (yoriswind: two same-named actions in different sets).
- Shape Blur's run admission counts each worker's f64 prefix sums and, for a
  custom shape, the kernel mask kept for the run plus its worst-case coverage
  runs (yoriswind).
- Every sampler that takes the primary release (Color Picker, Curves/Levels and
  Properties eyedroppers, Curves Cmd-click, Color Range) ends a transform handle
  drag and a Move drag begun before it, not only the Color Picker (#11210 review).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

apps/photocraft-cli/src/document.rs

@@ -117,9 +117,15 @@ pub(crate) fn run_cmds(a: &RunArgs, out: &mut dyn Write, err: &mut dyn Write) ->
             continue;
         }
         let r = h.command_run(&id, params).map_err(|source| CliError::Command { id: id.clone(), source })?;
-        // A direct Batch reports per-file errors and continues, as in Scripts; failed
-        // action or script calls still stop the command list before export.
-        if let Some(message) = photocraft_engine::actions_cmds::nested_failure(&id, &r).filter(|_| id != "file.automate.batch") {
+        // A direct Batch reports per-file errors and continues, as in Scripts, unless it was
+        // cancelled, ran out of script steps or couldn't write its error log; failed action or
+        // script calls still stop the command list before export.
+        let failure = if id == "file.automate.batch" {
+            photocraft_engine::actions_cmds::batch_stop(&r)
+        } else {
+            photocraft_engine::actions_cmds::nested_failure(&id, &r)
+        };
+        if let Some(message) = failure {
             return Err(CliError::NestedFailure { id, message });
         }
         let new_warnings: Vec<String> =

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

@@ -404,6 +404,32 @@ fn run_continues_after_batch_file_errors_and_keeps_partial_results() {
     std::fs::remove_dir_all(d).unwrap();
 }
 
+/// A direct Batch whose error log can't be written stops the command list before export, also
+/// when a file failed as well (per-file errors alone would let it go on).
+#[test]
+fn run_stops_when_a_direct_batch_cannot_write_its_error_log() {
+    let d = tmp("run-batch-log-fails");
+    let bad = d.join("bad.png");
+    let out = d.join("result.png");
+    std::fs::write(&bad, b"not a PNG").unwrap();
+    // The folder itself as the log path: the log write fails.
+    let params = json!({"input":[bad],"output":d.join("batch"),"format":"png","steps":[],"errorLog":d}).to_string();
+    let result = bin()
+        .args(["run", "--new", r#"{"width":8,"height":8}"#])
+        .args(["--cmd", "file.automate.batch", "--params", &params])
+        .args(["--cmd", "document.inspect", "--out"])
+        .arg(&out)
+        .output()
+        .unwrap();
+    let error = String::from_utf8_lossy(&result.stderr);
+    assert_eq!(result.status.code(), Some(1), "stdout: {}\nstderr: {error}", String::from_utf8_lossy(&result.stdout));
+    assert!(error.contains("error log"), "{error}");
+    let rows: Vec<Value> = String::from_utf8_lossy(&result.stdout).lines().map(|line| serde_json::from_str(line).unwrap()).collect();
+    assert!(rows.iter().all(|row| row["command"] != "document.inspect"), "later commands must not run");
+    assert!(!out.exists(), "a stopped command list must not export");
+    std::fs::remove_dir_all(d).unwrap();
+}
+
 #[test]
 fn run_continues_after_successful_action_and_exports() {
     let d = tmp("run-successful-action");

crates/algo/src/lib.rs

@@ -1049,7 +1049,7 @@ pub fn working_bytes(params: &FilterParams, area: Rect, bounds: Rect, channels:
     let floats = (channels as u64).saturating_mul(4);
     // Result groups hold at most RESULT_BUDGET bytes of floats, and never more than the area.
     let base = area.size().area().saturating_mul(floats).min(RESULT_BUDGET as u64).saturating_mul(3);
-    match params.halo_for(bounds) {
+    let windows = match params.halo_for(bounds) {
         Halo::Bounds => base.saturating_add(bounds.union(&area).size().area().saturating_mul(floats)),
         Halo::Radius(r) => {
             let side = tile.saturating_add((r.max(0) as u64).saturating_mul(2));
@@ -1057,7 +1057,22 @@ pub fn working_bytes(params: &FilterParams, area: Rect, bounds: Rect, channels:
                 side.saturating_mul(side).saturating_add(tile.saturating_mul(tile).saturating_mul(3)).saturating_mul(floats).saturating_mul(threads),
             )
         }
-    }
+    };
+    windows.saturating_add(shape_blur_bytes(params, bounds, tile, channels as u64, threads))
+}
+
+/// What Shape Blur holds beyond the float windows: each worker's f64 prefix sums over its
+/// halo-expanded tile, and for a custom shape the kernel mask kept for the whole run plus its
+/// prepared coverage runs (at most [`shape_blur::max_spans`], 20 bytes each).
+fn shape_blur_bytes(params: &FilterParams, bounds: Rect, tile: u64, channels: u64, threads: u64) -> u64 {
+    let (FilterParams::ShapeBlur { .. } | FilterParams::CustomShapeBlur { .. }) = params else { return 0 };
+    let Halo::Radius(r) = params.halo_for(bounds) else { return 0 };
+    let side = tile.min(2048).saturating_add((r.max(0) as u64).saturating_mul(2)).saturating_add(1);
+    let prefix = side.saturating_mul(side).saturating_mul(channels).saturating_mul(8).saturating_mul(threads);
+    let FilterParams::CustomShapeBlur { kernel: Some(mask), .. } = params else { return prefix };
+    let size = (mask.rect.width() as usize).max(mask.rect.height() as usize);
+    let spans = (shape_blur::max_spans(size) as u64).saturating_mul(20);
+    prefix.saturating_add((mask.data.len() as u64).saturating_mul(4)).saturating_add(spans)
 }
 
 /// [`apply`] / [`apply_in`] with an explicit tile size (tests use it to check tile independence).

crates/algo/src/tests.rs

@@ -16,6 +16,31 @@ fn relight_admission_tracks_the_document_dependent_working_set() {
     assert!(working_bytes(&params, tiny, tiny, 4) < 64 << 20, "{}", working_bytes(&params, tiny, tiny, 4));
 }
 
+/// Shape Blur admits each worker's f64 prefix sums over its halo-expanded tile, and a custom
+/// shape also the kernel mask it keeps for the whole run and its worst-case coverage runs.
+#[test]
+fn shape_blur_admission_counts_prefix_sums_and_the_retained_kernel() {
+    let area = Rect::new(0, 0, 4096, 4096);
+    let radius = 100.0f32;
+    let plain = FilterParams::ShapeBlur { radius, shape: BlurShape::Circle };
+    let Halo::Radius(reach) = plain.halo_for(area) else { panic!("Shape Blur has a radius halo") };
+    let threads = rayon::current_num_threads().max(1) as u64;
+    let tile = auto_tile(&plain, area) as u64;
+    let window = tile + 2 * reach as u64;
+    // The float windows every radius filter admits (4 channels of f32), then the f64 prefix sums.
+    let windows = (area.size().area() * 16).min(RESULT_BUDGET as u64) * 3 + (window * window + tile * tile * 3) * 16 * threads;
+    let prefix = (window + 1) * (window + 1) * 4 * 8 * threads;
+    assert_eq!(working_bytes(&plain, area, area, 4), windows + prefix);
+
+    let size = (2 * shape_blur::radius(radius).ceil() as usize + 1) as i32;
+    let mask = Image { rect: Rect::new(0, 0, size, size), ch: 1, data: vec![1.0; (size * size) as usize] };
+    let custom = FilterParams::CustomShapeBlur { radius, kernel: Some(std::sync::Arc::new(mask)) };
+    let kernel = (size * size) as u64 * 4 + shape_blur::max_spans(size as usize) as u64 * 20;
+    assert_eq!(working_bytes(&custom, area, area, 4), working_bytes(&plain, area, area, 4) + kernel);
+    // Unresolved, it holds no kernel yet.
+    assert_eq!(working_bytes(&FilterParams::CustomShapeBlur { radius, kernel: None }, area, area, 4), working_bytes(&plain, area, area, 4));
+}
+
 fn fmt(s: SampleType) -> PixelFormat {
     PixelFormat::new(ColorMode::Rgb, s, true)
 }

crates/engine/src/cmd/actions_cmds.rs

@@ -704,6 +704,19 @@ pub fn restore_recording(s: &mut Session, idx: usize, action: &str, from: usize)
     s.journal.push(("actions.play".into(), params));
 }
 
+/// What stops the caller of a Batch even where per-file errors don't (a direct Batch in a script
+/// or a CLI command list): the script ran out of steps inside it, it was cancelled, or its
+/// requested error log could not be written.
+pub fn batch_stop(value: &Value) -> Option<String> {
+    if let Some(error) = value.get("scriptLimitError").and_then(Value::as_str) {
+        return Some(error.to_string());
+    }
+    if value.get("cancelled") == Some(&Value::Bool(true)) {
+        return Some("Batch was cancelled".into());
+    }
+    value.get("errorLogError").and_then(Value::as_str).map(|error| format!("Batch error log failed: {error}"))
+}
+
 /// Nested playback and Batch return structured failures even though the command succeeded.
 /// Promote it to the caller's failed step so no later parent steps run after a child fails.
 /// Batch text summarizes counts and the first failure; its full result retains every file outcome.
@@ -721,10 +734,7 @@ pub fn nested_failure(id: &str, value: &Value) -> Option<String> {
         let Some(first) = errors.first() else {
             // Every input succeeded, yet the run was stopped or its requested log is missing:
             // later parent steps must not run as if the Batch had finished.
-            if value.get("cancelled") == Some(&Value::Bool(true)) {
-                return Some("Batch was cancelled".into());
-            }
-            return value.get("errorLogError").and_then(Value::as_str).map(|error| format!("Batch error log failed: {error}"));
+            return batch_stop(value);
         };
         let error = first.get("error")?.as_str()?;
         let file = first.get("file").or_else(|| first.get("document")).and_then(Value::as_str).unwrap_or("unknown input");
@@ -917,14 +927,18 @@ fn rename(s: &mut Session, p: &Value) -> Result<Value> {
         return Err(bad("actions.rename", "an action with this name already exists"));
     }
     let pending = s.actions.recording.map_or(s.journal.len(), |(start, _)| start);
-    let old_set = s.actions.list.get(idx).and_then(|a| a.set.clone());
+    // A loaded library can hold two actions of one name (in different sets): a call follows the
+    // rename only if it resolved to this action before it, so one reaching the other stays put.
+    let before = s.actions.list.clone();
+    let by_name = resolve(&before, &json!({"action": old}), "actions.rename").ok() == Some(idx);
     let calls = s.actions.list.iter_mut().flat_map(|a| a.steps.iter_mut()).chain(s.journal.iter_mut().skip(pending));
     for (id, params) in calls {
         let keys: &[&str] = match id.as_str() {
-            "actions.play" => &["action"],
-            "actions.conditional" => &["then", "else"],
-            // A named Batch selects from the caller's list, by name alone or within its set.
-            "file.automate.batch" if params.get("set").is_none_or(|set| set.is_null() || set.as_str() == old_set.as_deref()) => &["action"],
+            "actions.play" if by_name => &["action"],
+            "actions.conditional" if by_name => &["then", "else"],
+            // A named Batch selects from the caller's list, by name alone or within its set;
+            // explicit `steps` run instead of any named action, so those are left alone.
+            "file.automate.batch" if params.get("steps").is_none() && crate::file_cmds::batch_action(&before, params).ok() == Some(idx) => &["action"],
             _ => &[],
         };
         for key in keys {

crates/engine/src/cmd/actions_cmds/tests.rs

@@ -1401,6 +1401,50 @@ fn record_keeps_an_empty_loaded_sets_exact_name() {
     assert_eq!(s.execute("actions.save", json!({"set": " Web "})).unwrap()["actions"], json!(["First"]));
 }
 
+/// Two actions of one name in different sets (a loaded library): renaming the second by index
+/// leaves the unqualified calls, which reach the first, and a Batch with explicit steps alone;
+/// only the Batch qualified with the second's set follows. Renaming the first moves them all.
+#[test]
+fn rename_leaves_calls_that_reach_a_same_named_action_in_another_set() {
+    let calls = || {
+        vec![
+            ("file.automate.batch".into(), json!({"action": "Target", "input": [], "output": "/out"})),
+            ("file.automate.batch".into(), json!({"action": "Target", "set": "B", "input": [], "output": "/out"})),
+            ("file.automate.batch".into(), json!({"action": "Target", "set": "B", "steps": [["image.adjustments.invert", {}]], "input": [], "output": "/out"})),
+            ("actions.play".into(), json!({"action": "Target"})),
+            ("actions.conditional".into(), json!({"if": "documentIsOpen", "then": "Target", "else": "Target"})),
+        ]
+    };
+    let session = || {
+        let mut s = Session::new();
+        for set in ["A", "B"] {
+            let mut target = Action::new("Target", vec![("image.adjustments.invert".into(), json!({}))]);
+            target.set = Some(set.into());
+            s.actions.list.push(target);
+        }
+        s.actions.list.push(Action::new("Caller", calls()));
+        s
+    };
+    let mut s = session();
+    s.execute("actions.rename", json!({"action": 1, "name": "Renamed"})).unwrap();
+    let caller = &s.actions.list[2].steps;
+    assert_eq!(caller[0].1["action"], "Target", "an unqualified Batch reaches set A's action");
+    assert_eq!(caller[1].1["action"], "Renamed", "the Batch qualified with set B follows");
+    assert_eq!(caller[2].1["action"], "Target", "explicit steps are left alone");
+    assert_eq!(caller[3].1["action"], "Target", "a nested Play reaches set A's action");
+    assert_eq!((&caller[4].1["then"], &caller[4].1["else"]), (&json!("Target"), &json!("Target")));
+    assert_eq!(crate::file_cmds::batch_action(&s.actions.list, &caller[0].1).unwrap(), 0);
+    assert_eq!(crate::file_cmds::batch_action(&s.actions.list, &caller[1].1).unwrap(), 1);
+
+    let mut s = session();
+    s.execute("actions.rename", json!({"action": 0, "name": "Renamed"})).unwrap();
+    let caller = &s.actions.list[2].steps;
+    assert_eq!(caller[0].1["action"], "Renamed");
+    assert_eq!(caller[1].1["action"], "Target", "the Batch qualified with set B still names B's action");
+    assert_eq!(caller[3].1["action"], "Renamed");
+    assert_eq!((&caller[4].1["then"], &caller[4].1["else"]), (&json!("Renamed"), &json!("Renamed")));
+}
+
 /// A saved named Batch step follows a renamed action, by name alone or within its own set;
 /// a Batch naming the same action in another set and an inline Batch stay as they were.
 #[test]

crates/engine/src/cmd/automate_cmds.rs

@@ -135,8 +135,9 @@ fn run_steps(s: &mut Session, steps: &[(String, Value)]) -> Vec<Value> {
         }
         match s.execute(id, params.clone()) {
             Ok(r) => {
-                // Direct Batch calls report file errors successfully to scripts. Called actions still fail the script.
-                let failure = if id == "file.automate.batch" { None } else { crate::actions_cmds::nested_failure(id, &r) };
+                // Direct Batch calls report file errors successfully to scripts, but running out of
+                // script steps, a cancel or a failed error log stops it. Called actions still fail it.
+                let failure = if id == "file.automate.batch" { crate::actions_cmds::batch_stop(&r) } else { crate::actions_cmds::nested_failure(id, &r) };
                 match failure {
                     Some(error) => {
                         results.push(json!({"command": id, "result": r, "error": error}));

crates/engine/src/cmd/file_cmds/batch_run.rs

@@ -967,6 +967,8 @@ pub(super) fn batch(s: &mut Session, p: &Value) -> Result<Value> {
                         if ctx.batch_interrupt_requested() {
                             return Err(EngineError::Cancelled.into());
                         }
+                        // A recorded Save is a step too: Batch writes the output for it.
+                        charge_script_step(scratch)?;
                         if batch_save_point(id) {
                             let st = scratch.active_mut().ok_or(EngineError::NoDocument)?;
                             saved = Some(st.doc.clone());
@@ -980,7 +982,6 @@ pub(super) fn batch(s: &mut Session, p: &Value) -> Result<Value> {
                             continue;
                         }
                         let params = crate::actions_cmds::playback_params(scratch, id, params)?;
-                        charge_script_step(scratch)?;
                         let r = scratch.execute(id, params)?;
                         ctx.publish_batch_preview(&source_path, scratch.active().map(|state| state.doc.clone()));
                         // A failed step inside a called action makes this file an error, not a saved result.
@@ -1032,6 +1033,8 @@ pub(super) fn batch(s: &mut Session, p: &Value) -> Result<Value> {
                 s.actions.playback_steps = carried;
             }
             s.actions.script_steps = script_steps;
+            let mut result = result;
+            mark_script_limit(s, &mut result);
             if let (Some(notices), Some(imports)) = (&mut s.batch_open_notices, result.get("notices").and_then(Value::as_array)) {
                 notices.extend(imports.iter().cloned());
             }
@@ -1115,6 +1118,8 @@ fn batch_open_documents(
             s.set_active(position);
             let mut saved: Option<Arc<Document>> = None;
             for (command, params) in steps {
+                // A recorded Save is a step too: Batch writes the output for it.
+                charge_script_step(s)?;
                 if batch_save_point(command) {
                     let state = s.active_mut().ok_or(EngineError::NoDocument)?;
                     saved = Some(state.doc.clone());
@@ -1128,7 +1133,6 @@ fn batch_open_documents(
                     continue;
                 }
                 let params = crate::actions_cmds::playback_params(s, command, params)?;
-                charge_script_step(s)?;
                 let response = s.execute(command, params)?;
                 if let Some(error) = crate::actions_cmds::nested_failure(command, &response) {
                     return Err(FileFailure { error: EngineError::Other(error), result: Some(response) });
@@ -1177,6 +1181,7 @@ fn batch_open_documents(
         result["errorLogError"] = json!(error);
     }
     result["documents"] = json!(originals.len());
+    mark_script_limit(s, &mut result);
     Ok(result)
 }
 
@@ -1194,6 +1199,17 @@ fn charge_script_step(s: &mut Session) -> Result<()> {
     Ok(())
 }
 
+/// A file that failed once the script's step budget is spent failed because of it, and every
+/// later script step would fail the same way: the result says so, so a script running this
+/// Batch directly stops with that error (`batch_stop`) instead of reporting success.
+fn mark_script_limit(s: &Session, result: &mut Value) {
+    let max = crate::automate_cmds::MAX_SCRIPT_STEPS;
+    let failed = result.get("errors").and_then(Value::as_array).is_some_and(|errors| !errors.is_empty());
+    if s.actions.script_depth > 0 && s.actions.script_steps >= max && failed {
+        result["scriptLimitError"] = json!(format!("the script ran more than {max} steps in all (scripts calling scripts)"));
+    }
+}
+
 /// Check source structure once at the common pre-write seam, after Open authorization and ownership.
 fn preserve_source(path: &str) -> Result<()> {
     let bytes = read_file(path)?;

crates/engine/src/cmd/file_cmds/tests.rs

@@ -4145,6 +4145,94 @@ fn batch_steps_inside_a_script_count_against_its_step_limit() {
     }
 }
 
+/// A recorded Save is a step too: with the script's budget spent, a Save-only Batch writes
+/// nothing, in both sources, and its result says the script ran out of steps.
+#[test]
+fn a_save_only_batch_writes_nothing_once_the_script_steps_are_spent() {
+    let dir = tmp("batch-script-save-only");
+    let input = png(&dir, "in.png", 4, 4, "#ff0000");
+    for source in ["open", "folder"] {
+        let mut s = session(4, 4, 8);
+        s.actions.script_depth = 1;
+        s.actions.script_steps = crate::automate_cmds::MAX_SCRIPT_STEPS;
+        let out = join(&dir, &format!("out-{source}"));
+        let mut p = json!({"source": source, "output": out, "format": "png", "steps": [["file.save", {}]]});
+        if source == "folder" {
+            p["input"] = json!([input]);
+        }
+        let result = s.execute("file.automate.batch", p).unwrap();
+        assert_eq!(result["files"], json!([]), "{source}: {result}");
+        assert!(result["errors"][0]["error"].as_str().unwrap_or_default().contains("steps in all"), "{source}: {result}");
+        assert!(result["scriptLimitError"].is_string(), "{source}: {result}");
+        let written = std::fs::read_dir(&out).map(|d| d.count()).unwrap_or(0);
+        assert_eq!(written, 0, "{source}: nothing may be written past the budget");
+    }
+}
+
+/// A direct Batch in a script that spends the script's last step makes the script fail there
+/// instead of reporting `ok: true` with the exhaustion hidden as a per-file error.
+#[test]
+fn a_direct_batch_that_spends_the_script_budget_fails_the_script() {
+    let dir = tmp("batch-script-budget-browse");
+    let input = png(&dir, "in.png", 4, 4, "#ff0000");
+    for source in ["open", "folder"] {
+        let mut s = session(4, 4, 8);
+        // A nested script: the outer one has already spent all but two steps.
+        s.actions.script_depth = 1;
+        s.actions.script_steps = crate::automate_cmds::MAX_SCRIPT_STEPS - 2;
+        let mut batch = json!({"source": source, "output": join(&dir, &format!("out-{source}")), "format": "png",
+            "steps": [["image.adjustments.invert", {}], ["image.adjustments.invert", {}]]});
+        if source == "folder" {
+            batch["input"] = json!([input]);
+        }
+        let r = s.execute("file.scripts.browse", json!({"steps": [["file.automate.batch", batch]]})).unwrap();
+        assert_eq!(r["ok"], false, "{source}: {r}");
+        assert!(r["results"][0]["error"].as_str().unwrap_or_default().contains("steps in all"), "{source}: {r}");
+        assert!(r["results"][0]["result"]["errors"].as_array().is_some_and(|e| !e.is_empty()), "{source}: {r}");
+    }
+}
+
+/// Per-file errors from a direct Batch stay script results, but a cancel or an error log that
+/// could not be written stops the script, also when files failed as well.
+#[test]
+fn a_direct_batch_in_a_script_stops_it_when_cancelled_or_its_log_fails() {
+    let dir = tmp("batch-script-stop");
+    let input = png(&dir, "in.png", 4, 4, "#ff0000");
+    let missing = join(&dir, "missing.png");
+    let next = json!(["layer.new.layer", {}]);
+
+    // Ordinary per-file errors: the script goes on.
+    let mut s = session(4, 4, 8);
+    let layers = doc(&s).layers.len();
+    let batch = json!({"input": [missing.clone()], "output": join(&dir, "out-plain"), "steps": []});
+    let r = s.execute("file.scripts.browse", json!({"steps": [["file.automate.batch", batch], next.clone()]})).unwrap();
+    assert_eq!(r["ok"], true, "{r}");
+    assert_eq!(doc(&s).layers.len(), layers + 1, "{r}");
+
+    // A failed error log next to a failed file: the script stops at the Batch.
+    let mut s = session(4, 4, 8);
+    let layers = doc(&s).layers.len();
+    let batch = json!({"input": [missing], "output": join(&dir, "out-log"), "steps": [], "errorLog": dir.clone()});
+    let r = s.execute("file.scripts.browse", json!({"steps": [["file.automate.batch", batch], next.clone()]})).unwrap();
+    assert_eq!(r["ok"], false, "{r}");
+    assert!(r["results"][0]["error"].as_str().unwrap_or_default().contains("error log"), "{r}");
+    assert_eq!(doc(&s).layers.len(), layers, "the step after the Batch must not run: {r}");
+
+    // A cancel: the script stops at the Batch.
+    let mut s = session(4, 4, 8);
+    let ctx = crate::jobs::JobCtx::new();
+    s.set_inline_job_ctx(Some(ctx.clone()));
+    REVIEW_CANCEL.with(|slot| *slot.borrow_mut() = Some(ctx.clone()));
+    s.authorize = Some(cancel_at_marker);
+    let layers = doc(&s).layers.len();
+    let batch = json!({"input": [input], "output": join(&dir, "out-cancel"), "steps": [["layer.renameLayer", {"name": "Cancel here"}]]});
+    let r = s.execute("file.scripts.browse", json!({"steps": [["file.automate.batch", batch], next]})).unwrap();
+    REVIEW_CANCEL.with(|slot| *slot.borrow_mut() = None);
+    assert_eq!(r["ok"], false, "{r}");
+    assert_eq!(r["results"][0]["error"], "Batch was cancelled", "{r}");
+    assert_eq!(doc(&s).layers.len(), layers, "the step after the Batch must not run: {r}");
+}
+
 /// Two spellings of one destination through a symlinked folder are one output claim.
 #[cfg(not(target_arch = "wasm32"))]
 #[test]

crates/ui-egui/src/control/dispatch.rs

@@ -1320,7 +1320,22 @@ fn parse_pointer_event(i: usize, e: &Value) -> std::result::Result<PointerSample
 
 /// Whether a modal tool, picker or context menu took the event instead of the canvas.
 fn pointer_event_intercepted(app: &mut PhotocraftApp, ctx: &egui::Context, g: &PointerGesture, s: &PointerSample) -> bool {
-    pointer_eyedropper_took(app, ctx, g, s) || color_range_took(app, g, s) || (g.secondary && secondary_menu_took(app, ctx, g, s))
+    let took = pointer_eyedropper_took(app, ctx, g, s) || color_range_took(app, g, s) || (g.secondary && secondary_menu_took(app, ctx, g, s));
+    if took && matches!(s.ev, ToolEvent::Up { .. }) && !g.secondary && !g.middle {
+        release_taken_drag(app, g);
+    }
+    took
+}
+
+/// A sampler (Color Picker, Curves/Levels or Properties eyedroppers, Curves ⌘-click, Color
+/// Range) took the primary release: a transform handle drag or Move drag begun before it
+/// ends, so later moves can't resume either without a new press. The Move drag ends where it
+/// last was, since the release's coordinates belong to the sampler.
+fn release_taken_drag(app: &mut PhotocraftApp, g: &PointerGesture) {
+    crate::tools::transform_tool::release_handle_drag(app);
+    if let Some([x, y]) = app.drag.as_ref().map(|d| d.points.last().map_or(d.start, |p| [p[0], p[1]])) {
+        tool_event(app, ToolEvent::Up { x, y }, g.mods);
+    }
 }
 
 fn pointer_eyedropper_took(app: &mut PhotocraftApp, ctx: &egui::Context, g: &PointerGesture, s: &PointerSample) -> bool {
@@ -1328,14 +1343,7 @@ fn pointer_eyedropper_took(app: &mut PhotocraftApp, ctx: &egui::Context, g: &Poi
     let down = matches!(s.ev, ToolEvent::Down { .. });
     // With the Color Picker on top the image is its eyedropper, as for the mouse.
     if crate::color_picker_ui::top(app).is_some() {
-        if matches!(s.ev, ToolEvent::Up { .. }) {
-            crate::tools::transform_tool::release_handle_drag(app);
-            // A drag begun before the picker opened ends where it last was (the release's
-            // coordinates are the picker's), so later moves can't resume it without a press.
-            if let Some([x, y]) = app.drag.as_ref().map(|d| d.points.last().map_or(d.start, |p| [p[0], p[1]])) {
-                tool_event(app, ToolEvent::Up { x, y }, g.mods);
-            }
-        } else {
+        if !matches!(s.ev, ToolEvent::Up { .. }) {
             crate::color_picker_ui::sample_at(app, x, y);
         }
         return true;

crates/ui-egui/src/transform_controls_pointer_tests.rs

@@ -533,6 +533,37 @@ fn color_picker_release_clears_the_handle_drags_snap_lines() {
     assert!(app.prefs_rt.snap.is_none());
 }
 
+/// Every sampler that takes the primary release ends a transform handle drag begun before it,
+/// not only the Color Picker: the Curves eyedroppers and Color Range too. A move after the
+/// sampler closes needs a new press.
+#[test]
+fn other_samplers_releases_end_an_existing_transform_handle_drag() {
+    for sampler in ["curves", "colorRange"] {
+        let (mut app, ctx, _) = fixture();
+        pointer(&mut app, &ctx, "down", [800.0, 500.0]);
+        pointer(&mut app, &ctx, "move", [700.0, 440.0]);
+        let preview = app.ui.transform.as_ref().unwrap().quad;
+        let dialog = match sampler {
+            "curves" => {
+                let id = crate::adjust_dialog::open(&mut app, "image.adjustments.curves").unwrap();
+                let d = app.ui.dialog_mut(id).unwrap();
+                let base = crate::filter_dialog::params_of(&d.fields);
+                d.fields.insert("__curvePicker".into(), json!("black"));
+                d.fields.insert("__curvePickerBase".into(), base);
+                assert!(crate::adjust_dialog::picker_armed(&app), "{sampler}");
+                id
+            }
+            _ => crate::color_range_ui::open(&mut app),
+        };
+        pointer(&mut app, &ctx, "down", [200.0, 150.0]);
+        pointer(&mut app, &ctx, "up", [999.0, 690.0]);
+        assert_eq!(app.ui.transform.as_ref().unwrap().quad, preview, "{sampler}: the sampler's coordinates do not move the box");
+        app.ui.dialogs.retain(|d| d.id != dialog);
+        pointer(&mut app, &ctx, "move", [650.0, 410.0]);
+        assert_eq!(app.ui.transform.as_ref().unwrap().quad, preview, "{sampler}: later moves need a new press");
+    }
+}
+
 /// The Color Picker takes the release of an ordinary Move drag: the drag ends where it last was, and
 /// moving after the picker closes needs a new press.
 #[test]