mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-11 16:36:13 -06:00
M13: common MCP tools through the confined automation path
Preserve task tools and compact mode, add complete hints and strict argument-key errors, and catch escaped tool panics. Co-Authored-By: OpenAI Codex <noreply@openai.com>
This commit is contained in:
co-authored by
OpenAI Codex
parent
a6bc63a497
commit
894e81affd
@@ -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.
|
||||
|
||||
@@ -41,7 +41,7 @@ fn listed_tools(flags: &[&str]) -> Vec<String> {
|
||||
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());
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<Vec<Content>> {
|
||||
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::<Vec<_>>().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<Value> {
|
||||
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<Value> {
|
||||
if args.opt_int("doc")?.is_some() {
|
||||
return Ok(info(self.doc(args)?));
|
||||
}
|
||||
Ok(json!({"documents":self.session.docs().iter().map(info).collect::<Vec<_>>()}))
|
||||
}
|
||||
|
||||
pub(crate) fn render_preview(&mut self, args: &Args) -> Result<Content> {
|
||||
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 })
|
||||
}
|
||||
}
|
||||
@@ -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<Value> = 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<Value> = 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 }))
|
||||
}
|
||||
|
||||
|
||||
@@ -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<Value, (i64, String)>) -> Result<Value, (i64, String)> {
|
||||
pdfcraft_engine::guard(f).unwrap_or_else(|message| Ok(tool_error(&format!("internal error: {message}"))))
|
||||
}
|
||||
|
||||
fn tool_content(name: &str, content: Vec<Content>) -> 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<Content>) -> 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.");
|
||||
|
||||
@@ -1049,7 +1049,15 @@ pub fn tools() -> Vec<ToolDef> {
|
||||
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}}), &[])),
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
@@ -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:"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+34
@@ -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.
|
||||
@@ -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]]
|
||||
|
||||
Reference in New Issue
Block a user