diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index d5cfa9166..d583f6fe3 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -270,7 +270,8 @@ impl FilterRun { /// Runs the plug-in in `s`, making the command calls it asks for on the active document /// ([`super::plugin_call::call`]) → its output. fn run_calling(&self, s: &mut Session) -> Result> { - let calls = super::plugin_call::Calls::of(s, self.plugin.manifest().permissions().contains(&Permission::DocWrite)); + let may = self.plugin.manifest().permissions(); + let calls = super::plugin_call::Calls::of(s, may.contains(&Permission::DocRead), may.contains(&Permission::DocWrite)); let mut bridge = |id: &str, args: &Value| super::plugin_call::call(s, &calls, id, args); self.plugin.run_calling(&self.input, &self.params, self.plugin.limits(), &mut bridge).0.map_err(err) } @@ -349,8 +350,10 @@ fn run(s: &mut Session, p: &Value) -> Result { // An output handed in is for this run only (the app hands it in just before), whatever it does. let handed = s.plugin_output.take(); let r = FilterRun::prepare(s, p)?; - // One that calls commands runs here, where it can make them. + // One that calls commands runs here, where it can make them, on the document with any edit in + // progress committed. if r.calls() { + let r = if super::plugin_call::settle(s)? { FilterRun::prepare(s, p)? } else { r }; return r.run_with_calls(s); } // Handed in for this very run (the app ran it in the background), it is what running the @@ -501,6 +504,13 @@ pub(crate) fn run_command(s: &mut Session, id: &str, p: &Value) -> Result }; let r = FilterRun::prepare_with(s, plugin, Some(command), id, p)?; if r.calls() { + let r = match super::plugin_call::settle(s)? { + true => { + let (plugin, command) = command_of(id).ok_or_else(|| EngineError::UnknownCommand(id.into()))?; + FilterRun::prepare_with(s, plugin, Some(command), id, p)? + } + false => r, + }; return r.run_with_calls(s).map_err(failed); } let output = r.run().map_err(&failed)?; diff --git a/crates/engine/src/cmd/plugin_call.rs b/crates/engine/src/cmd/plugin_call.rs index 76786daeb..db52f3112 100644 --- a/crates/engine/src/cmd/plugin_call.rs +++ b/crates/engine/src/cmd/plugin_call.rs @@ -202,20 +202,35 @@ fn names_plugin_in(v: &Value, named: bool) -> bool { } } +/// Commits an edit in progress in the active document (not a preview's), as [`transaction`] does → +/// whether that changed the document or its selection. A run that calls commands is prepared again +/// then, so its input is the document its calls change. +pub(crate) fn settle(s: &mut Session) -> Result { + if s.previewing { + return Ok(false); + } + let Some(before) = s.active().map(|d| (d.doc.clone(), d.selection.clone())) else { return Ok(false) }; + super::edit::settle_interaction(s)?; + Ok(s.active().is_none_or(|d| !Arc::ptr_eq(&before.0, &d.doc) || before.1 != d.selection)) +} + /// What a run that makes command calls keeps fixed: the document it runs in, the documents open, /// whether an interaction holds its changes (its own, or a preview's), and whether the plug-in may -/// change the document. +/// read and change the document. pub(crate) struct Calls { document: Option, open: Vec, interaction: bool, + may_read: bool, may_write: bool, } impl Calls { - /// The run's fixed state in `s` as it is now; `may_write` is whether it has `doc.write`. - pub(crate) fn of(s: &Session, may_write: bool) -> Calls { - Calls { document: s.active().map(|d| d.uid), open: s.docs.iter().map(|d| d.uid).collect(), interaction: s.in_interaction(), may_write } + /// The run's fixed state in `s` as it is now; `may_read` and `may_write` are whether it has + /// `doc.read` and `doc.write`. + pub(crate) fn of(s: &Session, may_read: bool, may_write: bool) -> Calls { + let (document, open) = (s.active().map(|d| d.uid), s.docs.iter().map(|d| d.uid).collect()); + Calls { document, open, interaction: s.in_interaction(), may_read, may_write } } /// Is `s` still as the run needs it: the same documents open, the same one active and the @@ -230,9 +245,13 @@ impl Calls { /// Makes plug-in call `id` with `args` in `s` → its result, or why it didn't run or failed. A /// command a plug-in may never call ([`denied`]), one whose args name a plug-in, one that changes /// which documents are open or active or ends the run's edit, and one that changes the document for -/// a plug-in without `doc.write` are refused. +/// a plug-in without `doc.write` are refused, and so is every call of a plug-in without `doc.read`: +/// a command's result describes the document. pub(crate) fn call(s: &mut Session, run: &Calls, id: &str, args: &Value) -> std::result::Result { let refused = || CallFailure { code: CALL_DENIED_COMMAND, why: EngineError::Other("a plug-in may not call this command".into()).to_string() }; + if !run.may_read { + return Err(refused()); + } let Some(spec) = super::find_command(id) else { return Err(CallFailure { code: CALL_UNKNOWN_COMMAND, why: super::params::unknown_command(id).to_string() }); }; diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index a705f05b9..aca18dee1 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -3407,3 +3407,29 @@ fn another_module_under_an_id_with_settings_needs_replace() { install(&mut s, json!({})).unwrap(); run(&mut s, "plugin.remove", json!({"id": id})); } + +/// A filter that calls commands runs on the document with the edit in progress kept first: a move +/// still being previewed is its own undo step, and the filter is given the moved rectangle, which +/// its empty output then removes. +#[test] +fn a_filter_that_calls_commands_is_given_the_edit_in_progress() { + let id = "test.engine.callsettle"; + let mut s = session(); + let grant = json!(["doc.read", "doc.write", "commands"]); + let manifest = format!(r#"{{"id":"{id}","name":"Test {id}","kind":"filter","api":2,"permissions":{grant},"commandAllowlist":["select.all"]}}"#); + let wasm = wat::parse_str(tpl::returning(&manifest, r#"{"objects":[]}"#)).unwrap(); + run(&mut s, "plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&wasm), "grant": grant, "replace": true})); + let paths = |s: &Session| s.doc().unwrap().doc.layers.first().and_then(|l| l.children()).map_or(0, |c| c.len()); + run(&mut s, "shape.rectangle", json!({"x": 10, "y": 10, "width": 50, "height": 50})); + s.begin_interaction("Move").unwrap(); + s.preview("object.move", &json!({"dx": 7, "dy": 0})).unwrap(); + run(&mut s, "plugin.run", json!({"id": id})); + assert!(!s.in_interaction()); + assert_eq!(paths(&s), 0, "the filter was given the moved rectangle and removed it"); + let labels: Vec<&str> = s.doc().unwrap().history.undo.iter().rev().take(2).map(|e| e.label.as_str()).collect(); + assert_eq!(labels, [format!("Test {id}").as_str(), "Move"]); + run(&mut s, "edit.undo", json!({})); + let x = run(&mut s, "document.inspect", json!({}))["layers"][0]["children"][0]["bounds"]["x"].clone(); + assert_eq!(x, json!(17.0), "undoing the filter brings back the moved rectangle"); + run(&mut s, "plugin.remove", json!({"id": id})); +} diff --git a/crates/engine/tests/plugin_call.rs b/crates/engine/tests/plugin_call.rs index 00fca06c4..f30a8b812 100644 --- a/crates/engine/tests/plugin_call.rs +++ b/crates/engine/tests/plugin_call.rs @@ -175,3 +175,20 @@ fn without_doc_write_a_call_that_edits_is_refused() { run_plugin(&mut s, "test.call.reader").unwrap(); run(&mut s, "plugin.remove", json!({"id": "test.call.reader"})); } + +/// A command's result describes the document, so a plug-in without `doc.read` may call nothing. +#[test] +fn without_doc_read_every_call_is_refused() { + let mut s = document(); + let before = state(&mut s); + install(&mut s, "test.call.blind", r#"["commands"]"#, r#"["select.all"]"#, SELECT_ALL, EMPTY).unwrap(); + let e = run_plugin(&mut s, "test.call.blind").unwrap_err(); + assert!(e.contains("may not call") && e.contains("select.all"), "{e}"); + assert_eq!(state(&mut s), before); + // The same for an id no command has: without `doc.read` the answer never depends on the command. + let unknown = "(drop (call $vc_call (i32.const 0x2020) (i32.const 11) (i32.const 0) (i32.const 0)))"; + install(&mut s, "test.call.blind", r#"["commands"]"#, r#"["no.such.cmd"]"#, unknown, EMPTY).unwrap(); + let e = run_plugin(&mut s, "test.call.blind").unwrap_err(); + assert!(e.contains("may not call") && e.contains("no.such.cmd"), "{e}"); + run(&mut s, "plugin.remove", json!({"id": "test.call.blind"})); +} diff --git a/docs/mcp.md b/docs/mcp.md index 512fb6403..2159a0951 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -6937,9 +6937,9 @@ each of them as `{"id": …}` or exactly as given, otherwise `plugin.run` return field, and the document and undo history are unchanged. A filter at plug-in API level 2 whose manifest lists the `commands` permission and a `commandAllowlist` (both in its `plugin.list` entry) runs the commands that list names while it runs, through `vc_call`: the calls and its output are one undo step, and a run that fails or makes a call that -fails leaves the documents, selections and history as they were. A command that reads or writes files, uses the -clipboard, changes preferences or the view, runs a script, an action or a plug-in, switches documents or undoes is -never called; `plugin.install` refuses a manifest that lists one, and `plugin.inspect` lists such ids as +fails leaves the documents, selections and history as they were. Without `doc.read` every call fails. A command that +reads or writes files, uses the clipboard, changes preferences or the view, runs a script, an action or a plug-in, +switches documents or undoes is never called; `plugin.install` refuses a manifest that lists one, and `plugin.inspect` lists such ids as `refusedCommands` and ids no command has as `unknownCommands` (`plugin.install` lists those too). A live-effect plug-in is an effect with id `plugin.`: `effect.apply`, `effect.setParams` and `effect.expandAppearance` work as for built-in effects, and `effect.list` lists it. One whose manifest declares `paintOutput` (at plug-in API level 2; `plugin.list` gives `api` diff --git a/docs/plugins.md b/docs/plugins.md index c93e3266a..31185ab1e 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -414,16 +414,20 @@ call commands on. traps, runs out of budget or is cancelled leaves every document, its selection and its history as they were. - Each call runs on the document the run is for, with its params checked as any request's are, and sees what the calls before it changed: a `select.all` followed by `object.move` moves everything. The filter's own input is the one it - was given when the run started. + was given when the run started; an edit still in progress (typing, or a shape being drawn) is kept as its own undo + step before that input is made. - A plug-in never calls a command that reads or writes a file or folder, uses the clipboard, changes preferences or the view, runs a script, an action or a plug-in (a `plugin.*` command, or arguments naming one, such as an `effect.apply` of `plugin.`), opens, closes or switches documents, or undoes or redoes, whatever its manifest lists. The same goes for a command that changes a tool's options, keeps a drag's state or adds font names for the - whole app, and for a command VectorCraft adds later until it has been checked for plug-in calls. Installing a manifest that lists such a command is refused, and the error names it; `plugin.inspect` lists - them as `refusedCommands`. + whole app, and for a command VectorCraft adds later until it has been checked for plug-in calls. Installing a + manifest that lists such a command is refused, and the error names it; `plugin.inspect` lists them as + `refusedCommands`. - An id in `commandAllowlist` that no command has installs, so a plug-in can list a command a later version adds; - `plugin.install` and `plugin.inspect` list such ids as `unknownCommands`, and calling one returns `-5`. -- Without `doc.write`, a call that changes the document fails with `-6`. + `plugin.install` and `plugin.inspect` list such ids as `unknownCommands`, and calling one returns `-5` (`-6` without + `doc.read`). +- Without `doc.read`, every call of a listed id fails with `-6`, since a command's result describes the document. + Without `doc.write`, a call that changes the document fails with `-6`. - These limits are VectorCraft's own values: at most 1,000 calls a run, 8 MiB of arguments or result a call and 64 MiB of arguments and results a run. Each call costs 10,000 instructions of the run's budget, plus one for every 64 bytes of its arguments and its result, and the run checks Cancel and its time budget before and after each call.