fix: capture shared action descendants at minimum call depth

Use a FIFO dependency queue so a deep route cannot hide descendants reachable through a shallower call. Cover packaged Batch and Conditional replay, retain Custom exported enabled flags in regression coverage, and document the existing concurrent-writer limitation of Save and Close.
This commit is contained in:
yoriswind committed 2026-11-02 18:14:50 -08:00
1 parent 6182b5b0d2
commit 9011c6d704
3 files changed
+45 -6

No files matched your search

+7 -6
View File
@@ -14,7 +14,7 @@
//! step, because the outer `actions.play` id would otherwise hide a recorded `file.*` command from
//! the top-level check.
use std::collections::BTreeSet;
use std::collections::{BTreeSet, VecDeque};
use serde::{Deserialize, Serialize};
use serde_json::{Value, json};
@@ -273,7 +273,7 @@ pub fn called_actions(list: &[Action], steps: &mut [(String, Value)]) -> Vec<Act
_ => None,
}
}
fn targets(list: &[Action], steps: &mut [(String, Value)], depth: usize, queue: &mut Vec<(usize, usize)>) {
fn targets(list: &[Action], steps: &mut [(String, Value)], depth: usize, queue: &mut VecDeque<(usize, usize)>) {
let mut pending = Vec::new();
for (id, params) in steps.iter_mut() {
pending.push((id.clone(), params));
@@ -303,14 +303,14 @@ pub fn called_actions(list: &[Action], steps: &mut [(String, Value)]) -> Vec<Act
if let (Some(action), Some(obj)) = (list.get(i), params.as_object_mut()) {
obj.insert("action".into(), json!(action.name));
}
queue.push((i, depth));
queue.push_back((i, depth));
}
// Conditional branches only take names, so there is nothing to rewrite.
"actions.conditional" => {
for key in ["then", "else"] {
let Some(name) = params.get(key).and_then(Value::as_str) else { continue };
if let Some(i) = list.iter().position(|a| a.name == name) {
queue.push((i, depth));
queue.push_back((i, depth));
}
}
}
@@ -319,10 +319,11 @@ pub fn called_actions(list: &[Action], steps: &mut [(String, Value)]) -> Vec<Act
}
}
}
let mut queue = Vec::new();
let mut queue = VecDeque::new();
targets(list, steps, 1, &mut queue);
let mut found = std::collections::BTreeMap::new();
while let Some((i, depth)) = queue.pop() {
// Visit each action at its shallowest call depth so a deeper path cannot hide its children.
while let Some((i, depth)) = queue.pop_front() {
// Playback stops at 16 levels, so deeper calls never run.
if depth > 16 || found.contains_key(&i) {
continue;
@@ -173,6 +173,8 @@ fn custom_duplicate_kernel_fields_are_kept_with_their_enabled_state() {
assert_eq!(action.steps[0].0, atn_cmds::PHOTOSHOP_STEP);
assert_eq!(action.disabled.contains(&0), !enabled);
assert_eq!(exported(action), d);
let (bytes, _) = atn_cmds::export("Parent", &[action], &[], false).unwrap();
assert_eq!(atn::parse(&bytes).unwrap().actions[0].items[0].enabled, enabled);
}
}
}
@@ -711,6 +713,39 @@ fn called_actions_packages_explicit_batch_children_and_replays_them() {
}
}
/// A deep route to a shared action must not hide children reached by a shallower playable branch.
#[test]
fn called_actions_packages_shared_descendants_at_the_shallowest_depth() {
let dir = tempfile::tempdir().unwrap();
let input = dir.path().join("input.png");
std::fs::write(&input, small_image(photocraft_codecs::Format::Png)).unwrap();
let mut list = vec![
Action::new("Shared", vec![("actions.play".into(), json!({"action":1}))]),
Action::new("Child", vec![("image.imageSize".into(), json!({"width":3,"height":1,"resample":"nearest"}))]),
];
// The long branch reaches Shared at depth16 and its child beyond the playback limit.
for i in 0..15 {
list.push(Action::new(format!("A{i}"), vec![("actions.play".into(), json!({"action":if i == 14 { 0 } else { i + 3 }}))]));
}
for (condition, then, otherwise) in [("always", "Shared", "A0"), ("never", "A0", "Shared")] {
let mut steps = vec![(
"file.automate.batch".into(),
json!({"input":[input],"output":dir.path().join(condition),"steps":[["actions.conditional",{"if":condition,"then":then,"else":otherwise}]]}),
)];
let captured = photocraft_engine::actions_cmds::called_actions(&list, &mut steps);
assert_eq!(captured.iter().map(|a| &a.name).collect::<Vec<_>>(), list.iter().map(|a| &a.name).collect::<Vec<_>>());
assert_eq!(captured[0].steps[0].1["action"], "Child");
let mut s = Session::new();
s.actions.list = captured;
let result = s.execute(&steps[0].0, steps[0].1.clone()).unwrap();
assert!(result["errors"].as_array().unwrap().is_empty(), "{result}");
assert_eq!(result["files"].as_array().unwrap().len(), 1, "{result}");
let saved = std::fs::read(result["files"][0].as_str().unwrap()).unwrap();
let doc = photocraft_io::import("saved.png", &saved).unwrap().document;
assert_eq!(doc.size, photocraft_doc::Size::new(3, 1));
}
}
/// JSON containers do not consume action-call depth; the existing cycle and sixteen-call limits remain.
#[test]
fn called_actions_nested_batch_shapes_keep_cycle_and_action_depth_bounds() {
+3
View File
@@ -370,6 +370,9 @@ Save and Close keeps TIFF layers by default; explicit tiffLayers false is
refused when it would lose layer data. TIFF color modes the writer cannot
preserve are refused. Existing Folder save defaults are unchanged.
Save and Close assumes that other processes do not modify or replace its inputs
during the batch. The source-data checks do not provide concurrent-write protection.
Save and Close refuses format conversion, sources already open in the caller,
and a result from a different document. An input must have an encoder for its
exact extension; import-only/unknown/extensionless inputs require Save As and