From 39137fbd784ca202142b349fa3c9db544b04f58c Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Sat, 24 Oct 2026 23:28:42 +0000 Subject: [PATCH] #1743: report refused queue items to CLI callers --- .../workflows/render-source-protection.yml | 2 + apps/effectcraft-cli/src/main.rs | 3 +- apps/effectcraft-cli/tests/it/cli.rs | 43 +++++++++++++++++++ crates/engine/src/commands/render_queue.rs | 12 +++++- crates/engine/src/tests_render_overwrite.rs | 35 +++++++++++++++ docs/render-source-protection.md | 4 ++ 6 files changed, 97 insertions(+), 2 deletions(-) diff --git a/.github/workflows/render-source-protection.yml b/.github/workflows/render-source-protection.yml index 3c040d831..a61c3bd4c 100644 --- a/.github/workflows/render-source-protection.yml +++ b/.github/workflows/render-source-protection.yml @@ -28,6 +28,8 @@ jobs: CARGO_INCREMENTAL: 0 steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@1.95.0 - run: cargo test --locked -p effectcraft-engine --lib output_guard::tests - run: cargo test --locked -p effectcraft-engine --lib tests_render_overwrite diff --git a/apps/effectcraft-cli/src/main.rs b/apps/effectcraft-cli/src/main.rs index 6e5b8d6cc..c858dec18 100644 --- a/apps/effectcraft-cli/src/main.rs +++ b/apps/effectcraft-cli/src/main.rs @@ -783,6 +783,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { r["outputModuleSummary"].as_str().unwrap_or("") ); } + let attempted: std::collections::HashSet = s.project.render_queue.iter().filter(|i| i.is_queued()).map(|i| i.id).collect(); s.execute("renderQueue.render", json!({"wait": false})).map_err(err)?; let tty = std::io::IsTerminal::is_terminal(&std::io::stderr()); // `--progress`: one line per update for a program reading stderr (a terminal gets the live line). @@ -821,7 +822,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { } let mut results = vec![]; let mut failed = vec![]; - for it in s.project.render_queue.iter().filter(|i| i.render && i.started.is_some()) { + for it in s.project.render_queue.iter().filter(|i| attempted.contains(&i.id)) { let name = s.project.item(it.comp).map(|i| i.name.clone()).unwrap_or_else(|| "?".into()); match &it.status { RenderStatus::Done => { diff --git a/apps/effectcraft-cli/tests/it/cli.rs b/apps/effectcraft-cli/tests/it/cli.rs index 538887d9b..97c604ce4 100644 --- a/apps/effectcraft-cli/tests/it/cli.rs +++ b/apps/effectcraft-cli/tests/it/cli.rs @@ -533,6 +533,49 @@ fn a_failed_render_under_run_and_exec_exits_1() { assert!(v["error"].as_str().is_some_and(|e| e.starts_with("T: ")), "{v}"); } +/// A source refusal happens before encoding and has no started timestamp. It must still +/// fail the CLI, while the valid queue item finishes and the imported frame stays intact. +#[test] +fn a_partly_protected_queue_exits_1_under_render_run_and_exec() { + let root = tmp("protected-mixed-queue"); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).unwrap(); + let source = root.join("source_00000.png"); + image::RgbaImage::from_pixel(4, 4, image::Rgba([40, 80, 120, 255])).save(&source).unwrap(); + let original = std::fs::read(&source).unwrap(); + let project = root.join("q.ecproj"); + let good = root.join("good.gif"); + let import = json!({"paths": [source]}).to_string(); + let blocked = json!({"format": "png", "output": root.join("source_[#####].png")}).to_string(); + let valid = json!({"format": "gif", "output": good}).to_string(); + ok_json(&[ + "run", + "--empty", + "--save-as", + project.to_str().unwrap(), + "comp.new", + r#"{"name":"Main","width":16,"height":16,"frameRate":10,"duration":0.1}"#, + "layer.newSolid", + "{}", + "file.import", + &import, + "renderQueue.add", + &blocked, + "renderQueue.add", + &valid, + ]); + let p = project.to_str().unwrap(); + for args in [vec!["render", "--queue", "--project", p], vec!["run", p, "renderQueue.render", "{}"], vec!["exec", "renderQueue.render", "--project", p]] { + std::fs::write(&good, b"previous output").unwrap(); + let (code, result) = run_json(&args); + assert_eq!(code, 1, "{args:?}: {result}"); + assert!(result["error"].as_str().is_some_and(|e| e.contains("footage") && e.contains("source_00000.png")), "{result}"); + assert_eq!(std::fs::read(&source).unwrap(), original); + assert!(std::fs::read(&good).unwrap().starts_with(b"GIF8"), "the valid item still rendered"); + } + std::fs::remove_dir_all(root).unwrap(); +} + /// `render --bitrate` (#440) targets the bitrate in VP9 WebM. #[test] fn render_bitrate_applies_to_vp9_webm() { diff --git a/crates/engine/src/commands/render_queue.rs b/crates/engine/src/commands/render_queue.rs index d7e914b1a..5b71cfca5 100644 --- a/crates/engine/src/commands/render_queue.rs +++ b/crates/engine/src/commands/render_queue.rs @@ -681,8 +681,18 @@ fn render(s: &mut Session, p: &Value) -> Result { { log::warn!("{e}"); } + let queued: std::collections::HashSet = s.project.render_queue.iter().filter(|i| i.is_queued()).map(|i| i.id).collect(); let ids = s.start_render(wait).map_err(EngineError::Other)?; - let items: Vec = s.project.render_queue.iter().enumerate().filter(|(_, i)| ids.contains(&i.id)).map(|(k, i)| item_json(s, i, k)).collect(); + // Source refusals fail before the exporter starts. Report them just like encoding + // failures, so CLI/MCP callers do not mistake a partly refused queue for full success. + let items: Vec = s + .project + .render_queue + .iter() + .enumerate() + .filter(|(_, i)| ids.contains(&i.id) || (queued.contains(&i.id) && matches!(i.status, RenderStatus::Failed(_)))) + .map(|(k, i)| item_json(s, i, k)) + .collect(); Ok(json!({"rendering": s.is_rendering(), "items": items})) } diff --git a/crates/engine/src/tests_render_overwrite.rs b/crates/engine/src/tests_render_overwrite.rs index a2473c6e8..420763da2 100644 --- a/crates/engine/src/tests_render_overwrite.rs +++ b/crates/engine/src/tests_render_overwrite.rs @@ -230,6 +230,7 @@ fn the_rest_of_the_queue_renders_when_one_item_would_replace_footage() { assert_eq!(f.read("main.mp4"), RENDERED); assert!(matches!(f.s.project.render_queue[0].status, RenderStatus::Failed(_))); assert_eq!(f.s.project.render_queue[1].status, RenderStatus::Done); + assert!(r["items"].as_array().unwrap().iter().any(|i| i["status"].get("Failed").is_some()), "the reply includes the refused item: {r}"); assert!(r["items"].as_array().unwrap().iter().any(|i| i["status"] == "Done"), "{r}"); } @@ -313,6 +314,40 @@ fn creating_a_proxy_again_chooses_a_new_default_and_attaches_it() { } } +/// A pre-render becomes imported footage. Its old output stays protected even from the +/// same queue item; issuing Pre-render again supplies a fresh default output instead. +#[test] +fn refreshing_a_pre_render_uses_a_new_default_without_exempting_imported_footage() { + struct Probe; + impl crate::Importer for Probe { + fn probe(&self, path: &str) -> Result { + if !Path::new(path).is_file() { + return Err("pre-render missing".into()); + } + Ok(Footage { path: path.into(), kind: FootageKind::Video, width: 16, height: 16, has_video: true, ..Default::default() }) + } + } + let mut f = Fixture::new("refresh-pre-render"); + f.s.importer = Some(Arc::new(Probe)); + f.comp("Main"); + f.s.execute("render.preRender", json!({"format": "gif"})).unwrap(); + let first = f.output(0); + f.render(json!({})).unwrap(); + f.s.execute("renderQueue.setRender", json!({"index": 1, "render": true})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("footage"), "{error}"); + assert_eq!(std::fs::read(&first).unwrap(), RENDERED); + assert_eq!(f.exported().len(), 1); + f.s.execute("renderQueue.setRender", json!({"index": 1, "render": false})).unwrap(); + f.s.execute("render.preRender", json!({"format": "gif"})).unwrap(); + let second = f.output(1); + assert_ne!(first, second); + f.render(json!({})).unwrap(); + assert_eq!(std::fs::read(first).unwrap(), RENDERED); + assert_eq!(std::fs::read(second).unwrap(), RENDERED); + assert_eq!(f.exported().len(), 2); +} + /// Exercise the engine's import and project serialization paths, rather than injecting a /// Footage item directly. The importer reads dimensions from a real, generated PNG. #[test] diff --git a/docs/render-source-protection.md b/docs/render-source-protection.md index 238684ed4..cf15f6797 100644 --- a/docs/render-source-protection.md +++ b/docs/render-source-protection.md @@ -15,6 +15,10 @@ Create Proxy: recreating a proxy chooses a new default name and attaches the new the previous proxy file intact. An explicit output path is kept as entered and refused if it would replace a protected file. +After Pre-render imports its result, that file is footage too. Re-queueing the same item +with the same output is refused; run Pre-render again for a fresh default output, or choose +another Output To. An item's `last_output` is not permission to replace imported footage. + Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves existing frames alone. If a frame number appears in a folder name, the checker probes each frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix.