mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-24 09:57:08 -06:00
Merge PR #8150: fix(automation): doc.replaceSmartObject replaces Smart Object contents from the read root (#8061)
This commit is contained in:
@@ -152,6 +152,30 @@ impl Headless {
|
||||
})
|
||||
}
|
||||
|
||||
/// Replace a smart object's contents with a file from the read root: the capability-scoped
|
||||
/// `layer.smartObjects.replaceContents`, which takes an ambient `path` and is refused in
|
||||
/// automation (#8061). `params` is the command's own (`layer`).
|
||||
pub fn replace_smart_object(&mut self, index: Option<usize>, path: &str, params: &Value) -> Result<Value, AutomationError> {
|
||||
let i = self.doc_index(index)?;
|
||||
if self.session.active_index() != Some(i) && !self.session.set_active(i) {
|
||||
return Err(AutomationError::BadRequest(format!("no document at index {i}")));
|
||||
}
|
||||
let mut opts = params.as_object().cloned().unwrap_or_default();
|
||||
opts.insert("path".into(), json!(path));
|
||||
if matches!(&self.filesystem, Filesystem::TrustedLocal) {
|
||||
return self.command_run("layer.smartObjects.replaceContents", Value::Object(opts));
|
||||
}
|
||||
let bytes = match &self.filesystem {
|
||||
Filesystem::Workspace(workspace) => workspace.read(path)?,
|
||||
_ => {
|
||||
return Err(AutomationError::BadRequest("automation filesystem access is not granted: read authority is absent".into()));
|
||||
}
|
||||
};
|
||||
let name = Path::new(path).file_name().and_then(|name| name.to_str()).unwrap_or(path);
|
||||
self.sync_jobs();
|
||||
Ok(photocraft_engine::smart_cmds::replace_contents_from_bytes(&mut self.session, &Value::Object(opts), name, bytes)?)
|
||||
}
|
||||
|
||||
/// Export each exported layer of a document as its own file beneath the write root: the
|
||||
/// capability-scoped `file.export.layersToFiles`, which takes an ambient `dir` and is
|
||||
/// therefore refused in automation (#3216). The bytes come from the command's own core, so
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
//! - `session.list`, `doc.open {path}`, `doc.new {…file.new params}`,
|
||||
//! `doc.place {path, scale?, fit?, center?, index?}` (#2810: the read root is the scope,
|
||||
//! the counterpart of the refused ambient `file.placeEmbedded`),
|
||||
//! `doc.replaceSmartObject {path, layer?, index?}` (#8061: the same scope for
|
||||
//! `layer.smartObjects.replaceContents`),
|
||||
//! `doc.exportLayers {dir, format?, prefix?, visibleOnly?, quality?, index?}`,
|
||||
//! `doc.exportDataSets {dir, format?, dataSets?, naming?, index?}`,
|
||||
//! `doc.importDataSets {path, delimiter?, index?}` (#3216: the same treatment for the export
|
||||
@@ -43,6 +45,7 @@ pub const METHODS: &[&str] = &[
|
||||
"session.list",
|
||||
"doc.open",
|
||||
"doc.place",
|
||||
"doc.replaceSmartObject",
|
||||
"doc.new",
|
||||
"doc.save",
|
||||
"doc.exportLayers",
|
||||
@@ -108,6 +111,13 @@ impl Headless {
|
||||
opts.remove("index");
|
||||
self.place(index_of(&p)?, path, &Value::Object(opts))
|
||||
}
|
||||
"doc.replaceSmartObject" => {
|
||||
let path = str_of(&p, "path").ok_or_else(|| bad("doc.replaceSmartObject needs `path`"))?;
|
||||
let mut opts = p.as_object().cloned().unwrap_or_default();
|
||||
opts.remove("path");
|
||||
opts.remove("index");
|
||||
self.replace_smart_object(index_of(&p)?, path, &Value::Object(opts))
|
||||
}
|
||||
"doc.new" => self.command_run("file.new", p),
|
||||
"doc.save" => {
|
||||
let tga_bits = photocraft_engine::file_cmds::tga_bits_param(&p, "doc.save").map_err(|e| bad(e.to_string()))?;
|
||||
@@ -724,6 +734,76 @@ mod tests {
|
||||
std::fs::remove_dir_all(base).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn doc_replace_smart_object_reads_through_the_read_root_and_never_outside_it() {
|
||||
let base = std::env::temp_dir().join(format!("pc-doc-replace-so-{}", std::process::id()));
|
||||
let root = base.join("workspace");
|
||||
let outside = base.join("outside");
|
||||
for dir in [&root, &outside] {
|
||||
std::fs::create_dir_all(dir).unwrap();
|
||||
}
|
||||
let png = |w: u32, h: u32, v: u8| {
|
||||
let img = photocraft_codecs::Image::from_u8(w, h, photocraft_codecs::ChannelLayout::Rgb, vec![v; (w * h * 3) as usize]).unwrap();
|
||||
photocraft_codecs::encode(&img, photocraft_codecs::Format::Png, &Default::default()).unwrap()
|
||||
};
|
||||
std::fs::write(root.join("logo.png"), png(8, 6, 42)).unwrap();
|
||||
std::fs::write(root.join("red.png"), png(8, 6, 200)).unwrap();
|
||||
std::fs::write(outside.join("red.png"), png(8, 6, 200)).unwrap();
|
||||
let absolute = outside.join("red.png").to_string_lossy().into_owned();
|
||||
|
||||
let mut h = Headless::with_workspace(crate::AuthorizedWorkspace::new(Some(&root), Some(&root)).unwrap());
|
||||
h.handle("doc.new", json!({"width": 40, "height": 30})).unwrap();
|
||||
let layer = h.handle("doc.place", json!({"path": "logo.png"})).unwrap()["layer"].clone();
|
||||
// The smart object's file name and the composite at its centre: what Replace Contents changes.
|
||||
let contents = |h: &mut Headless| {
|
||||
let inspected = h.handle("doc.inspect", json!({})).unwrap();
|
||||
let layers = inspected["layers"].as_array().unwrap();
|
||||
let so = layers.iter().find(|l| l["id"] == layer).unwrap_or_else(|| panic!("{inspected}"));
|
||||
let name = so["smartSource"]["fileName"].as_str().unwrap_or_default().to_owned();
|
||||
let rendered = h.handle("doc.render", json!({"maxSide": 0})).unwrap();
|
||||
let png = base64::engine::general_purpose::STANDARD.decode(rendered["base64"].as_str().unwrap()).unwrap();
|
||||
let img = photocraft_codecs::decode(&png).unwrap();
|
||||
let (w, ht) = img.dimensions();
|
||||
let data = img.as_u8().unwrap();
|
||||
let channels = data.len() / (w * ht) as usize;
|
||||
(name, data[((ht / 2 * w + w / 2) as usize) * channels])
|
||||
};
|
||||
assert_eq!(contents(&mut h), ("logo.png".to_owned(), 42), "the placed contents");
|
||||
let replaced = h.handle("doc.replaceSmartObject", json!({"path": "red.png", "layer": layer})).unwrap();
|
||||
assert_eq!(replaced["layer"], layer, "{replaced}");
|
||||
assert_eq!(contents(&mut h), ("red.png".to_owned(), 200), "the contents were replaced");
|
||||
h.handle("engine.execute", json!({"command": "edit.undo"})).unwrap();
|
||||
assert_eq!(contents(&mut h), ("logo.png".to_owned(), 42), "one undo step restores the old contents");
|
||||
h.handle("engine.execute", json!({"command": "edit.redo"})).unwrap();
|
||||
assert_eq!(contents(&mut h), ("red.png".to_owned(), 200), "redo replaces them again");
|
||||
// Outside the read root, a bad layer, a missing path or a file that is no image: refused, nothing changed.
|
||||
for (params, reason) in [
|
||||
(json!({"path": "../outside/red.png"}), "parent traversal"),
|
||||
(json!({"path": absolute}), "must be inside the automation root"),
|
||||
(json!({"path": "logo.png", "layer": 999_999}), "no such layer"),
|
||||
(json!({}), "needs `path`"),
|
||||
] {
|
||||
let refused = h.handle("doc.replaceSmartObject", params).unwrap_err().to_string();
|
||||
assert!(refused.contains(reason), "{refused}");
|
||||
assert_eq!(contents(&mut h), ("red.png".to_owned(), 200), "nothing changed: {refused}");
|
||||
}
|
||||
std::fs::write(root.join("junk.png"), b"not an image").unwrap();
|
||||
assert!(h.handle("doc.replaceSmartObject", json!({"path": "junk.png", "layer": layer})).is_err());
|
||||
assert_eq!(contents(&mut h), ("red.png".to_owned(), 200), "a file that is no image changes nothing");
|
||||
// The ambient command keeps its refusal.
|
||||
let ambient = h.handle("engine.execute", json!({"command": "layer.smartObjects.replaceContents", "params": {"path": "red.png"}}));
|
||||
assert!(ambient.unwrap_err().to_string().contains("ambient filesystem paths"));
|
||||
// Trusted local keeps the ambient command.
|
||||
let mut t = Headless::trusted_local();
|
||||
t.handle("doc.new", json!({"width": 40, "height": 30})).unwrap();
|
||||
let tl = t.handle("doc.place", json!({"path": root.join("logo.png").to_string_lossy()})).unwrap()["layer"].clone();
|
||||
let trusted = t.handle("doc.replaceSmartObject", json!({"path": root.join("red.png").to_string_lossy(), "layer": tl})).unwrap();
|
||||
assert_eq!(trusted["layer"], tl, "{trusted}");
|
||||
drop(t);
|
||||
drop(h);
|
||||
std::fs::remove_dir_all(base).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn doc_export_layers_writes_only_beneath_the_write_root() {
|
||||
let base = std::env::temp_dir().join(format!("pc-doc-export-layers-{}", std::process::id()));
|
||||
|
||||
@@ -71,6 +71,19 @@ pub struct PlaceParams {
|
||||
pub center: Option<[f64; 2]>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct ReplaceSmartObjectParams {
|
||||
/// Path beneath the configured automation read root: relative (forward slashes) or absolute.
|
||||
pub path: String,
|
||||
/// Document index from `session_list` (default: the active document).
|
||||
#[serde(default)]
|
||||
pub index: Option<usize>,
|
||||
/// The Smart Object layer: an id or a name (default: the active layer).
|
||||
#[serde(default)]
|
||||
pub layer: Option<Value>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct ExportLayersParams {
|
||||
@@ -637,6 +650,27 @@ impl PhotocraftMcp {
|
||||
self.run_command("file.placeEmbedded".into(), bridge_params, true).await
|
||||
}
|
||||
|
||||
#[tool(
|
||||
description = "Replace a Smart Object's contents with the file at `path` beneath the read root, keeping its transform and filters (the capability-scoped Replace Contents). `layer` is the Smart Object's id or name (default: the active layer)."
|
||||
)]
|
||||
async fn doc_replace_smart_object(&self, Parameters(p): Parameters<ReplaceSmartObjectParams>) -> Result<CallToolResult, McpError> {
|
||||
let mut opts = serde_json::Map::new();
|
||||
if let Some(v) = p.layer {
|
||||
opts.insert("layer".into(), v);
|
||||
}
|
||||
let opts = Value::Object(opts);
|
||||
// Bridge mode runs the app on this machine, which reads the path itself like any command.
|
||||
let mut bridge_params = opts.clone();
|
||||
if let Some(o) = bridge_params.as_object_mut() {
|
||||
o.insert("path".into(), Value::String(p.path.clone()));
|
||||
}
|
||||
let (path, index) = (p.path, p.index);
|
||||
if let Some(r) = self.headless_op(move |h| h.replace_smart_object(index, &path, &opts)).await {
|
||||
return to_result(r);
|
||||
}
|
||||
self.run_command("layer.smartObjects.replaceContents".into(), bridge_params, true).await
|
||||
}
|
||||
|
||||
#[tool(
|
||||
description = "Export each exported layer of the document as its own file beneath the write root (the capability-scoped Layers to Files). A file that already exists is an error, not an overwrite."
|
||||
)]
|
||||
|
||||
@@ -80,6 +80,7 @@ async fn lists_expected_tools() {
|
||||
"doc_open",
|
||||
"doc_new",
|
||||
"doc_place",
|
||||
"doc_replace_smart_object",
|
||||
"doc_export_layers",
|
||||
"doc_export_data_sets",
|
||||
"doc_import_data_sets",
|
||||
@@ -706,6 +707,36 @@ async fn doc_place_is_capability_scoped() {
|
||||
cleanup(&base);
|
||||
}
|
||||
|
||||
/// #8061: Replace Contents reads from the read root through `doc_replace_smart_object`; the
|
||||
/// ambient `layer.smartObjects.replaceContents` stays refused.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn doc_replace_smart_object_is_capability_scoped() {
|
||||
let base = tmp("replace-so");
|
||||
let root = base.join("workspace");
|
||||
let outside = base.join("outside");
|
||||
std::fs::create_dir_all(&root).unwrap();
|
||||
std::fs::create_dir_all(&outside).unwrap();
|
||||
let img = photocraft_codecs::Image::from_u8(8, 6, photocraft_codecs::ChannelLayout::Rgb, vec![42; 144]).unwrap();
|
||||
let bytes = photocraft_codecs::encode(&img, photocraft_codecs::Format::Png, &Default::default()).unwrap();
|
||||
std::fs::write(root.join("inside.png"), &bytes).unwrap();
|
||||
std::fs::write(outside.join("outside.png"), bytes).unwrap();
|
||||
let client = connect(headless_in(&root)).await;
|
||||
|
||||
json_of(&call(&client, "doc_new", json!({"width": 40, "height": 30})).await);
|
||||
let placed = json_of(&call(&client, "doc_place", json!({"path": "inside.png"})).await);
|
||||
let layer = placed["layer"].clone();
|
||||
let replaced = call(&client, "doc_replace_smart_object", json!({"path": "inside.png", "layer": layer})).await;
|
||||
assert_ne!(replaced.is_error, Some(true), "{}", text(&replaced));
|
||||
let refused = call(&client, "doc_replace_smart_object", json!({"path": outside.join("outside.png").to_string_lossy(), "layer": layer})).await;
|
||||
assert_eq!(refused.is_error, Some(true));
|
||||
assert!(text(&refused).contains("must be inside the automation root"), "{}", text(&refused));
|
||||
let ambient = call(&client, "command_run", json!({"id": "layer.smartObjects.replaceContents", "params": {"path": "inside.png", "layer": layer}})).await;
|
||||
assert_eq!(ambient.is_error, Some(true));
|
||||
assert!(text(&ambient).contains("ambient filesystem paths"), "{}", text(&ambient));
|
||||
client.cancel().await.unwrap();
|
||||
cleanup(&base);
|
||||
}
|
||||
|
||||
/// #3216: the export half of #2810. The write root scopes both exports, the read root the CSV,
|
||||
/// and the three ambient commands keep their refusal.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
|
||||
@@ -956,6 +956,13 @@ fn replace_contents(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
replace_contents_bytes(s, id, &base_name(&path), bytes)
|
||||
}
|
||||
|
||||
/// Replace Contents with a file the caller already read, for the smart object `p` names (`layer`,
|
||||
/// else the active layer): the body of the command minus its ambient path read (#8061).
|
||||
pub fn replace_contents_from_bytes(s: &mut Session, p: &Value, file_name: &str, bytes: Vec<u8>) -> Result<Value> {
|
||||
let id = layer_param(s, p)?;
|
||||
replace_contents_bytes(s, id, file_name, bytes)
|
||||
}
|
||||
|
||||
/// Replace Contents from a file's name and bytes: what the desktop and web file pickers hand over.
|
||||
pub fn replace_contents_bytes(s: &mut Session, id: LayerId, file_name: &str, bytes: Vec<u8>) -> Result<Value> {
|
||||
decode_source(file_name, &bytes)?; // fail before touching the document
|
||||
|
||||
@@ -467,7 +467,7 @@ psd/src/
|
||||
|
||||
- **`automation`:** an MCP server built on `rmcp`. Its tool names use underscores (the dotted
|
||||
names such as `doc.open` belong to the JSON control protocol, `docs/control-protocol.md`). It exposes:
|
||||
- `session_list`, `doc_open`, `doc_new`, `doc_place` (place from the read root as a Smart Object),
|
||||
- `session_list`, `doc_open`, `doc_new`, `doc_place` (place from the read root as a Smart Object), `doc_replace_smart_object` (replace a Smart Object's contents from the read root),
|
||||
`doc_export_layers`, `doc_export_data_sets` (write beneath the write root), `doc_import_data_sets` (a CSV from the read root),
|
||||
`doc_save`, `doc_export`, `doc_inspect` (layer tree as JSON), `doc_render_preview` (PNG), `doc_select`, `doc_close`.
|
||||
- `command_list`, `command_run` and `command_batch`, generated from the registry; `jobs_list` and `jobs_cancel` for background jobs.
|
||||
|
||||
@@ -335,6 +335,7 @@ How each MCP tool maps onto control methods in bridge mode:
|
||||
| `doc_inspect` | `engine.execute {command: "document.inspect"}` |
|
||||
| `doc_open {path}` | `app.open {path}` |
|
||||
| `doc_place {path, …}` | `engine.execute {command: "file.placeEmbedded", params}` |
|
||||
| `doc_replace_smart_object {path, layer?, …}` | `engine.execute {command: "layer.smartObjects.replaceContents", params}`, which the desktop refuses (the command reads an ambient path, and no control method reads it through the automation root yet; `doc_place` is refused the same way). Use it headless or in the workspace server (`doc.replaceSmartObject`), where `path` resolves beneath the read root |
|
||||
| `doc_export_layers {dir, …}` | `engine.execute {command: "file.export.layersToFiles", params}` |
|
||||
| `doc_export_data_sets {dir, …}` | `engine.execute {command: "file.export.dataSetsAsFiles", params}` |
|
||||
| `doc_import_data_sets {path, …}` | `engine.execute {command: "file.import.variableDataSets", params}` |
|
||||
@@ -392,6 +393,7 @@ no MCP framing, no app start-up per command. Configure its file access with the
|
||||
| `session.list` | open documents and the active index |
|
||||
| `doc.open` / `doc.new` | `{path}` / `file.new` params |
|
||||
| `doc.place` | `{path, scale? (percent), fit? (true), center? ([x, y]), index?}`: places a file as a Smart Object layer above the active one and returns `{layer, scale, bounds}`. `path` resolves beneath the read root, so this is the capability-scoped `file.placeEmbedded` — that command itself still refuses an ambient path |
|
||||
| `doc.replaceSmartObject` | `{path, layer? (id or name; default the active layer), index?}` → the command's `{layer, instances}`: replaces a Smart Object's contents (one history step, transform and filters kept). `path` resolves beneath the read root, so this is the capability-scoped `layer.smartObjects.replaceContents`, which itself still refuses an ambient path (#8061) |
|
||||
| `doc.exportLayers` | `{dir, format? (png), prefix?, visibleOnly? (true), quality?, index?}` → `{files, count}`: one file per exported layer, named like `file.export.layersToFiles` names them. `dir` resolves beneath the write root, so the ambient command stays refused. The bytes come from the command's own core, so both paths write the same files; the write capability refuses to overwrite an existing file |
|
||||
| `doc.exportDataSets` | `{dir, format? (png), dataSets? (all), naming? ("{name}\|{index}\|{document}"), index?}` → `{files, count}`: the capability-scoped `file.export.dataSetsAsFiles`. The batch is planned first and colliding names are refused before any file is written, as in the command |
|
||||
| `doc.importDataSets` | `{path, delimiter? (,), index?}` → `{imported}`: the capability-scoped `file.import.variableDataSets`; the CSV is read beneath the read root |
|
||||
|
||||
+1
-1
@@ -257,7 +257,7 @@ For headless MCP clients, pass absolute paths to deliberately chosen trusted wor
|
||||
Tools:
|
||||
|
||||
- `session_list`
|
||||
- `doc_open`, `doc_new`, `doc_place`, `doc_export_layers`, `doc_export_data_sets`, `doc_import_data_sets`, `doc_save`, `doc_export`, `doc_inspect`, `doc_render_preview` (returns a PNG image), `doc_select`, `doc_close`
|
||||
- `doc_open`, `doc_new`, `doc_place`, `doc_replace_smart_object`, `doc_export_layers`, `doc_export_data_sets`, `doc_import_data_sets`, `doc_save`, `doc_export`, `doc_inspect`, `doc_render_preview` (returns a PNG image), `doc_select`, `doc_close`
|
||||
- `command_list`, `command_run`, `command_batch` (several commands per call)
|
||||
- `jobs_list` (running background jobs with progress, then recently finished ones with their result or error), `jobs_cancel` (one job by id, or every running job)
|
||||
- bridge only: `ui_inspect`, `ui_screenshot`, `ui_pointer`, `ui_menu_invoke`, `ui_set`, `control_call`
|
||||
|
||||
Reference in New Issue
Block a user