From 07423c22d6acbeb48832cf2fec5ab008d0f390d0 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 02:09:47 -0800 Subject: [PATCH] MCP: save and export replies keep the path the client gave under read roots Under automation read roots, a reply shows each path outside the read roots that the request sent by its file name. A save or export to a path the client gave inside the write roots then answered with the bare file name in `path`, where the save replies keep the given path. The `path` and `files` entries of the reply's result, a `command.batch` step's result included, that equal a path param the request gave inside the write roots now stay as given; every other occurrence is still shown by its file name, and loaded libraries outside the read roots keep their `loaded/` ids. --- crates/ui-egui/src/control.rs | 76 +++++++++++++++++++++++++- crates/ui-egui/src/tests/automation.rs | 39 +++++++++++++ docs/mcp.md | 5 +- 3 files changed, 117 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index ef747f8476..4ab3982607 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -1086,7 +1086,8 @@ const DOCKING_ACTION: &str = "action"; /// such library's id and path as `ui.inspect` does ([`hide_library_ids`]) in every string and key, /// errors included, an error from a panic too: the libraries hidden before it ran and after it. The /// reply also shows each path outside the read root that the request sent by its file name, whether it -/// succeeded or failed ([`sent_paths`]). A reply with no `/`, `\` or `%` in it is not searched, since every hidden path +/// succeeded or failed ([`sent_paths`]), except where a `path` or `files` entry of its result names the +/// file the request wrote inside the write roots, as the request gave it. A reply with no `/`, `\` or `%` in it is not searched, since every hidden path /// holds a separator, written or escaped. A screenshot's reply is hidden when it is sent /// ([`hide_in_deferred_reply`]); a `ui.pointer` reply sent after the tool's edit holds no library id /// or path. @@ -1122,17 +1123,88 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlReques } /// Hides in `reply` the libraries hidden now and in `before`, and the paths outside the read root that -/// `sent`, the request's params, holds ([`handle`]). +/// `sent`, the request's params, holds ([`handle`]). The `path` and `files` entries of the reply's +/// `result`, at any depth, that name a file the request gave inside the write roots, the file it +/// wrote, stay as given ([`written_paths`]). fn hide_in_reply(app: &VectorcraftApp, reply: &mut Value, sent: &Value, before: Hiding) { if !holds_a_separator(reply) { return; } + let mut given = Vec::new(); + written_paths(sent, false, &mut given); + let kept = kept_written_paths(reply, &given); let mut hiding = unreadable_library_ids(app); hiding.merge(before); let mut paths = Vec::new(); sent_paths(sent, &mut paths); hiding.set_hidden(paths, cfg!(windows)); hide_library_ids(reply, &hiding); + restore_written_paths(reply, kept); +} + +/// The strings that `v`'s path params hold at any depth ([`vectorcraft_engine::file_access::is_path_key`]) +/// that name a file inside the write roots; `v` is a path param's value when `path_param` is set. +fn written_paths(v: &Value, path_param: bool, out: &mut Vec) { + use vectorcraft_engine::file_access::{Access, current, is_path_key}; + match v { + Value::String(s) if path_param && !s.is_empty() && current().is_some_and(|r| r.check(s, Access::Write).is_ok()) => out.push(s.clone()), + Value::Array(items) => items.iter().for_each(|item| written_paths(item, path_param, out)), + Value::Object(fields) => { + for (key, item) in fields { + written_paths(item, is_path_key(key), out); + } + } + _ => {} + } +} + +/// The entries under `reply`'s `result`, at any depth (a `command.batch` step's result too), that a +/// `path` key or a `files` list holds and that are one of `given`: each entry's JSON pointer and text. +fn kept_written_paths(reply: &Value, given: &[String]) -> Vec<(String, String)> { + let mut kept = Vec::new(); + if !given.is_empty() { + kept_under(&reply["result"], "/result", given, &mut kept); + } + kept +} + +/// [`kept_written_paths`] for `v`, found at the JSON pointer `at`. +fn kept_under(v: &Value, at: &str, given: &[String], out: &mut Vec<(String, String)>) { + let is_given = |v: &Value| v.as_str().filter(|s| given.iter().any(|g| g == s)).map(str::to_string); + match v { + Value::Array(items) => { + for (i, item) in items.iter().enumerate() { + kept_under(item, &format!("{at}/{i}"), given, out); + } + } + Value::Object(fields) => { + for (key, item) in fields { + let here = format!("{at}/{}", key.replace('~', "~0").replace('/', "~1")); + match (key.as_str(), is_given(item), item.as_array()) { + ("path", Some(path), _) => out.push((here, path)), + ("files", _, Some(rows)) => { + for (i, row) in rows.iter().enumerate() { + match is_given(row) { + Some(path) => out.push((format!("{here}/{i}"), path)), + None => kept_under(row, &format!("{here}/{i}"), given, out), + } + } + } + _ => kept_under(item, &here, given, out), + } + } + } + _ => {} + } +} + +/// Writes back in `reply` the entries [`kept_written_paths`] took from it. +fn restore_written_paths(reply: &mut Value, kept: Vec<(String, String)>) { + for (at, path) in kept { + if let Some(slot) = reply.pointer_mut(&at) { + *slot = json!(path); + } + } } /// Hides in a reply sent after its request was handled (a screenshot's) what [`handle`] hides in a diff --git a/crates/ui-egui/src/tests/automation.rs b/crates/ui-egui/src/tests/automation.rs index 68cb169c8c..073cefbd82 100644 --- a/crates/ui-egui/src/tests/automation.rs +++ b/crates/ui-egui/src/tests/automation.rs @@ -128,6 +128,45 @@ fn control_saves_leave_out_the_documents_file_outside_the_read_root() { let _ = std::fs::remove_dir_all(base); } +/// In one session with a library loaded from outside the read root: the library keeps its file-name +/// id, a reply that names its path shows the file name, a save without a path leaves out the +/// document's file, and a save or export to a path the client gave in the write root keeps that path +/// in `path` and `files`, also in a `command.batch` step's result. A refused path outside the write root is still shown by its file name. +#[test] +fn control_saves_keep_the_given_path_beside_a_hidden_library() { + let (mut app, tx, base) = confined("save-and-library"); + let library = s(base.join("outside/b.svg")); + app.ui.color_guide_limit = format!("loaded/{library}"); + app.run("file.new", json!({"width": 100, "height": 50})).unwrap(); + let own = s(base.join("out/doc.vectorcraft")); + app.run("file.saveAs", json!({"path": own})).unwrap(); + app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 20, "height": 20})).unwrap(); + let r = request(&mut app, &tx, "app.save", json!({})); + assert!(r["result"].get("path").is_none() && !r.to_string().contains(&s(base.join("out"))), "{r}"); + let r = request(&mut app, &tx, "app.save", json!({"path": own})); + assert_eq!(r["result"]["path"], json!(own), "{r}"); + let split = s(base.join("out/split.vectorcraft")); + let r = request(&mut app, &tx, "engine.execute", json!({"command": "file.save", "params": {"path": split, "separateArtboards": true}})); + assert_eq!(r["result"]["path"], json!(split), "{r}"); + assert_eq!(r["result"]["files"][0], json!(split), "{r}"); + let batched = s(base.join("out/batched.vectorcraft")); + let steps = json!({"commands": [{"command": "document.save", "params": {"path": batched}}]}); + let r = request(&mut app, &tx, "engine.execute", json!({"command": "command.batch", "params": steps})); + assert_eq!(r["result"]["results"][0]["path"], json!(batched), "{r}"); + let svg = s(base.join("out/a.svg")); + let r = request(&mut app, &tx, "app.export", json!({"path": svg, "wait": true})); + assert_eq!(r["result"]["path"], json!(svg), "{r}"); + app.ui.status = format!("from {library}"); + let r = request(&mut app, &tx, "test.status", json!({})); + assert_eq!(r["result"], "from b.svg", "{r}"); + let r = request(&mut app, &tx, "ui.inspect", json!({})); + assert_eq!(r["result"]["ui"]["color_guide_limit"], "loaded/b.svg", "{r}"); + let refused = s(base.join("outside/c.vectorcraft")); + let r = request(&mut app, &tx, "app.save", json!({"path": refused, "wait": true})); + assert!(r["error"].as_str().is_some_and(|e| e.contains("outside the write root") && !e.contains(&refused)), "{r}"); + let _ = std::fs::remove_dir_all(base); +} + /// With Background Save and Background Export on, `app.save` and `app.export` with `wait: true` /// (MCP's `save_file` in the app) reply with the write root's refusal, and write a file inside it /// before they reply. Without `wait` the refusal reaches only the status bar. diff --git a/docs/mcp.md b/docs/mcp.md index 5cb8df4df8..03bb0b9665 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -178,7 +178,10 @@ the suite) keep the agent to the folders you give it: URL writes it, and on Windows in any case of its letters. A hidden id or path stays only where it begins another loaded library's whole id with no `.` or `..` part, so a readable library's id that begins with a hidden one stays. A reply also shows a path outside the read roots that the request sent by its file name, whether or not a library was loaded from it and whether the - request succeeded or failed, so that the reply reads the same either way. A path param (`path`, `file`, + request succeeded or failed, so that the reply reads the same either way. The one exception is the `path` + and `files` entries of a reply's result, a `command.batch` step's result included, that name the file the + request wrote inside the write roots at the path the request gave: they stay as + given, as for the save and export replies above. A path param (`path`, `file`, `destinationFolder` and the like) counts with a relative path too. The hidden libraries' ids and paths are matched before the paths the request sent, so a sent path written over part of a hidden library's path does not keep that path in the reply. A relative path is not replaced where it ends a longer path (`/work/data/icon.svg`