From 5c16320f6bdff936c4a65111b035fd7f9ace15a8 Mon Sep 17 00:00:00 2001 From: storybored379 Date: Thu, 8 Oct 2026 23:02:13 -0700 Subject: [PATCH 1/3] MCP: add shared core tools, hints and resources Keep documented helpers listed, reject unknown helper keys, and recover escaped backend panics. Cover core tools, resource snapshots and modern result metadata with regression tests. Co-Authored-By: OpenAI Codex --- crates/mcp/src/server.rs | 66 ++++++++++++++-- crates/mcp/src/tests.rs | 115 ++++++++++++++++++++++++++- crates/mcp/src/tools.rs | 167 +++++++++++++++++++++++++++++++++++++-- docs/mcp.md | 33 ++++++++ 4 files changed, 366 insertions(+), 15 deletions(-) diff --git a/crates/mcp/src/server.rs b/crates/mcp/src/server.rs index c3371be5..3f1f12f0 100644 --- a/crates/mcp/src/server.rs +++ b/crates/mcp/src/server.rs @@ -5,7 +5,7 @@ use std::io::{BufRead, Write}; use serde_json::{Value, json}; use crate::backend::Backend; -use crate::tools::{call_tool, tool_definitions}; +use crate::tools::{ToolResult, call_tool, check_args, doc_inspect, tool_definitions}; /// The MCP revision we implement. pub const PROTOCOL_VERSION: &str = "2025-06-18"; @@ -22,11 +22,13 @@ const RESOURCE_NOT_FOUND: i64 = -32002; const INSTRUCTIONS: &str = "LightCraft is a non-destructive photo library and raw developer (a Lightroom alternative). \ Photos have integer ids; most editing acts on the *active* photo (tools that take `id` make that photo active first). \ Typical loop: query_photos → select_photos (or pass id) → list_controls → set_develop {values: {\"light.exposure\": 0.5}} → \ -render_photo to look at the result → export. Positions (crop rects, mask points, brush strokes) are normalized image \ -coordinates 0..1 with the origin at the top-left. Every action is a command: list_commands shows ids and parameter \ -docs; run_command runs any of them. Edits are undoable (undo / redo)."; +render_preview to look at the result → export. Positions (crop rects, mask points, brush strokes) are normalized image \ +coordinates 0..1 with the origin at the top-left. Every action is a command: command_list shows ids and parameter \ +docs; command_run {id, params} runs any of them; command_batch runs steps in order. Inspect with doc_inspect. Edits are undoable (undo / redo)."; /// Resource URIs. +pub const DOCUMENT_URI: &str = "lightcraft://document"; +pub const COMMANDS_URI: &str = "lightcraft://commands"; pub const LIBRARY_URI: &str = "lightcraft://library"; pub const PHOTOS_URI: &str = "lightcraft://photos"; pub const PHOTO_URI: &str = "lightcraft://photo/active"; @@ -132,7 +134,10 @@ impl Server { return Some(error(id, INVALID_PARAMS, "`params` must be an object")); } Some(match self.request(method, ¶ms) { - Ok(r) => response(id, r), + Ok(mut r) => { + modern_result_fields(method, ¶ms, &mut r); + response(id, r) + } Err((code, m)) => error(id, code, m), }) } @@ -158,13 +163,20 @@ impl Server { })) } "ping" => Ok(json!({})), - "tools/list" => Ok(json!({"tools": tool_definitions(self.backend.as_mut(), self.command_tools)})), + "tools/list" => { + let command_tools = self.command_tools; + let tools = self.guarded(|b| tool_definitions(b, command_tools)).map_err(|e| (INTERNAL_ERROR, e))?; + Ok(json!({"tools": tools})) + } "tools/call" => { let name = params.get("name").and_then(Value::as_str).ok_or((INVALID_PARAMS, "missing tool `name`".to_string()))?; let args = params.get("arguments").cloned().unwrap_or(Value::Null); - Ok(call_tool(self.backend.as_mut(), name, &args).to_value()) + check_args(name, &args).map_err(|e| (INVALID_PARAMS, e))?; + Ok(self.guarded(|b| call_tool(b, name, &args)).unwrap_or_else(ToolResult::error).to_value()) } "resources/list" => Ok(json!({"resources": [ + resource(DOCUMENT_URI, "document", "Library document", "Library state and counts (same as doc_inspect)"), + resource(COMMANDS_URI, "commands", "Command catalog", "Every command (same as command_list)"), resource(LIBRARY_URI, "library", "Library state", "Current source, filter, sort, selection, active photo/mask and undo/redo labels (library.state)"), resource(PHOTOS_URI, "photos", "Photos in view", "The photos in the current view with ids, ratings, flags and metadata (catalog.query)"), resource(PHOTO_URI, "active-photo", "Active photo", "Everything about the active photo: file, metadata, develop settings, albums, history (photo.inspect)"), @@ -175,6 +187,7 @@ impl Server { "resources/read" => { let uri = params.get("uri").and_then(Value::as_str).ok_or((INVALID_PARAMS, "missing `uri`".to_string()))?; let cmd = match uri { + DOCUMENT_URI | COMMANDS_URI => "", LIBRARY_URI => "library.state", PHOTOS_URI => "catalog.query", PHOTO_URI => "photo.inspect", @@ -182,10 +195,47 @@ impl Server { CONTROLS_URI => "develop.controls", _ => return Err((RESOURCE_NOT_FOUND, format!("resource not found: {uri}"))), }; - let v = self.backend.call("engine.execute", json!({"command": cmd, "params": {}})).map_err(|e| (INTERNAL_ERROR, e))?; + let v = self + .guarded(|b| match uri { + DOCUMENT_URI => doc_inspect(b), + COMMANDS_URI => b.call("engine.commands", json!({})), + _ => b.call("engine.execute", json!({"command": cmd, "params": {}})), + }) + .and_then(|v| v) + .map_err(|e| (INTERNAL_ERROR, e))?; Ok(json!({"contents": [{"uri": uri, "mimeType": "application/json", "text": serde_json::to_string_pretty(&v).unwrap_or_default()}]})) } other => Err((METHOD_NOT_FOUND, format!("method not found: {other}"))), } } } + +impl Server { + /// Escaped backend panics become errors; subsequent requests can keep using the session. + fn guarded(&mut self, f: impl FnOnce(&mut dyn Backend) -> T) -> Result { + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(self.backend.as_mut()))).map_err(|p| { + let message = + p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()); + format!("internal error: {message}") + }) + } +} + +/// Per-request protocol metadata leaves initialize-based clients' result shapes unchanged. +fn modern_result_fields(method: &str, params: &Value, result: &mut Value) { + let modern = params.pointer("/_meta/io.modelcontextprotocol~1protocolVersion").and_then(Value::as_str).is_some_and(|v| v >= "2026-07-28"); + let ttl = match method { + "tools/list" | "resources/list" | "resources/templates/list" => Some(600_000), + "resources/read" => Some(0), + "tools/call" => None, + _ => return, + }; + let Some(obj) = result.as_object_mut().filter(|_| modern) else { + return; + }; + obj.insert("resultType".into(), json!("complete")); + if let Some(ttl) = ttl { + obj.insert("ttlMs".into(), json!(ttl)); + obj.insert("cacheScope".into(), json!("private")); + } +} diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index ee17e1ad..b58f195b 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -215,7 +215,7 @@ fn resources() { let mut s = server(); let r = rpc(&mut s, 1, "resources/list", json!({})); let list = r["result"]["resources"].as_array().unwrap(); - assert_eq!(list.len(), 5); + assert_eq!(list.len(), 7); for (i, res) in list.iter().enumerate() { let uri = res["uri"].as_str().unwrap(); let r = rpc(&mut s, 10 + i as u64, "resources/read", json!({"uri": uri})); @@ -245,3 +245,116 @@ fn select_photos_rejects_unknown_ids_and_keeps_the_active_photo() { assert!(r.is_error, "{r:?}"); assert_eq!(b.session.active(), Some(first)); } + +#[test] +fn core_tools_have_hints_and_reject_unknown_keys() { + for compact in [false, true] { + let mut s = server().with_command_tools(!compact); + let list = rpc(&mut s, 1, "tools/list", json!({})); + let tools = list["result"]["tools"].as_array().unwrap(); + for name in ["command_list", "command_run", "command_batch", "doc_inspect", "render_preview", "list_commands", "run_command", "render_photo"] + { + assert!(tools.iter().any(|t| t["name"] == name), "missing {name}"); + } + for t in tools { + assert!(!t["title"].as_str().unwrap().is_empty()); + for hint in ["readOnlyHint", "destructiveHint", "idempotentHint", "openWorldHint"] { + assert!(t["annotations"][hint].is_boolean(), "{t}"); + } + } + for name in ["command_list", "command_run", "command_batch", "doc_inspect", "render_preview", "query_photos", "run_command"] { + let r = rpc(&mut s, 2, "tools/call", json!({"name":name,"arguments":{"misspelled":true}})); + assert_eq!(r["error"]["code"], -32602, "{r}"); + assert!(r["error"]["message"].as_str().unwrap().contains("misspelled"), "{r}"); + } + let preview = tools.iter().find(|t| t["name"] == "render_preview").unwrap(); + assert_eq!(preview["annotations"]["readOnlyHint"], true); + assert!(preview["inputSchema"]["properties"].get("path").is_none()); + let writer = tools.iter().find(|t| t["name"] == "render_photo").unwrap(); + assert_eq!(writer["annotations"]["readOnlyHint"], false); + let export = tools.iter().find(|t| t["name"] == "export").unwrap(); + for key in lightcraft_engine::export::OPTION_PARAMS.iter().chain(lightcraft_engine::export::TARGET_PARAMS) { + assert!(export["inputSchema"]["properties"].get(*key).is_some(), "missing export argument {key}"); + } + let before = rpc(&mut s, 2, "resources/read", json!({"uri":"lightcraft://library"})); + let image = rpc(&mut s, 2, "tools/call", json!({"name":"render_preview","arguments":{"max_side":64}})); + assert_eq!(image["result"]["isError"], false, "{image}"); + assert!(image["result"]["content"].as_array().unwrap().iter().any(|c| c["type"] == "image")); + assert_eq!(before, rpc(&mut s, 2, "resources/read", json!({"uri":"lightcraft://library"}))); + let batch = json!({"steps":[{"id":"catalog.stats"},{"id":"no.such"},{"id":"catalog.stats"}],"stop_on_error":false}); + let r = rpc(&mut s, 3, "tools/call", json!({"name":"command_batch","arguments":batch})); + assert_eq!(r["result"]["isError"], true); + assert_eq!(r["result"]["structuredContent"]["completed"], 2); + assert_eq!(r["result"]["structuredContent"]["failed"], 1); + let r = rpc(&mut s, 4, "tools/call", json!({"name":"command_batch","arguments":{"steps":[{"id":"no.such"},{"id":"catalog.stats"}]}})); + assert_eq!(r["result"]["structuredContent"]["completed"], 0); + assert_eq!(r["result"]["structuredContent"]["results"].as_array().unwrap().len(), 1); + } +} + +#[test] +fn core_resources_match_tools_after_parse_error() { + let mut s = server(); + let bad: Value = serde_json::from_str(&s.handle_line("{bad").unwrap()).unwrap(); + assert_eq!(bad["error"]["code"], -32700); + assert!(bad["id"].is_null()); + let list = rpc(&mut s, 1, "resources/list", json!({})); + for (uri, tool) in [("lightcraft://document", "doc_inspect"), ("lightcraft://commands", "command_list")] { + assert!(list["result"]["resources"].as_array().unwrap().iter().any(|r| r["uri"] == uri)); + let resource = rpc(&mut s, 2, "resources/read", json!({"uri":uri})); + let call = rpc(&mut s, 3, "tools/call", json!({"name":tool,"arguments":{}})); + let a: Value = serde_json::from_str(resource["result"]["contents"][0]["text"].as_str().unwrap()).unwrap(); + let b: Value = serde_json::from_str(call["result"]["content"][0]["text"].as_str().unwrap()).unwrap(); + assert_eq!(a, b); + } + let old = rpc(&mut s, 4, "resources/read", json!({"uri":"lightcraft://library"})); + assert!(old["result"]["contents"].is_array(), "{old}"); +} + +#[test] +fn escaped_tool_panic_is_an_error_and_session_survives() { + struct Faulty { + inner: Headless, + fail: bool, + } + impl Backend for Faulty { + fn call(&mut self, method: &str, params: Value) -> Result { + if method == "engine.execute" && params["command"] == "catalog.stats" && std::mem::take(&mut self.fail) { + panic!("synthetic backend panic"); + } + self.inner.call(method, params) + } + fn has_ui(&self) -> bool { + false + } + fn describe(&self) -> String { + "test".into() + } + } + let mut s = Server::new(Box::new(Faulty { inner: Headless::demo(), fail: true })); + let params = json!({"name":"run_command","arguments":{"command":"catalog.stats"}}); + let r = rpc(&mut s, 1, "tools/call", params.clone()); + assert_eq!(r["result"]["isError"], true, "{r}"); + assert!(r.to_string().contains("synthetic backend panic")); + assert_eq!(rpc(&mut s, 2, "tools/call", params)["result"]["isError"], false); + assert!(rpc(&mut s, 3, "ping", json!({}))["result"].is_object()); +} + +#[test] +fn modern_requests_receive_result_and_cache_fields() { + let mut s = server(); + let meta = json!({"io.modelcontextprotocol/protocolVersion":"2026-07-28"}); + for (method, mut params) in [ + ("tools/list", json!({})), + ("resources/list", json!({})), + ("resources/templates/list", json!({})), + ("resources/read", json!({"uri":"lightcraft://library"})), + ] { + params["_meta"] = meta.clone(); + let r = rpc(&mut s, 1, method, params); + assert_eq!(r["result"]["resultType"], "complete", "{r}"); + assert_eq!(r["result"]["cacheScope"], "private"); + assert_eq!(r["result"]["ttlMs"], if method == "resources/read" { 0 } else { 600_000 }); + } + assert!(rpc(&mut s, 2, "tools/list", json!({}))["result"].get("resultType").is_none()); +} diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 1b41d86a..929a0d29 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -66,7 +66,7 @@ fn tool(name: &str, title: &str, description: &str, properties: Value, required: "name": name, "title": title, "description": description, - "inputSchema": {"type": "object", "properties": properties, "required": required}, + "inputSchema": {"type": "object", "properties": properties, "required": required, "additionalProperties": false}, }) } @@ -78,6 +78,51 @@ fn ids_schema(what: &str) -> Value { pub fn helper_tools(has_ui: bool) -> Vec { let photo_id = json!({"type": "integer", "description": "Photo id (default: the active photo). Makes that photo active first."}); let mut v = vec![ + tool( + "command_list", + "List commands", + "Every command (engine + UI when connected) with id, label, menu, shortcut, parameter doc and whether it is enabled now. Each id is also callable as tool `cmd_` or via command_run.", + json!({ + "filter": {"type": "string", "description": "Only commands whose id or label contains this text (case-insensitive)"}, + "enabled_only": {"type": "boolean", "description": "Only commands that are enabled now"} + }), + &[], + ), + tool( + "command_run", + "Run command", + "Run any LightCraft command by id with JSON params (see command_list for ids and parameter docs), e.g. {id: \"photo.rate\", params: {rating: 4}}.", + json!({"id": {"type": "string", "description": "Command id"}, "params": {"type": "object", "description": "Command parameters", "additionalProperties": true}}), + &["id"], + ), + tool( + "command_batch", + "Run commands", + "Run several commands in order: {steps: [{id, params?}], stop_on_error?: true}. Returns {completed, failed, results: [{ok, result | error}]}.", + json!({ + "steps": {"type": "array", "items": {"type": "object", "properties": {"id": {"type": "string"}, "params": {"type": "object", "additionalProperties": true}}, "required": ["id"], "additionalProperties": false}}, + "stop_on_error": {"type": "boolean", "description": "Stop at the first failing step (default true)"} + }), + &["steps"], + ), + tool( + "doc_inspect", + "Inspect library", + "The library as JSON: current source, filter, sort, selection, active photo and undo/redo labels (library.state) plus counts (catalog.stats).", + json!({}), + &[], + ), + tool( + "render_preview", + "Render preview", + "Render a photo with its current develop settings (cropped) and return it as an image (`max_side` is the long edge; does not change selection or save a file).", + json!({ + "id": {"type": "integer", "description": "Photo id (default: the active photo); selection is unchanged"}, + "max_side": {"type": "integer", "description": "Long edge in pixels (default 1024, max 4096)"}, + "format": {"type": "string", "enum": ["png", "jpeg"], "description": "Image encoding (default png; jpeg is smaller)"}, + }), + &[], + ), tool( "list_commands", "List commands", @@ -187,6 +232,13 @@ pub fn helper_tools(has_ui: bool) -> Vec { "ids": {"type": "array", "items": {"type": "integer"}, "description": "Photos to export (default: the given/active photo)"}, "path": {"type": "string"}, "dir": {"type": "string"}, + "preset": {"type": "string", "description": "Export preset id or name"}, + "background": {"type": "boolean", "description": "Desktop background export (headless exports remain blocking)"}, + "resize": {"type": "object", "description": "Full Resize options; alternatively use the size helpers below"}, + "subfolder": {"type": "string"}, + "conflict": {"type": "string", "enum": ["unique", "overwrite", "skip"]}, + "tiffCompression": {"type": "string", "enum": ["none", "lzw", "zip"]}, + "dngCompression": {"type": "string", "enum": ["lossless", "deflate", "uncompressed"]}, "format": {"type": "string", "enum": ["jpeg", "png", "tiff", "webp", "avif", "original", "dng"], "description": "original = the file as is + an XMP sidecar with the edits; dng = raw photos as DNG with the edits embedded"}, "longEdge": {"type": "integer", "description": "Output long edge in px. When no size param is given: 3000; 0 = full size (cropped, native resolution)"}, "shortEdge": {"type": "integer", "description": "Output short edge in px"}, @@ -213,6 +265,9 @@ pub fn helper_tools(has_ui: bool) -> Vec { ]; if has_ui { v.extend([ + tool("ui_inspect", "Inspect UI", "UI state and selection (same as inspect_ui).", json!({}), &[]), + tool("ui_screenshot", "Screenshot", "Return the window as an image without saving a file.", + json!({"maxSize": {"type": "integer"}, "format": {"type": "string", "enum": ["png", "jpeg"]}}), &[]), tool( "screenshot", "Screenshot", @@ -298,12 +353,60 @@ pub fn command_tools(backend: &mut dyn Backend) -> Vec { .collect() } +/// Conservative hints: generated command tools may edit; optional output paths make a helper a writer. +fn annotations(name: &str, title: &str) -> Value { + let read_only = matches!( + name, + "command_list" + | "list_commands" + | "doc_inspect" + | "render_preview" + | "query_photos" + | "list_controls" + | "get_develop" + | "inspect_ui" + | "list_widgets" + | "ui_inspect" + | "ui_screenshot" + ); + let files = matches!(name, "export" | "render_photo" | "screenshot"); + json!({"title": title, "readOnlyHint": read_only, "destructiveHint": !read_only && !files, + "idempotentHint": read_only || matches!(name, "render_photo" | "screenshot"), "openWorldHint": false}) +} + +/// Helpers have fixed keys; generated command tools retain the registry's free-form params. +pub(crate) fn check_args(name: &str, args: &Value) -> Result<(), String> { + if args.is_null() { + return Ok(()); + } + let Some(m) = args.as_object() else { + return Err("tool arguments must be an object".into()); + }; + let defs = helper_tools(true); + let Some(def) = defs.iter().find(|t| t["name"] == name) else { + return Ok(()); + }; + let Some(props) = def["inputSchema"]["properties"].as_object() else { + return Ok(()); + }; + if let Some(bad) = m.keys().find(|k| !props.contains_key(*k)) { + let accepted: Vec<&str> = props.keys().map(String::as_str).collect(); + return Err(format!("unknown argument \"{bad}\" for {name}; expected: {}", accepted.join(", "))); + } + Ok(()) +} + /// All tools: helpers, plus the generated command tools when `with_commands`. pub fn tool_definitions(backend: &mut dyn Backend, with_commands: bool) -> Vec { let mut v = helper_tools(backend.has_ui()); if with_commands { v.extend(command_tools(backend)); } + for t in &mut v { + let name = t["name"].as_str().unwrap_or_default().to_string(); + let title = t["title"].as_str().unwrap_or(&name).to_string(); + t["annotations"] = annotations(&name, &title); + } v } @@ -445,22 +548,67 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult { return ToolResult::from(exec(b, &rest.replace('_', "."), args.clone())); } match name { - "list_commands" => { + "list_commands" | "command_list" => { let filter = s("filter").unwrap_or("").to_lowercase(); + let enabled_only = args.get("enabled_only").and_then(Value::as_bool).unwrap_or(false); ToolResult::from(b.call("engine.commands", json!({})).map(|v| { match v { - Value::Array(a) if !filter.is_empty() => Value::Array( + Value::Array(a) => Value::Array( a.into_iter() .filter(|c| { - c["id"].as_str().unwrap_or("").to_lowercase().contains(&filter) + filter.is_empty() + || c["id"].as_str().unwrap_or("").to_lowercase().contains(&filter) || c["label"].as_str().unwrap_or("").to_lowercase().contains(&filter) }) + .filter(|c| !enabled_only || c["enabled"].as_bool() != Some(false)) .collect(), ), v => v, } })) } + "command_run" => match s("id") { + Some(c) => ToolResult::from(exec(b, c, args.get("params").cloned().filter(|p| !p.is_null()).unwrap_or(json!({})))), + None => ToolResult::error("missing `id`"), + }, + "command_batch" => { + let Some(steps) = args.get("steps").and_then(Value::as_array) else { return ToolResult::error("missing `steps`") }; + let stop = args.get("stop_on_error").and_then(Value::as_bool).unwrap_or(true); + let (mut completed, mut failed, mut results) = (0, 0, Vec::new()); + for st in steps { + let Some(id) = st["id"].as_str() else { + failed += 1; + results.push(json!({"ok": false, "error": "step without `id`"})); + if stop { + break; + } + continue; + }; + match exec(b, id, st.get("params").cloned().filter(|p| !p.is_null()).unwrap_or(json!({}))) { + Ok(r) => { + completed += 1; + results.push(json!({"ok": true, "result": r})); + } + Err(e) => { + failed += 1; + results.push(json!({"ok": false, "error": e})); + if stop { + break; + } + } + } + } + let r = json!({"completed": completed, "failed": failed, "results": results}); + if failed > 0 { ToolResult { is_error: true, ..ToolResult::json(r) } } else { ToolResult::json(r) } + } + "doc_inspect" => ToolResult::from(doc_inspect(b)), + "render_preview" => { + let mut a = obj(args, &["id", "format"]); + if let Some(m) = args.get("max_side") { + a["size"] = m.clone(); + } + render_photo(b, &a) + } "run_command" => match s("command") { Some(c) => ToolResult::from(exec(b, c, args.get("params").cloned().filter(|p| !p.is_null()).unwrap_or(json!({})))), None => ToolResult::error("missing `command`"), @@ -585,8 +733,8 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult { ), ) })), - "screenshot" => screenshot(b, args), - "inspect_ui" => ToolResult::from(b.call("ui.inspect", json!({}))), + "screenshot" | "ui_screenshot" => screenshot(b, args), + "inspect_ui" | "ui_inspect" => ToolResult::from(b.call("ui.inspect", json!({}))), "set_ui" => ToolResult::from(b.call("ui.set", args.get("state").cloned().unwrap_or(json!({})))), "list_widgets" => ToolResult::from(b.call("ui.widgets", obj(args, &["filter"]))), "click" => { @@ -606,3 +754,10 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult { other => ToolResult::error(format!("unknown tool `{other}` (see tools/list)")), } } + +/// The canonical document resource and tool share the same snapshot. +pub(crate) fn doc_inspect(b: &mut dyn Backend) -> Result { + let library = exec(b, "library.state", json!({}))?; + let stats = exec(b, "catalog.stats", json!({}))?; + Ok(json!({"library": library, "stats": stats})) +} diff --git a/docs/mcp.md b/docs/mcp.md index 7e815406..24f53c65 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -71,6 +71,39 @@ Every stdio MCP client takes the same shape: a `command` plus `args`. For exampl During development you can also point the client at `cargo run --release -p lightcraft-cli -- mcp` (with `"cwd"` set to the repository), at the cost of a slower start. +## Shared core tools + +These follow the same conventions as [filmcraft #28](https://github.com/storytold/filmcraft/pull/28). +They are listed in both full and compact mode; existing documented helpers remain listed too. +There are no hidden compatibility aliases. + +| Tool | Arguments | Result | +|---|---|---| +| `command_list` | `filter?`, `enabled_only?` | Command catalog | +| `command_run` | `id`, `params?` | Command result | +| `command_batch` | `steps: [{id, params?}]`, `stop_on_error?` | Counts and per-step results; stops on error by default | +| `doc_inspect` | none | Library state and catalog counts | +| `render_preview` | `id?`, `max_side?`, `format?` | Image; neither changes selection nor saves a file | + +Each edit in a batch has its own undo step. Connect mode also lists `ui_inspect` and +`ui_screenshot` (the latter has no output-path argument). The existing `inspect_ui`, +`screenshot`, `list_commands`, `run_command` and `render_photo` keep their documented arguments. +Ports and connect mode are unchanged. + +Every listed tool has a title and all four MCP hints. Generated command tools are conservatively +marked as edits. Helpers with optional output paths are marked as writers; `export` is not +idempotent because the default conflict policy chooses a new filename on repeated calls. +Unknown helper argument keys return JSON-RPC `-32602` naming the key and accepted arguments. +Generated `cmd_*` tools and nested command params remain free-form: the registry has prose +parameter docs, not machine-readable schemas. Existing command validation (including strict +export options) is preserved. Escaped tool panics return `isError: true`, and the session keeps +serving; backend panics during resource reads or tool listing return an internal error. +The headless backend owns its session directly, without a session mutex. + +`lightcraft://document` and `lightcraft://commands` contain JSON matching `doc_inspect` and +`command_list`. Existing resources remain available. Requests declaring MCP 2026-07-28 in +per-request `_meta` receive `resultType: complete` and list/read cache hints; reads are not cached. + ## Tools ### Helpers From dedc0496925f412d88690fea09a49afd52d228f9 Mon Sep 17 00:00:00 2001 From: storybored379 Date: Thu, 8 Oct 2026 23:02:21 -0700 Subject: [PATCH 2/3] MCP: add export progress and cancellation between photos Answer ping at photo boundaries, preserve queued requests and EOF completion, and suppress cancelled responses. Keep completed exports and the existing atomic file writer; cover cancellation, output preservation and transport poison recovery. Co-Authored-By: OpenAI Codex --- apps/lightcraft-cli/src/main.rs | 2 +- crates/mcp/src/backend.rs | 5 + crates/mcp/src/headless.rs | 37 +++++-- crates/mcp/src/lib.rs | 2 +- crates/mcp/src/progress.rs | 165 ++++++++++++++++++++++++++++++++ crates/mcp/src/server.rs | 47 ++++++--- crates/mcp/tests/e2e.rs | 112 ++++++++++++++++++++++ docs/mcp.md | 17 ++++ 8 files changed, 365 insertions(+), 22 deletions(-) create mode 100644 crates/mcp/src/progress.rs diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs index f8ddf550..7c9d255e 100644 --- a/apps/lightcraft-cli/src/main.rs +++ b/apps/lightcraft-cli/src/main.rs @@ -342,7 +342,7 @@ fn mcp(args: &[String]) -> Result<(), String> { eprintln!("lightcraft-cli mcp: serving MCP on stdio ({})", backend.describe()); let mut server = Server::new(backend).with_command_tools(!compact); let stdin = std::io::stdin(); - server.serve(BufReader::new(stdin.lock()), std::io::stdout().lock()).map_err(|e| e.to_string()) + server.serve(BufReader::new(stdin), std::io::stdout()).map_err(|e| e.to_string()) } fn merge(args: &[String]) -> Result<(), String> { diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index 5db2aaaa..4772363b 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -15,8 +15,13 @@ pub trait Backend { fn has_ui(&self) -> bool; /// Short human description ("headless", "remote 127.0.0.1:7980"). fn describe(&self) -> String; + /// Progress/cancellation for the next headless batch export. Other backends ignore it. + fn set_progress(&mut self, _hook: Option) {} } +/// `(completed photos, total photos, next filename)`; false stops before the next photo. +pub type ProgressHook = Box bool + Send>; + /// A running LightCraft app, reached through its loopback control port. pub struct Remote { addr: String, diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index 837302d6..0817b39d 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -8,7 +8,7 @@ use lightcraft_engine::catalog::PhotoId; use lightcraft_raster::Rgba8; use serde_json::{Value, json}; -use crate::backend::Backend; +use crate::backend::{Backend, ProgressHook}; /// File extensions recognised as photos when expanding folders. pub const PHOTO_EXTENSIONS: &[&str] = &[ @@ -20,6 +20,7 @@ pub const PHOTO_EXTENSIONS: &[&str] = &[ /// Headless backend: a [`Session`] with filesystem hooks. pub struct Headless { pub session: Session, + progress: Option, } impl Drop for Headless { @@ -37,7 +38,7 @@ impl Default for Headless { impl Headless { pub fn new(session: Session) -> Self { - Self { session } + Self { session, progress: None } } /// A headless session with the procedurally generated demo library. @@ -60,7 +61,7 @@ impl Headless { /// (see `lightcraft_engine::export::ExportOptions::from_json`, plus `ids`, `dir`, `path`). /// With `path` and no `format`, the format follows the path's extension. fn export(&mut self, p: &Value) -> Result { - use lightcraft_engine::export::{Destination, ExportFormat, ExportOptions, Resize, export_batch}; + use lightcraft_engine::export::{Destination, ExportFormat, ExportOptions, Resize, prepare_batch, run_batch}; let p = &self.session.export_params(p)?; let mut opts = ExportOptions::from_params(p).map_err(|e| e.to_string())?; if !ExportOptions::has_size_param(p) { @@ -82,10 +83,28 @@ impl Headless { }; let dir = p.get("dir").and_then(Value::as_str).unwrap_or(""); let write = &mut lightcraft_engine::export::write_file; - let files = - export_batch(&mut self.session, &ids, &opts, &Destination { dir: dir.to_string(), exact: exact.map(str::to_string) }, write, &|path| { - Path::new(path).exists() - })?; + let items = prepare_batch(&mut self.session, &ids, &opts)?; + let total = items.len(); + let mut hook = self.progress.take(); + let mut cancelled = false; + let files = run_batch( + items, + &opts, + &Destination { dir: dir.to_string(), exact: exact.map(str::to_string) }, + write, + &|path| Path::new(path).exists(), + true, + &mut |done, name| { + cancelled = hook.as_mut().is_some_and(|h| !h(done, total, name)); + !cancelled + }, + )?; + if !cancelled && let Some(hook) = hook.as_mut() { + cancelled = !hook(total, total, ""); + } + if cancelled { + return Err("cancelled".into()); + } // Single-photo exports also report path/width/height at the top level (back-compat). let mut out = files.first().cloned().unwrap_or_else(|| json!({})); out["files"] = json!(files); @@ -94,6 +113,10 @@ impl Headless { } impl Backend for Headless { + fn set_progress(&mut self, hook: Option) { + self.progress = hook; + } + fn call(&mut self, method: &str, params: Value) -> Result { let p = if params.is_null() { json!({}) } else { params }; match method { diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index a1fb5510..b2417fe0 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -21,7 +21,7 @@ mod headless; mod server; mod tools; -pub use backend::{Backend, Remote}; +pub use backend::{Backend, ProgressHook, Remote}; pub use base64::{base64_decode, base64_encode}; pub use headless::{Headless, PHOTO_EXTENSIONS, encode_image, expand_paths, write_image}; pub use server::{PROTOCOL_VERSION, Server}; diff --git a/crates/mcp/src/progress.rs b/crates/mcp/src/progress.rs new file mode 100644 index 00000000..2043dfd9 --- /dev/null +++ b/crates/mcp/src/progress.rs @@ -0,0 +1,165 @@ +//! Stdio progress and cancellation between photos. The backend keeps its session on the +//! serving thread; only stdin and stdout move to transport threads. Other calls wait in order. + +use std::collections::VecDeque; +use std::io::{BufRead, Write}; +use std::sync::atomic::Ordering; +use std::sync::mpsc::{Receiver, Sender, TryRecvError}; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; + +use serde_json::{Value, json}; + +use super::{Server, response}; +use crate::backend::ProgressHook; + +type Input = Option>; + +#[derive(Clone)] +pub(super) struct Wire { + inbox: Arc>>, + out: Sender, + deferred: Arc>>, +} + +fn lock(m: &Mutex) -> std::sync::MutexGuard<'_, T> { + m.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +} + +impl Server { + pub(super) fn serve_wire(&mut self, input: impl BufRead + Send, output: impl Write + Send) -> std::io::Result<()> { + std::thread::scope(|scope| { + let (in_tx, in_rx) = std::sync::mpsc::channel::(); + let (out_tx, out_rx) = std::sync::mpsc::channel::(); + let wire = Wire { inbox: Arc::new(Mutex::new(in_rx)), out: out_tx, deferred: Arc::new(Mutex::new(VecDeque::new())) }; + let writer = std::thread::Builder::new().name("mcp-output".into()).spawn_scoped(scope, move || -> std::io::Result<()> { + let mut output = output; + for line in out_rx { + output.write_all(line.as_bytes())?; + output.write_all(b"\n")?; + output.flush()?; + } + Ok(()) + })?; + let reader = match std::thread::Builder::new().name("mcp-input".into()).spawn_scoped(scope, move || { + for line in input.lines() { + let failed = line.is_err(); + if in_tx.send(Some(line)).is_err() || failed { + return; + } + } + let _ = in_tx.send(None); + }) { + Ok(reader) => reader, + Err(error) => { + drop(wire); + let _ = writer.join(); + return Err(error); + } + }; + self.wire = Some(wire.clone()); + let served = (|| -> std::io::Result<()> { + loop { + let queued = lock(&wire.deferred).pop_front(); + let line = match queued { + Some(line) => line, + None => lock(&wire.inbox).recv().ok().flatten(), + }; + let Some(line) = line else { return Ok(()) }; + if let Some(reply) = self.handle_line(&line?) { + wire.out.send(reply).map_err(|_| std::io::Error::new(std::io::ErrorKind::BrokenPipe, "MCP output closed"))?; + } + } + })(); + self.wire = None; + drop(wire); + let written = writer.join().map_err(|_| std::io::Error::other("MCP output thread panicked")).and_then(|r| r); + let read = reader.join().map_err(|_| std::io::Error::other("MCP input thread panicked")); + served.and(written).and(read) + }) + } + + /// Called at photo boundaries, including completion. EOF finishes the pending export; + /// cancellation stops before the next photo. No file scan or output deletion is needed: + /// the existing writer replaces each whole file atomically. + pub(super) fn progress_hook(&self) -> Option { + let wire = self.wire.clone()?; + let (id, token) = self.current.clone()?; + let suppress = self.suppress.clone(); + let mut closed = false; + let mut last = None; + let mut reported: Option = None; + Some(Box::new(move |done, total, name| { + while !closed { + let next = lock(&wire.inbox).try_recv(); + let line = match next { + Ok(Some(Ok(line))) => line, + Ok(Some(Err(error))) => { + lock(&wire.deferred).push_back(Some(Err(error))); + closed = true; + break; + } + Ok(None) | Err(TryRecvError::Disconnected) => { + lock(&wire.deferred).push_back(None); + closed = true; + break; + } + Err(TryRecvError::Empty) => break, + }; + let msg: Value = serde_json::from_str(&line).unwrap_or(Value::Null); + match msg.get("method").and_then(Value::as_str) { + Some("notifications/cancelled") if msg.get("id").is_none() && msg["params"]["requestId"] == id => { + suppress.store(true, Ordering::Relaxed); + } + Some("ping") + if msg.get("id").is_some_and(|i| i.is_string() || i.is_number()) + && msg.get("params").is_none_or(|p| p.is_null() || p.is_object()) => + { + let _ = wire.out.send(response(msg["id"].clone(), json!({})).to_string()); + } + _ => lock(&wire.deferred).push_back(Some(Ok(line))), + } + } + if suppress.load(Ordering::Relaxed) { + return false; + } + if let Some(token) = &token + && total > 0 + && last.is_none_or(|last| done > last) + && (done == total || reported.is_none_or(|at| at.elapsed() >= Duration::from_millis(100))) + { + last = Some(done); + reported = Some(Instant::now()); + let message = if done < total { + format!("Exporting {name} ({} of {total})", done.saturating_add(1)) + } else { + format!("Exported {total} photo(s)") + }; + let notification = json!({"jsonrpc":"2.0","method":"notifications/progress", + "params":{"progressToken":token,"progress":done,"total":total,"message":message}}); + let _ = wire.out.send(notification.to_string()); + } + true + })) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn transport_lock_recovers_after_panic() { + let state = Arc::new(Mutex::new(VecDeque::new())); + let held = state.clone(); + let _ = std::thread::spawn(move || { + held.lock().unwrap().push_back(7); + let _guard = held.lock().unwrap(); + panic!("synthetic transport panic"); + }) + .join(); + assert_eq!(lock(&state).pop_front(), Some(7)); + lock(&state).push_back(8); + assert_eq!(lock(&state).pop_front(), Some(8)); + } +} diff --git a/crates/mcp/src/server.rs b/crates/mcp/src/server.rs index 3f1f12f0..ba64ef45 100644 --- a/crates/mcp/src/server.rs +++ b/crates/mcp/src/server.rs @@ -1,6 +1,11 @@ //! JSON-RPC 2.0 framing and the MCP lifecycle / tools / resources methods. use std::io::{BufRead, Write}; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; + +#[path = "progress.rs"] +mod progress; use serde_json::{Value, json}; @@ -40,6 +45,9 @@ pub struct Server { backend: Box, initialized: bool, command_tools: bool, + wire: Option, + current: Option<(Value, Option)>, + suppress: Arc, } fn response(id: Value, result: Value) -> Value { @@ -56,7 +64,7 @@ fn resource(uri: &str, name: &str, title: &str, description: &str) -> Value { impl Server { pub fn new(backend: Box) -> Self { - Self { backend, initialized: false, command_tools: true } + Self { backend, initialized: false, command_tools: true, wire: None, current: None, suppress: Arc::new(AtomicBool::new(false)) } } /// Whether `tools/list` includes one `cmd_*` tool per registered command (default true). @@ -77,16 +85,8 @@ impl Server { /// Serve newline-delimited JSON-RPC until `input` closes. Logs go to stderr only (stdout is /// the protocol stream). - pub fn serve(&mut self, input: impl BufRead, mut output: impl Write) -> std::io::Result<()> { - for line in input.lines() { - let line = line?; - if let Some(reply) = self.handle_line(&line) { - output.write_all(reply.as_bytes())?; - output.write_all(b"\n")?; - output.flush()?; - } - } - Ok(()) + pub fn serve(&mut self, input: impl BufRead + Send, output: impl Write + Send) -> std::io::Result<()> { + self.serve_wire(input, output) } /// Handle one line; returns the reply line (None for notifications and blank lines). @@ -133,7 +133,15 @@ impl Server { if !(params.is_object() || params.is_null()) { return Some(error(id, INVALID_PARAMS, "`params` must be an object")); } - Some(match self.request(method, ¶ms) { + let token = params.get("_meta").and_then(|m| m.get("progressToken")).filter(|t| t.is_string() || t.is_number()).cloned(); + self.current = Some((id.clone(), token)); + self.suppress.store(false, Ordering::Relaxed); + let result = self.request(method, ¶ms); + self.current = None; + if self.suppress.swap(false, Ordering::Relaxed) { + return None; + } + Some(match result { Ok(mut r) => { modern_result_fields(method, ¶ms, &mut r); response(id, r) @@ -172,7 +180,20 @@ impl Server { let name = params.get("name").and_then(Value::as_str).ok_or((INVALID_PARAMS, "missing tool `name`".to_string()))?; let args = params.get("arguments").cloned().unwrap_or(Value::Null); check_args(name, &args).map_err(|e| (INVALID_PARAMS, e))?; - Ok(self.guarded(|b| call_tool(b, name, &args)).unwrap_or_else(ToolResult::error).to_value()) + let long = matches!(name, "export" | "cmd_app_export") + || (name == "command_run" && args["id"] == "app.export") + || (name == "run_command" && args["command"] == "app.export"); + let hook = if long && !self.backend.has_ui() { self.progress_hook() } else { None }; + let result = self.guarded(|b| { + b.set_progress(hook); + call_tool(b, name, &args) + }); + let clear = self.guarded(|b| b.set_progress(None)); + let result = match (result, clear) { + (Ok(result), Ok(())) => result, + (Err(error), _) | (_, Err(error)) => ToolResult::error(error), + }; + Ok(result.to_value()) } "resources/list" => Ok(json!({"resources": [ resource(DOCUMENT_URI, "document", "Library document", "Library state and counts (same as doc_inspect)"), diff --git a/crates/mcp/tests/e2e.rs b/crates/mcp/tests/e2e.rs index ea27919c..ef2e49b6 100644 --- a/crates/mcp/tests/e2e.rs +++ b/crates/mcp/tests/e2e.rs @@ -153,3 +153,115 @@ fn remote_unreachable_is_a_tool_error() { assert_eq!(r[0]["result"]["isError"], true); assert!(r[0]["result"]["content"][0]["text"].as_str().unwrap().contains("not reachable")); } + +/// Real stdio framing and real atomic exports: progress, ping during a batch, cancellation +/// between complete photos, unrelated output preservation, and a usable session afterwards. +#[test] +fn export_progress_and_cancel() { + let dir = std::env::temp_dir().join(format!("lc-mcp-progress-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let (input, mut send) = std::io::pipe().unwrap(); + let (output, write) = std::io::pipe().unwrap(); + let server = std::thread::spawn(move || Server::new(Box::new(Headless::demo())).serve(BufReader::new(input), write).unwrap()); + let (tx, rx) = std::sync::mpsc::channel(); + let reader = std::thread::spawn(move || { + for line in BufReader::new(output).lines() { + let value: Value = serde_json::from_str(&line.unwrap()).unwrap(); + if tx.send(value).is_err() { + break; + } + } + }); + let next = || rx.recv_timeout(std::time::Duration::from_secs(30)).expect("MCP reply/progress"); + writeln!(send, "{}", call(1, "query_photos", json!({"limit":8}))).unwrap(); + let query = next(); + let ids: Vec<_> = query["result"]["structuredContent"]["photos"].as_array().unwrap().iter().map(|p| p["id"].clone()).collect(); + assert_eq!(ids.len(), 8); + let mut request = call(2, "export", json!({"ids":ids,"dir":dir.join("complete"),"longEdge":1200,"format":"png"})); + request["params"]["_meta"] = json!({"progressToken":"photos","io.modelcontextprotocol/protocolVersion":"2026-07-28"}); + writeln!(send, "{request}").unwrap(); + let (mut last, mut notes, mut asked, mut answered) = (-1.0, 0, false, false); + loop { + let value = next(); + if value["method"] == "notifications/progress" { + assert_eq!(value["params"]["progressToken"], "photos"); + let progress = value["params"]["progress"].as_f64().unwrap(); + assert!(progress > last && progress <= 8.0, "{value}"); + assert_eq!(value["params"]["total"], 8); + last = progress; + notes += 1; + if !asked { + writeln!(send, "{}", json!({"jsonrpc":"2.0","id":3,"method":"ping"})).unwrap(); + asked = true; + } + } else if value["id"] == 3 { + answered = true; + } else { + assert_eq!(value["id"], 2, "{value}"); + assert_eq!(value["result"]["isError"], false, "{value}"); + assert_eq!(value["result"]["resultType"], "complete"); + break; + } + } + assert!(notes >= 2 && answered, "{notes} progress messages; ping answered {answered}"); + assert_eq!(last, 8.0); + let out = dir.join("cancelled"); + std::fs::create_dir_all(&out).unwrap(); + let decoy = out.join("photo999.png"); + std::fs::write(&decoy, b"unrelated existing output").unwrap(); + let mut request = call(4, "command_run", json!({"id":"app.export","params":{"ids":ids,"dir":out,"longEdge":1200,"format":"png"}})); + request["params"]["_meta"] = json!({"progressToken":44}); + writeln!(send, "{request}").unwrap(); + loop { + let value = next(); + assert_ne!(value["id"], 4, "must be cancellable before completion: {value}"); + assert_eq!(value["params"]["progressToken"], 44, "{value}"); + if value["params"]["progress"].as_f64().unwrap() > 0.0 { + break; + } + } + writeln!(send, "{}", json!({"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":4}})).unwrap(); + writeln!(send, "{}", call(5, "doc_inspect", json!({}))).unwrap(); + loop { + let value = next(); + if value["method"] == "notifications/progress" { + continue; + } + assert_eq!(value["id"], 5, "cancelled request must not reply: {value}"); + assert_eq!(value["result"]["isError"], false); + break; + } + assert_eq!(std::fs::read(&decoy).unwrap(), b"unrelated existing output"); + let photos: Vec<_> = std::fs::read_dir(&out).unwrap().map(|e| e.unwrap().path()).filter(|p| *p != decoy).collect(); + assert!(!photos.is_empty() && photos.len() < ids.len(), "only completed photos remain: {photos:?}"); + for path in &photos { + lightcraft_codecs::decode(&std::fs::read(path).unwrap(), Default::default()).expect("complete image, no partial/temp file"); + } + assert_eq!(std::fs::read_dir(dir.join("complete")).unwrap().count(), 8, "earlier export retained"); + drop(send); + server.join().unwrap(); + reader.join().unwrap(); + std::fs::remove_dir_all(dir).unwrap(); +} + +#[test] +fn export_without_token_finishes_after_eof() { + let dir = std::env::temp_dir().join(format!("lc-mcp-eof-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let mut headless = Headless::demo(); + let ids: Vec<_> = headless.session.visible_cloned().iter().take(2).map(|id| id.0).collect(); + let request = call(1, "cmd_app_export", json!({"ids":ids,"dir":dir,"longEdge":64,"format":"png"})); + // Headless sessions remain movable to a worker after installing progress support. + let replies = std::thread::spawn(move || { + let mut s = Server::new(Box::new(headless)); + session(&mut s, &[request, call(2, "doc_inspect", json!({}))]) + }) + .join() + .unwrap(); + assert_eq!(replies.len(), 2, "no token means no notifications"); + assert_eq!(replies[0]["result"]["isError"], false, "{replies:?}"); + assert_eq!(replies[1]["id"], 2, "queued request survives EOF"); + assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 2); + std::fs::remove_dir_all(dir).unwrap(); +} diff --git a/docs/mcp.md b/docs/mcp.md index 24f53c65..3134468e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -219,3 +219,20 @@ lightcraft-cli calibrate --max 300 ~/Pictures/2026 # camera colour profiles (d (`Remote`) against a stand-in control server; also import → render → JPEG export of a real file. - `apps/lightcraft-cli/tests/cli.rs` — spawns `lightcraft-cli mcp` with real pipes; `render`; `commands`. + +## Export progress and cancellation + +Headless direct `export`, `command_run` / `run_command` with `app.export`, and `cmd_app_export` +calls report photo-count progress when `params._meta.progressToken` is a string or number. +Notifications are strictly increasing, at most ten per second plus the final total. No token +means no notifications. `ping` is answered at photo boundaries; other requests wait in order +until the export ends. EOF lets a pending export finish and preserves queued requests. + +`notifications/cancelled` with `params.requestId` stops the matching export before its next +photo, suppressing its response. Unknown/completed request ids are ignored. A photo already +being processed finishes first: completed photos remain, each written with the existing atomic +file writer, and there are no partial files to delete. Unrelated outputs and earlier exports +are untouched. A failed export is an `isError` tool result. Transport mutexes recover poisoning. + +Connect mode and exports inside `command_batch` remain synchronous and do not report MCP +progress or cancellation. Use a direct headless export call for this behavior. From 7a42f99cf06b47fd4aec71c12480ac6e54586e3b Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Fri, 9 Oct 2026 09:05:04 -0400 Subject: [PATCH 3/3] MCP: the export tool forwards every option it advertises preset, background, resize, subfolder, conflict, tiffCompression and dngCompression were in the export tool's schema (and so passed the new argument check) but were dropped before reaching app.export. Co-Authored-By: Claude Opus 5.5 --- crates/mcp/src/tests.rs | 13 +++++++++++++ crates/mcp/src/tools.rs | 7 +++++++ 2 files changed, 20 insertions(+) diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index b58f195b..3393e504 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -139,6 +139,19 @@ fn export_refuses_unknown_params_and_bad_values() { assert!(e.contains("did you mean `quality`"), "{e}"); } +/// Every option the `export` tool advertises reaches `app.export` (they were accepted, then dropped). +#[test] +fn export_tool_forwards_its_advertised_options() { + let mut b = Headless::demo(); + let base = std::env::temp_dir().join(format!("lc-mcp-export-fwd-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let r = call_tool(&mut b, "export", &json!({"dir": base.to_string_lossy(), "subfolder": "sub", "longEdge": 64, "format": "png"})); + assert!(!r.is_error, "{r:?}"); + let files: Vec<_> = std::fs::read_dir(base.join("sub")).unwrap().collect(); + assert_eq!(files.len(), 1, "written into the subfolder"); + let _ = std::fs::remove_dir_all(&base); +} + /// The `import` helper moves: renamed into the folder template, the source removed. #[test] fn import_tool_moves_with_a_folder_template() { diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 929a0d29..4a114f18 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -729,6 +729,13 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult { "watermark", "colorSpace", "bitDepth", + "preset", + "background", + "resize", + "subfolder", + "conflict", + "tiffCompression", + "dngCompression", ], ), )