MCP: add_effect is one undo step and leaves nothing applied when it fails (#175)

add_effect ran effect.apply and then one prop.set per `values` entry as
separate commands: each became its own undo step, and a bad value (an
unknown parameter id) returned an error but left the effect, and any
values set before it, on the layer, so a corrected retry landed at
effects/#2.

It now runs them as one engine.batch, named like effect.apply's own undo
step ("Apply Gaussian Blur"): one undo removes the effect with its values,
and a failing step rolls the whole call back. The reply is unchanged.

Test: add_effect_is_one_undo_step_and_all_or_nothing.

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

crates/automation/src/tests.rs

@@ -416,6 +416,32 @@ fn high_level_tools_add_effect_and_report_state() {
     assert!(err && c[0]["text"].as_str().unwrap().contains("unknown effect"), "{c:?}");
 }
 
+/// add_effect is one undo step, and a call that fails leaves the layer and the history as they
+/// were (#175).
+#[test]
+fn add_effect_is_one_undo_step_and_all_or_nothing() {
+    let mut s = server();
+    call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"name": "A", "width": 64, "height": 64}}));
+    call_json(&mut s, "execute_command", json!({"command": "layer.newSolid", "params": {"name": "S", "color": "#ff0000"}}));
+    let undo = |s: &mut McpServer| call_json(s, "get_project", json!({}))["undo"].as_array().unwrap().clone();
+    let effects = |s: &mut McpServer| {
+        let l = call_json(s, "get_layer", json!({"layer": "S", "flat": true}));
+        l["properties"].as_array().unwrap().iter().filter(|p| p["path"].as_str().unwrap().starts_with("effects/")).count()
+    };
+    let before = undo(&mut s);
+    let (c, err) = call(&mut s, "add_effect", json!({"layer": "S", "effect": "Gaussian Blur", "values": {"blurriness": 8, "repeatEdgePixels": true}}));
+    assert!(err && c[0]["text"].as_str().unwrap().contains("effects/#1/repeatEdgePixels"), "{c:?}");
+    assert_eq!((undo(&mut s), effects(&mut s)), (before.clone(), 0), "nothing applied");
+    // The corrected retry lands at #1, in one undo step named like effect.apply's.
+    let r = call_json(&mut s, "add_effect", json!({"layer": "S", "effect": "Gaussian Blur", "values": {"blurriness": 8, "repeatEdge": true}}));
+    assert_eq!((&r["path"], &r["set"]), (&json!("effects/#1"), &json!(["effects/#1/blurriness", "effects/#1/repeatEdge"])), "{r}");
+    let after = undo(&mut s);
+    assert_eq!(after.len(), before.len() + 1, "{after:?}");
+    assert_eq!(after.last(), Some(&json!("Apply Gaussian Blur")));
+    call_json(&mut s, "undo", json!({}));
+    assert_eq!((undo(&mut s), effects(&mut s)), (before, 0), "one undo removes the effect and its values");
+}
+
 #[test]
 fn list_fonts_lists_families_with_styles() {
     let mut s = server();

crates/automation/src/tools.rs

@@ -154,7 +154,8 @@ fn list_fonts(b: &mut Backend, a: &Value) -> Result<Reply> {
     json_reply(b.exec("text.fonts", obj(&[("query", get(a, "query")), ("rescan", get(a, "rescan"))]))?)
 }
 
-/// `effect.apply` on one layer, then set parameters on the new instance by param id.
+/// `effect.apply` on one layer, then set parameters on the new instance by param id: one batch,
+/// so one undo step, and a failing value leaves nothing applied.
 fn add_effect(b: &mut Backend, a: &Value) -> Result<Reply> {
     let (layer, effect, comp) = (need(a, "layer")?, need(a, "effect")?, get(a, "comp"));
     let count = |t: &Value| effects_group(t).and_then(|g| g.get("children")).and_then(Value::as_array).map_or(0, Vec::len);
@@ -162,16 +163,22 @@ fn add_effect(b: &mut Backend, a: &Value) -> Result<Reply> {
     let index = count(&before) + 1;
     let mut p = obj(&[("effect", Some(effect)), ("comp", comp)]);
     p["layers"] = json!([layer]);
-    b.exec("effect.apply", p)?;
+    let mut steps = vec![json!({"command": "effect.apply", "params": p})];
     let prefix = format!("effects/#{index}");
     let mut set = vec![];
     if let Some(vals) = get(a, "values").and_then(Value::as_object) {
         for (k, v) in vals {
             let path = format!("{prefix}/{k}");
-            b.exec("prop.set", obj(&[("layer", Some(layer)), ("comp", comp), ("path", Some(&json!(path))), ("value", Some(v))]))?;
+            steps.push(
+                json!({"command": "prop.set", "params": obj(&[("layer", Some(layer)), ("comp", comp), ("path", Some(&json!(path))), ("value", Some(v))])}),
+            );
             set.push(path);
         }
     }
+    // The undo step is named like effect.apply's own ("Apply Gaussian Blur").
+    let name = effect.as_str().map(|e| effectcraft_engine::effects::lookup(e).map_or(e, |s| s.name));
+    let label = name.map(|n| json!(format!("Apply {n}")));
+    b.exec("engine.batch", obj(&[("steps", Some(&json!(steps))), ("label", label.as_ref())]))?;
     let tree = b.exec("layer.tree", obj(&[("layer", Some(layer)), ("comp", comp)]))?;
     let inst = effects_group(&tree).and_then(|g| g.get("children")).and_then(Value::as_array).and_then(|c| c.get(index - 1)).cloned();
     let mut params = vec![];
@@ -590,7 +597,7 @@ static TOOLS: &[ToolDef] = &[
     },
     ToolDef {
         name: "add_effect",
-        description: "Apply an effect to a layer and optionally set its parameters in one undoable call. `effect` is an id (`ec.blur.gaussian`) or After Effects name (`Gaussian Blur`; see list_effects); `values` maps parameter ids to values, e.g. {\"blurriness\": 12}. Returns the instance `path` (`effects/#n`, for set_property / add_keyframe) and its parameters with their paths and current values.",
+        description: "Apply an effect to a layer and optionally set its parameters in one undoable call. `effect` is an id (`ec.blur.gaussian`) or After Effects name (`Gaussian Blur`; see list_effects); `values` maps parameter ids to values, e.g. {\"blurriness\": 12}. One undo step; if a value fails, nothing is applied. Returns the instance `path` (`effects/#n`, for set_property / add_keyframe) and its parameters with their paths and current values.",
         bridge_only: false,
         schema: || {
             schema(

docs/agents.md

@@ -59,7 +59,7 @@ The server speaks JSON-RPC 2.0 over stdio, one message per line, and supports MC
 | `add_keyframe {layer, path, time+value \| keys:[...], interpolation?, comp?}` | Adds keys, then optionally applies linear/bezier/hold/easyEase. |
 | `list_effects {filter?}` | Effect ids, names, categories, GPU / 32-bpc support and parameters. |
 | `list_fonts {query?, rescan?}` | Font families text layers can use, bundled and installed, with their styles, origin and own-language name; `rescan` picks up fonts installed since launch. |
-| `add_effect {layer, effect, values?, comp?}` | Apply an effect and set its parameters in one call; returns the instance path (`effects/#n`) and its parameter paths. |
+| `add_effect {layer, effect, values?, comp?}` | Apply an effect and set its parameters in one call and one undo step (a failing value leaves nothing applied); returns the instance path (`effects/#n`) and its parameter paths. |
 | `render_frame {comp?, time?, max_side?, path?, inline?, transparent?}` | Returns a PNG image of a frame; `transparent: true` keeps the alpha (as an RGB + Alpha render writes it) instead of compositing over the comp background. |
 | `open_project {path \| demo \| new}` / `save_project {path?}` | Open and save files. |
 | `undo {steps?}` / `redo {steps?}` | History. |