From 95c3af68c33b9e94cf8091242e661f3b0500718b Mon Sep 17 00:00:00 2001 From: David Mann Date: Thu, 22 Oct 2026 21:36:42 +0200 Subject: [PATCH] Integration: prove doc.replaceSmartObject replaces and undoes (#8150) The rpc test only checked the returned layer id, so a no-op that echoed {"layer": id} passed, its undo check passed on doc.place's own step, and the bad-layer case asserted on an empty reason. It now checks the smart object's file name and rendered centre pixel (42 -> 200 after the replace, back to 42 on undo, 200 on redo), that every refusal changes nothing, and that layer 999999 fails with "no such layer". The bridge row in docs/control-protocol.md claimed doc_replace_smart_object maps onto layer.smartObjects.replaceContents; the desktop refuses that command (ambient path), like doc_place's file.placeEmbedded. The row now says so and points to the headless / workspace server method. Co-authored-by: VishnuByrraju <112673081+VishnuByrraju@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/automation/src/rpc.rs | 26 +++++++++++++++++++++++--- docs/control-protocol.md | 2 +- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/automation/src/rpc.rs b/crates/automation/src/rpc.rs index b52ecc078a..1498898008 100644 --- a/crates/automation/src/rpc.rs +++ b/crates/automation/src/rpc.rs @@ -712,22 +712,42 @@ mod tests { 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}"); - let undo = h.handle("engine.execute", json!({"command": "edit.undo"})); - assert!(undo.is_ok(), "the replacement is one history step: {undo:?}"); + 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": "red.png", "layer": 999_999}), ""), + (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")); diff --git a/docs/control-protocol.md b/docs/control-protocol.md index f752aab3f0..7632c3a72e 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -318,7 +318,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}` | +| `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}` |