From d1e766173e7216346b3dbcd3935df55d4c7c13a4 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 18:59:31 -0800 Subject: [PATCH] MCP: a library saved with another platform's absolute path is hidden under a read root Outside Windows, a library path that starts with a drive or a backslash counts as outside the read roots, since the file access check would read it as a name in the working folder. --- crates/engine/src/cmd/swatchlib.rs | 19 ++++++-- crates/ui-egui/src/control/tests.rs | 12 +++++ crates/ui-egui/src/tests/automation.rs | 64 +++++++++++++------------- docs/mcp.md | 3 +- 4 files changed, 62 insertions(+), 36 deletions(-) diff --git a/crates/engine/src/cmd/swatchlib.rs b/crates/engine/src/cmd/swatchlib.rs index 45320066cf..bc48f9d323 100644 --- a/crates/engine/src/cmd/swatchlib.rs +++ b/crates/engine/src/cmd/swatchlib.rs @@ -475,9 +475,18 @@ fn readable(path: &str) -> bool { crate::file_access::check_read(path).is_ok() } +/// Is `path`, under a read root, written as an absolute path of another platform: outside Windows, one +/// that starts with a drive (`C:`) or a `\`? The file access check would read it as a name in the +/// working folder. +fn written_elsewhere(path: &str) -> bool { + let foreign = path.starts_with('\\') || matches!(path.as_bytes(), [drive, b':', ..] if drive.is_ascii_alphabetic()); + !cfg!(windows) && foreign && crate::file_access::current().is_some() +} + /// The loaded library ids (`loaded/`) whose file the request being served may not read, each /// with the id a reply shows instead: `loaded/`, as a library loaded without a path is -/// named. The ids are those of the session's swatch, brush, symbol and graphic style libraries, then +/// named. The file name is the last part of the path after either separator, and a path saved as +/// another platform's absolute path counts as one the request may not read. The ids are those of the session's swatch, brush, symbol and graphic style libraries, then /// `others` (ids the UI state refers to). When another id already is or shows that id, the next free /// one of `loaded/ (2)`, `(3)` and so on is shown, so that two libraries stay two. The /// session's libraries are numbered first, in the sorted order of their full ids, so a library shows @@ -493,9 +502,13 @@ pub fn concealed_library_ids(s: &Session, others: impl IntoIterator hidden.push((id, name)), + Some((name, path)) if name != path && (written_elsewhere(path) || !readable(path)) => hidden.push((id, name)), _ => { taken.insert(id); } diff --git a/crates/ui-egui/src/control/tests.rs b/crates/ui-egui/src/control/tests.rs index 3d2a68b2be..13890c28bb 100644 --- a/crates/ui-egui/src/control/tests.rs +++ b/crates/ui-egui/src/control/tests.rs @@ -220,6 +220,18 @@ fn a_reply_replaces_each_hidden_text_once() { assert_eq!(reply["error"], "file:///art/in%20work/a.svg", "{reply}"); } +/// A library saved with another platform's absolute path is hidden, also when the working folder is +/// inside the read root. +#[test] +fn a_library_saved_with_another_platforms_path_is_hidden() { + let mut app = VectorcraftApp::new(Session::new(), Services::default()); + app.ui.color_guide_limit = r"loaded/C:\Users\someone\b.svg".to_string(); + let here = std::env::current_dir().unwrap(); + let roots = vectorcraft_engine::file_access::AutomationRoots::new(&[&here], &[]).unwrap(); + let hiding = vectorcraft_engine::file_access::confine(roots.as_ref(), || super::unreadable_library_ids(&app)); + assert_eq!(hiding.hidden, [(r"loaded/C:\Users\someone\b.svg".to_string(), "loaded/b.svg".to_string())], "{hiding:?}"); +} + /// A hidden relative path is hidden where it stands alone or after `./`, and stays where it ends a /// longer path. #[test] diff --git a/crates/ui-egui/src/tests/automation.rs b/crates/ui-egui/src/tests/automation.rs index 57d63e9f69..e36388bf31 100644 --- a/crates/ui-egui/src/tests/automation.rs +++ b/crates/ui-egui/src/tests/automation.rs @@ -446,33 +446,6 @@ fn ui_inspect_leaves_out_paths_outside_the_read_root() { let _ = std::fs::remove_dir_all(base); } -/// Two libraries loaded from outside the read root under the same file name, only the second of them -/// in a library panel: `ui.inspect` and `swatch.library.list` show the second under the same id, and -/// that id finds it. -#[test] -fn ui_inspect_and_library_commands_show_a_hidden_library_under_one_id() { - let (mut app, tx, base) = confined("one-id"); - let (x, y) = (base.join("outside/x/Kit.gpl"), base.join("outside/y/Kit.gpl")); - for (path, colors) in [(&x, "0 0 0 Black\n"), (&y, "0 0 0 Black\n255 255 255 White\n")] { - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - std::fs::write(path, format!("GIMP Palette\nName: Kit\n{colors}")).unwrap(); - app.session.execute("swatch.library.load", &json!({"path": s(path.clone())})).unwrap(); - } - let second = format!("loaded/{}", s(y.clone())); - app.ui.library_panels = vec![serde_json::from_value(json!({"kind": "swatches", "id": second, "path": s(y.clone())})).unwrap()]; - - let r = request(&mut app, &tx, "ui.inspect", json!({})); - assert_eq!(r["result"]["ui"]["library_panels"][0]["id"], "loaded/Kit.gpl (2)", "{r}"); - let r = request(&mut app, &tx, "engine.execute", json!({"command": "swatch.library.list", "params": {}})); - let libs = r["result"]["libraries"].as_array().unwrap_or_else(|| panic!("{r}")); - let loaded: Vec<&Value> = libs.iter().filter(|l| l["category"] == "loaded").map(|l| &l["id"]).collect(); - assert_eq!(loaded, [&json!("loaded/Kit.gpl"), &json!("loaded/Kit.gpl (2)")], "{r}"); - let r = request(&mut app, &tx, "engine.execute", json!({"command": "swatch.library.get", "params": {"library": "loaded/Kit.gpl (2)"}})); - assert_eq!(r["result"]["swatches"].as_array().map(Vec::len), Some(2), "the second library: {r}"); - assert!(!r.to_string().contains("outside"), "{r}"); - let _ = std::fs::remove_dir_all(base); -} - /// `ui.inspect` checks a folder preference as it is written: the read root's path with a space before /// it is a relative path, outside the root, and is left out. #[test] @@ -675,10 +648,10 @@ fn ui_inspect_keeps_a_hidden_library_off_a_loaded_librarys_id() { let _ = std::fs::remove_dir_all(base); } -/// Hidden libraries with the same file name are numbered in the order they were loaded, not in the -/// order of their paths. +/// Hidden libraries with the same file name that are shown for the first time in the same reply are +/// numbered in the order of their paths, as the library commands number them. #[test] -fn hidden_libraries_are_numbered_in_load_order() { +fn hidden_libraries_are_numbered_in_the_order_of_their_paths() { let (mut app, tx, base) = confined("hidden-load-order"); let gpl = "GIMP Palette\nName: p\n0 0 0 Black\n"; for dir in ["z", "a"] { @@ -687,7 +660,7 @@ fn hidden_libraries_are_numbered_in_load_order() { std::fs::write(&path, gpl).unwrap(); app.session.execute("swatch.library.load", &json!({"path": s(path)})).unwrap(); } - app.ui.color_guide_limit = format!("loaded/{}", s(base.join("outside").join("a").join("b.gpl"))); + app.ui.color_guide_limit = format!("loaded/{}", s(base.join("outside").join("z").join("b.gpl"))); let r = request(&mut app, &tx, "ui.inspect", json!({})); assert_eq!(r["result"]["ui"]["color_guide_limit"], "loaded/b.gpl (2)", "{r}"); let _ = std::fs::remove_dir_all(base); @@ -736,7 +709,7 @@ fn an_error_hides_a_sent_path_whether_or_not_a_library_was_loaded_from_it() { let (mut app, tx, base) = confined("client-echo"); let path = s(base.join("outside/b.svg")); // Another hidden library with the same file name comes first, so this one shows as `b.svg (2)`. - let first = format!("loaded/{}", s(base.join("outside/z/b.svg"))); + let first = format!("loaded/{}", s(base.join("outside/a/b.svg"))); let panel: crate::panels::library_panel::LibraryPanel = serde_json::from_value(json!({"kind": "swatches", "id": first})).unwrap(); app.ui.library_panels = vec![panel]; app.ui.color_guide_limit = format!("loaded/{path}"); @@ -808,6 +781,33 @@ fn ui_inspect_leaves_out_relative_paths_in_recorded_steps() { let _ = std::fs::remove_dir_all(base); } +/// Two libraries loaded from outside the read root under the same file name, only the second of them +/// in a library panel: `ui.inspect` and `swatch.library.list` show the second under the same id, and +/// that id finds it. +#[test] +fn ui_inspect_and_library_commands_show_a_hidden_library_under_one_id() { + let (mut app, tx, base) = confined("one-id"); + let (x, y) = (base.join("outside/x/Kit.gpl"), base.join("outside/y/Kit.gpl")); + for (path, colors) in [(&x, "0 0 0 Black\n"), (&y, "0 0 0 Black\n255 255 255 White\n")] { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, format!("GIMP Palette\nName: Kit\n{colors}")).unwrap(); + app.session.execute("swatch.library.load", &json!({"path": s(path.clone())})).unwrap(); + } + let second = format!("loaded/{}", s(y.clone())); + app.ui.library_panels = vec![serde_json::from_value(json!({"kind": "swatches", "id": second, "path": s(y.clone())})).unwrap()]; + + let r = request(&mut app, &tx, "ui.inspect", json!({})); + assert_eq!(r["result"]["ui"]["library_panels"][0]["id"], "loaded/Kit.gpl (2)", "{r}"); + let r = request(&mut app, &tx, "engine.execute", json!({"command": "swatch.library.list", "params": {}})); + let libs = r["result"]["libraries"].as_array().unwrap_or_else(|| panic!("{r}")); + let loaded: Vec<&Value> = libs.iter().filter(|l| l["category"] == "loaded").map(|l| &l["id"]).collect(); + assert_eq!(loaded, [&json!("loaded/Kit.gpl"), &json!("loaded/Kit.gpl (2)")], "{r}"); + let r = request(&mut app, &tx, "engine.execute", json!({"command": "swatch.library.get", "params": {"library": "loaded/Kit.gpl (2)"}})); + assert_eq!(r["result"]["swatches"].as_array().map(Vec::len), Some(2), "the second library: {r}"); + assert!(!r.to_string().contains("outside"), "{r}"); + let _ = std::fs::remove_dir_all(base); +} + /// A closed panel's former place, as `ui.docking_hidden` keeps it. fn hidden_place() -> craft_ui::docking::Location { serde_json::from_value(json!({"floating": null, "anchor": null, "placement": {"Tab": {"before": null}}, "size": null, "stack": null})).unwrap() diff --git a/docs/mcp.md b/docs/mcp.md index a3c47ec71d..1881416381 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -155,7 +155,8 @@ the suite) keep the agent to the folders you give it: roots shows as `loaded/` wherever `ui_inspect` and `ui.docking` name it: its id, its panel's id, the Color Guide's `ui.color_guide_limit`, a closed panel's key in `ui.docking_hidden` and the `library` and `panel` params of a recorded action's steps in `ui.action_sets`, also when its panel is closed. A path written with - either separator counts as a path, also one saved on another platform. When another hidden library has the same + either separator counts as a path, also one saved on another platform, and outside Windows a path that starts + with a drive (`C:`) or a `\` counts as outside the roots. When another hidden library has the same file name, the libraries take `loaded/`, `loaded/ (2)`, `(3)` and so on in the order of their full ids (the libraries loaded in this session first, then those only the UI state names), and each one keeps its id in both replies and in the library commands below. A hidden library skips an id that a library