Merge pull request #5888 from dotCooCoo/fix/flag-params-switch-or

Commands: true/false flags reject other values everywhere

crates/engine/src/cmd/appearance.rs

@@ -190,7 +190,7 @@ impl Session {
         let ids: Vec<u64> = ids.into_iter().map(|id| id.0).collect();
         let aligned = match item {
             ItemTarget::Item { aligned, .. } => aligned,
-            ItemTarget::Top => aligned(params),
+            ItemTarget::Top => aligned(params, "effect.apply").ok()?,
         };
         Some((json!({"ids": ids, "item": at, "aligned": aligned}), effects.as_slice()))
     }
@@ -241,11 +241,12 @@ pub(crate) fn item_target(s: &Session, p: &Value, cmd: &str) -> Result<ItemTarge
 
 /// [`item_target`] read from param `key` (e.g. `fromItem`).
 pub(crate) fn item_target_at(s: &Session, p: &Value, key: &str, cmd: &str) -> Result<ItemTarget> {
+    let aligned = aligned(p, cmd)?;
     match p.get(key) {
         Some(Value::Null) => Ok(ItemTarget::Top),
         Some(v) => v
             .as_u64()
-            .map(|i| ItemTarget::Item { index: index(i), explicit: true, aligned: aligned(p) })
+            .map(|i| ItemTarget::Item { index: index(i), explicit: true, aligned })
             .ok_or_else(|| bad(cmd, format!("`{key}` must be an appearance item index (paint order) or null"))),
         // The active item is a row of the first selected object's own stack.
         None if p.get("ids").is_some() || p.get("id").is_some() || targets_contents(p)? => Ok(ItemTarget::Top),
@@ -337,8 +338,8 @@ impl ItemTarget {
 
 /// Does `p` say its item and effect indices are rows of the first target's stack (`aligned`),
 /// which each other target edits where it has them (see [`ItemTarget::per_target`])?
-pub(crate) fn aligned(p: &Value) -> bool {
-    p.get("aligned").and_then(Value::as_bool).unwrap_or(false)
+pub(crate) fn aligned(p: &Value, cmd: &str) -> Result<bool> {
+    switch_or(p, "aligned", false, cmd)
 }
 
 /// What the appearances of `ids` share, when there are several of them and they all exist.
@@ -745,7 +746,7 @@ fn set_item(s: &mut Session, p: &Value) -> Result<Value> {
     // When every object comes out as it was (for example, when none has item `index`), the document
     // and its undo history stay as they are.
     let before = s.doc()?.doc.clone();
-    let per = items_per_target(&before, &ids, &[idx], aligned(p));
+    let per = items_per_target(&before, &ids, &[idx], aligned(p, "appearance.setItem")?);
     super::overprint::edit_counted(s, "Appearance", |d| {
         let mut edited = Vec::new();
         for (id, at) in ids.iter().zip(&per) {
@@ -818,7 +819,7 @@ fn remove_item(s: &mut Session, p: &Value) -> Result<Value> {
     // When no object has an item at any of the indices, the document and its undo history stay as
     // they are.
     let before = s.doc()?.doc.clone();
-    let per = items_per_target(&before, &ids, &idx, aligned(p));
+    let per = items_per_target(&before, &ids, &idx, aligned(p, "appearance.removeItem")?);
     super::overprint::edit_counted(s, "Remove Item", |d| {
         for (id, idx) in ids.iter().zip(&per) {
             if let Some(n) = d.node_mut(*id) {
@@ -854,11 +855,11 @@ fn duplicate_item(s: &mut Session, p: &Value) -> Result<Value> {
     let active = s.active_appearance_item.as_ref().and_then(|a| a.objects.first().copied());
     let mut active_at = None;
     // Aligned rows of a mixed selection are copied right above themselves in each object.
-    if aligned(p) && to.is_some() && shared_stack(&s.doc()?.doc, &ids).is_some() {
+    if aligned(p, C)? && to.is_some() && shared_stack(&s.doc()?.doc, &ids).is_some() {
         return Err(bad(C, "`to` takes the rows of one object's stack, not aligned rows"));
     }
     let before = s.doc()?.doc.clone();
-    let per = items_per_target(&before, &ids, &idx, aligned(p));
+    let per = items_per_target(&before, &ids, &idx, aligned(p, C)?);
     s.edit("Duplicate Item", |d, _| {
         let mut any = false;
         for (id, idx) in ids.iter().zip(&per) {
@@ -932,11 +933,12 @@ fn move_item(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "appearance.moveItem";
     let to = index_param(p, "to", C)?;
     let ids = require_targets(s, p, C)?;
+    let aligned = aligned(p, C)?;
     if p.get("from").and_then(Value::as_str) == Some("contents") {
         return move_contents(s, &ids, to);
     }
     // Where an aligned row would go in each of several different stacks isn't one place.
-    if aligned(p) && appearances_differ(&s.doc()?.doc, &ids) {
+    if aligned && appearances_differ(&s.doc()?.doc, &ids) {
         return Err(bad(C, "rows of objects whose appearances differ can't be reordered together"));
     }
     if p.get("indices").is_some() {

crates/engine/src/cmd/create.rs

@@ -721,7 +721,7 @@ fn text_create(s: &mut Session, p: &Value) -> Result<Value> {
     if rotation != 0.0 {
         t.xf *= vectorcraft_geom::rotate_deg(rotation);
     }
-    t.vertical = p.get("vertical").and_then(Value::as_bool).unwrap_or(false);
+    t.vertical = switch_or(p, "vertical", false, "text.create")?;
     t.para = new_type_para();
     new_type_alignment(s, p, "text.create", &mut t)?;
     if let Some(a) = p.get("area") {

crates/engine/src/cmd/edit.rs

@@ -11,7 +11,7 @@ use super::clipboard::SwatchChoices;
 use super::{
     CommandSpec, Result, Session, always, bad, bool_or, can_clear_history, can_redo, can_undo, checked_ids_param, cmd, f64_or, f64_req,
     finite_numbers, has_clipboard, has_doc, has_selection, has_selection_guides_or_artboard_tool, has_selection_or_guides, id_param, index, ok,
-    point_param, str_param,
+    point_param, str_param, switch_or,
 };
 use crate::{Clipboard, EngineError, inspect};
 
@@ -817,6 +817,7 @@ fn cut_direct_selection(s: &mut Session, roots: &[NodeId]) -> Result<Value> {
 }
 
 fn clear(s: &mut Session, p: &Value) -> Result<Value> {
+    let guides = switch_or(p, "guides", false, "edit.clear")?;
     let explicit = if p.get("ids").is_some() { Some(checked_ids_param(s, p, "ids", "edit.clear")?) } else { None };
     // Direct-selected anchors or segments: remove those, and any objects selected whole with them.
     if explicit.is_none() && (!s.doc()?.selection.anchors.is_empty() || !s.doc()?.selection.segments.is_empty()) {
@@ -829,7 +830,7 @@ fn clear(s: &mut Session, p: &Value) -> Result<Value> {
     let st = s.doc()?;
     // Guides selected with the art go with it (not when ids are given, unless `guides` says so:
     // Cut, which names the objects it copied).
-    let with_guides = explicit.is_none() || bool_or(p, "guides", false);
+    let with_guides = explicit.is_none() || guides;
     let is_explicit = explicit.is_some();
     let targets = explicit.unwrap_or_else(|| st.selection.in_paint_order(&st.doc));
     // Keep the selected ids themselves, without promoting compound members. Delete a selected

crates/engine/src/cmd/effectcmd.rs

@@ -16,7 +16,7 @@ use vectorcraft_render::effects;
 use super::appearance::{ItemTarget, aligned, appearance_targets, index_param, item_indices, item_target, item_target_at};
 use super::expand::has_seeded;
 use super::rasterfx;
-use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, fileio, has_doc, str_param};
+use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, fileio, has_doc, str_param, switch_or};
 
 /// The effect commands of the Effect menu and the Appearance panel.
 pub fn specs() -> Vec<CommandSpec> {
@@ -116,12 +116,13 @@ pub(crate) fn effect_targets(s: &Session, p: &Value, item: ItemTarget, index: us
     // Whether `item` names an item in the stack it is a row of (else the edit is object-level there).
     let anchor = roots.get(item.anchor(s, &roots)).and_then(|a| d.node(*a));
     let anchor_level = anchor.map(|a| item.effects_item(&a.appearance, cmd)).transpose()?.flatten();
+    let aligned = aligned(p, cmd)?;
     let mut out = Vec::with_capacity(roots.len());
     for (id, item) in roots.iter().zip(per) {
         let (Some(item), Some(n)) = (item, d.node(*id)) else { continue };
         let at = item.effects_item(&n.appearance, cmd)?;
         // An object-level effect this target shares with the first is where it is in this target.
-        let index = match (first.filter(|_| aligned(p)), at, anchor_level) {
+        let index = match (first.filter(|_| aligned), at, anchor_level) {
             (Some(f), None, None) => SharedStack::of(&[&*f.appearance, &*n.appearance]).effect_in(index, 1).unwrap_or(index),
             _ => index,
         };
@@ -236,6 +237,7 @@ pub(crate) fn apply(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let effect = effects::new_effect(id, &params).ok_or_else(|| bad(C, format!("unknown effect `{id}`")))?;
     let label = effects::effect_info(id).map(|e| e.label.trim_end_matches('…').to_string()).unwrap_or_default();
+    aligned(p, C)?;
     if let Some(loose) = loose_for_pathfinder(s, p, id)? {
         let gid = s.edit(&label, |d, sel| {
             let gid = super::object::group_nodes(d, &loose)?;
@@ -375,10 +377,10 @@ fn move_effect(s: &mut Session, p: &Value) -> Result<Value> {
     let to = index_param(p, "to", C)?;
     let src = item_target_at(s, p, "fromItem", C)?;
     let dst = if p.get("toItem").is_some() { item_target_at(s, p, "toItem", C)? } else { src };
-    let copy = p.get("copy").and_then(Value::as_bool).unwrap_or(false);
+    let copy = switch_or(p, "copy", false, C)?;
     let roots = appearance_targets(s, p)?;
     // Where an aligned effect would go in each of several different stacks isn't one place.
-    if aligned(p) && super::appearance::appearances_differ(&s.doc()?.doc, &roots) {
+    if aligned(p, C)? && super::appearance::appearances_differ(&s.doc()?.doc, &roots) {
         return Err(bad(C, "effects of objects whose appearances differ can't be moved together"));
     }
     // Where the effect landed in each object it moved in, in the order of `ids`.

crates/engine/src/cmd/maskedit.rs

@@ -14,7 +14,7 @@ use std::sync::Arc;
 use serde_json::{Value, json};
 use vectorcraft_doc::{Document, MaskEdit, Node, NodeId, NodeKind, Selection};
 
-use super::{CommandSpec, EngineError, Result, Session, bad, cmd, has_doc, id_param};
+use super::{CommandSpec, EngineError, Result, Session, bad, cmd, has_doc, id_param, switch_or};
 
 /// Name of the temporary mask-editing layer.
 pub const MASK_EDIT_LAYER: &str = "Opacity Mask Editing";
@@ -138,7 +138,7 @@ fn view_mask(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "transparency.viewOpacityMask";
     let st = s.doc()?;
     let editing = st.doc.mask_edit.map(|m| m.object);
-    if !p.get("on").and_then(Value::as_bool).unwrap_or(st.shown_mask().is_none()) {
+    if !switch_or(p, "on", st.shown_mask().is_none(), C)? {
         let st = s.doc_mut()?;
         let id = st.mask_view.take();
         st.revision += 1;

crates/engine/src/cmd/path.rs

@@ -1191,7 +1191,7 @@ fn convert_anchors(s: &mut Session, p: &Value) -> Result<Value> {
 
 pub(crate) fn delete_anchors(s: &mut Session, p: &Value) -> Result<Value> {
     // `guides: true` (Cut, #3226): the selected ruler guides go in the same undo step.
-    let with_guides = p.get("guides").and_then(Value::as_bool).unwrap_or(false);
+    let with_guides = switch_or(p, "guides", false, "path.deleteAnchors")?;
     let st = s.doc()?;
     let sel = st.selection.anchors.clone();
     let segments = st.selection.segments.clone();

crates/engine/src/cmd/plugin.rs

@@ -20,7 +20,7 @@ use serde_json::{Map, Value, json};
 use vectorcraft_doc::{Effect, Node};
 use vectorcraft_plugins::{Budget, Kind, Plugin, effect, export, objects, registry};
 
-use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, has_doc, ids_param, str_param};
+use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, has_doc, ids_param, str_param, switch_or};
 
 /// What a plug-in error looks like to the user. A run the caller cancelled changed nothing.
 fn err(e: vectorcraft_plugins::Error) -> EngineError {
@@ -288,12 +288,13 @@ fn load_given(c: &str, p: &Value) -> Result<(Plugin, Vec<u8>)> {
 /// `plugin.list` entry.
 fn install(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "plugin.install";
+    let (replace, allow_heavy) = (switch_or(p, "replace", true, C)?, switch_or(p, "allowHeavy", false, C)?);
     let (plugin, bytes) = load_given(C, p)?;
-    if !p.get("replace").and_then(Value::as_bool).unwrap_or(true) && registry::get(plugin.id()).is_some() {
+    if !replace && registry::get(plugin.id()).is_some() {
         return Err(bad(C, format!("a plug-in with id {:?} is already installed", plugin.id())));
     }
     // A heavy plug-in is installed once the user agrees: whoever installs it asks them first.
-    if plugin.manifest().budget == Budget::Heavy && !p.get("allowHeavy").and_then(Value::as_bool).unwrap_or(false) {
+    if plugin.manifest().budget == Budget::Heavy && !allow_heavy {
         let (minutes, times) = vectorcraft_plugins::heavy_terms();
         return Err(bad(
             C,

crates/engine/src/cmd/printtiling.rs

@@ -10,7 +10,7 @@ use vectorcraft_pdf::{TileOrigin, TilingPage};
 
 use super::fileio::pdf::pdf_error;
 use super::print::{settings, to_json};
-use super::{CommandSpec, Result, Session, bad, bool_or, cmd, has_doc, index};
+use super::{CommandSpec, Result, Session, bad, bool_or, cmd, has_doc, index, switch_or};
 
 const SET: &str = "print.tiling.set";
 
@@ -48,7 +48,7 @@ pub fn specs() -> Vec<CommandSpec> {
 
 fn show(s: &mut Session, p: &Value) -> Result<Value> {
     let st = s.doc_mut()?;
-    st.print_tiling = p.get("on").and_then(Value::as_bool).unwrap_or(!st.print_tiling);
+    st.print_tiling = switch_or(p, "on", !st.print_tiling, "view.printTiling")?;
     Ok(json!({ "on": st.print_tiling }))
 }
 

crates/engine/src/cmd/textedit.rs

@@ -1585,7 +1585,7 @@ fn create_in_path(s: &mut Session, p: &Value) -> Result<Value> {
     };
     let kind = if on_path { TextKind::OnPath { path: path.clone(), start, end: None } } else { TextKind::Area { frame: path.clone() } };
     let mut t = TextObject {
-        vertical: p.get("vertical").and_then(Value::as_bool).unwrap_or(false),
+        vertical: switch_or(p, "vertical", false, C)?,
         kind,
         xf: Affine::IDENTITY,
         runs: vec![TextRun { text, style, inline: None }],

crates/engine/src/tests/appearance.rs

@@ -981,3 +981,36 @@ fn removing_shared_and_unshared_rows_removes_only_the_shared_ones_elsewhere() {
     assert!(node(&s, a).appearance.items.is_empty(), "a loses both");
     assert_eq!(node(&s, b).appearance.items, fills_b, "b loses only its stroke, keeping both fills");
 }
+
+#[test]
+fn invalid_aligned_flags_leave_the_appearance_alone() {
+    let mut s = session();
+    let id = rect(&mut s, 100.0);
+    run(&mut s, "effect.apply", json!({"effect": "distort.twist"}));
+    let before = node(&s, id);
+    let before_undo = s.doc().unwrap().history.undo.len();
+    for (command, params) in [
+        ("appearance.setItem", json!({"index": 0, "opacity": 50, "aligned": "true"})),
+        ("appearance.removeItem", json!({"index": 0, "aligned": 1})),
+        ("appearance.duplicateItem", json!({"index": 0, "aligned": "false"})),
+        ("appearance.moveItem", json!({"from": 0, "to": 1, "aligned": 0})),
+        ("stroke.set", json!({"item": 1, "weight": 5, "aligned": "true"})),
+        ("stroke.set", json!({"weight": 5, "aligned": "true"})),
+        ("transparency.set", json!({"item": 0, "opacity": 50, "aligned": "true"})),
+        ("effect.apply", json!({"effect": "distort.roughen", "item": 0, "aligned": "true"})),
+        ("effect.remove", json!({"index": 0, "aligned": "true"})),
+    ] {
+        assert!(s.execute(command, &params).is_err(), "{command} accepted {params}");
+        assert_eq!(node(&s, id), before, "{command} changed the object");
+        assert_eq!(s.doc().unwrap().history.undo.len(), before_undo, "{command} recorded an undo step");
+    }
+    // Null, as for every command, is the same as leaving `aligned` out.
+    run(&mut s, "appearance.removeItem", json!({"index": 0, "aligned": null}));
+    assert_eq!(node(&s, id).appearance.items.len(), before.appearance.items.len() - 1);
+    // A Pathfinder effect on loose objects groups them first: the flag is checked before that.
+    let other = rect(&mut s, 250.0);
+    run(&mut s, "select.set", json!({"ids": [id.0, other.0]}));
+    let before_undo = s.doc().unwrap().history.undo.len();
+    assert!(s.execute("effect.apply", &json!({"effect": "pathfinder.add", "aligned": "true"})).is_err());
+    assert_eq!(s.doc().unwrap().history.undo.len(), before_undo, "grouped the objects");
+}

crates/engine/src/tests/effectedit.rs

@@ -70,6 +70,29 @@ fn effects_move_and_copy_between_the_object_and_its_items() {
     assert!(s.execute("effect.move", &json!({"from": 0, "fromItem": 1, "to": 0, "toItem": 7})).is_err());
 }
 
+#[test]
+fn invalid_effect_move_flags_leave_the_effects_alone() {
+    let (mut s, id) = session_with_rect();
+    run(&mut s, "effect.apply", json!({"effect": "distort.twist"}));
+    let before_undo = s.doc().unwrap().history.undo.len();
+    let to_fill = json!({"from": 0, "fromItem": null, "to": 0, "toItem": 0});
+    for (key, value) in [("copy", json!("true")), ("copy", json!(1)), ("aligned", json!("false"))] {
+        let mut params = to_fill.clone();
+        params[key] = value;
+        assert!(s.execute("effect.move", &params).is_err(), "accepted {params}");
+        assert_eq!(fx(&s, id, None), ["distort.twist"], "moved the effect for {params}");
+        assert!(fx(&s, id, Some(0)).is_empty(), "copied the effect for {params}");
+        assert_eq!(s.doc().unwrap().history.undo.len(), before_undo, "recorded undo for {params}");
+    }
+    // Null, as for every command, is the same as leaving the flag out: a move.
+    let mut params = to_fill;
+    params["copy"] = Value::Null;
+    params["aligned"] = Value::Null;
+    run(&mut s, "effect.move", params);
+    assert!(fx(&s, id, None).is_empty());
+    assert_eq!(fx(&s, id, Some(0)), ["distort.twist"]);
+}
+
 /// `effect.duplicate` copies a fill's effect right after it.
 #[test]
 fn duplicate_selected_works_for_effects() {

crates/engine/src/tests/maskview.rs

@@ -69,3 +69,23 @@ fn view_opacity_mask_shows_its_luminance_and_hides_the_artwork() {
     assert!(s.execute("transparency.viewOpacityMask", &json!({"on": true, "id": other.0})).is_err());
     assert!(s.doc().unwrap().mask_view.is_none());
 }
+
+#[test]
+fn invalid_view_opacity_mask_parameters_leave_the_view_unchanged() {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"width": 300, "height": 300})).unwrap();
+    let obj = rect(&mut s, 0.0, 200.0, "#ff0000");
+    let m = rect(&mut s, 0.0, 100.0, "#00ff00");
+    s.execute("select.set", &json!({"ids": [obj.0, m.0]})).unwrap();
+    s.execute("transparency.makeOpacityMask", &json!({})).unwrap();
+    let undo = s.doc().unwrap().history.undo.len();
+    for params in [json!({"on": "true"}), json!({"on": 1})] {
+        assert!(s.execute("transparency.viewOpacityMask", &params).is_err(), "accepted {params}");
+        let st = s.doc().unwrap();
+        assert!(st.doc.mask_edit.is_none() && st.shown_mask().is_none(), "showed the mask for {params}");
+        assert_eq!(st.history.undo.len(), undo, "recorded undo for {params}");
+    }
+    // Left out, or null as for every command, it toggles.
+    assert_eq!(s.execute("transparency.viewOpacityMask", &json!(null)).unwrap(), json!({"on": true, "id": obj.0}));
+    assert_eq!(s.execute("transparency.viewOpacityMask", &json!({"on": null})).unwrap(), json!({"on": false, "id": obj.0}));
+}

crates/engine/src/tests/menucmds.rs

@@ -3827,6 +3827,30 @@ fn guides_and_art_are_shift_selected_together() {
     assert_eq!(guides(&s).len(), 2);
 }
 
+#[test]
+fn invalid_clear_guides_parameters_delete_nothing() {
+    let mut s = session();
+    let a = rect(&mut s, 10.0, 10.0, 40.0, 20.0);
+    s.execute("guide.add", &json!({"vertical": true, "pos": 200})).unwrap();
+    sel(&mut s, &[a]);
+    s.execute("guide.select", &json!({"indexes": [0], "toggle": true})).unwrap();
+    let n = undo_len(&s);
+    for params in [json!({"ids": [a.0], "guides": "true"}), json!({"guides": 1})] {
+        assert!(s.execute("edit.clear", &params).is_err(), "accepted {params}");
+        assert!(s.doc().unwrap().doc.node(a).is_some(), "deleted the art for {params}");
+        assert_eq!((guides(&s).len(), undo_len(&s)), (1, n), "changed the document for {params}");
+    }
+    // Direct-selected anchors take the same check.
+    s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 1]], "mode": "set"})).unwrap();
+    assert!(s.execute("edit.clear", &json!({"guides": "false"})).is_err());
+    assert!(!s.doc().unwrap().selection.anchors.is_empty());
+    assert_eq!(undo_len(&s), n);
+    // Null, as for every command, is the same as leaving `guides` out: with ids, the guides stay.
+    s.execute("edit.clear", &json!({"ids": [a.0], "guides": null})).unwrap();
+    assert!(s.doc().unwrap().doc.node(a).is_none());
+    assert_eq!(guides(&s).len(), 1);
+}
+
 /// When ruler guides and art are selected together, moving the art moves the guides as well;
 /// copy leaves both in place and moves the copies; single undo reverts both.
 #[test]

crates/engine/src/tests/plugins.rs

@@ -493,6 +493,36 @@ fn a_heavy_filter_installs_once_the_user_agrees() {
     assert!(install(&mut s, DESATURATE).get("budget").is_none());
 }
 
+#[test]
+fn invalid_install_flags_are_refused() {
+    let mut s = session();
+    let id = "test.engine.heavy.flags";
+    let data = vectorcraft_format::base64_encode(&heavy(id, 1));
+    for allow in [json!("true"), json!(1)] {
+        let e = s.execute("plugin.install", &json!({"dataBase64": data, "allowHeavy": allow})).unwrap_err().to_string();
+        assert!(e.contains("`allowHeavy` must be true or false"), "allowHeavy {allow}: {e}");
+        assert!(vectorcraft_plugins::registry::get(id).is_none(), "installed with allowHeavy {allow}");
+    }
+    // Null, as for every command, is the same as leaving the flag out.
+    let e = s.execute("plugin.install", &json!({"dataBase64": data, "allowHeavy": null})).unwrap_err().to_string();
+    assert!(e.contains("heavy budget"), "{e}");
+    run(&mut s, "plugin.install", json!({"dataBase64": data, "allowHeavy": true}));
+    for replace in [json!("false"), json!(0)] {
+        let e = s.execute("plugin.install", &json!({"dataBase64": data, "allowHeavy": true, "replace": replace})).unwrap_err().to_string();
+        assert!(e.contains("`replace` must be true or false"), "replace {replace}: {e}");
+    }
+    run(&mut s, "plugin.install", json!({"dataBase64": data, "allowHeavy": true, "replace": null}));
+    assert!(vectorcraft_plugins::registry::get(id).is_some());
+    // A plug-in with the usual budget is refused too.
+    let light = "test.engine.light.flags";
+    let manifest = format!(r#"{{"id":"{light}","name":"Test {light}","version":"1","kind":"filter"}}"#);
+    let output = json!({"objects": [{"path": triangle(), "name": "Plug-in"}]});
+    let data = vectorcraft_format::base64_encode(&wat::parse_str(tpl::spinning(&manifest, 1, &output.to_string())).unwrap());
+    let e = s.execute("plugin.install", &json!({"dataBase64": data, "allowHeavy": "false"})).unwrap_err().to_string();
+    assert!(e.contains("`allowHeavy` must be true or false"), "{e}");
+    assert!(vectorcraft_plugins::registry::get(light).is_none());
+}
+
 /// The user agreed once: a heavy filter kept in the plug-in store is installed again at the next
 /// start without asking, with its budget.
 #[test]

crates/engine/src/tests/printtiling.rs

@@ -182,3 +182,18 @@ fn settings_saved_before_the_tile_origin_still_load() {
     assert_eq!((set["copies"].clone(), set["tileOrigin"]["placed"].clone()), (json!(2), json!(false)));
     assert_eq!(tiling(&mut s, json!({}))["pages"].as_array().unwrap().len(), 1);
 }
+
+#[test]
+fn invalid_print_tiling_parameters_leave_the_view_unchanged() {
+    let mut s = session();
+    let undo = s.doc().unwrap().history.undo.len();
+    for params in [json!({"on": "true"}), json!({"on": 1})] {
+        assert!(s.execute("view.printTiling", &params).is_err(), "accepted {params}");
+        let st = s.doc().unwrap();
+        assert!(!st.print_tiling, "showed the tiling for {params}");
+        assert_eq!(st.history.undo.len(), undo, "recorded undo for {params}");
+    }
+    // Left out, or null as for every command, it toggles.
+    assert_eq!(s.execute("view.printTiling", &json!(null)).unwrap(), json!({"on": true}));
+    assert_eq!(s.execute("view.printTiling", &json!({"on": null})).unwrap(), json!({"on": false}));
+}

docs/mcp.md

@@ -1349,6 +1349,7 @@ then composite against the art below it). SVG has no non-isolated groups, so the
 (default: the mask being edited, else the first selected masked object) as greyscale coverage (white = opaque) and the
 mask is edited; called again it shows the artwork, still editing. It is view state of the open document: the canvas
 (and `ui.screenshot`) shows it, exports never do, and it ends when mask editing ends.
+The command rejects an `on` value that is not a boolean. Omitting `on` toggles the current value.
 
 Isolation mode and mask editing exclude each other. In isolation mode `transparency.editOpacityMask` and
 `transparency.viewOpacityMask` are off, and `transparency.makeOpacityMask` refuses one target, an object or a layer,
@@ -1433,6 +1434,8 @@ or unknown id leaves the document unchanged. An empty `ids` array does nothing.
 are kept, and selecting an ancestor together with its descendants deletes that subtree once.
 Without `ids`, direct-selected anchors or selected ruler guides retain their usual Clear behavior.
 Cut still removes the objects it copied, including a whole compound when a member is selected.
+With `ids`, `guides: true` also deletes the selected ruler guides. The command rejects a `guides` value that is not a
+boolean.
 
 `object.setProps` with `visible: false` or `locked: true` deselects every selected object that is hidden or
 locked after the call: the selected objects it hides or locks, selected objects inside them, and selected objects
@@ -2058,6 +2061,8 @@ it (Alt-drag). `appearance.duplicateItem {index, to?}` / `{indices}` and `appear
 act on several fills/strokes at once, and `appearance.showAllHidden` makes every hidden fill, stroke and effect
 visible again. In remote mode, `effect.dialog {effect, index, item}` opens an applied effect's dialog prefilled
 (`ui.dialog.confirm` runs `effect.setParams`); see the control protocol for the "already applied" question.
+`effect.move` rejects a `copy` value that is not a boolean, and every command that takes `aligned` rejects an `aligned`
+value that is not a boolean.
 
 Effect dialogs use the targeted object, group, or layer shown in Appearance. The Transform dialog previews valid
 numbers as they are typed, including Copies, distances with units, and rotation. With Preview off, changes apply
@@ -4940,6 +4945,7 @@ outside `tileRange` dimmer). The Print Tiling tool (`printTiling`) shows them to
 what is drawn, from the layout `print.preview` and both writers (PDF and PostScript) use, so the canvas and the
 printed job agree: `{tileOrigin, pages: [{artboard, number, printed, page, imageable}]}` (`[x0, y0, x1, y1]` in document space; every tile when tiling, else
 each page once, not once per ink).
+`view.printTiling` rejects an `on` value that is not a boolean. Omitting `on` toggles the current value.
 
 Dragging with the tool puts the top-left corner of the first page's imageable area where the pointer is (snapping to
 the artboard's edges): `print.tiling.set {origin: [x, y], artboard?}` saves it with the print settings as
@@ -5834,6 +5840,7 @@ that no built-in format has. A failing plug-in returns an error and leaves the d
 no file). A filter whose manifest asks for a heavy budget (minutes rather than a minute, `budget: "heavy"` in its
 `plugin.inspect {path | dataBase64}` entry) installs only with `allowHeavy: true`: ask the user first. `plugin.run`
 runs it where it is called, so the call may take minutes; `notifications/cancelled` stops it.
+`plugin.install` rejects a `replace` or `allowHeavy` value that is not a boolean.
 
 ```json
 {"name":"run_command","arguments":{"command":"plugin.install","params":{"path":"/plugins/desaturate.wasm"}}}