From 0f70015b93d7d78f308466e37d9fc60c1691d210 Mon Sep 17 00:00:00 2001 From: storybored379 Date: Sun, 4 Oct 2026 11:06:08 -0700 Subject: [PATCH 1/4] MCP: tool titles and annotations, command_batch / doc_inspect / render_preview, strict arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Every tool has a title and readOnly / destructive / idempotent / openWorld hints, set in its #[tool] attribute, so clients can run the read-only tools without asking and ask before edits. - New tools: command_batch (several commands, stop at the first error by default), doc_inspect (project tree + active sequence in one call) and render_preview (= render_frame). - An unknown tool argument is a -32602 error naming it and the accepted ones, instead of being ignored. - A line that is not JSON gets a -32700 reply (id null) instead of being dropped without an answer: serve_io filters lines in front of rmcp. - A panicking command no longer poisons the session lock for the rest of the session ("session lock poisoned"); it is reported as "internal error: …" (AGENTS.md §0.6). - docs/agents.md § Conventions. Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/server.rs | 371 +++++++++++++++++++++++++++++--- docs/agents.md | 22 ++ docs/control-protocol.md | 6 +- 3 files changed, 373 insertions(+), 26 deletions(-) diff --git a/crates/automation/src/server.rs b/crates/automation/src/server.rs index 816f6606..790b4a3b 100644 --- a/crates/automation/src/server.rs +++ b/crates/automation/src/server.rs @@ -1,6 +1,6 @@ //! The MCP server. Tool names are snake_case. -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, PoisonError}; use base64::Engine as _; use filmcraft_engine::Session; @@ -19,6 +19,19 @@ pub enum Backend { Bridge(Arc), } +/// A panic payload as text. +fn panic_message(p: Box) -> String { + p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()) +} + +/// A failed blocking task: a panic is reported as an internal error and the session stays usable. +fn join_error(e: tokio::task::JoinError) -> AutomationError { + match e.try_into_panic() { + Ok(p) => AutomationError::Other(format!("internal error: {}", panic_message(p))), + Err(e) => AutomationError::Other(e.to_string()), + } +} + #[derive(Clone)] pub struct FilmcraftMcp { backend: Arc, @@ -44,6 +57,24 @@ pub struct RunParams { pub params: Option, } +#[derive(Debug, Deserialize, JsonSchema)] +pub struct BatchStep { + /// Command id. + pub id: String, + /// Parameters object. + #[serde(default)] + pub params: Option, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct BatchParams { + /// Commands to run in order. + pub steps: Vec, + /// Stop at the first failing step (default true). + #[serde(default)] + pub stop_on_error: Option, +} + #[derive(Debug, Default, Deserialize, JsonSchema)] pub struct RenderParams { /// Timeline time in seconds (default: the playhead). @@ -148,9 +179,60 @@ impl FilmcraftMcp { } pub async fn serve_stdio(self) -> Result<(), AutomationError> { - let running = self.serve(rmcp::transport::stdio()).await.map_err(|e| AutomationError::Other(format!("MCP init: {e}")))?; - running.waiting().await.map_err(|e| AutomationError::Other(e.to_string()))?; - Ok(()) + self.serve_io(tokio::io::stdin(), tokio::io::stdout()).await + } + + /// Serve MCP over any line stream. A line that isn't JSON gets a JSON-RPC parse error + /// (`-32700`, id null) instead of being dropped silently; the rest go to the rmcp service. One + /// writer task owns `output`, so replies never interleave. + pub async fn serve_io( + self, + input: impl tokio::io::AsyncRead + Unpin + Send + 'static, + output: impl tokio::io::AsyncWrite + Unpin + Send + 'static, + ) -> Result<(), AutomationError> { + use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; + let (mut to_service, service_in) = tokio::io::duplex(1 << 20); + let (service_out, from_service) = tokio::io::duplex(1 << 20); + let (err_tx, mut err_rx) = tokio::sync::mpsc::unbounded_channel::(); + let reader = tokio::spawn(async move { + let mut lines = BufReader::new(input).lines(); + while let Ok(Some(line)) = lines.next_line().await { + if line.trim().is_empty() { + continue; + } + match serde_json::from_str::(&line) { + Ok(_) => { + if to_service.write_all(format!("{line}\n").as_bytes()).await.is_err() { + break; + } + } + Err(e) => { + let reply = json!({"jsonrpc": "2.0", "id": null, "error": {"code": -32700, "message": format!("parse error: {e}")}}); + let _ = err_tx.send(reply.to_string()); + } + } + } + // EOF: closing the pipe ends the service + let _ = to_service.shutdown().await; + }); + let writer = tokio::spawn(async move { + let mut output = output; + let mut replies = BufReader::new(from_service).lines(); + loop { + let line = tokio::select! { + l = replies.next_line() => match l { Ok(Some(l)) => l, _ => break }, + Some(e) = err_rx.recv() => e, + }; + if output.write_all(format!("{line}\n").as_bytes()).await.is_err() || output.flush().await.is_err() { + break; + } + } + }); + let running = self.serve((service_in, service_out)).await.map_err(|e| AutomationError::Other(format!("MCP init: {e}")))?; + let r = running.waiting().await.map_err(|e| AutomationError::Other(e.to_string())); + reader.abort(); + let _ = writer.await; + r.map(|_| ()) } /// Run an engine command on whichever backend. @@ -160,16 +242,24 @@ impl FilmcraftMcp { let s = s.clone(); let id = id.to_string(); tokio::task::spawn_blocking(move || { - let mut g = s.lock().map_err(|_| AutomationError::Other("session lock poisoned".into()))?; + // A command that panicked earlier poisoned the lock; the session is still usable. + let mut g = s.lock().unwrap_or_else(PoisonError::into_inner); g.execute(&id, params).map_err(AutomationError::from) }) .await - .map_err(|e| AutomationError::Other(e.to_string()))? + .map_err(join_error)? } Backend::Bridge(b) => b.call("engine.execute", json!({"command": id, "params": params})).await, } } + /// The project tree plus the active sequence (null when there is none). + async fn document(&self) -> Result { + let project = self.run("project.inspect", json!({})).await?; + let sequence = self.run("sequence.inspect", json!({})).await.ok(); + Ok(json!({"project": project, "sequence": sequence})) + } + fn bridge_client(&self) -> Option> { match &*self.backend { Backend::Bridge(b) => Some(b.clone()), @@ -190,7 +280,9 @@ impl FilmcraftMcp { #[tool_router] impl FilmcraftMcp { #[tool( - description = "List every command (id, label, menu path, shortcut, parameter doc, enabled now). Menus, shortcuts, panels and timeline gestures all map to these ids." + title = "List commands", + description = "List every command (id, label, menu path, shortcut, parameter doc, enabled now). Menus, shortcuts, panels and timeline gestures all map to these ids.", + annotations(read_only_hint = true, destructive_hint = false, idempotent_hint = true, open_world_hint = false) )] async fn command_list(&self, Parameters(p): Parameters) -> Result { let r = self.run("command.list", json!({})).await.map(|v| { @@ -213,38 +305,99 @@ impl FilmcraftMcp { } #[tool( - description = "Run a command by id with JSON params, e.g. {\"id\":\"timeline.razor\",\"params\":{\"seconds\":3.5}} or {\"id\":\"effects.apply\",\"params\":{\"effect\":\"Gaussian Blur\"}}. Undoable edits land in History." + title = "Run a command", + description = "Run a command by id with JSON params, e.g. {\"id\":\"timeline.razor\",\"params\":{\"seconds\":3.5}} or {\"id\":\"effects.apply\",\"params\":{\"effect\":\"Gaussian Blur\"}}. Undoable edits land in History.", + annotations(read_only_hint = false, destructive_hint = true, idempotent_hint = false, open_world_hint = false) )] async fn command_run(&self, Parameters(p): Parameters) -> Result { wrap(self.run(&p.id, p.params.unwrap_or(json!({}))).await) } - #[tool(description = "Project tree (bins and items with ids, types, durations), active sequence, source clip.")] + #[tool( + title = "Run several commands", + description = "Run several commands in order: {steps: [{id, params?}], stop_on_error?: true}. Returns {completed, failed, results: [{ok, result | error}]}; each edit is its own undo step.", + annotations(read_only_hint = false, destructive_hint = true, idempotent_hint = false, open_world_hint = false) + )] + async fn command_batch(&self, Parameters(p): Parameters) -> Result { + let stop = p.stop_on_error.unwrap_or(true); + let (mut completed, mut failed, mut results) = (0u32, 0u32, Vec::new()); + for st in p.steps { + match self.run(&st.id, st.params.unwrap_or(json!({}))).await { + Ok(v) => { + completed += 1; + results.push(json!({"ok": true, "result": v})); + } + Err(e) => { + failed += 1; + results.push(json!({"ok": false, "error": e.to_string()})); + if stop { + break; + } + } + } + } + let v = json!({"completed": completed, "failed": failed, "results": results}); + Ok(if failed > 0 { CallToolResult::error(vec![Content::text(v.to_string())]) } else { ok_json(&v) }) + } + + #[tool( + title = "Inspect the document", + description = "The project as JSON: the tree (as project_inspect) plus the active sequence (as sequence_inspect; null when there is none).", + annotations(read_only_hint = true, destructive_hint = false, idempotent_hint = true, open_world_hint = false) + )] + async fn doc_inspect(&self) -> Result { + wrap(self.document().await) + } + + #[tool( + title = "Render a preview", + description = "The active sequence's frame at `seconds` (default: the playhead) as PNG, same as render_frame (bridge mode: Program monitor screenshot).", + annotations(read_only_hint = true, destructive_hint = false, idempotent_hint = true, open_world_hint = false) + )] + async fn render_preview(&self, p: Parameters) -> Result { + self.render_frame(p).await + } + + #[tool( + title = "Inspect the project tree", + description = "Project tree (bins and items with ids, types, durations), active sequence, source clip.", + annotations(read_only_hint = true, destructive_hint = false, idempotent_hint = true, open_world_hint = false) + )] async fn project_inspect(&self) -> Result { wrap(self.run("project.inspect", json!({})).await) } #[tool( - description = "The active sequence as JSON: settings, tracks, clips (ids, start/duration in ticks and frames, effects), transitions, markers, in/out, playhead, selection. 254016000000 ticks = 1 second." + title = "Inspect the active sequence", + description = "The active sequence as JSON: settings, tracks, clips (ids, start/duration in ticks and frames, effects), transitions, markers, in/out, playhead, selection. 254016000000 ticks = 1 second.", + annotations(read_only_hint = true, destructive_hint = false, idempotent_hint = true, open_world_hint = false) )] async fn sequence_inspect(&self) -> Result { wrap(self.run("sequence.inspect", json!({})).await) } - #[tool(description = "Import media files by absolute path (MP4/MOV, WAV/MP3/FLAC/AIFF/Ogg, PNG/JPEG/…).")] + #[tool( + title = "Import media", + description = "Import media files by absolute path (MP4/MOV, WAV/MP3/FLAC/AIFF/Ogg, PNG/JPEG/…).", + annotations(read_only_hint = false, destructive_hint = false, idempotent_hint = false, open_world_hint = false) + )] async fn media_import(&self, Parameters(p): Parameters) -> Result { let paths: Vec<&str> = p.text.split('\n').map(str::trim).filter(|s| !s.is_empty()).collect(); wrap(self.run("file.import", json!({"paths": paths})).await) } - #[tool(description = "Render the active sequence's frame (at `seconds` or the playhead) and return it as PNG.")] + #[tool( + title = "Render a frame", + description = "Render the active sequence's frame (at `seconds` or the playhead) and return it as PNG.", + annotations(read_only_hint = true, destructive_hint = false, idempotent_hint = true, open_world_hint = false) + )] async fn render_frame(&self, Parameters(p): Parameters) -> Result { let max = p.max_side.unwrap_or(960); match &*self.backend { Backend::Headless(s) => { let s = s.clone(); let r = tokio::task::spawn_blocking(move || -> Result, AutomationError> { - let mut g = s.lock().map_err(|_| AutomationError::Other("lock".into()))?; + let mut g = s.lock().unwrap_or_else(PoisonError::into_inner); if let Some(sec) = p.seconds { g.set_playhead(filmcraft_time::Tick::from_seconds_f64(sec)); } @@ -255,7 +408,7 @@ impl FilmcraftMcp { png_rgba(img.w as u32, img.h as u32, img.over_black_rgba8(), max) }) .await - .map_err(|e| McpError::internal_error(e.to_string(), None))?; + .unwrap_or_else(|e| Err(join_error(e))); Ok(match r { Ok(png) => png_result(&png, "rendered frame"), Err(e) => fail(e), @@ -272,17 +425,29 @@ impl FilmcraftMcp { } } - #[tool(description = "Live app: UI state (tool, workspace, panels, timeline zoom, playback, selection, fps).")] + #[tool( + title = "Live app: UI state", + description = "Live app: UI state (tool, workspace, panels, timeline zoom, playback, selection, fps).", + annotations(read_only_hint = true, destructive_hint = false, idempotent_hint = true, open_world_hint = false) + )] async fn ui_inspect(&self) -> Result { self.ui("ui.inspect", json!({})).await } - #[tool(description = "Live app: every on-screen interactive element with id, label and rect (points). Use ids with ui_click/ui_drag.")] + #[tool( + title = "Live app: on-screen elements", + description = "Live app: every on-screen interactive element with id, label and rect (points). Use ids with ui_click/ui_drag.", + annotations(read_only_hint = true, destructive_hint = false, idempotent_hint = true, open_world_hint = false) + )] async fn ui_elements(&self, Parameters(p): Parameters) -> Result { self.ui("ui.elements", json!({"prefix": p.prefix.unwrap_or_default()})).await } - #[tool(description = "Live app: click an element by id or at x,y (points). Supports right/middle button, double-click (count=2) and modifiers.")] + #[tool( + title = "Live app: click", + description = "Live app: click an element by id or at x,y (points). Supports right/middle button, double-click (count=2) and modifiers.", + annotations(read_only_hint = false, destructive_hint = true, idempotent_hint = false, open_world_hint = false) + )] async fn ui_click(&self, Parameters(p): Parameters) -> Result { let mut v = json!({"button": p.button, "count": p.count, "modifiers": p.modifiers}); if let Some(id) = p.id { @@ -295,23 +460,37 @@ impl FilmcraftMcp { } #[tool( - description = "Live app: press-drag-release from one point/element to another (move clips, trim edges, scrub, resize panels, drop project items onto tracks)." + title = "Live app: drag", + description = "Live app: press-drag-release from one point/element to another (move clips, trim edges, scrub, resize panels, drop project items onto tracks).", + annotations(read_only_hint = false, destructive_hint = true, idempotent_hint = false, open_world_hint = false) )] async fn ui_drag(&self, Parameters(p): Parameters) -> Result { self.ui("ui.drag", json!({"from": p.from, "to": p.to, "steps": p.steps, "modifiers": p.modifiers})).await } - #[tool(description = "Live app: press a key or shortcut (e.g. `Space`, `L`, `Cmd+K`, `Shift+Delete`, `Cmd+Z`).")] + #[tool( + title = "Live app: press a key", + description = "Live app: press a key or shortcut (e.g. `Space`, `L`, `Cmd+K`, `Shift+Delete`, `Cmd+Z`).", + annotations(read_only_hint = false, destructive_hint = true, idempotent_hint = false, open_world_hint = false) + )] async fn ui_key(&self, Parameters(p): Parameters) -> Result { self.ui("ui.key", json!({"key": p.key})).await } - #[tool(description = "Live app: type text into the focused field.")] + #[tool( + title = "Live app: type text", + description = "Live app: type text into the focused field.", + annotations(read_only_hint = false, destructive_hint = true, idempotent_hint = false, open_world_hint = false) + )] async fn ui_type(&self, Parameters(p): Parameters) -> Result { self.ui("ui.type", json!({"text": p.text})).await } - #[tool(description = "Live app: screenshot of the window or one panel (PNG).")] + #[tool( + title = "Live app: screenshot", + description = "Live app: screenshot of the window or one panel (PNG).", + annotations(read_only_hint = true, destructive_hint = false, idempotent_hint = true, open_world_hint = false) + )] async fn ui_screenshot(&self, Parameters(p): Parameters) -> Result { match self.bridge_client() { Some(b) => self.screenshot(&b, p.panel, p.max_side.unwrap_or(1600)).await, @@ -320,7 +499,9 @@ impl FilmcraftMcp { } #[tool( - description = "Live app: call any control-channel method directly (ui.set, ui.scroll, ui.timeline.hit, ui.timeline.locate, ui.playback, ui.panel.show, ui.menu.list, …)." + title = "Live app: control-channel call", + description = "Live app: call any control-channel method directly (ui.set, ui.scroll, ui.timeline.hit, ui.timeline.locate, ui.playback, ui.panel.show, ui.menu.list, …).", + annotations(read_only_hint = false, destructive_hint = true, idempotent_hint = false, open_world_hint = false) )] async fn ui_control(&self, Parameters(p): Parameters) -> Result { self.ui(&p.method, p.params.unwrap_or(json!({}))).await @@ -350,12 +531,37 @@ impl FilmcraftMcp { } } +impl FilmcraftMcp { + /// The first argument `tool` doesn't take, as a message naming the ones it does. + fn unknown_arg(&self, tool: &str, args: Option<&rmcp::model::JsonObject>) -> Option { + let def = self.tool_router.get(tool)?; + let props = def.input_schema.get("properties").and_then(Value::as_object).cloned().unwrap_or_default(); + let bad = args?.keys().find(|k| !props.contains_key(*k))?; + let accepted: Vec<&str> = props.keys().map(String::as_str).collect(); + let accepted = if accepted.is_empty() { "no arguments".to_string() } else { accepted.join(", ") }; + Some(format!("unknown argument \"{bad}\" for {tool}; expected: {accepted}")) + } +} + #[tool_handler( router = self.tool_router, name = "filmcraft", - instructions = "FilmCraft video editor (Premiere Pro-class). Every edit is an engine command: `command_list` to discover ids/params, `command_run` to execute (undoable). `project_inspect`/`sequence_inspect` return ids you can pass to commands; `render_frame` shows the result. In bridge mode the `ui_*` tools drive the live app: `ui_elements` lists clickable ids, `ui_click`/`ui_drag`/`ui_key` operate it, `ui_screenshot` shows it. Time is in ticks: 254016000000 per second (commands also accept `seconds`, `frame` or `timecode`)." + instructions = "FilmCraft video editor (Premiere Pro-class). Every edit is an engine command: `command_list` to discover ids/params, `command_run` to execute (undoable; `command_batch` runs several). `doc_inspect` (or `project_inspect`/`sequence_inspect`) returns ids you can pass to commands; `render_preview` shows the result. In bridge mode the `ui_*` tools drive the live app: `ui_elements` lists clickable ids, `ui_click`/`ui_drag`/`ui_key` operate it, `ui_screenshot` shows it. Time is in ticks: 254016000000 per second (commands also accept `seconds`, `frame` or `timecode`)." )] -impl ServerHandler for FilmcraftMcp {} +impl ServerHandler for FilmcraftMcp { + /// Strict arguments in front of the tool router. + async fn call_tool( + &self, + request: rmcp::model::CallToolRequestParams, + context: rmcp::service::RequestContext, + ) -> Result { + if let Some(m) = self.unknown_arg(&request.name, request.arguments.as_ref()) { + return Err(McpError::invalid_params(m, None)); + } + let tcc = rmcp::handler::server::tool::ToolCallContext::new(self, request, context); + self.tool_router.call(tcc).await + } +} #[cfg(test)] mod tests { @@ -371,4 +577,121 @@ mod tests { let r = m.render_frame(Parameters(RenderParams { seconds: Some(1.0), max_side: Some(320) })).await.unwrap(); assert_ne!(r.is_error, Some(true)); } + + /// One request/response exchange over `serve_io`, as an MCP client sees it. + struct Client { + tx: tokio::io::DuplexStream, + rx: tokio::io::Lines>, + } + + impl Client { + fn start(s: Session) -> Self { + use tokio::io::AsyncBufReadExt; + let (tx, server_in) = tokio::io::duplex(1 << 22); + let (server_out, rx) = tokio::io::duplex(1 << 22); + tokio::spawn(FilmcraftMcp::headless(s).serve_io(server_in, server_out)); + Self { tx, rx: tokio::io::BufReader::new(rx).lines() } + } + async fn send(&mut self, line: &str) { + use tokio::io::AsyncWriteExt; + self.tx.write_all(format!("{line}\n").as_bytes()).await.unwrap(); + } + async fn next(&mut self) -> Value { + serde_json::from_str(&self.rx.next_line().await.unwrap().unwrap()).unwrap() + } + async fn ask(&mut self, msg: Value) -> Value { + self.send(&msg.to_string()).await; + self.next().await + } + async fn init(&mut self) -> Value { + let r = self.ask(json!({"jsonrpc":"2.0","id":0,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"t","version":"1"}}})).await; + self.send(&json!({"jsonrpc":"2.0","method":"notifications/initialized"}).to_string()).await; + r + } + async fn call(&mut self, id: u64, name: &str, arguments: Value) -> Value { + self.ask(json!({"jsonrpc":"2.0","id":id,"method":"tools/call","params":{"name":name,"arguments":arguments}})).await + } + } + + fn demo() -> Session { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + s + } + + /// Every listed tool has a title and the four hints, and the core tools are listed. + #[tokio::test(flavor = "multi_thread")] + async fn tools_are_titled_and_annotated() { + let mut c = Client::start(demo()); + let init = c.init().await; + assert_eq!(init["result"]["serverInfo"]["name"], "filmcraft"); + let tools = c.ask(json!({"jsonrpc":"2.0","id":1,"method":"tools/list"})).await["result"]["tools"].as_array().cloned().unwrap(); + for t in &tools { + assert!(t["title"].is_string(), "{} has no title", t["name"]); + for h in ["readOnlyHint", "destructiveHint", "idempotentHint", "openWorldHint"] { + assert!(t["annotations"][h].is_boolean(), "{} lacks {h}", t["name"]); + } + } + let names: Vec<&str> = tools.iter().filter_map(|t| t["name"].as_str()).collect(); + for want in ["command_list", "command_run", "command_batch", "doc_inspect", "render_preview", "project_inspect", "render_frame", "ui_click"] { + assert!(names.contains(&want), "{want} not listed"); + } + let ro = |n: &str| tools.iter().find(|t| t["name"] == n).map(|t| t["annotations"]["readOnlyHint"].clone()); + assert_eq!((ro("command_list"), ro("command_run")), (Some(json!(true)), Some(json!(false)))); + } + + /// Strict arguments, malformed JSON, batch, doc_inspect and render_preview over the raw line + /// protocol. + #[tokio::test(flavor = "multi_thread")] + async fn strict_arguments_and_parse_errors() { + let mut c = Client::start(demo()); + c.init().await; + let r = c.call(1, "command_list", json!({"filter": "razor", "enabled_only": true})).await; + assert!(r["result"]["content"][0]["text"].as_str().unwrap().contains("timeline.razor"), "{r}"); + // an unknown argument names itself and the accepted ones + let r = c.call(2, "command_list", json!({"filtr": "x"})).await; + assert_eq!(r["error"]["code"], -32602, "{r}"); + let m = r["error"]["message"].as_str().unwrap(); + assert!(m.contains("\"filtr\"") && m.contains("filter") && m.contains("enabled_only"), "{m}"); + let r = c.call(3, "doc_inspect", json!({"verbose": true})).await; + assert_eq!(r["error"]["code"], -32602, "{r}"); + // command params stay free-form + let r = c.call(4, "command_run", json!({"id": "playhead.set", "params": {"seconds": 0.5}})).await; + assert_eq!(r["result"]["isError"], false, "{r}"); + // malformed JSON: a parse error, and the server keeps serving + c.send("{\"jsonrpc\": \"2.0\", \"id\": 5, BROKEN").await; + let r = c.next().await; + assert_eq!((r["error"]["code"].as_i64(), r["id"].is_null()), (Some(-32700), true), "{r}"); + let r = c.ask(json!({"jsonrpc":"2.0","id":6,"method":"ping"})).await; + assert!(r["result"].is_object(), "{r}"); + // batch: stops at the first failure and reports both + let r = + c.call(7, "command_batch", json!({"steps": [{"id": "playhead.set", "params": {"seconds": 0.5}}, {"id": "no.such"}, {"id": "playhead.set"}]})).await; + assert_eq!(r["result"]["isError"], true, "{r}"); + let v: Value = serde_json::from_str(r["result"]["content"][0]["text"].as_str().unwrap()).unwrap(); + assert_eq!((v["completed"].as_u64(), v["failed"].as_u64(), v["results"].as_array().map(Vec::len)), (Some(1), Some(1), Some(2)), "{v}"); + let r = c.call(8, "doc_inspect", json!({})).await; + let v: Value = serde_json::from_str(r["result"]["content"][0]["text"].as_str().unwrap()).unwrap(); + assert!(v["project"].is_object() && v["sequence"].is_object(), "{v}"); + let r = c.call(9, "render_preview", json!({"max_side": 64})).await; + assert_eq!(r["result"]["content"][0]["type"], "image", "{r}"); + } + + /// A command that panicked used to poison the session lock, and every later call failed with + /// "session lock poisoned" until the server was restarted. + #[tokio::test(flavor = "multi_thread")] + async fn a_poisoned_session_lock_keeps_serving() { + let m = FilmcraftMcp::headless(demo()); + let Backend::Headless(s) = &*m.backend else { panic!("headless backend") }; + let s = s.clone(); + let _ = std::thread::spawn(move || { + let _g = s.lock().unwrap(); + panic!("a command panicked"); + }) + .join(); + let v = m.run("sequence.inspect", json!({})).await.unwrap(); + assert!(v.is_object(), "{v}"); + let r = m.render_frame(Parameters(RenderParams { seconds: Some(0.5), max_side: Some(64) })).await.unwrap(); + assert_ne!(r.is_error, Some(true)); + } } diff --git a/docs/agents.md b/docs/agents.md index 0689ff85..c5108c81 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -39,6 +39,9 @@ claude mcp add filmcraft-headless -- /abs/path/filmcraft/target/release/filmcraf |---|---|---| | `command_list` | both | every command: id, label, menu, shortcut, params, enabled now (`filter`, `enabled_only`) | | `command_run` | both | run a command `{id, params}`; edits are undoable | +| `command_batch` | both | run several commands in order `{steps: [{id, params}], stop_on_error}` → `{completed, failed, results}` | +| `doc_inspect` | both | the project tree and the active sequence in one call (`project_inspect` + `sequence_inspect`) | +| `render_preview` | both | same as `render_frame` | | `project_inspect` | both | bins and items with ids, types, durations; active sequence | | `sequence_inspect` | both | the active sequence: tracks, clips (ticks and frames), effects, transitions, markers, playhead, selection | | `media_import` | both | import files by absolute path (`text`, one path per line) | @@ -55,6 +58,25 @@ claude mcp add filmcraft-headless -- /abs/path/filmcraft/target/release/filmcraf Typical loop: `project_inspect` / `sequence_inspect` → get ids → `command_run` → `render_frame` or `ui_screenshot` → look at the result → `edit.undo` if it's wrong. +### Conventions + +The server follows a few conventions that make it predictable for agents. They are not +FilmCraft-specific: the sibling craft apps' MCP servers can follow the same ones, so an agent that has +driven one app can drive the others. + +- **Core tools.** `command_list`, `command_run`, `command_batch`, `doc_inspect` and `render_preview` + work in both modes. The FilmCraft tools above stay as they are. +- **Titles and annotations.** Every tool has a `title` and the four MCP hints (`readOnlyHint`, + `destructiveHint`, `idempotentHint`, `openWorldHint`), so a client can let the read-only tools + (lists, inspects, renders, screenshots) run without asking and ask before edits. When you add a + tool, set them in its `#[tool(...)]` attribute; a test checks every listed tool. +- **Strict arguments.** An unknown tool argument is a JSON-RPC `-32602` error that names it and the + accepted ones (`unknown argument "filtr" for command_list; expected: enabled_only, filter`), not a + silently ignored key. `command_run.params` goes to the command unchanged. +- **Errors.** A failing command is an `isError` result with the engine's message. A line that is not + JSON gets a `-32700` parse error (id `null`) and the server keeps serving. A command that panics is + reported as `internal error: …`, and the session stays usable. + ## 2. Control channel `filmcraft --control 9876` (or `FILMCRAFT_CONTROL_PORT=9876`) listens on `127.0.0.1` only. Send one diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 67637920..203d4f28 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -103,7 +103,9 @@ Ctrl), `Ctrl` (macOS ⌃), `Alt`, `Shift`. The dialog (Edit ▸ Keyboard Shortcu ## MCP `filmcraft-cli mcp` serves MCP on stdio: headless (in-process session; `--demo` / `--project p.fcproj`) or `--bridge 127.0.0.1:9876` to drive the running app. Tools: `command_list`, `command_run`, -`project_inspect`, `sequence_inspect`, `media_import`, `render_frame`, `ui_inspect`, `ui_elements`, -`ui_click`, `ui_drag`, `ui_key`, `ui_type`, `ui_screenshot`, `ui_control`. `.mcp.json` registers both. +`command_batch`, `doc_inspect`, `render_preview`, `project_inspect`, `sequence_inspect`, `media_import`, +`render_frame`, `ui_inspect`, `ui_elements`, `ui_click`, `ui_drag`, `ui_key`, `ui_type`, `ui_screenshot`, +`ui_control`. `.mcp.json` registers both. Annotations, argument checking and errors: +[agents.md § Conventions](agents.md#conventions). Time is in ticks (254 016 000 000 per second); commands also accept `seconds`, `frame` or `timecode`. From 8ee7658303b5912cface8e660c16e8db66ee6962 Mon Sep 17 00:00:00 2001 From: storybored379 Date: Sun, 4 Oct 2026 11:07:06 -0700 Subject: [PATCH 2/4] MCP: filmcraft://document and filmcraft://commands resources, with MCP 2026-07-28 cache hints - resources/list and resources/read serve the project (tree + active sequence, as doc_inspect) and the command catalog (as command_list) as JSON, for clients that attach context from resources. - Results are built with rmcp's types, so resultType follows the negotiated revision; for 2026-07-28 clients (current Claude Code) they also carry ttlMs and cacheScope, like the generated tools/list. The resource list is fixed (10 min), document reads are never cached (0). - get_info now declares the resources capability; the instructions text moves to a const, unchanged. Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/server.rs | 117 ++++++++++++++++++++++++++++++-- docs/agents.md | 4 ++ docs/control-protocol.md | 4 +- 3 files changed, 118 insertions(+), 7 deletions(-) diff --git a/crates/automation/src/server.rs b/crates/automation/src/server.rs index 790b4a3b..fdb51808 100644 --- a/crates/automation/src/server.rs +++ b/crates/automation/src/server.rs @@ -543,12 +543,27 @@ impl FilmcraftMcp { } } -#[tool_handler( - router = self.tool_router, - name = "filmcraft", - instructions = "FilmCraft video editor (Premiere Pro-class). Every edit is an engine command: `command_list` to discover ids/params, `command_run` to execute (undoable; `command_batch` runs several). `doc_inspect` (or `project_inspect`/`sequence_inspect`) returns ids you can pass to commands; `render_preview` shows the result. In bridge mode the `ui_*` tools drive the live app: `ui_elements` lists clickable ids, `ui_click`/`ui_drag`/`ui_key` operate it, `ui_screenshot` shows it. Time is in ticks: 254016000000 per second (commands also accept `seconds`, `frame` or `timecode`)." -)] +const INSTRUCTIONS: &str = "FilmCraft video editor (Premiere Pro-class). Every edit is an engine command: `command_list` to discover ids/params, `command_run` to execute (undoable; `command_batch` runs several). `doc_inspect` (or `project_inspect`/`sequence_inspect`) returns ids you can pass to commands; `render_preview` shows the result. In bridge mode the `ui_*` tools drive the live app: `ui_elements` lists clickable ids, `ui_click`/`ui_drag`/`ui_key` operate it, `ui_screenshot` shows it. Time is in ticks: 254016000000 per second (commands also accept `seconds`, `frame` or `timecode`)."; + +/// Resources: the project (as `doc_inspect`) and the command catalog (as `command_list`). +const DOCUMENT_URI: &str = "filmcraft://document"; +const COMMANDS_URI: &str = "filmcraft://commands"; + +/// MCP 2026-07-28 clients (current Claude Code) require `ttlMs` and `cacheScope` on list and read +/// results; rmcp fills `resultType` and leaves the cache hints to the server, as the generated +/// `tools/list` does. The resource list never changes; the document does, so reads aren't cached. +fn cache_hints(context: &rmcp::service::RequestContext, ttl_ms: u64) -> Option<(u64, rmcp::model::CacheScope)> { + context.protocol_version().is_some_and(|v| v >= rmcp::model::ProtocolVersion::V_2026_07_28).then_some((ttl_ms, rmcp::model::CacheScope::Private)) +} + +#[tool_handler(router = self.tool_router)] impl ServerHandler for FilmcraftMcp { + fn get_info(&self) -> rmcp::model::ServerConfig { + rmcp::model::ServerConfig::new(rmcp::model::ServerCapabilities::builder().enable_tools().enable_resources().build()) + .with_server_info(rmcp::model::Implementation::new("filmcraft", env!("CARGO_PKG_VERSION"))) + .with_instructions(INSTRUCTIONS.to_string()) + } + /// Strict arguments in front of the tool router. async fn call_tool( &self, @@ -561,6 +576,47 @@ impl ServerHandler for FilmcraftMcp { let tcc = rmcp::handler::server::tool::ToolCallContext::new(self, request, context); self.tool_router.call(tcc).await } + + async fn list_resources( + &self, + _request: Option, + context: rmcp::service::RequestContext, + ) -> Result { + use rmcp::model::Resource; + let mut r = rmcp::model::ListResourcesResult::with_all_items(vec![ + Resource::new(DOCUMENT_URI, "document") + .with_title("Project") + .with_description("The project tree and the active sequence (same as doc_inspect).") + .with_mime_type("application/json"), + Resource::new(COMMANDS_URI, "commands") + .with_title("Command catalog") + .with_description("Every command with id, label, menu path, shortcut, params and enabled state (same as command_list).") + .with_mime_type("application/json"), + ]); + if let Some((ttl, scope)) = cache_hints(&context, 600_000) { + r = r.with_ttl_ms(ttl).with_cache_scope(scope); + } + Ok(r) + } + + async fn read_resource( + &self, + request: rmcp::model::ReadResourceRequestParams, + context: rmcp::service::RequestContext, + ) -> Result { + let v = match request.uri.as_str() { + DOCUMENT_URI => self.document().await, + COMMANDS_URI => self.run("command.list", json!({})).await, + other => return Err(McpError::resource_not_found(format!("resource not found: {other}"), None)), + } + .map_err(|e| McpError::internal_error(e.to_string(), None))?; + let text = serde_json::to_string_pretty(&v).map_err(|e| McpError::internal_error(e.to_string(), None))?; + let mut r = rmcp::model::ReadResourceResult::new(vec![rmcp::model::ResourceContents::text(text, request.uri).with_mime_type("application/json")]); + if let Some((ttl, scope)) = cache_hints(&context, 0) { + r = r.with_ttl_ms(ttl).with_cache_scope(scope); + } + Ok(r.into()) + } } #[cfg(test)] @@ -677,6 +733,57 @@ mod tests { assert_eq!(r["result"]["content"][0]["type"], "image", "{r}"); } + /// `filmcraft://document` and `filmcraft://commands` are listed and readable as JSON. + #[tokio::test(flavor = "multi_thread")] + async fn resources_document_and_commands() { + let mut c = Client::start(demo()); + let init = c.init().await; + assert!(init["result"]["capabilities"]["resources"].is_object(), "{init}"); + assert!(init["result"]["instructions"].as_str().unwrap().contains("doc_inspect"), "{init}"); + let r = c.ask(json!({"jsonrpc":"2.0","id":1,"method":"resources/list"})).await; + let uris: Vec<&str> = r["result"]["resources"].as_array().unwrap().iter().filter_map(|x| x["uri"].as_str()).collect(); + assert_eq!(uris, [DOCUMENT_URI, COMMANDS_URI], "{r}"); + let r = c.ask(json!({"jsonrpc":"2.0","id":2,"method":"resources/read","params":{"uri":DOCUMENT_URI}})).await; + assert_eq!(r["result"]["contents"][0]["mimeType"], "application/json", "{r}"); + let v: Value = serde_json::from_str(r["result"]["contents"][0]["text"].as_str().unwrap()).unwrap(); + assert!(v["project"].is_object() && v["sequence"].is_object(), "{v}"); + let r = c.ask(json!({"jsonrpc":"2.0","id":3,"method":"resources/read","params":{"uri":COMMANDS_URI}})).await; + assert!(r["result"]["contents"][0]["text"].as_str().unwrap().contains("timeline.razor"), "{r}"); + let r = c.ask(json!({"jsonrpc":"2.0","id":4,"method":"resources/read","params":{"uri":"filmcraft://nope"}})).await; + assert!(r["error"]["code"].is_i64(), "{r}"); + } + + /// MCP 2026-07-28 clients (current Claude Code) negotiate per request and reject list and read + /// results without `resultType`, `ttlMs` and `cacheScope`; sessions that negotiated an older + /// revision through `initialize` get the old shape. + #[tokio::test(flavor = "multi_thread")] + async fn modern_clients_get_result_type_and_cache_hints() { + let mut c = Client::start(Session::default()); + let meta = json!({ + "io.modelcontextprotocol/protocolVersion": "2026-07-28", + "io.modelcontextprotocol/clientInfo": {"name": "t", "version": "1"}, + "io.modelcontextprotocol/clientCapabilities": {}, + }); + let r = c.ask(json!({"jsonrpc":"2.0","id":1,"method":"server/discover","params":{"_meta":meta}})).await; + assert!(r["result"]["supportedVersions"].as_array().unwrap().contains(&json!("2026-07-28")), "{r}"); + for (id, method, params) in [ + (2, "tools/list", json!({"_meta": meta})), + (3, "resources/list", json!({"_meta": meta})), + (4, "resources/read", json!({"_meta": meta, "uri": DOCUMENT_URI})), + ] { + let r = c.ask(json!({"jsonrpc":"2.0","id":id,"method":method,"params":params})).await; + assert_eq!(r["result"]["resultType"], "complete", "{method}: {r}"); + assert!(r["result"]["ttlMs"].is_u64(), "{method}: {r}"); + assert!(matches!(r["result"]["cacheScope"].as_str(), Some("public" | "private")), "{method}: {r}"); + } + let mut c = Client::start(Session::default()); + c.init().await; + for (id, method, params) in [(2, "tools/list", json!({})), (3, "resources/list", json!({})), (4, "resources/read", json!({"uri": DOCUMENT_URI}))] { + let r = c.ask(json!({"jsonrpc":"2.0","id":id,"method":method,"params":params})).await; + assert!(r["result"].is_object() && r["result"].get("resultType").is_none() && r["result"].get("ttlMs").is_none(), "{method}: {r}"); + } + } + /// A command that panicked used to poison the session lock, and every later call failed with /// "session lock poisoned" until the server was restarted. #[tokio::test(flavor = "multi_thread")] diff --git a/docs/agents.md b/docs/agents.md index c5108c81..80751185 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -76,6 +76,10 @@ driven one app can drive the others. - **Errors.** A failing command is an `isError` result with the engine's message. A line that is not JSON gets a `-32700` parse error (id `null`) and the server keeps serving. A command that panics is reported as `internal error: …`, and the session stays usable. +- **Resources.** `filmcraft://document` (the project tree and active sequence, as `doc_inspect`) and + `filmcraft://commands` (the command catalog, as `command_list`), both `application/json`. For MCP + 2026-07-28 clients such as current Claude Code, list and read results carry the `ttlMs` and + `cacheScope` hints that revision requires. ## 2. Control channel diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 203d4f28..d6fb94ef 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -105,7 +105,7 @@ Ctrl), `Ctrl` (macOS ⌃), `Alt`, `Shift`. The dialog (Edit ▸ Keyboard Shortcu or `--bridge 127.0.0.1:9876` to drive the running app. Tools: `command_list`, `command_run`, `command_batch`, `doc_inspect`, `render_preview`, `project_inspect`, `sequence_inspect`, `media_import`, `render_frame`, `ui_inspect`, `ui_elements`, `ui_click`, `ui_drag`, `ui_key`, `ui_type`, `ui_screenshot`, -`ui_control`. `.mcp.json` registers both. Annotations, argument checking and errors: -[agents.md § Conventions](agents.md#conventions). +`ui_control`; resources `filmcraft://document` and `filmcraft://commands`. `.mcp.json` registers both. +Annotations, argument checking and errors: [agents.md § Conventions](agents.md#conventions). Time is in ticks (254 016 000 000 per second); commands also accept `seconds`, `frame` or `timecode`. From cad16825e31201da12644b18fcd49b6acb27edd8 Mon Sep 17 00:00:00 2001 From: storybored379 Date: Sun, 4 Oct 2026 11:08:45 -0700 Subject: [PATCH 3/4] MCP: progress notifications and cancellation for blocking exports command_run file.exportMedia with wait: true (headless) runs as the background export job and uses the MCP progress and cancellation utilities: - with _meta.progressToken, notifications/progress reports frames done / total with the job status, at most every 100 ms (none without a token); - notifications/cancelled stops the job at the next batch (jobs.cancel), deletes the partial output and sends no response; - the session lock is only held for short polls, so ping, doc_inspect, jobs.list etc. are answered while it encodes. Without wait, and in bridge mode, nothing changes. Test export_progress_and_cancel. Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/lib.rs | 1 + crates/automation/src/long_job.rs | 126 ++++++++++++++++++++++++++++++ crates/automation/src/server.rs | 90 ++++++++++++++++++++- docs/agents.md | 18 +++++ docs/control-protocol.md | 3 +- 5 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 crates/automation/src/long_job.rs diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 76428c9e..39362ccc 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -9,6 +9,7 @@ //! and type in the live UI — every menu, panel, timeline gesture and keystroke. pub mod bridge; +pub mod long_job; pub mod server; pub use bridge::BridgeClient; diff --git a/crates/automation/src/long_job.rs b/crates/automation/src/long_job.rs new file mode 100644 index 00000000..cd86488d --- /dev/null +++ b/crates/automation/src/long_job.rs @@ -0,0 +1,126 @@ +//! Long exports over MCP (docs/agents.md § Long exports): `file.exportMedia` with `wait: true` +//! runs as a background engine job; the server reports its progress as MCP +//! `notifications/progress` (when the request carried a `progressToken`) and stops it on +//! `notifications/cancelled`, deleting the partial output. The session lock is only held for +//! short polls, so other requests are answered while the export runs. + +use std::path::Path; +use std::time::{Duration, SystemTime}; + +use rmcp::RoleServer; +use rmcp::model::{CallToolResult, ContentBlock as Content, ProgressNotificationParam, ProgressToken}; +use rmcp::service::{Peer, RequestContext}; +use serde_json::{Value, json}; + +use crate::server::FilmcraftMcp; + +/// Engine commands that block until a render is written when called with `wait: true`. +pub const LONG_COMMANDS: &[&str] = &["file.exportMedia"]; + +/// How often the job is polled (and at most how often progress is reported). +const POLL: Duration = Duration::from_millis(100); + +/// Whether `command_run {id, params}` is a long call this module runs. +pub fn is_long(id: &str, params: &Value) -> bool { + LONG_COMMANDS.contains(&id) && params.get("wait").and_then(Value::as_bool) == Some(true) +} + +/// Sends `notifications/progress` for one request; progress only ever increases. +struct Reporter { + peer: Peer, + token: Option, + last: f64, +} + +impl Reporter { + async fn report(&mut self, done: f64, total: f64, message: &str) { + let Some(token) = self.token.clone() else { return }; + if total <= 0.0 || done <= self.last { + return; + } + self.last = done; + let mut p = ProgressNotificationParam::new(token, done).with_total(total); + if !message.is_empty() { + p = p.with_message(message); + } + let _ = self.peer.notify_progress(p).await; + } +} + +impl FilmcraftMcp { + /// Run `id` (a [`LONG_COMMANDS`] entry, `wait: true`) as a job, reporting progress and + /// honouring cancellation. + pub(crate) async fn run_long(&self, id: &str, mut params: Value, context: &RequestContext) -> CallToolResult { + let started = SystemTime::now(); + if let Some(p) = params.as_object_mut() { + p.insert("wait".into(), json!(false)); + } + let start = match self.run(id, params).await { + Ok(v) => v, + Err(e) => return CallToolResult::error(vec![Content::text(e.to_string())]), + }; + let Some(job) = start.get("job").and_then(Value::as_u64) else { + return CallToolResult::error(vec![Content::text(format!("{id} started no job: {start}"))]); + }; + let path = start.get("path").and_then(Value::as_str).unwrap_or_default().to_string(); + let mut rep = Reporter { peer: context.peer.clone(), token: context.meta.get_progress_token(), last: 0.0 }; + loop { + let state = self.job_state(job).await; + let finished = state.as_ref().is_none_or(|j| j["finished"].as_bool() == Some(true)); + if let Some(j) = &state { + let (done, total) = (j["done"].as_f64().unwrap_or(0.0), j["total"].as_f64().unwrap_or(0.0)); + rep.report(done, total, j["status"].as_str().unwrap_or_default()).await; + } + if finished { + let result = state.map(|j| j["result"].clone()).unwrap_or(Value::Null); + if let Some(e) = result.get("error").and_then(Value::as_str) { + return CallToolResult::error(vec![Content::text(format!("export failed: {e}"))]); + } + let mut out = start; + if let Some(o) = out.as_object_mut() { + o.insert("result".into(), result); + } + return CallToolResult::success(vec![Content::text(serde_json::to_string_pretty(&out).unwrap_or_default())]); + } + tokio::select! { + _ = context.ct.cancelled() => break, + _ = tokio::time::sleep(POLL) => {} + } + } + // Cancelled: stop the encode at the next batch, wait for the worker, delete the partial file. + let _ = self.run("jobs.cancel", json!({"job": job})).await; + for _ in 0..600 { + if self.job_state(job).await.is_none_or(|j| j["finished"].as_bool() == Some(true)) { + break; + } + tokio::time::sleep(POLL).await; + } + remove_partial(&path, started); + CallToolResult::error(vec![Content::text("cancelled")]) + } + + /// The job's `jobs.list` entry. + async fn job_state(&self, job: u64) -> Option { + let jobs = self.run("jobs.list", json!({})).await.ok()?; + jobs.as_array()?.iter().find(|j| j["id"].as_u64() == Some(job)).cloned() + } +} + +/// Delete what an interrupted export wrote: the output file and, for image sequences and caption +/// sidecars, the files next to it named `[digits].` — only those written since `since`. +pub fn remove_partial(path: &str, since: SystemTime) { + let p = Path::new(path); + let (Some(dir), Some(stem)) = (p.parent(), p.file_stem().map(|s| s.to_string_lossy().to_string())) else { return }; + let dir = if dir.as_os_str().is_empty() { Path::new(".") } else { dir }; + let Ok(entries) = std::fs::read_dir(dir) else { return }; + // file times are coarser than the clock + let since = since.checked_sub(Duration::from_secs(1)).unwrap_or(since); + for e in entries.flatten() { + let name = e.file_name().to_string_lossy().to_string(); + let sibling = name.strip_prefix(&stem).is_some_and(|rest| rest.trim_start_matches(|c: char| c.is_ascii_digit()).starts_with('.')); + let ours = sibling && e.metadata().is_ok_and(|m| m.is_file() && m.modified().is_ok_and(|t| t >= since)); + if ours { + let _ = std::fs::remove_file(e.path()); + } + } +} diff --git a/crates/automation/src/server.rs b/crates/automation/src/server.rs index fdb51808..55da4a40 100644 --- a/crates/automation/src/server.rs +++ b/crates/automation/src/server.rs @@ -564,7 +564,7 @@ impl ServerHandler for FilmcraftMcp { .with_instructions(INSTRUCTIONS.to_string()) } - /// Strict arguments in front of the tool router. + /// Strict arguments and long exports in front of the tool router. async fn call_tool( &self, request: rmcp::model::CallToolRequestParams, @@ -573,6 +573,17 @@ impl ServerHandler for FilmcraftMcp { if let Some(m) = self.unknown_arg(&request.name, request.arguments.as_ref()) { return Err(McpError::invalid_params(m, None)); } + // A blocking export reports progress and can be cancelled (docs/agents.md § Long exports). + if request.name == "command_run" + && matches!(&*self.backend, Backend::Headless(_)) + && let Some(a) = &request.arguments + && let Some(id) = a.get("id").and_then(Value::as_str) + { + let params = a.get("params").cloned().filter(Value::is_object).unwrap_or_else(|| json!({})); + if crate::long_job::is_long(id, ¶ms) { + return Ok(self.run_long(id, params, &context).await.into()); + } + } let tcc = rmcp::handler::server::tool::ToolCallContext::new(self, request, context); self.tool_router.call(tcc).await } @@ -784,6 +795,83 @@ mod tests { } } + /// docs/agents.md § Long exports: a blocking export reports `notifications/progress` for its + /// token (none without one), answers `ping` meanwhile, and `notifications/cancelled` stops + /// it, deletes the partial file and sends no response. + #[tokio::test(flavor = "multi_thread")] + async fn export_progress_and_cancel() { + let dir = std::env::temp_dir().join(format!("filmcraft-mcp-progress-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let mut c = Client::start(demo()); + c.init().await; + let export = |id: u64, name: &str, token: Option, seconds: f64| { + let path = dir.join(name).to_string_lossy().to_string(); + let params = json!({"path": path, "format": "h264", "width": 640, "height": 360, "audio": false, "range": "custom", "startSeconds": 0, "endSeconds": seconds, "wait": true}); + let mut call = + json!({"jsonrpc":"2.0","id":id,"method":"tools/call","params":{"name":"command_run","arguments":{"id":"file.exportMedia","params":params}}}); + if let Some(t) = token { + call["params"]["_meta"] = json!({"progressToken": t}); + } + call.to_string() + }; + // no token: no notifications, the same response as before + c.send(&export(1, "quiet.mp4", None, 0.5)).await; + let m = c.next().await; + assert_eq!((m["id"].as_u64(), &m["result"]["isError"]), (Some(1), &json!(false)), "{m}"); + // progress, strictly increasing, with a total; ping answered while the export runs + c.send(&export(2, "a.mp4", Some(json!("tok")), 1.0)).await; + let (mut last, mut notes, mut pinged, mut pong) = (-1.0, 0, false, false); + let done = loop { + let m = c.next().await; + if m["method"] == "notifications/progress" { + assert_eq!(m["params"]["progressToken"], "tok", "{m}"); + let p = m["params"]["progress"].as_f64().unwrap(); + assert!(p > last && m["params"]["total"].as_f64().unwrap() >= p, "{m}"); + last = p; + notes += 1; + if !pinged { + pinged = true; + c.send(r#"{"jsonrpc":"2.0","id":3,"method":"ping"}"#).await; + } + } else if m["id"] == 3 { + pong = true; + } else if m["id"] == 2 { + break m; + } + }; + assert_eq!(done["result"]["isError"], false, "{done}"); + assert!(notes >= 2 && pong, "{notes} notifications, ping answered during the export: {pong}"); + assert!(dir.join("a.mp4").exists()); + // cancel after the first notification: no response, no partial file + c.send(&export(4, "b.mp4", Some(json!(7)), 8.0)).await; + loop { + let m = c.next().await; + assert_ne!(m["id"], 4, "finished before the cancel: {m}"); + if m["method"] == "notifications/progress" { + break; + } + } + c.send(r#"{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":4}}"#).await; + let t0 = std::time::Instant::now(); + while dir.join("b.mp4").exists() || t0.elapsed() < std::time::Duration::from_millis(500) { + assert!(t0.elapsed().as_secs() < 30, "partial file still there"); + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + c.send(r#"{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"command_run","arguments":{"id":"jobs.list"}}}"#).await; + let m = loop { + let m = c.next().await; + assert_ne!(m["id"], 4, "no response for a cancelled request: {m}"); + if m["id"] == 5 { + break m; + } + }; + let jobs: Value = serde_json::from_str(m["result"]["content"][0]["text"].as_str().unwrap()).unwrap(); + let last = jobs.as_array().and_then(|a| a.last()).cloned().unwrap_or_default(); + assert_eq!(last["result"]["error"], "cancelled", "{jobs}"); + assert!(!dir.join("b.mp4").exists()); + let _ = std::fs::remove_dir_all(&dir); + } + /// A command that panicked used to poison the session lock, and every later call failed with /// "session lock poisoned" until the server was restarted. #[tokio::test(flavor = "multi_thread")] diff --git a/docs/agents.md b/docs/agents.md index 80751185..1ef1e7e5 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -81,6 +81,24 @@ driven one app can drive the others. 2026-07-28 clients such as current Claude Code, list and read results carry the `ttlMs` and `cacheScope` hints that revision requires. +### Long exports: progress and cancellation + +`command_run {"id": "file.exportMedia", "params": {…, "wait": true}}` in headless mode blocks until +the file is written. It uses the MCP progress and cancellation utilities, so a client can show +progress and stop it: + +- The export runs as the same background job `jobs.list` shows. The session is locked only for short + polls, so other requests (`ping`, `doc_inspect`, `jobs.list`, …) are answered while it encodes. +- With `"_meta": {"progressToken": T}` on the `tools/call`, the server sends + `notifications/progress` `{progressToken: T, progress: , total: , message: + }` at most every 100 ms while frames advance. Without a token it sends none. +- `notifications/cancelled` for the request stops the encode at the next batch (`jobs.cancel`), + deletes the partial output (the movie, or the frames and caption sidecar written so far) and sends + no response, as the MCP cancellation utility asks. +- Without `wait` the command returns `{job, path}` at once, as before: poll `jobs.list` and stop it + with `jobs.cancel`. Other exports (`file.exportFrame`, interchange formats, …) finish quickly and + ignore the token. Bridge mode forwards the call unchanged; the app shows its own progress. + ## 2. Control channel `filmcraft --control 9876` (or `FILMCRAFT_CONTROL_PORT=9876`) listens on `127.0.0.1` only. Send one diff --git a/docs/control-protocol.md b/docs/control-protocol.md index d6fb94ef..f04ec395 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -106,6 +106,7 @@ or `--bridge 127.0.0.1:9876` to drive the running app. Tools: `command_list`, `c `command_batch`, `doc_inspect`, `render_preview`, `project_inspect`, `sequence_inspect`, `media_import`, `render_frame`, `ui_inspect`, `ui_elements`, `ui_click`, `ui_drag`, `ui_key`, `ui_type`, `ui_screenshot`, `ui_control`; resources `filmcraft://document` and `filmcraft://commands`. `.mcp.json` registers both. -Annotations, argument checking and errors: [agents.md § Conventions](agents.md#conventions). +Annotations, argument checking, errors and export progress / cancellation: +[agents.md § Conventions](agents.md#conventions). Time is in ticks (254 016 000 000 per second); commands also accept `seconds`, `frame` or `timecode`. From 5d1165d06b525b33cec1b8f697d5fd3a9a5b7cfd Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 5 Oct 2026 16:54:24 -0400 Subject: [PATCH 4/4] MCP: cancelled exports delete only their own files remove_partial removed every '[digits].' file in the output folder modified since the export started, so cancelling an export to trailer.mp4 also deleted trailer.aep or trailer2.psd if they were saved during the export. It now removes only the output file, numbered stills with the output's extension, and the .srt/.vtt caption sidecar. Regression test with decoy files. Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/long_job.rs | 71 ++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 7 deletions(-) diff --git a/crates/automation/src/long_job.rs b/crates/automation/src/long_job.rs index cd86488d..d03c4067 100644 --- a/crates/automation/src/long_job.rs +++ b/crates/automation/src/long_job.rs @@ -106,21 +106,78 @@ impl FilmcraftMcp { } } -/// Delete what an interrupted export wrote: the output file and, for image sequences and caption -/// sidecars, the files next to it named `[digits].` — only those written since `since`. +/// Delete what an interrupted export wrote, and nothing else: the output file itself, the numbered +/// stills of an image sequence (`.`) and the caption sidecar +/// (`.srt` / `.vtt`), each only if it was written since `since`. Other files that +/// merely share the name's stem (`.aep`, `2.psd`, …) are never touched, even when they +/// were saved during the export. pub fn remove_partial(path: &str, since: SystemTime) { let p = Path::new(path); - let (Some(dir), Some(stem)) = (p.parent(), p.file_stem().map(|s| s.to_string_lossy().to_string())) else { return }; + let (Some(dir), Some(stem), Some(file)) = (p.parent(), p.file_stem().and_then(|s| s.to_str()), p.file_name().and_then(|s| s.to_str())) else { + return; + }; + let ext = p.extension().and_then(|s| s.to_str()).unwrap_or_default(); let dir = if dir.as_os_str().is_empty() { Path::new(".") } else { dir }; let Ok(entries) = std::fs::read_dir(dir) else { return }; // file times are coarser than the clock let since = since.checked_sub(Duration::from_secs(1)).unwrap_or(since); for e in entries.flatten() { - let name = e.file_name().to_string_lossy().to_string(); - let sibling = name.strip_prefix(&stem).is_some_and(|rest| rest.trim_start_matches(|c: char| c.is_ascii_digit()).starts_with('.')); - let ours = sibling && e.metadata().is_ok_and(|m| m.is_file() && m.modified().is_ok_and(|t| t >= since)); - if ours { + let name = e.file_name(); + let Some(name) = name.to_str() else { continue }; + if is_export_output(name, file, stem, ext) && e.metadata().is_ok_and(|m| m.is_file() && m.modified().is_ok_and(|t| t >= since)) { let _ = std::fs::remove_file(e.path()); } } } + +/// Whether `name` is a file an export to `file` (`stem` + `.ext`) writes. +fn is_export_output(name: &str, file: &str, stem: &str, ext: &str) -> bool { + if name == file { + return true; + } + let Some(rest) = name.strip_prefix(stem) else { return false }; + if let Some(sidecar) = rest.strip_prefix('.') { + return sidecar.eq_ignore_ascii_case("srt") || sidecar.eq_ignore_ascii_case("vtt"); + } + // image sequence: one or more digits, then the output's own extension + let digits = rest.len() - rest.trim_start_matches(|c: char| c.is_ascii_digit()).len(); + digits > 0 && !ext.is_empty() && rest.get(digits..).is_some_and(|r| r.strip_prefix('.').is_some_and(|x| x == ext)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_the_exports_own_files_are_removed() { + let dir = std::env::temp_dir().join(format!("fc-remove-partial-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let started = SystemTime::now(); + let ours = ["trailer.png", "trailer000.png", "trailer123.png", "trailer.srt", "trailer.vtt"]; + // same stem, written during the export, but not the export's + let theirs = ["trailer.aep", "trailer2.psd", "trailer.docx", "trailer000.jpg", "trailers.png", "trailer.png.bak", "other.png"]; + for f in ours.iter().chain(theirs.iter()) { + std::fs::write(dir.join(f), b"x").unwrap(); + } + remove_partial(&dir.join("trailer.png").to_string_lossy(), started); + for f in ours { + assert!(!dir.join(f).exists(), "{f} should be removed"); + } + for f in theirs { + assert!(dir.join(f).exists(), "{f} must not be removed"); + } + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn files_older_than_the_export_are_kept() { + let dir = std::env::temp_dir().join(format!("fc-remove-partial-old-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("clip.mp4"), b"x").unwrap(); + remove_partial(&dir.join("clip.mp4").to_string_lossy(), SystemTime::now() + Duration::from_secs(60)); + assert!(dir.join("clip.mp4").exists()); + let _ = std::fs::remove_dir_all(&dir); + } +}