fix(export): report skipped layers in doc_export_layers (#8491) (#8713)

* fix(export): report skipped layers across automation surfaces

* test(export): use the MCP visible_only parameter spelling

* test(export): restore old adapter for hosted regression proof

* Revert "test(export): restore old adapter for hosted regression proof"

This reverts commit 397a5ffa2a.

* docs(export): qualify workspace-only response guarantees
This commit is contained in:
JGF
2026-10-23 17:41:19 -04:00
committed by GitHub
parent 2cd1bd8664
commit 35cb32be20
8 changed files with 218 additions and 16 deletions
+8 -2
View File
@@ -171,10 +171,16 @@ impl Headless {
_ => return Err(AutomationError::BadRequest("automation filesystem access is not granted: write authority is absent".into())),
};
let mut outputs = workspace.export_sequence(dir)?;
let written = file_cmds::export_layers_with(&this.session, params, |name, bytes| {
let report = file_cmds::export_layers_report_with(&this.session, params, |name, bytes| {
outputs.write(name, bytes).map_err(|e| photocraft_engine::EngineError::Other(e.to_string()))
});
exported(dir, written, &mut outputs)
let (written, skipped) = match report {
Ok(report) => (Ok(report.files), report.skipped),
Err(error) => (Err(error), Vec::new()),
};
let mut result = exported(dir, written, &mut outputs)?;
result["skipped"] = json!(skipped);
Ok(result)
})
}
+53
View File
@@ -724,6 +724,59 @@ mod tests {
std::fs::remove_dir_all(base).unwrap();
}
#[test]
fn doc_export_layers_reports_skips_in_workspace_and_trusted_sessions() {
for trusted in [false, true] {
let base = std::env::temp_dir().join(format!("pc-export-skips-{}-{trusted}", std::process::id()));
let _ = std::fs::remove_dir_all(&base);
std::fs::create_dir_all(&base).unwrap();
let mut h =
if trusted { Headless::trusted_local() } else { Headless::with_workspace(crate::AuthorizedWorkspace::new(Some(&base), Some(&base)).unwrap()) };
h.handle("doc.new", json!({"width": 4, "height": 4, "background": "white", "name": "card"})).unwrap();
let adjustment = h.handle("engine.execute", json!({"command": "layer.newAdjustmentLayer.levels"})).unwrap()["layer"].clone();
h.handle("engine.execute", json!({"command": "layer.setProps", "params": {"name": "Levels"}})).unwrap();
let hidden = h.handle("engine.execute", json!({"command": "layer.new.layer", "params": {"name": "Hidden"}})).unwrap()["layer"].clone();
h.handle("engine.execute", json!({"command": "layer.setProps", "params": {"visible": false}})).unwrap();
// Exercise the actual JSON-lines serve envelope, not only direct dispatch.
let h = Mutex::new(h);
let out_dir = base.join("out").to_string_lossy().into_owned();
let request = json!({"id": 8491, "method": "doc.exportLayers", "params": {"dir": out_dir}}).to_string() + "\n";
let mut output = Vec::new();
serve_lines(&h, request.as_bytes(), &mut output).unwrap();
let reply: Value = serde_json::from_slice(&output).unwrap();
assert_eq!(reply["id"], 8491);
assert_eq!(reply["ok"], true, "{reply}");
assert_eq!(reply["result"]["files"], json!([format!("{out_dir}/card_0001_Background.png")]));
if !trusted {
assert_eq!(reply["result"]["count"], 1);
}
assert_eq!(
reply["result"]["skipped"],
json!([
{"layer": hidden, "name": "Hidden", "reason": "hidden by visibleOnly"},
{"layer": adjustment, "name": "Levels", "reason": "adjustment layer has no pixels of its own"}
])
);
assert_eq!(std::fs::read_dir(base.join("out")).unwrap().count(), 1);
let mut h = h.into_inner().unwrap();
let all_dir = base.join("all").to_string_lossy().into_owned();
let all = h.export_layers(None, &all_dir, &json!({"visibleOnly": false})).unwrap();
assert_eq!(all["files"].as_array().unwrap().len(), 2);
assert_eq!(all["skipped"].as_array().unwrap().len(), 1);
// Workspace no-overwrite errors still fail and remove only newly created files.
if !trusted {
let collision = base.join("collision");
std::fs::create_dir_all(&collision).unwrap();
std::fs::write(collision.join("card_0002_Background.png"), b"keep").unwrap();
assert!(h.export_layers(None, &collision.to_string_lossy(), &json!({"visibleOnly": false})).is_err());
assert_eq!(std::fs::read_dir(&collision).unwrap().count(), 1);
assert_eq!(std::fs::read(collision.join("card_0002_Background.png")).unwrap(), b"keep");
}
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()));
+1 -1
View File
@@ -645,7 +645,7 @@ impl PhotocraftMcp {
}
#[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."
description = "Export top-level layers as files (groups merged). Returns files and skipped: [{layer, name, reason}] for hidden layers (visible_only) and adjustments with no pixels of their own. Workspace exports resolve beneath the write root, also return count of written files, and reject existing files instead of overwriting them. Trusted-local/bridge exports use the engine command's file access policy."
)]
async fn doc_export_layers(&self, Parameters(p): Parameters<ExportLayersParams>) -> Result<CallToolResult, McpError> {
let mut opts = serde_json::Map::new();
+37
View File
@@ -768,6 +768,43 @@ async fn doc_exports_and_data_set_import_are_capability_scoped() {
cleanup(&base);
}
/// Skipped-layer details survive the MCP tool result, including the all-skipped case.
#[tokio::test(flavor = "multi_thread")]
async fn doc_export_layers_reports_skipped_layers() {
let base = tmp("skipped-layers");
let client = connect(headless_in(&base)).await;
json_of(&call(&client, "doc_new", json!({"width": 4, "height": 4, "background": "white", "name": "card"})).await);
let background = json_of(&call(&client, "doc_inspect", json!({})).await)["layers"][0]["id"].clone();
let adjustment = json_of(&call(&client, "command_run", json!({"id": "layer.newAdjustmentLayer.levels"})).await)["layer"].clone();
json_of(&call(&client, "command_run", json!({"id": "layer.setProps", "params": {"name": "Levels/α"}})).await);
let hidden = json_of(&call(&client, "command_run", json!({"id": "layer.new.layer", "params": {"name": "Hidden"}})).await)["layer"].clone();
json_of(&call(&client, "command_run", json!({"id": "layer.setProps", "params": {"visible": false}})).await);
let result = json_of(&call(&client, "doc_export_layers", json!({"dir": "layers"})).await);
assert_eq!(result["count"], 1);
assert_eq!(result["files"], json!(["layers/card_0001_Background.png"]));
assert_eq!(
result["skipped"],
json!([
{"layer": hidden, "name": "Hidden", "reason": "hidden by visibleOnly"},
{"layer": adjustment, "name": "Levels/α", "reason": "adjustment layer has no pixels of its own"}
])
);
let all = json_of(&call(&client, "doc_export_layers", json!({"dir": "all", "visible_only": false})).await);
assert_eq!(all["count"], 2);
assert_eq!(all["skipped"].as_array().unwrap().len(), 1);
assert_eq!(std::fs::read_dir(base.join("layers")).unwrap().count(), 1);
assert_eq!(std::fs::read_dir(base.join("all")).unwrap().count(), 2);
// Every top-level layer is still accounted for when none can be exported.
json_of(&call(&client, "command_run", json!({"id": "layer.setProps", "params": {"layer": background, "visible": false}})).await);
let none = json_of(&call(&client, "doc_export_layers", json!({"dir": "none"})).await);
assert_eq!(none["files"], json!([]));
assert_eq!(none["count"], 0);
assert_eq!(none["skipped"].as_array().unwrap().len(), 3);
assert_eq!(std::fs::read_dir(base.join("none")).unwrap().count(), 0);
client.cancel().await.unwrap();
cleanup(&base);
}
/// The template workflow stays in memory, including saving nested contents back to the parent.
#[tokio::test(flavor = "multi_thread")]
async fn embedded_smart_objects_can_be_edited_saved_and_unpacked_without_file_authority() {
+45 -12
View File
@@ -1668,16 +1668,41 @@ fn flatten_all_masks(s: &mut Session) -> Result<Value> {
fn layers_to_files(s: &mut Session, p: &Value) -> Result<Value> {
let cmd = "file.export.layersToFiles";
let dir = str_param(p, "dir", cmd).or_else(|_| str_param(p, "output", cmd))?.to_string();
let names = export_layers_with(s, p, |name, bytes| write_file(&join(&dir, name), bytes))?;
let files: Vec<String> = names.iter().map(|name| join(&dir, name)).collect();
Ok(json!({"files": files}))
let report = export_layers_report_with(s, p, |name, bytes| write_file(&join(&dir, name), bytes))?;
let files: Vec<String> = report.files.iter().map(|name| join(&dir, name)).collect();
Ok(json!({"files": files, "skipped": report.skipped}))
}
/// The bodies of `file.export.layersToFiles`, streamed to `write` as a single-component filename
/// plus its bytes: one file per exported layer (top to bottom, like Photoshop's numbering),
/// encoded but never touched as a path. Automation writes exactly these bytes beneath its write
/// root (#3216), so the two paths cannot drift. Returns the filenames written, in order.
pub fn export_layers_with(s: &Session, p: &Value, mut write: impl FnMut(&str, &[u8]) -> Result<()>) -> Result<Vec<String>> {
pub fn export_layers_with(s: &Session, p: &Value, write: impl FnMut(&str, &[u8]) -> Result<()>) -> Result<Vec<String>> {
export_layers_report_with(s, p, write).map(|report| report.files)
}
/// A top-level layer omitted from a successful Layers to Files export.
#[derive(Debug, serde::Serialize)]
pub struct SkippedExportLayer {
/// Session layer id (names need not be unique).
pub layer: u64,
/// Original name, not the sanitized filename component.
pub name: String,
/// Why the layer produced no file.
pub reason: &'static str,
}
/// Filenames written and layers skipped, each in top-to-bottom order.
#[derive(Debug, Default)]
pub struct LayerExportReport {
pub files: Vec<String>,
pub skipped: Vec<SkippedExportLayer>,
}
/// The same export as [`export_layers_with`], with an explanation for every omitted top-level
/// layer. Groups export merged, so their children are not separate exports or skips. Encoding
/// and write failures remain errors, never successful skips. Existing numbering is preserved.
pub fn export_layers_report_with(s: &Session, p: &Value, mut write: impl FnMut(&str, &[u8]) -> Result<()>) -> Result<LayerExportReport> {
let cmd = "file.export.layersToFiles";
let format = p.get("format").and_then(Value::as_str).unwrap_or("png").trim_start_matches('.').to_ascii_lowercase();
let visible_only = p.get("visibleOnly").and_then(Value::as_bool).unwrap_or(true);
@@ -1688,24 +1713,32 @@ pub fn export_layers_with(s: &Session, p: &Value, mut write: impl FnMut(&str, &[
let d = s.active().ok_or(EngineError::NoDocument)?;
let prefix = p.get("prefix").and_then(Value::as_str).map(str::to_string).unwrap_or_else(|| stem(&d.doc.name));
let doc = d.doc.clone();
let mut written = Vec::new();
// Top-level layers (groups export merged), top to bottom like Photoshop's numbering.
let layers: Vec<&Layer> = doc.layers.iter().rev().filter(|l| l.visible || !visible_only).collect();
for (i, l) in layers.iter().enumerate() {
let mut report = LayerExportReport::default();
// Hidden layers never consumed a filename index; adjustments that pass the visibility
// filter did. Keep those filenames stable while reporting both kinds of omission.
let mut i = 0usize;
for l in doc.layers.iter().rev() {
if !l.visible && visible_only {
report.skipped.push(SkippedExportLayer { layer: l.id.0, name: l.name.clone(), reason: "hidden by visibleOnly" });
continue;
}
let index = i;
i += 1;
if matches!(l.content, LayerContent::Adjustment(_)) {
report.skipped.push(SkippedExportLayer { layer: l.id.0, name: l.name.clone(), reason: "adjustment layer has no pixels of its own" });
continue;
}
let mut one = (*doc).clone();
let mut only = (*l).clone();
let mut only = l.clone();
only.visible = true;
only.clipped = false;
one.layers = vec![only];
let name = format!("{}_{:04}_{}.{format}", sanitize(&prefix), i, sanitize(&l.name));
let name = format!("{}_{:04}_{}.{format}", sanitize(&prefix), index, sanitize(&l.name));
let (bytes, _warnings) = encode(&one, &name, save)?;
write(&name, &bytes)?;
written.push(name);
report.files.push(name);
}
Ok(written)
Ok(report)
}
/// Adjustment layers that can be represented as an RGB-only LUT, in document stack order.
+63
View File
@@ -1080,6 +1080,69 @@ fn flatten_all_layer_effects_keeps_a_pattern_overlay() {
}
}
#[test]
fn layers_to_files_reports_skipped_layers_without_changing_files() {
for depth in [8, 16, 32] {
let dir = tmp(&format!("skipped-layers-{depth}"));
let mut s = session(4, 4, depth);
let adjustment = s.execute("layer.newAdjustmentLayer.levels", json!({})).unwrap()["layer"].clone();
s.execute("layer.setProps", json!({"name": "Levels/α"})).unwrap();
let hidden = s.execute("layer.new.layer", json!({"name": "Levels/α"})).unwrap()["layer"].clone();
s.execute("layer.setProps", json!({"visible": false})).unwrap();
let before = s.active().unwrap().doc.clone();
let result = s.execute("file.export.layersToFiles", json!({"dir": dir, "prefix": "doc"})).unwrap();
assert_eq!(
result["skipped"],
json!([
{"layer": hidden, "name": "Levels/α", "reason": "hidden by visibleOnly"},
{"layer": adjustment, "name": "Levels/α", "reason": "adjustment layer has no pixels of its own"}
])
);
assert_eq!(result["files"], json!([join(&dir, "doc_0001_Background.png")]));
assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 1);
assert_eq!(s.active().unwrap().doc.layers.len(), before.layers.len());
assert!(!s.active().unwrap().doc.layer(LayerId(hidden.as_u64().unwrap())).unwrap().visible);
let all = s.execute("file.export.layersToFiles", json!({"dir": dir, "prefix": "all", "visibleOnly": false})).unwrap();
assert_eq!(
all["skipped"],
json!([
{"layer": adjustment, "name": "Levels/α", "reason": "adjustment layer has no pixels of its own"}
])
);
assert_eq!(all["files"], json!([join(&dir, "all_0000_Levels_α.png"), join(&dir, "all_0002_Background.png")]));
}
}
#[test]
fn layers_to_files_reports_groups_as_exports_and_failures_as_errors() {
let mut s = session(4, 4, 8);
s.execute("layer.newAdjustmentLayer.levels", json!({})).unwrap();
s.execute("layer.new.groupFromLayers", json!({"name": "Pack"})).unwrap();
let params = json!({"prefix": "doc"});
let mut names = Vec::new();
let report = export_layers_report_with(&s, &params, |name, bytes| {
assert!(!bytes.is_empty());
names.push(name.to_string());
Ok(())
})
.unwrap();
assert_eq!(names, vec!["doc_0000_Pack.png", "doc_0001_Background.png"]);
assert_eq!(report.files, names);
assert!(report.skipped.is_empty(), "a group's adjustment is not an omitted top-level layer");
assert_eq!(export_layers_with(&s, &params, |_, _| Ok(())).unwrap(), names, "legacy Rust helper remains compatible");
assert!(export_layers_report_with(&s, &params, |_, _| Err(EngineError::Other("disk full".into()))).unwrap_err().to_string().contains("disk full"));
assert!(export_layers_report_with(&s, &json!({"format": "not-a-format"}), |_, _| panic!("must not write")).is_err());
assert!(export_layers_report_with(&Session::new(), &params, |_, _| panic!("must not write")).is_err());
s.edit("Empty", |doc, _| {
doc.layers.clear();
Ok(())
})
.unwrap();
let empty = export_layers_report_with(&s, &params, |_, _| panic!("must not write")).unwrap();
assert!(empty.files.is_empty());
assert!(empty.skipped.is_empty());
}
#[test]
fn layers_to_files_writes_each_visible_layer() {
let dir = tmp("layers");
+1 -1
View File
@@ -378,7 +378,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.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.exportLayers` | `{dir, format? (png), prefix?, visibleOnly? (true), quality?, index?}` → `{files, skipped}` (plus `count` for workspace exports): one file per exported top-level layer, named like `file.export.layersToFiles` names them. `count` is present only for workspace exports and counts written files only. `skipped` is always an array (empty when nothing is omitted), in top-to-bottom order, of `{layer, name, reason}`: the session layer id, original unsanitized name, and `"hidden by visibleOnly"` or `"adjustment layer has no pixels of its own"`. Visibility takes precedence for hidden adjustments; `visibleOnly:false` exports hidden non-adjustment layers. Groups export merged; their children are not separate exports or skips. Existing filenames/numbering are unchanged. `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. Encode/write failures are errors, not skips |
| `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 |
| `doc.save` | `{path?, format?, quality?, index?, tiffLayers?, tgaBits?, ddsCompression?, ddsMipmaps?}` (`.pcraft` native, else export by extension; set `tiffLayers: true` to preserve TIFF layers rather than exporting a flattened TIFF; `tgaBits` 24 or 32 sets a TGA's bits per pixel, and `ddsCompression`/`ddsMipmaps` a DDS texture's compression and mip chain, as `app.save` does). Without `path` only a PSD, PSB or `.pcraft` document is written back to its own file, in its own format; anything else is an error and the file is left unchanged. A successful layered save (PSD, PSB or `.pcraft`) records the current revision as saved and makes the file the document's path, so `session.list` reports `dirty: false`; a flat export is a copy and leaves the document dirty |
+10
View File
@@ -261,6 +261,16 @@ Tools:
- `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`
`doc_export_layers` in workspace headless mode returns `{files, count, skipped}`. `count` is
only the number of files written. Each skipped top-level layer is reported as
`{layer, name, reason}` (session id, original name, explanation); `skipped` is `[]` when every
layer exports. Hidden layers are reported when `visible_only` is true (the default; `visibleOnly` in the engine/serve params), and
standalone adjustments are reported because they have no pixels of their own. Groups still
export merged, so their children are not separate skips. Filename numbering is unchanged;
encoding or write failures remain errors. The engine command and trusted-local exports add
the same `skipped` array to their existing `{files}` response. See the
[`doc.exportLayers` protocol entry](control-protocol.md#headless-server) for details.
Claude Code (`.mcp.json` in the repo root, or `claude mcp add`):
```json