Merge #400: Let batch steps reference ids created by earlier steps (@edivanteixeira)

crates/engine/src/batch_step_ref_tests.rs

@@ -0,0 +1,112 @@
+use serde_json::json;
+
+use crate::{EngineError, Session};
+
+fn id(value: &serde_json::Value) -> crate::NodeId {
+    crate::params::id_of(&value["id"]).unwrap()
+}
+
+#[test]
+fn later_steps_target_layers_created_by_earlier_steps() {
+    let mut s = Session::new();
+    let results = s
+        .execute(
+            "command.batch",
+            &json!({
+                "label": "Card",
+                "commands": [
+                    {"command": "node.create", "params": {"type": "frame", "x": 0, "y": 0, "width": 200, "height": 100}},
+                    {"command": "node.create", "params": {"type": "rectangle", "parent": {"$step": 0}, "x": 10, "y": 10, "width": 20, "height": 20}},
+                    {"command": "node.set", "params": {"ids": [{"$step": 1}], "props": {"name": "Swatch"}}}
+                ]
+            }),
+        )
+        .unwrap();
+
+    let frame = id(&results[0]);
+    let rect = id(&results[1]);
+    let doc = s.doc().unwrap();
+    assert_eq!(doc.node(rect).unwrap().parent, Some(frame));
+    assert_eq!(doc.node(rect).unwrap().name, "Swatch");
+    assert_eq!(s.ds().unwrap().history.undo.len(), 1, "still one undo step");
+}
+
+#[test]
+fn field_picks_another_part_of_the_result() {
+    let mut s = Session::new();
+    let results = s
+        .execute(
+            "command.batch",
+            &json!({"commands": [
+                {"command": "node.create", "params": {"type": "frame", "width": 50, "height": 50}},
+                {"command": "node.create", "params": {"type": "text", "text": "hi", "parent": {"$step": 0, "field": "id"}}}
+            ]}),
+        )
+        .unwrap();
+    assert_eq!(s.doc().unwrap().node(id(&results[1])).unwrap().parent, Some(id(&results[0])));
+}
+
+#[test]
+fn bad_references_fail_and_roll_back() {
+    let mut s = Session::new();
+    let before = s.doc().unwrap().node(s.page().unwrap()).unwrap().children.len();
+    let frame = json!({"command": "node.create", "params": {"type": "frame", "width": 10, "height": 10}});
+    for (bad, needle) in [
+        (json!({"$step": 1}), "earlier step"),
+        (json!({"$step": 5}), "earlier step"),
+        (json!({"$step": -1}), "step index"),
+        (json!({"$step": "0"}), "step index"),
+        (json!({"$step": 0, "field": "nope"}), "no `nope`"),
+        (json!({"$step": 0, "field": 3}), "`field` must be a string"),
+        (json!({"$step": 0, "extra": true}), "not `extra`"),
+    ] {
+        let r = s.execute("command.batch", &json!({"commands": [frame, {"command": "node.set", "params": {"ids": [bad], "props": {"name": "x"}}}]}));
+        match r {
+            Err(EngineError::BadParams { msg, .. }) => assert!(msg.contains(needle), "{msg} should mention {needle}"),
+            other => panic!("expected BadParams for {bad}, got {other:?}"),
+        }
+        assert_eq!(s.doc().unwrap().node(s.page().unwrap()).unwrap().children.len(), before, "rolled back");
+    }
+}
+
+#[test]
+fn deep_params_are_refused_not_overflowed() {
+    let mut s = Session::new();
+    let mut deep = json!(1);
+    for _ in 0..200 {
+        deep = json!([deep]);
+    }
+    let r = s.execute("command.batch", &json!({"commands": [{"command": "select.none", "params": {"x": deep}}]}));
+    assert!(matches!(r, Err(EngineError::BadParams { .. })));
+}
+
+#[test]
+fn text_that_looks_like_a_reference_is_left_alone() {
+    let mut s = Session::new();
+    let results = s
+        .execute("command.batch", &json!({"commands": [{"command": "node.create", "params": {"type": "text", "text": "$step 0 {\"$step\": 0}"}}]}))
+        .unwrap();
+    let n = s.doc().unwrap().node(id(&results[0])).unwrap().clone();
+    assert!(matches!(n.kind, figcraft_doc::NodeKind::Text(t) if t.characters == "$step 0 {\"$step\": 0}"));
+}
+
+#[test]
+fn a_nested_batch_refers_to_its_own_steps() {
+    let mut s = Session::new();
+    let frame = |name: &str| json!({"command": "node.create", "params": {"type": "frame", "name": name, "width": 100, "height": 100}});
+    let results = s
+        .execute(
+            "command.batch",
+            &json!({"commands": [
+                frame("Outer"),
+                {"command": "command.batch", "params": {"commands": [
+                    frame("Inner"),
+                    {"command": "node.create", "params": {"type": "rectangle", "parent": {"$step": 0}, "width": 10, "height": 10}}
+                ]}}
+            ]}),
+        )
+        .unwrap();
+    let inner = &results[1];
+    let rect = s.doc().unwrap().node(id(&inner[1])).unwrap().clone();
+    assert_eq!(rect.parent, Some(id(&inner[0])), "in the nested batch's own first frame, not the outer one");
+}

crates/engine/src/lib.rs

@@ -107,6 +107,47 @@ pub fn bad(cmd: &str, msg: impl Into<String>) -> EngineError {
     EngineError::BadParams { cmd: cmd.to_string(), msg: msg.into() }
 }
 
+/// Deepest params nesting `resolve_step_refs` walks before giving up.
+const STEP_REF_MAX_DEPTH: usize = 64;
+
+/// Replace every `{"$step": n}` / `{"$step": n, "field": "…"}` in `v` with that field (default
+/// `id`) of `results[n]`, the result of an earlier batch step.
+fn resolve_step_refs(v: &Value, results: &[Value], step: usize, depth: usize) -> Result<Value> {
+    if depth > STEP_REF_MAX_DEPTH {
+        return Err(bad("command.batch", format!("step {step}: params nest deeper than {STEP_REF_MAX_DEPTH} levels")));
+    }
+    match v {
+        Value::Object(o) if o.contains_key("$step") => {
+            if let Some(k) = o.keys().find(|k| *k != "$step" && *k != "field") {
+                return Err(bad("command.batch", format!("step {step}: a `$step` reference takes only `field`, not `{k}`")));
+            }
+            let n = o
+                .get("$step")
+                .and_then(Value::as_u64)
+                .ok_or_else(|| bad("command.batch", format!("step {step}: `$step` must be a step index (0, 1, …)")))?;
+            let field = match o.get("field") {
+                None => "id",
+                Some(Value::String(f)) => f.as_str(),
+                Some(_) => return Err(bad("command.batch", format!("step {step}: `field` must be a string"))),
+            };
+            let Some(result) = usize::try_from(n).ok().and_then(|n| results.get(n)) else {
+                let range = if step == 0 { "there is none before step 0".to_string() } else { format!("0 to {}", step - 1) };
+                return Err(bad("command.batch", format!("step {step}: `$step` {n} must be an earlier step ({range})")));
+            };
+            result.get(field).cloned().ok_or_else(|| bad("command.batch", format!("step {step}: step {n}'s result has no `{field}`")))
+        }
+        Value::Object(o) => {
+            let mut out = serde_json::Map::with_capacity(o.len());
+            for (k, x) in o {
+                out.insert(k.clone(), resolve_step_refs(x, results, step, depth + 1)?);
+            }
+            Ok(Value::Object(out))
+        }
+        Value::Array(a) => a.iter().map(|x| resolve_step_refs(x, results, step, depth + 1)).collect::<Result<Vec<_>>>().map(Value::Array),
+        _ => Ok(v.clone()),
+    }
+}
+
 /// One undo step.
 #[derive(Clone, Debug)]
 pub struct HistoryEntry {
@@ -1085,9 +1126,20 @@ impl Session {
         self.ui_requests.push(UiRequest { kind: kind.to_string(), data });
     }
 
-    /// Run several commands as one undo step.
+    /// Run several commands as one undo step. A param value `{"$step": n}` is replaced by the `id`
+    /// in the result of the earlier step `n` (`{"$step": n, "field": "…"}` picks another field),
+    /// so a step can target a layer that an earlier step of the same batch created. Any object
+    /// with a `$step` key in a step's params is such a reference, except inside a nested
+    /// `command.batch` step: its references are to its own steps.
     pub fn batch(&mut self, label: &str, cmds: &[(String, Value)]) -> Result<Vec<Value>> {
-        self.grouped(label, |s| cmds.iter().map(|(id, p)| s.execute(id, p)).collect())
+        self.grouped(label, |s| {
+            let mut results: Vec<Value> = Vec::with_capacity(cmds.len());
+            for (step, (id, p)) in cmds.iter().enumerate() {
+                let p = if id == "command.batch" { p.clone() } else { resolve_step_refs(p, &results, step, 0)? };
+                results.push(s.execute(id, &p)?);
+            }
+            Ok(results)
+        })
     }
 
     /// Run `f` as one undo step called `label`: the commands it executes push no steps of their
@@ -1410,6 +1462,8 @@ mod autosave_tests;
 #[cfg(test)]
 mod batch_rollback_tests;
 #[cfg(test)]
+mod batch_step_ref_tests;
+#[cfg(test)]
 mod binding_lists_tests;
 #[cfg(test)]
 mod booleans_tests;

crates/mcp/src/tools.rs

@@ -128,7 +128,7 @@ pub fn tool_definitions() -> Vec<Value> {
         tool(
             "batch",
             "Run commands as one step",
-            "Run several commands in order as ONE undo step (engine `command.batch`). If any command fails, everything is rolled back and the error names the failing step. Returns an array of the results in order. Note: later commands cannot reference ids created by earlier ones in the same batch (ids are only known afterwards), so create containers first, then batch the edits.",
+            "Run several commands in order as ONE undo step (engine `command.batch`). If any command fails, everything is rolled back and the error names the failing step. Returns an array of the results in order. A later step can use a layer an earlier step created: anywhere in its params, {\"$step\": n} stands for the `id` in step n's result (zero-based; {\"$step\": n, \"field\": \"…\"} picks another field). Example: [{\"command\":\"node.create\",\"params\":{\"type\":\"frame\"}}, {\"command\":\"node.create\",\"params\":{\"type\":\"text\",\"text\":\"Hi\",\"parent\":{\"$step\":0}}}] builds a frame and a text inside it in one call.",
             obj(
                 json!({
                     "commands": {"type": "array", "minItems": 1, "description": "Steps to run", "items": obj(json!({"command": string("Command id"), "params": {"type": "object"}}), &["command"])},

crates/mcp/tests/agent_tasks.rs

@@ -307,3 +307,40 @@ fn task_vector_edit_keys() {
     assert_eq!(c.page_children().len(), 1);
     assert!(c.call("run_command", json!({"command": "vector.edit.state", "params": {}}))["isError"] == true, "Enter left edit mode");
 }
+
+/// A whole card in one `batch` call: later steps reach the frames earlier steps created through
+/// `{"$step": n}`, so the tree is built in one round-trip and one undo step.
+#[test]
+fn task_card_in_one_batch() {
+    let mut c = Client::new();
+    let out = c.ok(
+        "batch",
+        json!({"label": "Card", "commands": [
+            {"command": "node.create", "params": {"type": "frame", "name": "Card", "width": 320, "height": 50,
+                "props": {"layout": "vertical", "padding": 24, "gap": 12, "sizingV": "hug"}}},
+            {"command": "node.create", "params": {"type": "text", "parent": {"$step": 0}, "text": "Ada Lovelace"}},
+            {"command": "node.create", "params": {"type": "frame", "parent": {"$step": 0}, "name": "Button", "height": 44,
+                "props": {"layout": "horizontal", "sizingH": "fill"}}},
+            {"command": "node.create", "params": {"type": "text", "parent": {"$step": 2}, "text": "Follow"}},
+            {"command": "node.set", "params": {"ids": [{"$step": 2}], "props": {"fill": "#3B5BDB"}}},
+        ]}),
+    );
+    let results = ids(&out);
+    assert_eq!(results.len(), 5);
+    let (card, button) = (results[0]["id"].clone(), results[2]["id"].clone());
+    let n = c.node(&card);
+    assert_eq!(ids(&n["children"]).len(), 2, "title and button are inside the card");
+    let b = c.node(&button);
+    assert_eq!(ids(&b["children"]).len(), 1, "the label is inside the button");
+    assert_eq!(b["fills"][0]["color"].as_str().unwrap().to_ascii_uppercase(), "#3B5BDB");
+    assert_eq!(c.doc()["history"]["undo"][0], "Card", "one undo step");
+    // A forward reference fails the whole batch and names the step.
+    let e = c.err(
+        "batch",
+        json!({"commands": [
+            {"command": "node.create", "params": {"type": "frame", "parent": {"$step": 1}}},
+            {"command": "node.create", "params": {"type": "frame"}},
+        ]}),
+    );
+    assert!(e.contains("step 0") && e.contains("earlier step"), "{e}");
+}

docs/control-protocol.md

@@ -69,6 +69,15 @@ JSON or the gradient shorthand `{"type":"linearGradient","stops":["#hex","#hex"]
 {"id":3,"method":"engine.execute","params":{"command":"command.batch","params":{"label":"Tidy","commands":[{"command":"align.left","params":{"ids":[2,3]}},{"command":"distribute.vertical","params":{"ids":[2,3]}}]}}}
 ```
 
+Inside `command.batch`, `{"$step": n}` in a step's params stands for the `id` in step `n`'s result
+(zero-based; `{"$step": n, "field": "…"}` for another field; any object with a `$step` key is read
+as one, and a nested `command.batch` step keeps its own references to its own steps), so one batch
+can create a frame and fill it:
+
+```json
+{"id":4,"method":"engine.execute","params":{"command":"command.batch","params":{"commands":[{"command":"node.create","params":{"type":"frame","width":200,"height":100}},{"command":"node.create","params":{"type":"rectangle","parent":{"$step":0},"width":20,"height":20}}]}}}
+```
+
 ### `engine.commands {}`
 
 Every command: `[{id, label, params, menu, shortcut, undoable}]`.

docs/mcp.md

@@ -219,7 +219,7 @@ Or in a project's `.mcp.json`:
 |---|---|
 | `list_commands {filter?}` | Every command: id, label, menu, shortcut, undoable, params doc. |
 | `run_command {command, params}` | Run any command (e.g. `align.left`, `layout.add`, `component.create`, `instance.place` (drop an instance at a page point into the frame or auto layout slot under it), `component.exposeInstance` / `instance.exposed` (nested instance properties on instances), `component.makeSlot` / `instance.slotInsert` / `instance.resetSlot` (slots, below), `inspect.code` (CSS, HTML + CSS for a whole frame, React, Leptos, SwiftUI or Jetpack Compose for a layer; components take their properties as parameters and instances are calls, see Dev Mode below), `inspect.properties` (layout, auto layout, typography, fills, strokes and effects with their style and variable names; `colorFormat`: hex, rgb, css, hsl or hsb), `inspect.assets` (layers under the selection with export settings, icons and images, each with its `kind`), `inspect.code` takes `unit: rem` and `rootFontSize` for CSS, HTML, React and Leptos, `style.create`, `variable.bind`, `variable.update` (rename, describe, scope), `variable.set` with `{alias}`, `variable.renameMode` / `variable.removeMode`, `variable.deleteCollection`, `proto.interaction.add`, `codeConnect.set` (components linked to real code, see Dev Mode below)). |
-| `batch {commands: [{command, params}], label?}` | Several commands as one undo step (`command.batch`); all-or-nothing for open documents and their document state. Failed inner commands are removed from the journal; preferences, clipboard, tool state and UI requests are not rolled back. |
+| `batch {commands: [{command, params}], label?}` | Several commands as one undo step (`command.batch`); all-or-nothing for open documents and their document state. Failed inner commands are removed from the journal; preferences, clipboard, tool state and UI requests are not rolled back. A step can use what an earlier step returned: `{"$step": n}` anywhere in its params becomes the `id` in step n's result (zero-based), and `{"$step": n, "field": "…"}` another field of it (see Step references below). |
 | `create_node {type, x, y, width, height, parent, index, name, text, path, props}` | Create a frame, rectangle, ellipse, polygon, star, line, arrow, text, vector, section, component or slice; returns `{id, node}`. |
 | `set_properties {ids, props}` | `node.set` on layers (geometry, fills, strokes, effects, corner radius, auto layout, sizing, constraints, masks (`isMask`, `maskType: alpha\|vector\|luminance`), handoff status (`devStatus: none\|readyForDev\|completed`; marking again clears "Changed"), a section's `sectionContentsHidden`, text…); returns the updated layers. |
 | `inspect_document {depth, allPages}` | The document as JSON (layer tree, styles, selection, history). |
@@ -1517,6 +1517,29 @@ Align and distribute as one undo step:
   {"command":"distribute.horizontal","params":{"ids":[3,4,5]}}]}}}
 ```
 
-The end-to-end tests in `crates/mcp/tests/agent_tasks.rs` are worked examples of ten realistic
-tasks (auto layout, components and overrides, gradients and shadows, paint styles, Light/Dark
-variables, align/distribute/group, boolean subtract + flatten, export, undo/redo, drawing with tools).
+### Step references
+
+A step in a `batch` can use what an earlier step returned. `{"$step": n}` anywhere in a step's
+params (a value, an item in `ids`, a nested field) is replaced by the `id` in the result of step `n`
+(zero-based) before the step runs; `{"$step": n, "field": "name"}` takes another field of that
+result. This builds a nested tree in one call and one undo step, without guessing ids:
+
+```json
+{"jsonrpc":"2.0","id":9,"method":"tools/call","params":{"name":"batch","arguments":{"label":"Card","commands":[
+  {"command":"node.create","params":{"type":"frame","name":"Card","props":{"layout":"vertical","gap":12}}},
+  {"command":"node.create","params":{"type":"text","parent":{"$step":0},"text":"Ada Lovelace"}},
+  {"command":"node.create","params":{"type":"frame","parent":{"$step":0},"name":"Button"}},
+  {"command":"node.create","params":{"type":"text","parent":{"$step":2},"text":"Follow"}}]}}}
+```
+
+A reference to the same or a later step, a negative or non-integer index, a missing `field` or any
+other key next to `$step` fails the whole batch, which is rolled back. Only an object whose keys are
+`$step` (and optionally `field`) is a reference: text such as `"$step 0"` is left as written. The
+`$step` key is reserved in step params: an object holding it anywhere (in plugin data or a variable
+value too) is read as a reference. A step that is itself a `command.batch` is passed on as written,
+so the references in its steps are to its own steps, not the outer batch's.
+
+The end-to-end tests in `crates/mcp/tests/agent_tasks.rs` are worked examples of realistic tasks
+(auto layout, a card built in one batch with step references, components and overrides, gradients
+and shadows, paint styles, Light/Dark variables, align/distribute/group, boolean subtract + flatten,
+export, undo/redo, drawing with tools).