Merge pull request #1460 from MortenHusted/fix/unknown-params

M0.9: commands, MCP tools and batch steps refuse names they don't take (#1413)

apps/figcraft-cli/src/files.rs

@@ -366,6 +366,7 @@ pub fn parse_script(text: &str) -> Result<Vec<BatchStep>, String> {
         .enumerate()
         .map(|(i, s)| {
             let cmd = s.get("command").and_then(Value::as_str).ok_or_else(|| format!("step {i}: `command` (a string) is required"))?;
+            figcraft_engine::batch::refuse_unknown_step_keys(i, &s).map_err(|e| e.to_string())?;
             let params = match s.get("params") {
                 None | Some(Value::Null) => json!({}),
                 Some(p @ Value::Object(_)) => p.clone(),
@@ -443,6 +444,9 @@ mod tests {
         assert!(parse_script("[{\"command\":\"a\",\"params\":3}]").is_err());
         assert!(parse_script("not json\n").is_err());
         assert!(parse_script("42").is_err());
+        // `args` for `params` would run the step without them (#1413).
+        let e = parse_script(r#"[{"command":"a"},{"command":"b","args":{"x":1}}]"#).unwrap_err().to_string();
+        assert!(e.contains("step 1: unknown key `args`"), "{e}");
     }
 
     #[test]

crates/engine/src/automation/batch.rs

@@ -39,6 +39,18 @@ pub fn step_name(step: usize, json: &Value) -> Result<Option<String>> {
     }
 }
 
+/// Refuse a step key other than `command`, `params` and `as` (`step` is its index, for the
+/// error): `args` in place of `params` would run the command without them.
+pub fn refuse_unknown_step_keys(step: usize, json: &Value) -> Result<()> {
+    let Some(o) = json.as_object() else { return Ok(()) };
+    let unknown: Vec<String> = o.keys().filter(|k| !matches!(k.as_str(), "command" | "params" | "as")).map(|k| format!("`{k}`")).collect();
+    if unknown.is_empty() {
+        return Ok(());
+    }
+    let s = if unknown.len() == 1 { "" } else { "s" };
+    Err(bad("command.batch", format!("step {step}: unknown key{s} {} (a step takes command, params and as)", unknown.join(", "))))
+}
+
 /// The steps of one batch run so far: their results, and which step each `as` name belongs to.
 /// A nested `command.batch` step gets its own scope, so its references are to its own steps.
 #[derive(Clone, Debug, Default)]

crates/engine/src/canvas/vectoredit/width.rs

@@ -244,7 +244,7 @@ pub(crate) fn specs() -> Vec<CommandSpec> {
     vec![spec!(
         "vector.edit.widthPoint",
         "Width point",
-        "{action?: add|set|remove, index?, t? (0-1 along the path), width? (px) | scale? (× stroke weight)} — add needs t (width defaults to the current one); set moves or resizes point `index`; with `side` only that side of the path changes (looking along it): `width` is then the distance from the centre line to that edge and `scale` that side's scale (× stroke weight, as if both sides were that wide)",
+        "{action?: add|set|remove, index?, t? (0-1 along the path), width? (px) | scale? (× stroke weight), side? (left|right)} — add needs t (width defaults to the current one); set moves or resizes point `index`; with `side` only that side of the path changes (looking along it): `width` is then the distance from the centre line to that edge and `scale` that side's scale (× stroke weight, as if both sides were that wide)",
         true,
         width_point
     )]

crates/engine/src/cmd.rs

@@ -63,6 +63,8 @@ macro_rules! spec {
 struct Registry {
     list: Vec<CommandSpec>,
     index: HashMap<&'static str, usize>,
+    /// The parameter names each command's doc lists (`list` order; see [`doc_param_names`]).
+    names: Vec<Option<Vec<&'static str>>>,
 }
 
 fn registry() -> &'static Registry {
@@ -123,7 +125,8 @@ fn registry() -> &'static Registry {
                 log::warn!("duplicate command id {}", s.id);
             }
         }
-        Registry { list, index }
+        let names = list.iter().map(|s| doc_param_names(s.params)).collect();
+        Registry { list, index, names }
     })
 }
 
@@ -136,6 +139,113 @@ pub fn find_command(id: &str) -> Option<&'static CommandSpec> {
     r.index.get(id).and_then(|i| r.list.get(*i))
 }
 
+/// The parameter names command `id` takes, from its doc; `None` when the doc isn't a `{…}` list
+/// (`node.set` documents its props, and refuses the ones it doesn't know itself).
+pub fn param_names(id: &str) -> Option<&'static [&'static str]> {
+    let r = registry();
+    r.index.get(id).and_then(|i| r.names.get(*i)).and_then(|n| n.as_deref())
+}
+
+/// Refuse params naming something the command doesn't take: a misspelt name would otherwise be
+/// ignored and the command answer as if it hadn't been given. `ids` and `id` are always taken:
+/// the layers a command acts on (`params::targets` reads either).
+pub fn refuse_unknown_params(id: &str, params: &Value) -> Result<()> {
+    let (Some(names), Some(given)) = (param_names(id), params.as_object()) else { return Ok(()) };
+    let taken = |k: &str| matches!(k, "ids" | "id") || names.contains(&k);
+    let unknown: Vec<String> = given.keys().filter(|k| !taken(k)).map(|k| format!("`{k}`")).collect();
+    if unknown.is_empty() {
+        return Ok(());
+    }
+    let s = if unknown.len() == 1 { "" } else { "s" };
+    let takes = if names.is_empty() { "none".to_string() } else { names.join(", ") };
+    Err(crate::bad(id, format!("unknown parameter{s} {} (it takes {takes})", unknown.join(", "))))
+}
+
+/// The top-level items of a `{…}` params doc (`{ids?, direction? (horizontal|vertical)}` →
+/// `ids?`, ` direction? (horizontal|vertical)`), with those of the alternatives after it
+/// (`{x, y} | {points}`); `None` when the doc doesn't start with one.
+pub(crate) fn doc_items(doc: &'static str) -> Option<Vec<&'static str>> {
+    let mut body = doc.trim_start().strip_prefix('{')?;
+    let (mut items, mut depth, mut start) = (Vec::new(), 0usize, 0);
+    loop {
+        let mut rest = None;
+        for (i, c) in body.char_indices() {
+            match c {
+                '{' | '[' | '(' => depth += 1,
+                '}' | ']' | ')' if depth > 0 => depth -= 1,
+                '}' | ',' if depth == 0 => {
+                    items.push(body.get(start..i).unwrap_or_default());
+                    start = i + c.len_utf8();
+                    if c == '}' {
+                        rest = body.get(start..);
+                        break;
+                    }
+                }
+                _ => {}
+            }
+        }
+        let rest = rest?;
+        match rest.trim_start().strip_prefix('|').and_then(|r| r.trim_start().strip_prefix('{')) {
+            Some(next) => (body, start) = (next, 0),
+            None => return Some(items),
+        }
+    }
+}
+
+/// The names one doc item gives: its first word and the words joined to it by `|`, `/` or `+`
+/// (`id?/ids?`, `path | data (base64) + name`, `value? + newValue?`), without the `?` of optional
+/// ones. Bracketed text describes a name, and what follows a `:`, `=` or `→` its value.
+pub(crate) fn item_names(item: &'static str) -> Vec<&'static str> {
+    let mut outside = Vec::new();
+    let (mut depth, mut start) = (0usize, None);
+    for (i, c) in item.char_indices() {
+        let value_follows = depth == 0 && matches!(c, ':' | '=' | '→');
+        let in_word = depth == 0 && !value_follows && !c.is_whitespace() && !matches!(c, '(' | '[' | '{' | ')' | ']' | '}');
+        match c {
+            '(' | '[' | '{' => depth += 1,
+            ')' | ']' | '}' => depth = depth.saturating_sub(1),
+            _ => {}
+        }
+        match (in_word, start) {
+            (true, None) => start = Some(i),
+            (false, Some(s)) => {
+                outside.push(item.get(s..i).unwrap_or_default());
+                start = None;
+            }
+            _ => {}
+        }
+        if value_follows {
+            break;
+        }
+    }
+    if let Some(s) = start {
+        outside.push(item.get(s..).unwrap_or_default());
+    }
+    let is_name =
+        |n: &&str| n.chars().next().is_some_and(|c| c.is_ascii_alphabetic() || c == '_') && n.chars().all(|c| c.is_ascii_alphanumeric() || c == '_');
+    let mut words = outside.into_iter();
+    let (mut names, mut word) = (Vec::new(), words.next());
+    while let Some(w) = word {
+        names.extend(w.split(['/', '|']).map(|n| n.trim_end_matches('?')).filter(is_name));
+        word = match words.next() {
+            Some("|" | "/" | "+") => words.next(),
+            _ => None,
+        };
+    }
+    names
+}
+
+/// Every name a `{…}` params doc lists, once each (see [`item_names`]).
+fn doc_param_names(doc: &'static str) -> Option<Vec<&'static str>> {
+    let mut names = Vec::new();
+    for n in doc_items(doc)?.into_iter().flat_map(item_names) {
+        if !names.contains(&n) {
+            names.push(n);
+        }
+    }
+    Some(names)
+}
+
 /// `{"ok": true}` shorthand.
 pub fn ok() -> Result<Value> {
     Ok(serde_json::json!({"ok": true}))

crates/engine/src/cmd/comment.rs

@@ -576,7 +576,7 @@ pub fn specs() -> Vec<CommandSpec> {
                 Ok(json!({"id": id, "link": link(id)}))
             }
         ),
-        spec!("comment.reply", "Reply to comment", "{id, text, author?}", true, |s, p| {
+        spec!("comment.reply", "Reply to comment", "{id | link, text, author?}", true, |s, p| {
             let id = req_comment("comment.reply", s, p)?;
             let who = author(s, p);
             let r = Message::new(who.clone(), text("comment.reply", p)?, now_secs());
@@ -586,7 +586,7 @@ pub fn specs() -> Vec<CommandSpec> {
             c.touched_by(&who);
             Ok(json!({"reply": c.replies.len() - 1}))
         }),
-        spec!("comment.edit", "Edit comment", "{id, text, reply? (index; omit for the first message), author?}", true, |s, p| {
+        spec!("comment.edit", "Edit comment", "{id | link, text, reply? (index; omit for the first message), author?}", true, |s, p| {
             let id = req_comment("comment.edit", s, p)?;
             let t = text("comment.edit", p)?;
             let who = author(s, p);
@@ -607,7 +607,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "comment.react",
             "React to comment",
-            "{id, emoji, reply? (index; omit for the first message), on? (default: toggle), author?}",
+            "{id | link, emoji, reply? (index; omit for the first message), on? (default: toggle), author?}",
             true,
             |s, p| {
                 let id = req_comment("comment.react", s, p)?;
@@ -629,7 +629,7 @@ pub fn specs() -> Vec<CommandSpec> {
                 Ok(json!({"on": on}))
             }
         ),
-        spec!("comment.resolve", "Resolve comment", "{id, resolved? (default true)}", true, |s, p| {
+        spec!("comment.resolve", "Resolve comment", "{id | link, resolved? (default true)}", true, |s, p| {
             let id = req_comment("comment.resolve", s, p)?;
             let v = boolean(p, "resolved").unwrap_or(true);
             if let Some(c) = s.doc_mut()?.comment_mut(id) {
@@ -640,7 +640,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "comment.move",
             "Move comment",
-            "{id, x, y, anchor? (default true: pin to the innermost frame under the point; false: to the page; or a node id)}",
+            "{id | link, x, y, anchor? (default true: pin to the innermost frame under the point; false: to the page; or a node id)}",
             true,
             |s, p| {
                 let id = req_comment("comment.move", s, p)?;
@@ -655,24 +655,30 @@ pub fn specs() -> Vec<CommandSpec> {
                 ok()
             }
         ),
-        spec!("comment.delete", "Delete comment", "{id, reply? (index: delete one reply; omit to delete the thread), author?}", true, |s, p| {
-            let id = req_comment("comment.delete", s, p)?;
-            let who = author(s, p);
-            let doc = s.doc_mut()?;
-            let c = doc.comment_mut(id).ok_or_else(|| bad("comment.delete", "no comment"))?;
-            match message_index("comment.delete", c, p)? {
-                0 => {
-                    own("comment.delete", &c.first, &who, "delete this thread")?;
-                    doc.comments.retain(|c| c.id != id);
-                }
-                i => {
-                    let m = c.message(i).ok_or_else(|| bad("comment.delete", "no message"))?;
-                    own("comment.delete", m, &who, "delete this reply")?;
-                    c.replies.remove(i - 1);
+        spec!(
+            "comment.delete",
+            "Delete comment",
+            "{id | link, reply? (index: delete one reply; omit to delete the thread), author?}",
+            true,
+            |s, p| {
+                let id = req_comment("comment.delete", s, p)?;
+                let who = author(s, p);
+                let doc = s.doc_mut()?;
+                let c = doc.comment_mut(id).ok_or_else(|| bad("comment.delete", "no comment"))?;
+                match message_index("comment.delete", c, p)? {
+                    0 => {
+                        own("comment.delete", &c.first, &who, "delete this thread")?;
+                        doc.comments.retain(|c| c.id != id);
+                    }
+                    i => {
+                        let m = c.message(i).ok_or_else(|| bad("comment.delete", "no message"))?;
+                        own("comment.delete", m, &who, "delete this reply")?;
+                        c.replies.remove(i - 1);
+                    }
                 }
+                ok()
             }
-            ok()
-        }),
+        ),
         spec!(
             "comment.markRead",
             "Mark comment as read",
@@ -711,7 +717,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "comment.next",
             "Next comment",
-            "{id? (the thread to step from; omitted: the first or last), dir? (1 next, -1 previous; default 1), page?, filter?, sort?, query? (the list, as comment.list), author?}",
+            "{id? (the thread to step from; omitted: the first or last), dir? (1 next, -1 previous; default 1), page?, filter?, sort?, query?, frame?, resolved? (the list, as comment.list), author?}",
             false,
             |s, p| {
                 let dir = num(p, "dir").unwrap_or(1.0);
@@ -748,7 +754,7 @@ pub fn specs() -> Vec<CommandSpec> {
             }
             Ok(json!({"zoom": v.zoom}))
         }),
-        spec!("comment.link", "Copy link to comment", "{id}", false, |s, p| {
+        spec!("comment.link", "Copy link to comment", "{id | link}", false, |s, p| {
             let id = req_comment("comment.link", s, p)?;
             Ok(json!({"link": link(id)}))
         }),

crates/engine/src/cmd/create.rs

@@ -53,37 +53,61 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "node.create",
             "Create layer",
-            "{type: rectangle|ellipse|polygon|star|line|arrow|vector|text|frame|section|component|slice, x?, y? (relative to the parent frame; groups don't count), width?, height?, parent? (id; default: the page), index?, name?, text? (for text), path? (SVG path data for vector), props? {node.set keys}, select? (default true)}",
+            "{type: rectangle|ellipse|polygon|star|line|arrow|vector|text|frame|section|component|slice, x?, y? (relative to the parent frame; groups don't count), width?, height?, parent? (id; default: the page), index?, name?, text? | characters? (for text), path? | points? (vector: SVG path data | [[x,y],..]), closed? (vector), props? {node.set keys}, select? (default true)}",
             true,
             create
         ),
         spec!(
             "frame.create",
             "Create frame",
-            "{preset? (name from FRAME_PRESETS), x?, y?, width?, height?, parent?, name?, props?} (a preset without x/y lands to the right of the page's frames)",
+            "{preset? (name from FRAME_PRESETS), x?, y?, width?, height?, parent?, index?, name?, props?, select? (default true)} (a preset without x/y lands to the right of the page's frames)",
             true,
             frame_create
         ),
-        spec!("shape.rectangle", "Rectangle", "{x, y, width, height, parent?, props?}", true, |s, p| create_kind(s, p, "rectangle")),
-        spec!("shape.ellipse", "Ellipse", "{x, y, width, height, parent?, props?}", true, |s, p| create_kind(s, p, "ellipse")),
-        spec!("shape.polygon", "Polygon", "{x, y, width, height, parent?, props? {pointCount}}", true, |s, p| create_kind(s, p, "polygon")),
-        spec!("shape.star", "Star", "{x, y, width, height, parent?, props? {pointCount, innerRadius}}", true, |s, p| create_kind(s, p, "star")),
-        spec!("shape.line", "Line", "{x, y, width (length), parent?, props? {rotation}}", true, |s, p| create_kind(s, p, "line")),
-        spec!("shape.arrow", "Arrow", "{x, y, width (length), parent?, props?}", true, |s, p| create_kind(s, p, "arrow")),
-        spec!("text.create", "Text", "{text, x, y, parent?, width? (fixed width wraps), props? (fontSize, fontFamily, …)}", true, |s, p| {
-            create_kind(s, p, "text")
+        spec!("shape.rectangle", "Rectangle", "{x, y, width, height, parent?, index?, name?, props?, select? (default true)}", true, |s, p| {
+            create_kind(s, p, "rectangle")
         }),
+        spec!("shape.ellipse", "Ellipse", "{x, y, width, height, parent?, index?, name?, props?, select? (default true)}", true, |s, p| create_kind(
+            s, p, "ellipse"
+        )),
+        spec!(
+            "shape.polygon",
+            "Polygon",
+            "{x, y, width, height, parent?, index?, name?, props? {pointCount}, select? (default true)}",
+            true,
+            |s, p| create_kind(s, p, "polygon")
+        ),
+        spec!(
+            "shape.star",
+            "Star",
+            "{x, y, width, height, parent?, index?, name?, props? {pointCount, innerRadius}, select? (default true)}",
+            true,
+            |s, p| create_kind(s, p, "star")
+        ),
+        spec!("shape.line", "Line", "{x, y, width (length), parent?, index?, name?, props? {rotation}, select? (default true)}", true, |s, p| {
+            create_kind(s, p, "line")
+        }),
+        spec!("shape.arrow", "Arrow", "{x, y, width (length), parent?, index?, name?, props?, select? (default true)}", true, |s, p| create_kind(
+            s, p, "arrow"
+        )),
+        spec!(
+            "text.create",
+            "Text",
+            "{text | characters, x, y, parent?, index?, name? (default: the first line), width? (fixed width wraps), props? (fontSize, fontFamily, …), select? (default true)}",
+            true,
+            |s, p| { create_kind(s, p, "text") }
+        ),
         spec!(
             "vector.create",
             "Vector",
-            "{path (SVG path data) | points [[x,y],..], closed?, x?, y?, width?, height? (scale the path; one keeps its proportion), parent?, props?}",
+            "{path (SVG path data) | points [[x,y],..], closed?, x?, y?, width?, height? (scale the path; one keeps its proportion), parent?, index?, name?, props?, select? (default true)}",
             true,
             |s, p| create_kind(s, p, "vector")
         ),
         spec!(
             "image.place",
             "Place image",
-            "{data (base64) | path, x?, y?, width?, height?, parent?, target? (an existing layer: the image is added on top of its fills instead of making a layer), mode? (fill|fit|crop|tile)} → {id, image, width, height, imageProfile (its embedded colour profile; null: taken as sRGB)}",
+            "{data (base64) | path, x?, y?, width?, height?, parent?, name? (default Image), target? (an existing layer: the image is added on top of its fills instead of making a layer), mode? (fill|fit|crop|tile)} → {id, image, width, height, imageProfile (its embedded colour profile; null: taken as sRGB)}",
             true,
             place_image
         ),

crates/engine/src/cmd/edit.rs

@@ -248,8 +248,8 @@ pub fn specs() -> Vec<CommandSpec> {
             s.doc_mut()?.reorder(i, idx)?;
             ok()
         }),
-        spec!("fill.add", "Add fill", "{ids?, paint? (default #D9D9D9 or white for frames)}", true, |s, p| paint_list(s, p, true, true)),
-        spec!("stroke.add", "Add stroke", "{ids?, paint? (default black)}", true, |s, p| paint_list(s, p, false, true)),
+        spec!("fill.add", "Add fill", "{ids?, paint? | color? (default #D9D9D9 or white for frames)}", true, |s, p| paint_list(s, p, true, true)),
+        spec!("stroke.add", "Add stroke", "{ids?, paint? | color? (default black)}", true, |s, p| paint_list(s, p, false, true)),
         spec!("fill.remove", "Remove fill", "{ids?, index}", true, |s, p| paint_list(s, p, true, false)),
         spec!("stroke.remove", "Remove stroke", "{ids?, index}", true, |s, p| paint_list(s, p, false, false)),
         spec!("fill.set", "Set fill", "{ids?, index?, paint | color, opacity?, visible?, blend?}", true, |s, p| paint_set(s, p, true)),
@@ -273,7 +273,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "fill.pattern",
             "Pattern fill",
-            "{source (layer id), ids?, index? (default the top fill), tile? (grid|horizontalHex|verticalHex), scale? (1 = the source's size), spacing? [x, y], align? (start|center|end), opacity?}  the source layer tiled; it updates as the source changes",
+            "{source (layer id), ids?, index? (default the top fill), tile? (grid|horizontalHex|verticalHex), scale? (1 = the source's size), spacing? [x, y], align? (start|center|end), opacity?, visible?, blend?}  the source layer tiled; it updates as the source changes",
             true,
             pattern_fill
         ),
@@ -354,7 +354,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "export.update",
             "Edit export setting",
-            "{ids?, index, size? (2x|512w|512h|{by, value}), suffix?, format? (png|jpg|svg|pdf|webp|xml: Android Vector Drawable), bleed? (PDF: points, or with a unit: \"3mm\", \"0.125in\", \"9pt\"), cropMarks?, bleedMarks?, cmyk? (PDF print options), vdCompat? (XML: bestEffort|exact|ignore)}",
+            "{ids?, index, size? (2x|512w|512h|{by, value}), suffix?, format? (png|jpg|svg|pdf|webp|xml: Android Vector Drawable), bleed? (PDF: points, or with a unit: \"3mm\", \"0.125in\", \"9pt\"), cropMarks?, bleedMarks?, cmyk? (PDF print options), vdCompat? (XML: bestEffort|exact|ignore), contentsOnly?, includeBoundingBox?, svgOutlineText?, svgIdAttribute?, svgSimplifyStroke?}",
             true,
             |s, p| {
                 let cmd = "export.update";
@@ -387,6 +387,7 @@ pub fn specs() -> Vec<CommandSpec> {
                     for (i, c) in a.iter().enumerate() {
                         let cmd =
                             c.get("command").and_then(Value::as_str).ok_or_else(|| bad("command.batch", format!("step {i} has no `command`")))?;
+                        crate::batch::refuse_unknown_step_keys(i, c)?;
                         let params = c.get("params").cloned().unwrap_or(json!({}));
                         out.push(BatchStep { name: crate::batch::step_name(i, c)?, ..BatchStep::new(cmd, params) });
                     }

crates/engine/src/cmd/file.rs

@@ -110,7 +110,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "file.export",
             "Export",
-            "{ids? (default selection, else the page), format? (png|jpg|svg|pdf|webp|xml (an Android Vector Drawable, with `warnings` for what it changed or left out), or fig: the whole file as .fig, experimental, with `warnings` for what was left out; other apps may not open every file), scale? (default 1) | size? (2x|512w|512h or {by: scale|width|height, value}: a scale, or a width or height in pixels), path? (else returns base64 `data`), bleed? (PDF: points, or with a unit: \"3mm\", \"0.125in\", \"9pt\"), cropMarks?, bleedMarks?, cmyk? (PDF print options), svgIdAttribute? (SVG ids, XML android:name), vdCompat? (XML: bestEffort (default: approximate with a warning)|exact (leave out what can't be exact, with a warning)|ignore (no warnings))}",
+            "{ids? (default selection, else the page), format? (png|jpg|svg|pdf|webp|xml (an Android Vector Drawable, with `warnings` for what it changed or left out), or fig: the whole file as .fig, experimental, with `warnings` for what was left out; other apps may not open every file), scale? (default 1) | size? (2x|512w|512h or {by: scale|width|height, value}: a scale, or a width or height in pixels), path? (else returns base64 `data`), bleed? (PDF: points, or with a unit: \"3mm\", \"0.125in\", \"9pt\"), cropMarks?, bleedMarks?, cmyk? (PDF print options), contentsOnly? (default true; false also draws the layers that show in their area), includeBoundingBox? (true: only the layers' own boxes, cutting off shadows and outside strokes), svgOutlineText? (default true), svgSimplifyStroke? (default true), svgIdAttribute? (SVG ids, XML android:name), vdCompat? (XML: bestEffort (default: approximate with a warning)|exact (leave out what can't be exact, with a warning)|ignore (no warnings))}",
             false,
             export
         ),
@@ -121,13 +121,19 @@ pub fn specs() -> Vec<CommandSpec> {
             false,
             export_layers
         ),
-        spec!("file.exportSvg", "Export selection as SVG", "{ids?, path?}", false, |s, p| {
-            let mut q = p.clone();
-            if let Some(o) = q.as_object_mut() {
-                o.insert("format".into(), json!("svg"));
+        spec!(
+            "file.exportSvg",
+            "Export selection as SVG",
+            "{ids?, path?, scale? | size?, contentsOnly?, includeBoundingBox?, svgOutlineText?, svgIdAttribute?, svgSimplifyStroke? (file.export's SVG options)}",
+            false,
+            |s, p| {
+                let mut q = p.clone();
+                if let Some(o) = q.as_object_mut() {
+                    o.insert("format".into(), json!("svg"));
+                }
+                export(s, &q)
             }
-            export(s, &q)
-        }),
+        ),
         spec!(
             "file.exportFramesPdf",
             "Export frames to PDF",
@@ -142,7 +148,13 @@ pub fn specs() -> Vec<CommandSpec> {
             true,
             import
         ),
-        spec!("file.placeImage", "Place image", "{path | data, x?, y?}", true, super::create::place_image),
+        spec!(
+            "file.placeImage",
+            "Place image",
+            "{path | data, x?, y?, width?, height?, parent?, name?, target?, mode? (as image.place)}",
+            true,
+            super::create::place_image
+        ),
         spec!("file.close", "Close tab", "{index?, force? (discard unsaved changes)}", false, close),
         spec!("file.moveTab", "Move tab", "{from, to} (indices among open documents)", false, |s, p| {
             let from = req_index("file.moveTab", p, "from")?;

crates/engine/src/cmd/proto.rs

@@ -16,7 +16,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "proto.interaction.add",
             "Add interaction",
-            "{id?, trigger? {type: onClick|onHover|onPress|onDrag|mouseEnter|mouseLeave|mouseDown|mouseUp|afterDelay {ms}|keyPress {key (a key or a combination: Shift+K, Ctrl+Alt+Delete; or a controller button: Gamepad A|B|X|Y|LB|RB|LT|RT|View|Menu|LS|RS|Up|Down|Left|Right|Home)}}, action {type: navigate {preserveScroll?}|back|scrollTo {offset? [x, y] (where the layer ends up from the top-left)}|openOverlay {position?, anchor? screen|triggerInside|triggerOutside (default screen; the trigger ones measure the position preset on the layer that opens the overlay: outside puts top presets above it and bottom presets below it, edges lined up), offset? [x, y] (position manual: the overlay's top-left from the top-left of the layer that opens it; without it, right below that layer), close_on_outside? (default true), background?}|swapOverlay|closeOverlay|openUrl {url}|changeTo|setVariable {variable, value | expression (text, e.g. \"count + 1\")}|setVariableMode {collection, mode}|conditional {blocks [{condition? | expression? (text, e.g. \"count >= 3\"), actions}]}, dest?}, actions? [..], transition? {type: instant|dissolve|smartAnimate|moveIn|moveOut|push|slideIn|slideOut, direction?: left|right|top|bottom}, durationMs?, easing? linear|easeIn|easeOut|easeInOut|easeInBack|easeOutBack|easeInOutBack|gentle|quick|bouncy|slow|{customBezier {x1, y1, x2, y2}}|{customSpring {mass, stiffness, damping}}}, delayMs? (mouse enter, leave, down and up: how long after the gesture the actions run, up to 600000)} — or {id, dest} for a click-navigate. Returns {index}",
+            "{id?, trigger? {type: onClick|onHover|onPress|onDrag|mouseEnter|mouseLeave|mouseDown|mouseUp|afterDelay {ms}|keyPress {key (a key or a combination: Shift+K, Ctrl+Alt+Delete; or a controller button: Gamepad A|B|X|Y|LB|RB|LT|RT|View|Menu|LS|RS|Up|Down|Left|Right|Home)}}, action {type: navigate {preserveScroll?}|back|scrollTo {offset? [x, y] (where the layer ends up from the top-left)}|openOverlay {position?, anchor? screen|triggerInside|triggerOutside (default screen; the trigger ones measure the position preset on the layer that opens the overlay: outside puts top presets above it and bottom presets below it, edges lined up), offset? [x, y] (position manual: the overlay's top-left from the top-left of the layer that opens it; without it, right below that layer), close_on_outside? (default true), background?}|swapOverlay|closeOverlay|openUrl {url}|changeTo|setVariable {variable, value | expression (text, e.g. \"count + 1\")}|setVariableMode {collection, mode}|conditional {blocks [{condition? | expression? (text, e.g. \"count >= 3\"), actions}]}, dest?}, actions? [..], dest? (without action: a click-navigate to it), transition? {type: instant|dissolve|smartAnimate|moveIn|moveOut|push|slideIn|slideOut, direction?: left|right|top|bottom}, durationMs? | duration_ms?, easing? linear|easeIn|easeOut|easeInOut|easeInBack|easeOutBack|easeInOutBack|gentle|quick|bouncy|slow|{customBezier {x1, y1, x2, y2}}|{customSpring {mass, stiffness, damping}}, delayMs? | delay_ms? (mouse enter, leave, down and up: how long after the gesture the actions run, up to 600000)} — or {id, dest} for a click-navigate. Returns {index}",
             true,
             add
         ),
@@ -51,7 +51,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "proto.interaction.set",
             "Edit interaction",
-            "{id?, index, interaction {..}} replaces it; or patch fields: {id?, index, trigger?, action? (first action), actions?, transition?, durationMs?, easing?, delayMs?}",
+            "{id?, index, interaction? {..} (replaces it whole), trigger?, action? (first action), actions?, transition?, durationMs? | duration_ms?, easing?, delayMs? | delay_ms?}  patch fields on top of the current interaction (or of interaction)",
             true,
             set
         ),

crates/engine/src/cmd/structure.rs

@@ -83,7 +83,7 @@ pub fn specs() -> Vec<CommandSpec> {
             }
             ok()
         }),
-        spec!("layout.suggest", "Suggest auto layout", "{ids?}", true, layout_add),
+        spec!("layout.suggest", "Suggest auto layout", "{ids?, direction? (horizontal|vertical|grid)}", true, layout_add),
         spec!("object.moveToPage", "Move to page", "{ids?, page}", true, move_to_page),
         spec!("object.setThumbnail", "Set as thumbnail", "{id?}", true, |s, p| {
             let id = targets(s, p).first().copied().ok_or_else(|| bad("object.setThumbnail", "select a frame"))?;

crates/engine/src/cmd/styles.rs

@@ -164,7 +164,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "variable.create",
             "Create variable",
-            "{collection, name, type: color|number|string|boolean, value (default mode) | values {modeName: value}}",
+            "{collection, name, type: color|number|string|boolean, value (default mode) | values {modeName: value}, description?}",
             true,
             var_create
         ),

crates/engine/src/cmd/text.rs

@@ -279,13 +279,19 @@ pub fn specs() -> Vec<CommandSpec> {
                 .ok_or_else(|| bad("text.decoration", "`decoration`: none|underline|strikethrough"))?;
             style(s, p, |st| st.decoration = d)
         }),
-        spec!("text.listStyle", "List style", "{style: none|bulleted|numbered, ids?}  while editing: the selected paragraphs", true, |s, p| {
-            let l: ListStyle = get(p, "style")
-                .or_else(|| get(p, "list"))
-                .and_then(super::props::enum_of)
-                .ok_or_else(|| bad("text.listStyle", "`style`: none|bulleted|numbered"))?;
-            set_list(s, p, l, false)
-        }),
+        spec!(
+            "text.listStyle",
+            "List style",
+            "{style | list: none|bulleted|numbered, ids?}  while editing: the selected paragraphs",
+            true,
+            |s, p| {
+                let l: ListStyle = get(p, "style")
+                    .or_else(|| get(p, "list"))
+                    .and_then(super::props::enum_of)
+                    .ok_or_else(|| bad("text.listStyle", "`style`: none|bulleted|numbered"))?;
+                set_list(s, p, l, false)
+            }
+        ),
         spec!("text.paragraphIndent", "Paragraph indent", "{indent (px), ids?}", true, |s, p| {
             let v = finite(req_num("text.paragraphIndent", p, "indent")?).clamp(0.0, 10_000.0);
             para(s, p, |t| t.paragraph_indent = v)
@@ -374,7 +380,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!("text.endPreview", "End text preview", "{}  puts the document back as it was before text.preview → {ended}", false, |s, _| {
             Ok(json!({"ended": crate::textpreview::end(s)}))
         }),
-        spec!("text.set", "Set text", "{id?, characters}", true, |s, p| {
+        spec!("text.set", "Set text", "{id?, characters | text}", true, |s, p| {
             let c = string(p, "characters").or_else(|| string(p, "text")).ok_or_else(|| bad("text.set", "`characters` required"))?;
             let ids = req_targets("text.set", s, p)?;
             let doc = s.doc_mut()?;
@@ -390,7 +396,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "text.style",
             "Style text range",
-            "{id?, start?, end?, fontFamily?, fontStyle?, fontWeight?, fontSize?, lineHeight?, letterSpacing?, textDecoration?, textCase?, hyperlink?, textFills?}",
+            "{id?, start?, end?, fontFamily?, fontStyle?, fontWeight?, fontVariations? | variationSettings?, fontSize?, lineHeight?, letterSpacing?, textDecoration?, textDecorationStyle?, textDecorationThickness?, textDecorationOffset?, textDecorationSkipInk?, textDecorationColor?, textCase?, hyperlink?, textFills?}",
             true,
             |s, p| {
                 let ids = req_targets("text.style", s, p)?;

crates/engine/src/cmd/view.rs

@@ -373,7 +373,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "pref.uiScale",
             "Interface size",
-            "{scale?: a size from 0.75 to 3 relative to the system's display scale, 1 being the system's own size, so 1.25 is 125 %, not 125 (outside the range it stops at the nearest end; \"auto\" is 1; omit: just read it)} → {scale}: how large the whole interface is drawn; the canvas zoom is separate",
+            "{scale? (a size from 0.75 to 3 relative to the system's display scale: 1 is the system's own size, so 1.25 is 125 %, not 125; outside the range it stops at the nearest end; \"auto\" is 1; omit: just read it)} → {scale}: how large the whole interface is drawn; the canvas zoom is separate",
             false,
             |s, p| {
                 match get(p, "scale") {

crates/engine/src/dev/annotations.rs

@@ -247,7 +247,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "annotation.add",
             "Add annotation",
-            "{id? (default the first selected layer), label? (the note), properties?: [width|height|top|left|rotation|opacity|cornerRadius|direction|gap|padding|alignment|fontFamily|fontStyle|fontSize|lineHeight|letterSpacing|textAlign|fill|stroke|strokeWeight|effects], category? (id or name)} → {id, index}  (a note, pinned properties or both)",
+            "{id? (default the first selected layer), label? | text? (the note), properties?: [width|height|top|left|rotation|opacity|cornerRadius|direction|gap|padding|alignment|fontFamily|fontStyle|fontSize|lineHeight|letterSpacing|textAlign|fill|stroke|strokeWeight|effects], category? (id or name)} → {id, index}  (a note, pinned properties or both)",
             true,
             |s, p| {
                 let cmd = "annotation.add";
@@ -270,7 +270,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "annotation.edit",
             "Edit annotation",
-            "{id?, index, label?, properties?: [..] (replaces the pinned properties), category? (id or name; null: none)}",
+            "{id?, index, label? | text?, properties?: [..] (replaces the pinned properties), category? (id or name; null: none)}",
             true,
             |s, p| {
                 let cmd = "annotation.edit";

crates/engine/src/history_behaviour_tests.rs

@@ -188,7 +188,7 @@ fn threads_replies_and_resolving() {
             |c| c.replies.iter().map(|m| m.text.as_str()).eq(["Sure", "Done"]),
             "replies stay in the order they were written",
         ),
-        ("comment.resolve", json!({"id": c, "author": "Sam"}), |c| c.resolved, "anyone in the file can resolve a thread, not just its author"),
+        ("comment.resolve", json!({"id": c}), |c| c.resolved, "anyone in the file can resolve a thread, not just its author (it takes no author)"),
         ("comment.resolve", json!({"id": c, "resolved": false}), |c| !c.resolved, "a resolved thread can be reopened"),
         ("comment.resolve", json!({"id": c}), |c| c.resolved, "resolving again marks it resolved"),
         (

crates/engine/src/lib.rs

@@ -495,6 +495,8 @@ mod tiff_tests;
 #[cfg(test)]
 mod transform_behaviour_tests;
 #[cfg(test)]
+mod unknown_params_tests;
+#[cfg(test)]
 mod variable_tests;
 #[cfg(test)]
 mod variables_behaviour_tests;

crates/engine/src/live_tests.rs

@@ -158,8 +158,8 @@ fn viewport(page: NodeId, x: f64, y: f64, w: f64, h: f64) -> Presence {
 #[test]
 fn without_a_link_live_sessions_are_unavailable() {
     let mut s = Session::new();
-    for cmd in ["collab.host", "collab.join"] {
-        let e = s.execute(cmd, &json!({"link": "figcraft://join?host=a:1&file=f&token=t"})).unwrap_err();
+    for (cmd, p) in [("collab.host", json!({})), ("collab.join", json!({"link": "figcraft://join?host=a:1&file=f&token=t"}))] {
+        let e = s.execute(cmd, &p).unwrap_err();
         assert!(matches!(e, EngineError::Disabled(..)), "{cmd}: {e}");
     }
     assert_eq!(s.execute("collab.leave", &json!({})).unwrap()["ended"], false);

crates/engine/src/session/exec.rs

@@ -49,6 +49,7 @@ impl Session {
     /// as it was.
     pub fn execute(&mut self, id: &str, params: &Value) -> Result<Value> {
         let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.to_string()))?;
+        crate::cmd::refuse_unknown_params(id, params)?;
         board::refuse_on_board(self, id)?;
         // Revert replaces the document's uid: refuse before ending previews or syncing branches.
         if id == "file.revert" && self.live().doc_uid.is_some() {

crates/engine/src/sharing/live/join.rs

@@ -168,7 +168,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "collab.share",
             "Share with a join code",
-            "{via?: \"online\" (default: the open file is kept on the server in Preferences, `pref.publishServer`, and the server hands out the codes) | \"local\" (this app hosts it for people on the same network; no server), access?: \"edit\" (default) | \"view\" (online only: which code `code` is), server? (online: another server's address, with ?key=… when it needs its key), name? (online: the file's name there), port?, bind?: \"lan\" (default) | \"local\" (this machine only) | an IP address (local)} → the open file's session as `collab.peers` gives it: `code` is what others type to join (`collab.join {code}`, the Join field), `link` an invite link. Local: at once, the code carries the host's IPv4 address, port and secret (K7M2Q-…, 20 symbols). Online: status \"connecting\" first; once live `code` is the server's (K7M-2QX-9TD) and `share` is {via, access, shared, editCode, viewCode, editLink, viewLink}: links made for others (the file's own links in `published` stay yours); `code` stays null when the server is older than join codes (give `share.editLink` / `share.viewLink` instead). Called again while shared: only switches which code `code` is (`access`). Called after `collab.stop` on a file that is still on the server: new links and codes for that same file (nothing is published twice). `collab.stop` stops sharing",
+            "{via?: \"online\" (default: the open file is kept on the server in Preferences, `pref.publishServer`, and the server hands out the codes) | \"local\" (this app hosts it for people on the same network; no server), access?: \"edit\" (default) | \"view\" (online only: which code `code` is), server? (online: another server's address, with ?key=… when it needs its key), key? (online: the server's key, when the address has none), name? (online: the file's name there), port?, bind?: \"lan\" (default) | \"local\" (this machine only) | an IP address (local)} → the open file's session as `collab.peers` gives it: `code` is what others type to join (`collab.join {code}`, the Join field), `link` an invite link. Local: at once, the code carries the host's IPv4 address, port and secret (K7M2Q-…, 20 symbols). Online: status \"connecting\" first; once live `code` is the server's (K7M-2QX-9TD) and `share` is {via, access, shared, editCode, viewCode, editLink, viewLink}: links made for others (the file's own links in `published` stay yours); `code` stays null when the server is older than join codes (give `share.editLink` / `share.viewLink` instead). Called again while shared: only switches which code `code` is (`access`). Called after `collab.stop` on a file that is still on the server: new links and codes for that same file (nothing is published twice). `collab.stop` stops sharing",
             false,
             share
         ),
@@ -182,7 +182,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "collab.join",
             "Join live session",
-            "{code: a join code (K7M-2QX-9TD from a server, asked at the server in Preferences; a longer one for the local network; spaces, dashes and case don't matter, O reads as 0 and I or L as 1) | link: figcraft://join?host=…&file=…&token=…, figcraft://join?server=ws(s)://…&file=…&token=… or ws(s)://host:port/?file=…&token=… (a session an app hosts, or a server's share link; a code here works too)} → the new session: {session, status: \"connecting\", …}; its file opens in a tab of its own (other files' sessions go on) and `collab.peers` tells when it's live (`sessions` lists it until then, and if it fails)",
+            "{code | link (a join code: K7M-2QX-9TD from a server, asked at the server in Preferences; a longer one for the local network; spaces, dashes and case don't matter, O reads as 0 and I or L as 1; or an invite link: figcraft://join?host=…&file=…&token=…, figcraft://join?server=ws(s)://…&file=…&token=… or ws(s)://host:port/?file=…&token=…, from a session an app hosts or a server's share link; either key takes either)} → the new session: {session, status: \"connecting\", …}; its file opens in a tab of its own (other files' sessions go on) and `collab.peers` tells when it's live (`sessions` lists it until then, and if it fails)",
             false,
             join
         ),
@@ -210,7 +210,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "collab.manageFile",
             "Manage published file",
-            "{file (its id), server?, owner? (the server's address and the owner token you got when you published the file without the key; each defaults to what is recorded for one of your online files, see `collab.files`), action: links | createLink {access?: view (default) | edit, label?} | revoke {link (a link's id)} | share (others get new links and join codes) | unshare (the links and codes others got are revoked; the file stays yours on the server) | rename {name} | remove (the file is deleted from the server and leaves your online files)} → {links: [{id, access, label, created}]} | {id, access, link (a new share link, shown once)} | {shared, code, viewCode, editLink, viewLink} | {done}",
+            "{file (its id), server?, owner? (the server's address and the owner token you got when you published the file without the key; each defaults to what is recorded for one of your online files, see `collab.files`), action: links | createLink | revoke | share (others get new links and join codes) | unshare (the links and codes others got are revoked; the file stays yours on the server) | rename | remove (the file is deleted from the server and leaves your online files), access? (createLink: view (default) | edit), label? (createLink), link? (revoke: a link's id), name? (rename)} → {links: [{id, access, label, created}]} | {id, access, link (a new share link, shown once)} | {shared, code, viewCode, editLink, viewLink} | {done}",
             false,
             manage_file
         ),

crates/engine/src/tests.rs

@@ -478,11 +478,19 @@ fn every_command_survives_junk_params() {
             continue;
         }
         for j in &junk {
-            let mut s = sess();
-            let _ = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10}));
-            let _ = s.execute("text.create", &json!({"text": "t"}));
-            if let Err(EngineError::Internal { cmd, msg }) = s.execute(spec.id, j) {
-                panic!("{cmd} panicked on {j}: {msg}");
+            // Each junk object as given (names the command doesn't take are refused) and cut down
+            // to the names it takes, so the junk values still reach its parsers.
+            let mut taken = j.clone();
+            if let (Some(names), Some(o)) = (crate::cmd::param_names(spec.id), taken.as_object_mut()) {
+                o.retain(|k, _| matches!(k.as_str(), "ids" | "id") || names.contains(&k.as_str()));
+            }
+            for params in if taken == *j { vec![j] } else { vec![j, &taken] } {
+                let mut s = sess();
+                let _ = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10}));
+                let _ = s.execute("text.create", &json!({"text": "t"}));
+                if let Err(EngineError::Internal { cmd, msg }) = s.execute(spec.id, params) {
+                    panic!("{cmd} panicked on {params}: {msg}");
+                }
             }
         }
     }

crates/engine/src/unknown_params_tests.rs

@@ -0,0 +1,91 @@
+//! A parameter name a command doesn't take is refused, not ignored: a misspelt name would make the
+//! command answer as if it hadn't been given (#1413).
+
+use serde_json::{Value, json};
+
+use crate::Session;
+use crate::cmd::{all_commands, param_names};
+
+fn err(s: &mut Session, cmd: &str, params: Value) -> String {
+    match s.execute(cmd, &params) {
+        Err(e) => e.to_string(),
+        Ok(v) => panic!("{cmd} {params} ran: {v}"),
+    }
+}
+
+/// The issue's repro: a misspelt `filter` listed the resolved thread as if no filter were given.
+#[test]
+fn a_misspelt_parameter_is_refused_and_named() {
+    let mut s = Session::new();
+    s.execute("comment.add", &json!({"x": 10, "y": 10, "text": "a"})).unwrap();
+    s.execute("comment.resolve", &json!({"id": 1})).unwrap();
+    let e = err(&mut s, "comment.list", json!({"filterr": "open"}));
+    assert!(e.contains("unknown parameter `filterr`") && e.contains("it takes page, filter, sort"), "{e}");
+    assert_eq!(s.execute("comment.list", &json!({"filter": "open"})).unwrap()["comments"].as_array().map(Vec::len), Some(0));
+    let e = err(&mut s, "edit.undo", json!({"steps": 2, "stepz": 1}));
+    assert!(e.contains("unknown parameters `steps`, `stepz` (it takes none)"), "{e}");
+}
+
+/// `ids` and `id` name the layers a command acts on, documented or not (`params::targets` reads
+/// either), so `{id}` keeps working where a doc only says `ids?`.
+#[test]
+fn ids_and_id_are_always_taken() {
+    let mut s = Session::new();
+    let r = crate::params::id_of(&s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()["id"]).unwrap();
+    assert_eq!(param_names("align.left"), Some(&["ids"][..]));
+    s.execute("align.left", &json!({"id": r})).unwrap();
+    s.execute("edit.undo", &json!({"ids": []})).unwrap();
+}
+
+/// Names a doc gives as alternatives (`id? | ids?`, `id?/ids?`, `path | data`) are all taken, and
+/// `null` params or no params at all stay fine.
+#[test]
+fn documented_alternatives_and_empty_params_are_taken() {
+    assert_eq!(param_names("vector.fillRule").map(|n| n.contains(&"id") && n.contains(&"ids")), Some(true));
+    assert_eq!(param_names("text.setFeatures").map(|n| n.contains(&"id") && n.contains(&"ids")), Some(true));
+    assert_eq!(param_names("file.import").map(|n| n.contains(&"path") && n.contains(&"data") && n.contains(&"name")), Some(true));
+    assert_eq!(param_names("file.export").map(|n| n.contains(&"scale") && n.contains(&"size")), Some(true));
+    assert_eq!(param_names("collab.share").map(|n| n.contains(&"via") && !n.contains(&"local")), Some(true));
+    assert_eq!(param_names("component.editProperty").map(|n| n.contains(&"value") && n.contains(&"newValue")), Some(true));
+    // Docs whose shape used to hide names: a stray brace, a comma in an example, a second object.
+    assert_eq!(param_names("proto.interaction.add").map(|n| n.contains(&"delayMs") && n.contains(&"dest")), Some(true));
+    assert_eq!(param_names("collab.join"), Some(&["code", "link"][..]));
+    assert_eq!(param_names("proto.interaction.set").map(|n| n.contains(&"trigger") && n.contains(&"interaction")), Some(true));
+    // Commands that run another command's code take its names too.
+    assert_eq!(param_names("file.placeImage").map(|n| ["width", "height", "target", "mode"].iter().all(|k| n.contains(k))), Some(true));
+    assert_eq!(param_names("file.exportSvg").map(|n| ["scale", "size", "contentsOnly", "svgOutlineText"].iter().all(|k| n.contains(k))), Some(true));
+    // Alternative groups: `{x, y} | {points: …}`.
+    assert_eq!(param_names("vector.edit.paint"), Some(&["x", "y", "points"][..]));
+    assert_eq!(param_names("vector.edit.cut"), Some(&["segment", "t", "vertex", "from", "to"][..]));
+    // `node.set` documents its props and checks them itself.
+    assert_eq!(param_names("node.set"), None);
+    let mut s = Session::new();
+    for p in [json!(null), json!({})] {
+        s.execute("select.all", &p).unwrap();
+    }
+}
+
+/// Every top-level item of every `{…}` params doc gives a name, so no parameter a doc describes is
+/// refused.
+#[test]
+fn every_doc_item_names_a_parameter() {
+    let nameless: Vec<String> = all_commands()
+        .iter()
+        .flat_map(|c| crate::cmd::doc_items(c.params).unwrap_or_default().into_iter().map(move |i| (c.id, i)))
+        .filter(|(_, i)| !i.trim().is_empty() && crate::cmd::item_names(i).is_empty())
+        .map(|(id, i)| format!("{id}: `{i}`"))
+        .collect();
+    assert!(nameless.is_empty(), "doc items without a name: {nameless:#?}");
+}
+
+/// A batch step key other than `command`, `params` and `as` (`args` for `params`) is refused, and
+/// so is a step's unknown parameter, naming the step.
+#[test]
+fn batch_steps_refuse_unknown_keys_and_parameters() {
+    let mut s = Session::new();
+    let e = err(&mut s, "command.batch", json!({"commands": [{"command": "select.all"}, {"command": "inspect.code", "args": {"id": 2}}]}));
+    assert!(e.contains("step 1: unknown key `args`"), "{e}");
+    let e =
+        err(&mut s, "command.batch", json!({"commands": [{"command": "select.all"}, {"command": "comment.list", "params": {"filterr": "open"}}]}));
+    assert!(e.contains("comment.list") && e.contains("step 1: unknown parameter `filterr`"), "{e}");
+}

crates/mcp/src/tools.rs

@@ -155,6 +155,9 @@ pub fn tool_definitions() -> Vec<Value> {
                     "name": string("Layer name"),
                     "text": string("Characters, for type text"),
                     "path": string("SVG path data, for type vector (e.g. \"M0 0 L100 0 L50 80 Z\")"),
+                    "points": {"type": "array", "items": {"type": "array", "items": {"type": "number"}}, "description": "For type vector, instead of path: the points [[x, y], ...]"},
+                    "closed": boolean("For type vector: close the path"),
+                    "select": boolean("Select the new layer (default true)"),
                     "props": {"type": "object", "description": "Any set_properties keys applied at creation, e.g. {\"fill\":\"#3366FF\",\"cornerRadius\":12,\"fontSize\":24,\"layout\":\"vertical\",\"gap\":8,\"padding\":16}"},
                 }),
                 &["type"],
@@ -289,6 +292,7 @@ pub fn tool_definitions() -> Vec<Value> {
                     "shift": boolean("Shift held"),
                     "alt": boolean("Alt/Option held"),
                     "cmd": boolean("Cmd (macOS) / Ctrl held"),
+                    "ctrl": boolean("Ctrl held (the Control key on macOS)"),
                     "pressure": num("Pen pressure 0-1 (the Pencil records it as the stroke's width profile; a drag starts with it)"),
                     "pressureTo": num("Pen pressure 0-1 at a drag's end (interpolated along it)"),
                 }),
@@ -301,7 +305,7 @@ pub fn tool_definitions() -> Vec<Value> {
             "Press key",
             "Press a key or shortcut as if typed on the canvas: a menu shortcut (e.g. {\"key\":\"G\",\"cmd\":true} groups, {\"key\":\"A\",\"alt\":true,\"shift\":false} …), a tool key (R, O, T, F, V…), Escape, Delete, Enter, arrows (nudge; shift = big nudge). While editing text, keys go to the text editor (Backspace, Delete, Left, Right, Home, End, Enter, Escape, SelectAll).",
             obj(
-                json!({"key": string("Key name: a letter/digit, Escape, Delete, Backspace, Enter, ArrowLeft…"), "shift": boolean("Shift"), "alt": boolean("Alt/Option"), "cmd": boolean("Cmd/Ctrl")}),
+                json!({"key": string("Key name: a letter/digit, Escape, Delete, Backspace, Enter, ArrowLeft…"), "shift": boolean("Shift"), "alt": boolean("Alt/Option"), "cmd": boolean("Cmd/Ctrl"), "ctrl": boolean("Ctrl (the Control key on macOS)")}),
                 &["key"],
             ),
             false,
@@ -434,12 +438,45 @@ fn friendly_params(p: &mut Value) {
 
 // ---------- dispatch ----------
 
+/// Refuse argument names `schema` doesn't list, at the top and in the objects it describes inside
+/// (a batch's steps); objects it leaves open (`params`) take any. `path` leads the names in the
+/// error (`commands[0].`).
+fn refuse_unknown_args(schema: &Value, args: &Value, path: &str) -> Result<(), String> {
+    if let (Some(props), Some(given)) = (schema.get("properties").and_then(Value::as_object), args.as_object()) {
+        let unknown: Vec<String> = given.keys().filter(|k| !props.contains_key(*k)).map(|k| format!("`{path}{k}`")).collect();
+        if !unknown.is_empty() {
+            let s = if unknown.len() == 1 { "" } else { "s" };
+            let takes: Vec<&str> = props.keys().map(String::as_str).collect();
+            let takes = if takes.is_empty() { "none".to_string() } else { takes.join(", ") };
+            return Err(format!("unknown argument{s} {} (it takes {takes})", unknown.join(", ")));
+        }
+        for (k, v) in given {
+            if let Some(inner) = props.get(k) {
+                refuse_unknown_args(inner, v, &format!("{path}{k}."))?;
+            }
+        }
+    }
+    if let (Some(items), Some(list)) = (schema.get("items"), args.as_array()) {
+        for (i, v) in list.iter().enumerate() {
+            refuse_unknown_args(items, v, &format!("{}[{i}].", path.trim_end_matches('.')))?;
+        }
+    }
+    Ok(())
+}
+
 /// Run one tool. Every failure becomes an `isError` result (never a panic).
 pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult {
     let a = match args_obj(args) {
         Ok(a) => a,
         Err(e) => return ToolResult::error(e),
     };
+    // Built once: the catalogue is the same on every call.
+    static TOOLS: std::sync::LazyLock<Vec<Value>> = std::sync::LazyLock::new(tool_definitions);
+    if let Some(schema) = TOOLS.iter().find(|t| t.get("name").and_then(Value::as_str) == Some(name)).and_then(|t| t.get("inputSchema"))
+        && let Err(e) = refuse_unknown_args(schema, &Value::Object(a.clone()), "")
+    {
+        return ToolResult::error(e);
+    }
     let r = match name {
         "list_commands" => list_commands(b, &a),
         "run_command" => run_command(b, &a),

crates/mcp/tests/it/protocol.rs

@@ -83,6 +83,13 @@ fn tools_call_happy_and_error_paths() {
     assert!(c.err("pointer", json!({"x": 1})).contains("`y`"));
     assert!(c.err("type_text", json!({"text": "hi"})).contains("not editing text"));
     assert!(c.err("open_file", json!({"path": "/definitely/not/here.figcraft"})).contains("here.figcraft"));
+    // Names a tool or a command doesn't take are refused, not ignored (#1413).
+    let e = c.err("run_command", json!({"command": "inspect.code", "args": {"id": id, "language": "html"}}));
+    assert!(e.contains("unknown argument `args` (it takes command, params)"), "{e}");
+    let e = c.err("batch", json!({"commands": [{"command": "comment.list", "args": {"filter": "open"}}]}));
+    assert!(e.contains("unknown argument `commands[0].args`"), "{e}");
+    let e = c.err("run_command", json!({"command": "comment.list", "params": {"filterr": "open"}}));
+    assert!(e.contains("unknown parameter `filterr`"), "{e}");
     // Protocol-level errors.
     let r = c.rpc("tools/call", json!({"name": "no_such_tool", "arguments": {}}));
     assert_eq!(r["error"]["code"], -32602);
@@ -92,6 +99,10 @@ fn tools_call_happy_and_error_paths() {
     assert_eq!(r["error"]["code"], -32602);
     // The document is unharmed by all of that.
     assert_eq!(c.page_children().len(), 1);
+    // Arguments the tools pass on are in their schemas, so they still go through.
+    c.ok("create_node", json!({"type": "vector", "points": [[0, 0], [10, 0], [5, 8]], "closed": true, "select": false}));
+    c.ok("key", json!({"key": "Escape", "ctrl": false}));
+    c.ok("pointer", json!({"kind": "hover", "x": 1, "y": 1, "ctrl": false}));
 }
 
 #[test]

crates/ui-egui/src/gradient_shorthand_tests.rs

@@ -14,7 +14,7 @@ fn engine_execute_takes_the_gradient_shorthand() {
     let r = exec(&mut u, "shape.rectangle", json!({"x": 0, "y": 0, "width": 200, "height": 100})).unwrap()["id"].clone();
     let short = json!({"type": "linearGradient", "stops": ["#ff0000", "#0000ff"], "from": [0, 0.5], "to": [1, 0.5]});
     exec(&mut u, "node.set", json!({"ids": [r], "fills": [short]})).unwrap();
-    let n = exec(&mut u, "node.get", json!({"id": r, "depth": 0})).unwrap();
+    let n = exec(&mut u, "node.get", json!({"id": r})).unwrap();
     let f = &n["fills"][0];
     assert_eq!((f["type"].clone(), f["kind"].clone(), f["width"].clone()), (json!("gradient"), json!("linear"), json!([0.0, 1.5])), "{n}");
     assert_eq!(f["stops"].as_array().map(Vec::len), Some(2), "{n}");

crates/ui-egui/src/lib.rs

@@ -202,6 +202,8 @@ mod text_enter_tests;
 #[cfg(test)]
 mod titlebar_tests;
 #[cfg(test)]
+mod ui_params_tests;
+#[cfg(test)]
 mod ui_scale_tests;
 #[cfg(test)]
 mod variables_export_ui_tests;

crates/ui-egui/src/panels/export_panel.rs

@@ -247,12 +247,12 @@ pub(crate) fn print_options(ui: &mut Ui, id: Id, p: &PrintSettings) -> Option<Pr
 }
 
 /// Print options as command params (`bleed`, `cropMarks`, `bleedMarks`, `cmyk`).
-fn print_params(p: &PrintSettings) -> Value {
+pub(crate) fn print_params(p: &PrintSettings) -> Value {
     json!({"bleed": p.bleed(), "cropMarks": p.crop_marks, "bleedMarks": p.bleed_marks, "cmyk": p.cmyk})
 }
 
 /// `file.export` params for `setting`: its size, format and options (PDF: its print options).
-fn setting_params(setting: &ExportSetting) -> Value {
+pub(crate) fn setting_params(setting: &ExportSetting) -> Value {
     let mut params = serde_json::to_value(setting).unwrap_or_else(|_| json!({}));
     if let Value::Object(m) = &mut params {
         m.remove("print");

crates/ui-egui/src/panels/typography.rs

@@ -158,10 +158,11 @@ fn path_row(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId], ids:
 }
 
 /// Set character `props` on the selected chars of the text being edited, else on whole layers
-/// (through `node.set`, which records overrides inside instances).
-fn commit_text(app: &mut FigApp, ids: &[NodeId], range: Option<(NodeId, usize, usize)>, props: Value) {
+/// (through `node.set`, which records overrides inside instances). Paragraph spacing is always
+/// the whole layer's: `text.style` doesn't take it.
+pub(crate) fn commit_text(app: &mut FigApp, ids: &[NodeId], range: Option<(NodeId, usize, usize)>, props: Value) {
     match range {
-        Some((_, a, b)) if a != b => {
+        Some((_, a, b)) if a != b && props.get("paragraphSpacing").is_none() => {
             app.run("text.style", text_style_params(ids, range, props));
         }
         _ => app.set_props(ids, props),
@@ -170,7 +171,7 @@ fn commit_text(app: &mut FigApp, ids: &[NodeId], range: Option<(NodeId, usize, u
 
 /// `text.style` params for `props` on the selected chars of the text being edited, else on the
 /// whole of every layer in `ids` (previews; commits go through [`commit_text`]).
-fn text_style_params(ids: &[NodeId], range: Option<(NodeId, usize, usize)>, mut props: Value) -> Value {
+pub(crate) fn text_style_params(ids: &[NodeId], range: Option<(NodeId, usize, usize)>, mut props: Value) -> Value {
     match range {
         Some((id, a, b)) if a != b => {
             props["id"] = json!(id);

crates/ui-egui/src/ui_params_tests.rs

@@ -0,0 +1,77 @@
+//! Params the panels build at run time name only what their commands take: a name a command
+//! doesn't list is refused (#1413), which would break the control that sent it.
+
+use figcraft_doc::{ExportFormat, ExportSetting, NodeId, PrintSettings, VdCompat};
+use figcraft_engine::Session;
+use serde_json::{Value, json};
+
+use crate::{FigApp, NoServices};
+
+fn app_with(cmd: &str, params: Value) -> (FigApp, NodeId) {
+    let mut s = Session::new();
+    let id = NodeId(s.execute(cmd, &params).unwrap()["id"].as_u64().unwrap());
+    (FigApp::new(s, Box::new(NoServices)), id)
+}
+
+/// The Export section: a setting with every option off its default exports in every format, and
+/// its edits (the setting's own keys, the PDF print options) apply.
+#[test]
+fn export_settings_with_every_option_are_taken() {
+    let (mut app, id) = app_with("shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10}));
+    app.session.execute("export.add", &json!({"ids": [id]})).unwrap();
+    let print = PrintSettings { bleed: 3.0, crop_marks: true, bleed_marks: true, cmyk: true };
+    for format in ExportFormat::ALL {
+        let setting = ExportSetting {
+            format,
+            print,
+            contents_only: false,
+            include_bounding_box: true,
+            svg_outline_text: false,
+            svg_id_attribute: true,
+            svg_simplify_stroke: false,
+            vd_compat: VdCompat::Exact,
+            ..ExportSetting::default()
+        };
+        let mut p = crate::export_panel::setting_params(&setting);
+        p["ids"] = json!([id]);
+        app.session.execute("file.export", &p).unwrap_or_else(|e| panic!("file.export {p}: {e}"));
+        p["index"] = json!(0);
+        app.session.execute("export.update", &p).unwrap_or_else(|e| panic!("export.update {p}: {e}"));
+    }
+    let mut p = crate::export_panel::print_params(&print);
+    p["ids"] = json!([id]);
+    p["index"] = json!(0);
+    app.session.execute("export.update", &p).unwrap_or_else(|e| panic!("export.update {p}: {e}"));
+}
+
+/// The Typography section on selected characters: every character property it sets goes through
+/// `text.style`, and paragraph spacing (the whole layer's) through `node.set`.
+#[test]
+fn typography_edits_on_selected_characters_are_taken() {
+    let (mut app, id) = app_with("text.create", json!({"text": "Typography", "x": 0, "y": 0}));
+    let range = Some((id, 0, 4));
+    for (key, v) in [
+        ("fontFamily", json!("Inter")),
+        ("fontStyle", json!("Bold")),
+        ("fontSize", json!(20)),
+        ("lineHeight", json!(24)),
+        ("letterSpacing", json!(1)),
+        ("textDecoration", json!("underline")),
+        ("textDecorationStyle", json!("wavy")),
+        ("textDecorationThickness", json!(2)),
+        ("textDecorationOffset", json!(3)),
+        ("textDecorationSkipInk", json!(false)),
+        ("textDecorationColor", json!("#FF0000")),
+        ("textCase", json!("upper")),
+    ] {
+        crate::typography::commit_text(&mut app, &[id], range, json!({ key: v }));
+        assert_eq!(app.ui.last_error, None, "{key}");
+    }
+    // A named instance of a variable font: its style, and the custom axis values cleared.
+    crate::typography::commit_text(&mut app, &[id], range, json!({"fontStyle": "Regular", "fontVariations": null}));
+    assert_eq!(app.ui.last_error, None, "fontVariations");
+    crate::typography::commit_text(&mut app, &[id], range, json!({"paragraphSpacing": 12}));
+    assert_eq!(app.ui.last_error, None, "paragraphSpacing");
+    let spacing = app.session.doc().unwrap().node(id).and_then(|n| n.kind.text()).map(|t| t.paragraph_spacing);
+    assert_eq!(spacing, Some(12.0));
+}

docs/cli.md

@@ -133,7 +133,9 @@ Runs commands in a headless session (a new document, or `--in`), printing one JS
 ```
 
 The whole script is **one undo step**, and it is **all-or-nothing**, exactly like running the same
-commands through `command.batch` (the MCP tool `batch`, the control channel, plugins). A step named
+commands through `command.batch` (the MCP tool `batch`, the control channel, plugins). A step key
+other than `command`, `params` and `as`, or a parameter the command's doc (`figcraft-cli commands`)
+doesn't list, fails the script with an error naming it (`ids` and `id` are always taken). A step named
 with `"as"` can be used by later steps: `{"$ref": "name"}` in a step's params is the `id` in that
 step's result (`{"$step": n}` does the same by zero-based position), and `"field"` takes another key
 or a slash path such as `ids/0`. So a step can target a layer an earlier step of the same script

docs/control-protocol.md

@@ -99,7 +99,8 @@ Run any command (the same ids as the menus, shortcuts, Actions palette, CLI and
 calls never open dialogs: dialog requests are dropped. Returns the command's result (creation
 commands return `{"id": N}`). Paints (`fills`, `strokes`, `paint`) take a colour, the full paint
 JSON or the gradient shorthand `{"type":"linearGradient","stops":["#hex","#hex"],"from"?,"to"?}`
-(as in `docs/mcp.md`).
+(as in `docs/mcp.md`). A parameter the command's doc (`engine.commands`) doesn't list is an error
+naming it and the ones it takes, never ignored; `ids` and `id` are always taken.
 
 ```json
 {"id":1,"method":"engine.execute","params":{"command":"shape.rectangle","params":{"x":10,"y":10,"width":100,"height":50}}}

docs/mcp.md

@@ -1894,6 +1894,10 @@ turned off 0°.
 
 Tool failures come back as results with `isError: true` and a message; protocol problems (unknown
 tool, malformed JSON, wrong parameter types) are JSON-RPC errors. Nothing crashes the server.
+A name a tool or command doesn't take (`run_command {args}` for `params`, a step key other than
+`command`, `params` and `as`, or a parameter its `list_commands` entry doesn't list, such as
+`comment.list {filterr}`) is an error that lists the names it does take, never ignored; `ids` and
+`id` are taken by every command.
 
 ## Resources