From 00624da7ad610cc64080cab91e3f8fa03b4b5a3d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 22:13:41 -0800 Subject: [PATCH] MCP: preferences replies leave out folders outside the read roots Under --automation-read-root, prefs.get, prefs.list, prefs.set and prefs.reset returned the folder preferences (fontsFolder, pluginsFolder, scratchPrimary, scratchSecondary, recoveryFolder, templatesFolder) and the export presets' folder even when they were outside the read roots. Such a folder preference is now null, as ui.inspect leaves it out, and such a preset's folder is left out. An empty folder and the scratch disk choices stay; without roots the replies are unchanged. --- crates/engine/src/cmd/prefscmds.rs | 46 +++++++++-- crates/engine/src/tests.rs | 1 + crates/engine/src/tests/prefs_roots.rs | 107 +++++++++++++++++++++++++ docs/mcp.md | 5 ++ docs/target-app-parity.md | 1 + 5 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 crates/engine/src/tests/prefs_roots.rs diff --git a/crates/engine/src/cmd/prefscmds.rs b/crates/engine/src/cmd/prefscmds.rs index a92b4095a9..014e825684 100644 --- a/crates/engine/src/cmd/prefscmds.rs +++ b/crates/engine/src/cmd/prefscmds.rs @@ -765,7 +765,7 @@ impl Session { pub fn specs() -> Vec { vec![ - cmd!(query "prefs.get", "Get Preferences", [], None, "{key?} → the value of one preference (or preference group: eyedropper), or {prefs: {…all…}}", always, get), + cmd!(query "prefs.get", "Get Preferences", [], None, "{key?} → the value of one preference (or preference group: eyedropper), or {prefs: {…all…}}; under automation read roots a folder preference outside them is null, and an export preset's folder outside them is left out", always, get), cmd!( query "prefs.set", "Set Preference", @@ -776,7 +776,7 @@ pub fn specs() -> Vec { set ), cmd!(query "prefs.reset", "Reset Preferences", [], None, "{category?} reset one category (or everything) to defaults", always, reset), - cmd!(query "prefs.list", "List Preferences", [], None, "{} → [{key, category, section, label, kind, value, …}] every preference", always, list), + cmd!(query "prefs.list", "List Preferences", [], None, "{} → [{key, category, section, label, kind, value, …}] every preference; under automation read roots a folder preference's value outside them is null", always, list), cmd!( query "prefs.export", "Export Preferences", @@ -882,8 +882,42 @@ fn import(s: &mut Session, p: &Value) -> Result { Ok(json!({ "imported": values.len() })) } +/// The preferences that name a folder: the [`FOLDER_PREFS`] and the scratch disks. +const PATH_PREFS: [&str; 6] = ["fontsFolder", "pluginsFolder", "scratchPrimary", "scratchSecondary", "recoveryFolder", "templatesFolder"]; + +/// The scratch disk choices that are not folders. +const SCRATCH_CHOICES: [&str; 2] = ["Startup", "None"]; + +/// The preferences as `prefs.get`, `prefs.list`, `prefs.set` and `prefs.reset` show them to the +/// request being served. Under automation read roots, a folder preference ([`PATH_PREFS`]) outside +/// them is `null`, as `ui.inspect` leaves it out, and an export preset's `folder` outside them is +/// left out. An empty value (the default folder) and a scratch +/// disk choice ([`SCRATCH_CHOICES`]) are shown. Without roots the preferences are shown as they are. +fn shown_prefs(prefs: &Prefs) -> Value { + let mut all = prefs.to_json(); + if crate::file_access::current().is_none() { + return all; + } + // The app trims a folder before it uses it. + let hidden = |path: &str| !path.trim().is_empty() && crate::file_access::check_read(path.trim()).is_err(); + for key in PATH_PREFS { + let Some(path) = all[key].as_str() else { continue }; + if hidden(path) && !(key.starts_with("scratch") && SCRATCH_CHOICES.contains(&path.trim())) { + all[key] = Value::Null; + } + } + if let Some(presets) = all.get_mut("screenExportPresets").and_then(Value::as_array_mut) { + for settings in presets.iter_mut().filter_map(|p| p.get_mut("settings").and_then(Value::as_object_mut)) { + if settings.get("folder").and_then(Value::as_str).is_some_and(hidden) { + settings.remove("folder"); + } + } + } + all +} + fn get(s: &mut Session, p: &Value) -> Result { - let all = s.prefs.to_json(); + let all = shown_prefs(&s.prefs); match str_param(p, "key") { Some(k) if spec(k).is_some() || PREF_GROUPS.contains(&k) => Ok(all[k].clone()), Some(k) => Err(bad("prefs.get", format!("unknown preference `{k}`"))), @@ -911,7 +945,7 @@ fn set(s: &mut Session, p: &Value) -> Result { if document_units && values.contains_key("unitsGeneral") && s.active().is_some() { s.set_document_units(s.default_units())?; } - let all = s.prefs.to_json(); + let all = shown_prefs(&s.prefs); Ok(Value::Object(values.keys().map(|k| (k.clone(), all[k.as_str()].clone())).collect())) } @@ -925,11 +959,11 @@ fn reset(s: &mut Session, p: &Value) -> Result { let mut next = s.prefs.clone(); next.reset(cat); s.apply_prefs(next); - Ok(json!({ "prefs": s.prefs.to_json() })) + Ok(json!({ "prefs": shown_prefs(&s.prefs) })) } fn list(s: &mut Session, _: &Value) -> Result { - let all = s.prefs.to_json(); + let all = shown_prefs(&s.prefs); Ok(Value::Array( PREF_SPECS .iter() diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f5bbd54220..67a7bc308d 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -133,6 +133,7 @@ mod placed_document; mod plugins; mod prefs; mod prefs_file; +mod prefs_roots; mod previewbounds; mod print; mod printadvanced; diff --git a/crates/engine/src/tests/prefs_roots.rs b/crates/engine/src/tests/prefs_roots.rs new file mode 100644 index 0000000000..b9818949f1 --- /dev/null +++ b/crates/engine/src/tests/prefs_roots.rs @@ -0,0 +1,107 @@ +//! The preferences commands under automation roots: a confined request sees a folder preference and +//! an export preset's folder only inside its read roots. + +use std::path::{Path, PathBuf}; + +use serde_json::{Value, json}; + +use crate::cmd::fileio::ScreenExportPreset; +use crate::file_access::{AutomationRoots, confine}; +use crate::*; + +fn run(s: &mut Session, id: &str, p: Value) -> Value { + s.execute(id, &p).unwrap_or_else(|e| panic!("{id} {p}: {e}")) +} + +fn text(p: &Path) -> String { + p.to_string_lossy().to_string() +} + +/// A fresh scratch folder for one test, with the folders `in` and `out`. +fn folder(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("vc-prefs-roots-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + for sub in ["in", "out"] { + std::fs::create_dir_all(dir.join(sub)).unwrap(); + } + dir +} + +/// The folder preferences that name the folder outside the read root. +const OUTSIDE: [&str; 3] = ["pluginsFolder", "scratchPrimary", "templatesFolder"]; + +/// A session whose folder preferences name `inside` (fonts, and the Data Recovery folder with +/// spaces around it) and `outside` (the [`OUTSIDE`] ones, the Templates folder with spaces around +/// it), with a scratch disk choice and an export preset kept with each folder. +fn session(inside: &str, outside: &str) -> Session { + let mut s = Session::new(); + s.prefs.fonts_folder = inside.to_string(); + s.prefs.recovery_folder = format!(" {inside} "); + s.prefs.plugins_folder = outside.to_string(); + s.prefs.scratch_primary = outside.to_string(); + s.prefs.scratch_secondary = "None".to_string(); + s.prefs.templates_folder = format!(" {outside} "); + for (name, folder) in [("Inside", inside), ("Outside", outside)] { + let settings = json!({"formats": [{"format": "svg"}], "folder": folder}); + s.prefs.screen_export_presets.push(ScreenExportPreset { name: name.into(), settings: settings.as_object().cloned().unwrap_or_default() }); + } + s +} + +/// Does a string anywhere in `v` hold `path`? +fn holds(v: &Value, path: &str) -> bool { + v.to_string().contains(&path.replace('\\', "\\\\")) +} + +#[test] +fn prefs_commands_leave_out_folders_outside_the_read_root() { + let base = folder("get"); + let (inside, outside) = (text(&base.join("in").join("fonts")), text(&base.join("out"))); + let roots = AutomationRoots::new(&[&base.join("in")], &[]).unwrap(); + let mut s = session(&inside, &outside); + + let all = confine(roots.as_ref(), || run(&mut s, "prefs.get", json!({}))); + let prefs = &all["prefs"]; + assert_eq!(prefs["fontsFolder"], json!(inside), "a folder inside the root is shown"); + assert_eq!(prefs["recoveryFolder"], json!(format!(" {inside} ")), "checked without the spaces, shown as kept"); + assert_eq!(prefs["scratchSecondary"], json!("None"), "a scratch disk choice is shown"); + for key in OUTSIDE { + assert_eq!(prefs[key], Value::Null, "{key}: {all}"); + } + let presets = prefs["screenExportPresets"].as_array().unwrap(); + assert_eq!(presets[0]["settings"]["folder"], json!(inside)); + assert_eq!(presets[1]["name"], json!("Outside"), "the preset stays"); + assert!(presets[1]["settings"].get("folder").is_none(), "{all}"); + assert!(!holds(&all, &outside), "{all}"); + + for key in OUTSIDE { + let one = confine(roots.as_ref(), || run(&mut s, "prefs.get", json!({"key": key}))); + assert_eq!(one, Value::Null, "{key}"); + } + assert_eq!(confine(roots.as_ref(), || run(&mut s, "prefs.get", json!({"key": "fontsFolder"}))), json!(inside)); + + let list = confine(roots.as_ref(), || run(&mut s, "prefs.list", json!({}))); + assert!(!holds(&list, &outside), "{list}"); + let row = |key: &str| list.as_array().unwrap().iter().find(|r| r["key"] == key).cloned().unwrap(); + assert_eq!(row("fontsFolder")["value"], json!(inside)); + assert_eq!(row("pluginsFolder")["value"], Value::Null); + assert_eq!(row("pluginsFolder")["kind"], json!("text"), "the row stays"); + + // A request that sets another preference, or resets a category without folders. + let set = confine(roots.as_ref(), || run(&mut s, "prefs.set", json!({"values": {"keyboardIncrement": 2, "pluginsFolder": outside}}))); + assert_eq!(set, json!({"keyboardIncrement": 2.0, "pluginsFolder": null}), "a folder kept as it was is left out too"); + let reset = confine(roots.as_ref(), || run(&mut s, "prefs.reset", json!({"category": "General"}))); + assert!(!holds(&reset, &outside), "{reset}"); + assert_eq!(reset["prefs"]["fontsFolder"], json!(inside)); + + // Unconfined and with only a write root. + let all = run(&mut s, "prefs.get", json!({})); + assert_eq!(all["prefs"]["pluginsFolder"], json!(outside), "unconfined, the folder is shown"); + assert_eq!(all["prefs"]["screenExportPresets"][1]["settings"]["folder"], json!(outside)); + let write_only = AutomationRoots::new(&[], &[&base.join("in")]).unwrap(); + let all = confine(write_only.as_ref(), || run(&mut s, "prefs.get", json!({}))); + assert_eq!(all["prefs"]["fontsFolder"], Value::Null, "without a read root no folder is shown"); + assert_eq!(all["prefs"]["recoveryFolder"], Value::Null); + assert_eq!(s.prefs.plugins_folder, outside, "the preferences are unchanged"); + let _ = std::fs::remove_dir_all(&base); +} diff --git a/docs/mcp.md b/docs/mcp.md index 7f6df79837..961d56b053 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -153,6 +153,11 @@ the suite) keep the agent to the folders you give it: and `modified` the document keeps of that file. Each row keeps its id, `name` (`fileName`), status and other fields. This applies to every document, also one the person or a client with wider roots opened. The paths a client gives `links.relink` and `links.unembed` stay in their replies, and the document's links are not changed. +- `prefs.get`, `prefs.list`, `prefs.set` and `prefs.reset` show a folder preference outside the read roots + (`fontsFolder`, `pluginsFolder`, `scratchPrimary`, `scratchSecondary`, `recoveryFolder`, `templatesFolder`) as + `null`; an empty folder and the scratch disk choices `Startup` and `None` stay. In `prefs.get {}` and + `prefs.reset`, an export preset in `screenExportPresets` leaves out its `folder` when the folder is outside the + read roots, and the preset stays. The preferences themselves are not changed. - Each flag may be given more than once (#3546): a path inside any of the read roots may be read, one inside any of the write roots written, so one session can read a project folder and a shared assets folder, or write to an output folder and a scratch folder, without widening a root to their common parent. A folder given twice diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 908895ad64..5fbe4157f0 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -753,6 +753,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - `ui.inspect` under `--automation-read-root`: the documents still opening, the recent files, library panels' files and folder preferences (saved, and in an open Preferences dialog) outside the read roots are left out of the reply, a library loaded from outside them shows by its file name in `ui.inspect` and `ui.docking`, and the app's state is not changed. `crates/ui-egui/src/tests/automation.rs` (`ui_inspect_leaves_out_paths_outside_the_read_root`). - Link replies under `--automation-read-root`: `links.list`, `links.check`, `links.info` and `document.open`'s link rows leave out the path, found path and folder of a linked file outside the read roots, and the stored size and date of it in `links.info`; each row keeps its id, file name and status, and the document's links are not changed. `crates/engine/src/tests/linkspanel.rs` (`link_paths_outside_the_read_roots_are_left_out`), `crates/engine/src/tests/placed_document.rs` (`link_info_leaves_out_a_placed_documents_path_outside_the_read_roots`). - Swatch, brush, symbol and graphic style library lists under `--automation-read-root`: library files and the user library folder outside the read roots are left out, a library loaded from outside them is listed and found by its file name, and the app's own list is not changed. `crates/engine/src/tests/library_roots.rs`. + - Preferences commands under `--automation-read-root`: `prefs.get`, `prefs.list`, `prefs.set` and `prefs.reset` show folder preferences outside the read roots as `null` and leave out an export preset's folder outside them, and the preferences are not changed. `crates/engine/src/tests/prefs_roots.rs`. - Scripts: `alert`, `confirm` and `prompt` open dialogs during a run, each with a Stop button that ends the run and puts every document back. In the app the run waits at the call for the person's answer; a run over MCP or the command line takes its answers in order from `script.run`'s `answers`, and the report lists each dialog with its answer. `crates/engine/src/tests/script_dialogs.rs`, `crates/ui-egui/src/tests/script_dialogs.rs`. - Parameter keys: the engine refuses a key that a command's params description doesn't name before the command runs, in every build and for every caller, including the app's own calls, the commands a command runs and the steps of `command.batch` and `action.batch`. An action recorded by an older version still plays from the Actions panel and the Batch dialog (`Session::replay`). `crates/engine/tests/it/param_keys.rs`. - MCP server with 33 tools (drawing, text, effects, Pathfinder, transforms, graphs, text wrap, export, screenshots, undo/redo, UI inspect/screenshot aliases, any command), attached to the running app or headless. Headless and remote `command_list` include desktop `UI_COMMANDS` (`ui: true`); a UI-only id still needs `--connect` to run. Eight prompts (poster, icon-set, recolor, trace-and-style, export-set, check-spelling, variables-bind, path-tools). Effect ids are not command ids. Prefer a CLI built from the checkout when a release binary lags (#6460). `vectorcraft://commands/reference` serves the API reference. Tool descriptions and replies agree with the engine: format lists come from its format table, `save_file` says it keeps the document's own format, `inspect_document` reports the paint type's characters show, and selection exports report the encoder's warnings. `--automation-read-root` / `--automation-write-root` (on `vectorcraft-cli mcp` and on `vectorcraft --control`) confine the files commands read and write to chosen folders, links followed, with the flags PhotoCraft takes (#832); each flag may be given more than once, a path inside any of its folders being allowed (#3546, `file_access::tests::several_roots_of_a_kind_each_grant_their_access`, `cli::mcp_automation_roots_take_several_folders`). A long export can be cancelled (`notifications/cancelled` stops it at the next page or group and writes nothing), reports page progress to a client that sent a progress token, and does not make other requests wait: engine-free ones answer at once, the rest are refused as busy after two seconds (#2165, #2171, #2172).