diff --git a/ROADMAP.md b/ROADMAP.md index e61995ad1b..3ceedcfd35 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -251,3 +251,5 @@ Newest first. One line per session: the date, what moved, and the new overall pe - Robustness sweep (963 of 983 files open, 0 crashes), text layer, find and select, tiles, web build, polish. - Overall ≈ 3–4%. - **2026-09-30 (session 1):** planning complete; viewer vertical slice. + +- M13: common command, batch, inspection and bounded preview tools share the existing confined automation path. MCP tools carry complete hints and reject unknown top-level keys as invalid parameters. diff --git a/apps/pdfcraft-cli/tests/mcp_compact.rs b/apps/pdfcraft-cli/tests/mcp_compact.rs index 8c12152a34..4869206804 100644 --- a/apps/pdfcraft-cli/tests/mcp_compact.rs +++ b/apps/pdfcraft-cli/tests/mcp_compact.rs @@ -41,7 +41,7 @@ fn listed_tools(flags: &[&str]) -> Vec { fn compact_flag_shortens_the_tool_list() { let full = listed_tools(&[]); let compact = listed_tools(&["--compact"]); - assert_eq!(compact.len(), 12, "{compact:?}"); + assert_eq!(compact.len(), 17, "{compact:?}"); assert!(compact.iter().any(|n| n == "tool_search") && compact.iter().any(|n| n == "tool_call")); assert!(compact.iter().all(|n| full.contains(n) || n == "tool_search" || n == "tool_call")); assert!(full.len() > 100 && !full.iter().any(|n| n == "tool_search"), "{} tools", full.len()); diff --git a/crates/automation/README.md b/crates/automation/README.md index 21dbc3f5ff..85bb2f7f09 100644 --- a/crates/automation/README.md +++ b/crates/automation/README.md @@ -32,7 +32,7 @@ mcp::McpServer::new(a).serve(stdin, stdout)? // newline-delimited JS **Opt-in only.** Nothing starts it automatically, and it opens no port. It runs while `pdfcraft-cli mcp [--root DIR]` runs, normally launched by an agent from its MCP configuration. It stops when stdin closes. The CLI's `mcp` Cargo feature (on by default) compiles it out entirely when disabled. -`pdfcraft-cli mcp --compact` (or `McpServer::with_compact(true)`) keeps `tools/list` short for agents with small context budgets. It lists only the core tools (`mcp::COMPACT_CORE_TOOLS`: open, info, save, close, render, text extract and find, combine, split, undo) plus two meta tools: `tool_search` (optional `query` and `category`, the name prefix; or `name` for one tool's full `input_schema`) and `tool_call` (`name` and `arguments`, run through the same `Automation::call`). `tools/call` by name keeps working for every tool, and the server instructions mention the meta tools. Without the flag nothing changes. +`pdfcraft-cli mcp --compact` (or `McpServer::with_compact(true)`) keeps `tools/list` short for agents with small context budgets. It lists only the core tools (`mcp::COMPACT_CORE_TOOLS`: open, info, save, close, render, text extract and find, combine, split, undo) plus the common command/inspection/preview tools and two meta tools: `tool_search` (optional `query` and `category`, the name prefix; or `name` for one tool's full `input_schema`) and `tool_call` (`name` and `arguments`, run through the same `Automation::call`). `tools/call` by name keeps working for every tool, and the server instructions mention the meta tools. Without the flag nothing changes. Implemented: `initialize` (protocol 2025-06-18, 2025-03-26, 2024-11-05), `ping`, `tools/list` (with `readOnlyHint`/`destructiveHint` annotations), `tools/call` (JSON results also returned as `structuredContent`; images as `image/png`), and `resources/list`, `resources/templates/list` and `resources/read`. The resources expose the open documents read-only: `pdfcraft://doc/{doc}/info` (JSON), `…/text`, `…/page/{page}/text` and `…/page/{page}/image{?dpi}` (PNG, 1–600 dpi). Tool failures come back as `isError: true` results, so the agent can read and recover from them. @@ -42,3 +42,27 @@ Implemented: `initialize` (protocol 2025-06-18, 2025-03-26, 2024-11-05), `ping`, 2. Add a `ToolDef` in `src/tools.rs`: a precise description, the schema, `ro()`/`destructive()`, and `cmd()` if a registry command exists. 3. Handle it in `Automation::call`, validating pages and positions with the existing helpers. 4. Add an end-to-end test in `tests/automation.rs`. `tool_table_is_well_formed` checks names, schemas and command links. + + +## Common command tools + +Following FilmCraft's MCP conventions, `command_list {doc?, filter?, enabled_only?}` keeps its +`{commands: [...]}` result and adds each mapped tool's parameter schema. `command_run {id, params?}` +runs that command through its existing headless tool: include `doc` and other tool arguments in +`params`. Commands without a headless mapping return a tool error. Unknown parameter keys are +ignored with a `warnings` array; required values and paths still go through the original validation. + +`command_batch {steps: [{id, params?}], stop_on_error?}` returns `completed`, `failed`, and per-step +`{ok, result?, error?}` entries. The default is to stop at the first error; earlier edits remain. +MCP marks a batch with any failed steps as `isError`, including through compact `tool_call`. +`doc_inspect {doc?}` returns one document's info, or `{documents: [...]}` for all open documents. +`render_preview {doc?, page?, max_side?}` returns a PNG (page defaults to 1, maximum side defaults +to 1024, allowed 1–4096). Omit `doc` only when exactly one document is open. + +All task tools remain available; compact mode includes the five common tools. Every MCP tool has +all four annotation hints. Unknown top-level argument keys return JSON-RPC `-32602` with the +accepted keys; failures in the invoked tool remain `isError` results. An escaped tool panic is +reported as an internal tool error, and the server continues serving. + +Exports complete synchronously. `progressToken` and cancellation notifications are harmlessly +ignored; no background-job or cancellation behavior is added. diff --git a/crates/automation/src/conventions.rs b/crates/automation/src/conventions.rs new file mode 100644 index 0000000000..e38227e88c --- /dev/null +++ b/crates/automation/src/conventions.rs @@ -0,0 +1,118 @@ +//! Shared command/inspection/preview entry points, using the existing confined tool path. +use serde_json::{Value, json}; + +use crate::{Args, Automation, Content, Result, ToolError, encode_png, failed, info, tools}; +use pdfcraft_render::{RenderRequest, RequestKind}; + +impl Automation { + pub(crate) fn command_run(&mut self, args: &Args) -> Result> { + let id = args.str("id")?; + let tool = tools::tool_for_command(id).ok_or_else(|| failed(format!("command {id:?} has no headless tool; see command_list")))?; + let def = tools::find(tool).ok_or_else(|| failed("command tool is unavailable"))?; + let mut params = match args.get("params") { + None | Some(Value::Null) => serde_json::Map::new(), + Some(Value::Object(params)) => params.clone(), + Some(_) => return Err(ToolError::InvalidArgs("params must be an object".into())), + }; + let props = def.input_schema.get("properties").and_then(Value::as_object).ok_or_else(|| failed("tool has no parameter schema"))?; + let accepted = props.keys().map(String::as_str).collect::>().join(", "); + let mut warnings = Vec::new(); + params.retain(|key, _| { + if props.contains_key(key) { + return true; + } + warnings.push(format!("unknown param {key:?} for {id} (accepted: {accepted})")); + false + }); + // This preserves the task tool's validation, atomic writes and --root confinement. + let mut content = self.call(tool, &Value::Object(params))?; + if !warnings.is_empty() { + if let Some(object) = content.iter_mut().find_map(|c| match c { + Content::Json(v) => v.as_object_mut(), + _ => None, + }) { + let mut all = match object.remove("warnings") { + Some(Value::Array(old)) => old, + Some(old) => vec![old], + None => Vec::new(), + }; + all.extend(warnings.into_iter().map(Value::String)); + object.insert("warnings".into(), Value::Array(all)); + } else { + content.push(Content::Json(json!({"warnings":warnings}))); + } + } + Ok(content) + } + + pub(crate) fn command_batch(&mut self, args: &Args) -> Result { + let steps = args.get("steps").and_then(Value::as_array).ok_or_else(|| ToolError::InvalidArgs("steps must be an array".into()))?; + // Check every envelope before applying any edit. + for step in steps { + let a = Args(step); + a.str("id")?; + if a.get("params").is_some_and(|p| !p.is_object() && !p.is_null()) { + return Err(ToolError::InvalidArgs("step params must be an object".into())); + } + } + let stop = args.opt_bool("stop_on_error")?.unwrap_or(true); + let (mut completed, mut failed, mut results) = (0usize, 0usize, Vec::new()); + for step in steps { + match self.command_run(&Args(step)) { + Ok(content) => { + completed += 1; + let result = content.into_iter().find_map(|c| match c { + Content::Json(v) => Some(v), + _ => None, + }); + results.push(json!({"ok":true,"result":result})); + } + Err(e) => { + failed += 1; + results.push(json!({"ok":false,"error":e.to_string()})); + if stop { + break; + } + } + } + } + Ok(json!({"completed":completed,"failed":failed,"results":results})) + } + + pub(crate) fn doc_inspect(&self, args: &Args) -> Result { + if args.opt_int("doc")?.is_some() { + return Ok(info(self.doc(args)?)); + } + Ok(json!({"documents":self.session.docs().iter().map(info).collect::>()})) + } + + pub(crate) fn render_preview(&mut self, args: &Args) -> Result { + let id = if args.opt_int("doc")?.is_some() { + self.doc(args)?.id + } else { + match self.session.docs() { + [doc] => doc.id, + _ => return Err(failed("give doc from doc_list (a single open document is selected automatically)")), + } + }; + let page = args.opt_int("page")?.unwrap_or(1); + let max = args.opt_int("max_side")?.unwrap_or(1024); + if !(1..=4096).contains(&max) { + return Err(ToolError::InvalidArgs("max_side must be between 1 and 4096".into())); + } + let page = usize::try_from(page).ok().and_then(|p| p.checked_sub(1)).ok_or_else(|| failed("page must be at least 1"))?; + let doc = self.session.get(id).ok_or_else(|| failed("no such document"))?; + let size = doc.info.pages.get(page).ok_or_else(|| failed("page is out of range"))?; + let side = size.width.max(size.height); + if !side.is_finite() || side <= 0.0 { + return Err(failed("page has invalid dimensions")); + } + let scale = (max as f32 / side).min(1.0); + let out = self.renderer(id)?.render(RenderRequest { page, kind: RequestKind::Pixels, tile: None, scale, tag: 0 }); + if let Some(error) = out.error { + return Err(failed(error)); + } + let data = encode_png(out.width, out.height, &out.rgba)?; + Ok(Content::Png { data, width: out.width, height: out.height }) + } +} diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 8563e94d8a..65235d24d3 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -14,6 +14,7 @@ mod a11y; mod comments; mod content; +mod conventions; mod forms; mod links; #[cfg(feature = "mcp")] @@ -215,6 +216,10 @@ impl Automation { json!({ "redone": label, "document": summary(self.doc(&a)?) }) } "command_list" => self.command_list(&a)?, + "command_run" => return self.command_run(&a), + "command_batch" => self.command_batch(&a)?, + "doc_inspect" => self.doc_inspect(&a)?, + "render_preview" => return self.render_preview(&a).map(|c| vec![c]), "page_number" => { use pdfcraft_organize::LabelStyle as L; let n = self.doc(&a)?.info.pages.len(); @@ -1433,6 +1438,8 @@ impl Automation { Some(_) => Some(self.doc(a)?.id), None => None, }; + let filter = a.opt_str("filter")?.unwrap_or("").to_lowercase(); + let enabled_only = a.opt_bool("enabled_only")?.unwrap_or(false); let list: Vec = commands::COMMANDS .iter() .map(|c| { @@ -1443,9 +1450,18 @@ impl Automation { "shortcut": c.shortcut.map(|s| s.label(cfg!(target_os = "macos"))), "enabled": commands::is_enabled(c, &self.session, active), "tool": tools::tool_for_command(c.id), + "params": tools::tool_for_command(c.id).and_then(tools::find).map(|t| &t.input_schema), }) }) .collect(); + let list: Vec = list + .into_iter() + .filter(|c| { + (!enabled_only || c.get("enabled").and_then(Value::as_bool) == Some(true)) + && (filter.is_empty() + || ["id", "label", "menu"].iter().any(|k| c.get(k).is_some_and(|v| v.to_string().to_lowercase().contains(&filter)))) + }) + .collect(); Ok(json!({ "commands": list })) } diff --git a/crates/automation/src/mcp.rs b/crates/automation/src/mcp.rs index 145190937b..375041eebe 100644 --- a/crates/automation/src/mcp.rs +++ b/crates/automation/src/mcp.rs @@ -29,15 +29,32 @@ const INVALID_PARAMS: i64 = -32602; const INSTRUCTIONS: &str = "PdfCraft edits PDFs. Open a file with doc_open to get a document id, then inspect \ (doc_info, text_extract, text_find, page_render) or edit it (page_*, doc_set_info). Edits are undoable \ (edit_undo) and stay in memory until doc_save. Page numbers are 1-based. Open documents are also \ -resources: pdfcraft://doc/{doc}/info, /text, /page/{page}/text and /page/{page}/image."; +resources: pdfcraft://doc/{doc}/info, /text, /page/{page}/text and /page/{page}/image. \ +command_list discovers command ids and tool schemas; command_run executes one, command_batch runs several. \ +doc_inspect returns open document state; render_preview returns a bounded PNG."; /// Appended to the instructions in compact mode. const COMPACT_INSTRUCTIONS: &str = " Only a core set of tools is listed. Every other tool is still available: find it with \ tool_search (optional query and category, or name for its full input schema) and run it with tool_call."; /// The tools `tools/list` returns in compact mode, besides the two meta tools. -pub const COMPACT_CORE_TOOLS: &[&str] = - &["doc_open", "doc_info", "doc_save", "doc_close", "page_render", "text_extract", "text_find", "doc_combine", "doc_split", "edit_undo"]; +pub const COMPACT_CORE_TOOLS: &[&str] = &[ + "doc_open", + "doc_info", + "doc_save", + "doc_close", + "page_render", + "text_extract", + "text_find", + "doc_combine", + "doc_split", + "edit_undo", + "command_list", + "command_run", + "command_batch", + "doc_inspect", + "render_preview", +]; /// Meta tools that exist only in compact mode. const TOOL_SEARCH: &str = "tool_search"; @@ -99,7 +116,8 @@ impl McpServer { }; let params = obj.get("params").cloned().unwrap_or(Value::Null); let id = id?; // notifications/initialized, notifications/cancelled, …: nothing to answer - Some(match self.dispatch(method, ¶ms) { + let result = if method == "tools/call" { guarded_tool(|| self.dispatch(method, ¶ms)) } else { self.dispatch(method, ¶ms) }; + Some(match result { Ok(result) => json!({ "jsonrpc": "2.0", "id": id, "result": result }), Err((code, message)) => error(id, code, &message), }) @@ -123,24 +141,35 @@ impl McpServer { let name = params.get("name").and_then(Value::as_str).ok_or((INVALID_PARAMS, "tools/call needs a tool name".to_string()))?; let args = params.get("arguments").cloned().unwrap_or(Value::Null); if self.compact && name == TOOL_SEARCH { + check_keys(name, &args, &["query", "category", "name"]).map_err(|e| (INVALID_PARAMS, e))?; return Ok(match search_tools(&args) { Ok(found) => call_result(vec![Content::Json(found)]), Err(message) => tool_error(&message), }); } if self.compact && name == TOOL_CALL { + check_keys(name, &args, &["name", "arguments"]).map_err(|e| (INVALID_PARAMS, e))?; let (inner, inner_args) = match unwrap_tool_call(&args) { Ok(pair) => pair, Err(message) => return Ok(tool_error(&message)), }; return Ok(match self.automation.call(&inner, &inner_args) { - Ok(content) => call_result(content), + Ok(content) => tool_content(&inner, content), Err(ToolError::UnknownTool(t)) => tool_error(&format!("unknown tool {t:?}; use tool_search to find the right name")), Err(e) => tool_error(&e.to_string()), }); } + if let Some(def) = crate::tools::find(name) { + let keys: Vec<&str> = def + .input_schema + .get("properties") + .and_then(Value::as_object) + .map(|p| p.keys().map(String::as_str).collect()) + .unwrap_or_default(); + check_keys(name, &args, &keys).map_err(|e| (INVALID_PARAMS, e))?; + } match self.automation.call(name, &args) { - Ok(content) => Ok(call_result(content)), + Ok(content) => Ok(tool_content(name, content)), Err(ToolError::UnknownTool(t)) => Err((INVALID_PARAMS, format!("unknown tool {t:?}"))), Err(e) => Ok(tool_error(&e.to_string())), } @@ -188,7 +217,7 @@ fn meta_tools() -> [Value; 2] { "required": [], "additionalProperties": false, }, - "annotations": { "title": "Find PdfCraft tools", "readOnlyHint": true, "destructiveHint": false, "openWorldHint": false }, + "annotations": { "title": "Find PdfCraft tools", "readOnlyHint": true, "destructiveHint": false, "idempotentHint": true, "openWorldHint": false }, }), json!({ "name": TOOL_CALL, @@ -204,7 +233,7 @@ fn meta_tools() -> [Value; 2] { "required": ["name"], "additionalProperties": false, }, - "annotations": { "title": "Run any PdfCraft tool", "readOnlyHint": false, "destructiveHint": true, "openWorldHint": false }, + "annotations": { "title": "Run any PdfCraft tool", "readOnlyHint": false, "destructiveHint": true, "idempotentHint": false, "openWorldHint": false }, }), ] } @@ -294,6 +323,24 @@ fn unwrap_tool_call(args: &Value) -> Result<(String, Value), String> { Ok((name.to_owned(), inner)) } +/// Last-resort boundary shared by every tools/call route, including compact dispatch. +fn guarded_tool(f: impl FnOnce() -> Result) -> Result { + pdfcraft_engine::guard(f).unwrap_or_else(|message| Ok(tool_error(&format!("internal error: {message}")))) +} + +fn tool_content(name: &str, content: Vec) -> Value { + let failed = name == "command_batch" + && content.iter().any(|c| match c { + Content::Json(v) => v.get("failed").and_then(Value::as_u64).is_some_and(|n| n > 0), + _ => false, + }); + let mut result = call_result(content); + if let Some(object) = result.as_object_mut() { + object.insert("isError".into(), json!(failed)); + } + result +} + /// A failed tool call, as the agent should read it. fn tool_error(message: &str) -> Value { json!({ "content": [{ "type": "text", "text": message }], "isError": true }) @@ -413,7 +460,7 @@ fn tool_json(t: &crate::ToolDef) -> Value { "title": t.title, "description": t.description, "inputSchema": t.input_schema, - "annotations": { "title": t.title, "readOnlyHint": t.read_only, "destructiveHint": t.destructive, "openWorldHint": false }, + "annotations": { "title": t.title, "readOnlyHint": t.read_only, "destructiveHint": t.destructive, "idempotentHint": t.read_only || matches!(t.name, "doc_save" | "doc_optimize" | "doc_export_data" | "measure_export" | "doc_export_images" | "doc_export_office" | "doc_export_all_images" | "doc_export_text" | "image_save"), "openWorldHint": false }, }) } @@ -445,6 +492,15 @@ fn call_result(content: Vec) -> Value { mod tests { use super::{Resource, first_sentence, parse_uri}; + #[test] + fn conventions_panic_is_a_tool_error() { + let r = super::guarded_tool(|| panic!("synthetic tool failure")).unwrap(); + assert_eq!(r["isError"], true); + assert!(r["content"][0]["text"].as_str().unwrap().contains("synthetic tool failure")); + let next = super::guarded_tool(|| Ok(serde_json::json!({"ok":true}))).unwrap(); + assert_eq!(next["ok"], true); + } + #[test] fn first_sentence_skips_abbreviations() { assert_eq!(first_sentence("Rotate pages. Undoable."), "Rotate pages."); diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 83d5a411dd..152cf724be 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -1049,7 +1049,15 @@ pub fn tools() -> Vec { t("edit_redo", "Redo", "Redo the last undone edit of a document.").cmd("edit.redo").with(schema(json!({ "doc": doc() }), &["doc"])), t("command_list", "List commands", "Every registered PdfCraft command with its menu, shortcut, whether it is enabled now, and the tool that automates it.") .ro() - .with(schema(json!({ "doc": doc() }), &[])), + .with(schema(json!({ "doc": doc(), "filter":{"type":"string"}, "enabled_only":{"type":"boolean"} }), &[])), + t("command_run", "Run a command", "Run a registry command through its headless tool. Pass that tool's arguments in params (see command_list). Unknown params are ignored with warnings; path confinement still applies.") + .destructive().with(schema(json!({"id":{"type":"string"},"params":{"type":"object"}}), &["id"])), + t("command_batch", "Run several commands", "Run steps in order. Returns completed/failed counts and per-step ok/result/error; stop_on_error defaults to true.") + .destructive().with(schema(json!({"steps":{"type":"array","items":schema(json!({"id":{"type":"string"},"params":{"type":"object"}}), &["id"])},"stop_on_error":{"type":"boolean"}}), &["steps"])), + t("doc_inspect", "Inspect open documents", "With doc, return doc_info; otherwise return information for every open document (empty documents array when none are open).") + .ro().with(schema(json!({"doc":doc()}), &[])), + t("render_preview", "Render a preview", "Render a PNG bounded by max_side (default 1024), without changing or saving the document. Page defaults to 1; doc may be omitted when exactly one document is open.") + .ro().with(schema(json!({"doc":doc(),"page":{"type":"integer","minimum":1},"max_side":{"type":"integer","minimum":1,"maximum":4096}}), &[])), ] } diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index 093a34431d..524d88a56e 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -636,7 +636,7 @@ fn mcp_compact_lists_the_core_tools_and_two_meta_tools() { let core_open = list["result"]["tools"][0].clone(); assert_eq!(Some(&core_open), full["result"]["tools"].as_array().unwrap().iter().find(|t| t["name"] == "doc_open")); assert!(list.to_string().len() * 5 < full.to_string().len(), "compact {} vs full {}", list.to_string().len(), full.to_string().len()); - for meta in &list["result"]["tools"].as_array().unwrap()[10..] { + for meta in &list["result"]["tools"].as_array().unwrap()[pdfcraft_automation::mcp::COMPACT_CORE_TOOLS.len()..] { assert_eq!(meta["inputSchema"]["type"], "object"); } let instructions = rpc(&mut s, 3, "initialize", json!({}))["result"]["instructions"].as_str().unwrap().to_string(); @@ -795,28 +795,28 @@ fn mcp_compact_meta_tools_reject_unknown_arguments() { let dir = workdir("mcp-compact-keys"); let mut s = compact_server(&dir); let list = rpc(&mut s, 1, "tools/list", json!({})); - let metas = &list["result"]["tools"].as_array().unwrap()[10..]; + let metas = &list["result"]["tools"].as_array().unwrap()[pdfcraft_automation::mcp::COMPACT_CORE_TOOLS.len()..]; assert_eq!(metas.len(), 2); for meta in metas { assert_eq!(meta["inputSchema"]["additionalProperties"], false, "{}", meta["name"]); } - let text = |r: &Value| r["result"]["content"][0]["text"].as_str().unwrap_or_default().to_string(); + let text = |r: &Value| r["error"]["message"].as_str().or_else(|| r["result"]["content"][0]["text"].as_str()).unwrap_or_default().to_string(); // A misspelled filter is an error, not an unfiltered listing. let typo = rpc(&mut s, 2, "tools/call", json!({ "name": "tool_search", "arguments": { "qurey": "rotate" } })); - assert_eq!(typo["result"]["isError"], true, "{typo}"); + assert_eq!(typo["error"]["code"], -32602, "{typo}"); assert!(text(&typo).contains("\"qurey\"") && text(&typo).contains("query"), "{}", text(&typo)); let extra = rpc(&mut s, 3, "tools/call", json!({ "name": "tool_search", "arguments": { "query": "rotate", "extra": 1 } })); - assert_eq!(extra["result"]["isError"], true, "{extra}"); + assert_eq!(extra["error"]["code"], -32602, "{extra}"); assert!(text(&extra).contains("\"extra\""), "{}", text(&extra)); // tool_call rejects extra outer keys before running anything. let misspelled = rpc(&mut s, 4, "tools/call", json!({ "name": "tool_call", "arguments": { "name": "doc_open", "argumentz": { "path": "a.pdf" } } })); - assert_eq!(misspelled["result"]["isError"], true, "{misspelled}"); + assert_eq!(misspelled["error"]["code"], -32602, "{misspelled}"); assert!(text(&misspelled).contains("\"argumentz\""), "{}", text(&misspelled)); let extra = rpc(&mut s, 5, "tools/call", json!({ "name": "tool_call", "arguments": { "name": "doc_list", "unexpected": "value" } })); - assert_eq!(extra["result"]["isError"], true, "{extra}"); + assert_eq!(extra["error"]["code"], -32602, "{extra}"); assert!(text(&extra).contains("\"unexpected\""), "{}", text(&extra)); assert_eq!(rpc(&mut s, 6, "tools/call", json!({ "name": "doc_list", "arguments": {} }))["result"]["structuredContent"]["documents"], json!([])); @@ -2923,3 +2923,66 @@ mod combine_argument_tests { } } } + +#[cfg(feature = "mcp")] +#[test] +fn conventions_core_tools_preserve_root_and_batch_errors() { + let dir = workdir("conventions-core"); + let mut s = McpServer::new(auto(&dir)); + let run = |s: &mut McpServer, name: &str, args: Value| rpc(s, 1, "tools/call", json!({"name":name,"arguments":args})); + assert_eq!(run(&mut s, "doc_inspect", json!({}))["result"]["structuredContent"]["documents"], json!([])); + let opened = run(&mut s, "command_run", json!({"id":"file.open","params":{"path":"a.pdf"}})); + assert_eq!(opened["result"]["isError"], false, "{opened}"); + let doc = opened["result"]["structuredContent"]["doc"].as_u64().unwrap(); + let filtered = run(&mut s, "command_list", json!({"doc":doc,"filter":"rotate","enabled_only":true})); + let commands = filtered["result"]["structuredContent"]["commands"].as_array().unwrap(); + assert!(!commands.is_empty()); + assert!(commands.iter().all(|c| c["enabled"] == true)); + assert!(commands.iter().find(|c| c["id"] == "page.rotate").unwrap()["params"].is_object()); + for stop in [true, false] { + let r = run( + &mut s, + "command_batch", + json!({"steps":[{"id":"page.rotate","params":{"doc":doc,"degrees":90}},{"id":"no.such.command"},{"id":"page.rotate","params":{"doc":doc,"degrees":90}}],"stop_on_error":stop}), + ); + assert_eq!(r["result"]["isError"], true, "{r}"); + assert_eq!(r["result"]["structuredContent"]["completed"], if stop { 1 } else { 2 }); + assert_eq!(r["result"]["structuredContent"]["failed"], 1); + } + let r = run(&mut s, "command_run", json!({"id":"file.save","params":{"doc":doc,"path":"../escaped.pdf"}})); + assert_eq!(r["result"]["isError"], true); + assert!(r["result"]["content"][0]["text"].as_str().unwrap().contains("outside the allowed directory")); + let r = run(&mut s, "command_run", json!({"id":"page.rotate","params":{"doc":doc,"degrees":90,"typo":1}})); + assert_eq!(r["result"]["isError"], false); + assert!(r["result"]["structuredContent"]["warnings"][0].as_str().unwrap().contains("typo")); + let before = run(&mut s, "doc_info", json!({"doc":doc})); + let r = run(&mut s, "render_preview", json!({"doc":doc,"page":1,"max_side":64})); + assert_eq!(r["result"]["isError"], false, "{r}"); + use base64::Engine as _; + let png = base64::engine::general_purpose::STANDARD.decode(r["result"]["content"][0]["data"].as_str().unwrap()).unwrap(); + let image = image::load_from_memory(&png).unwrap(); + assert!(image.width().max(image.height()) <= 64); + assert_eq!(before["result"], run(&mut s, "doc_info", json!({"doc":doc}))["result"]); +} + +#[cfg(feature = "mcp")] +#[test] +fn conventions_annotations_and_strict_keys_in_both_modes() { + for compact in [false, true] { + let mut s = McpServer::new(Automation::new()).with_compact(compact); + let r = rpc(&mut s, 1, "tools/list", json!({})); + let tools = r["result"]["tools"].as_array().unwrap(); + for name in ["command_list", "command_run", "command_batch", "doc_inspect", "render_preview"] { + assert!(tools.iter().any(|t| t["name"] == name), "missing {name}"); + } + for tool in tools { + for hint in ["readOnlyHint", "destructiveHint", "idempotentHint", "openWorldHint"] { + assert!(tool["annotations"][hint].is_boolean(), "{}: {hint}", tool["name"]); + } + let r = rpc(&mut s, 2, "tools/call", json!({"name":tool["name"],"arguments":{"bogus_arg":1}})); + assert_eq!(r["error"]["code"], -32602, "{}", tool["name"]); + let message = r["error"]["message"].as_str().unwrap(); + assert!(message.contains("bogus_arg") && message.contains("expected:")); + } + } +} diff --git a/docs/mcp.md b/docs/mcp.md new file mode 100644 index 0000000000..2ea1367e7e --- /dev/null +++ b/docs/mcp.md @@ -0,0 +1,34 @@ +# MCP conventions + +Start explicitly with `pdfcraft-cli mcp [--root DIR] [--compact]`. The server uses JSON lines on +stdin/stdout, opens no port, and stops on EOF. `--root` confines all reads and writes, including +commands invoked through the common tools. Logs stay on stderr. + +| Tool | Arguments | +|---|---| +| `command_list` | `doc?`, `filter?`, `enabled_only?`; returns `{commands: [...]}` with mapped tool schemas | +| `command_run` | `id`, `params?` (the mapped task tool's arguments, including `doc` when required) | +| `command_batch` | `steps: [{id, params?}]`, `stop_on_error?` (default true) | +| `doc_inspect` | `doc?`; one document's info or `{documents: [...]}` for the session | +| `render_preview` | `doc?`, `page?` (1-based, default 1), `max_side?` (1–4096, default 1024) | + +All existing task tools remain available. Compact mode lists these five tools alongside its +existing core set and `tool_search` / `tool_call`. There are no legacy-name aliases. The full +[automation guide](../crates/automation/README.md) describes the tools, resources, coordinate +system, root confinement, and errors. + +```text +command_run {"id":"file.open","params":{"path":"input.pdf"}} + → {"doc":1,...} +command_list {"doc":1,"filter":"rotate"} +command_run {"id":"page.rotate","params":{"doc":1,"degrees":90}} +render_preview {"doc":1,"page":1,"max_side":512} +``` + +Every tool has a title and the four MCP annotation hints. Unknown top-level argument keys return +JSON-RPC `-32602`; command parameter keys that do not belong to the mapped task tool are ignored +with warnings. Task failures and escaped tool panics return `isError: true`, and the server keeps +serving. A failed batch includes per-step errors and is marked `isError: true`. + +Exports complete synchronously. A call's `progressToken` and cancellation notifications are +harmlessly ignored; the server does not provide background export progress or cancellation. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 647ddbf180..2640b50976 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -9337,8 +9337,8 @@ tier = "P0" milestone = "M13" status = "shipped" commands = [] -tools = [] -evidence = ["crates/automation/tests/automation.rs::mcp_session_over_stdio", "crates/automation/tests/automation.rs::mcp_errors"] +tools = ["command_list", "command_run", "command_batch", "doc_inspect", "render_preview"] +evidence = ["crates/automation/tests/automation.rs::mcp_session_over_stdio", "crates/automation/tests/automation.rs::mcp_errors", "crates/automation/tests/automation.rs::conventions_core_tools_preserve_root_and_batch_errors", "crates/automation/tests/automation.rs::conventions_annotations_and_strict_keys_in_both_modes"] notes = "" [[feature]]