From 591c06ba33af93408bbf4d9ded033890ac400dc7 Mon Sep 17 00:00:00 2001 From: Nikita Date: Tue, 13 Oct 2026 13:14:07 +0300 Subject: [PATCH] MCP tools refuse an argument that is not in their schema (#371) A misspelled key used to be dropped, so the tool still succeeded with its defaults. Co-authored-by: Cursor --- crates/mcp/src/tests.rs | 22 ++++++++++++++++++++++ crates/mcp/src/tools.rs | 33 +++++++++++++++++++++++++++++++-- docs/mcp.md | 2 ++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index b1521521..e77d6c3d 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -79,6 +79,28 @@ fn agent_builds_a_deck() { assert_eq!(n2, n1 + 1); } +#[test] +fn unknown_tool_arguments_are_refused() { + let mut s = Server::new(Box::new(Headless::new())); + tool(&mut s, "new_presentation", json!({})); + let charts = |v: &Value| v["shapes"].as_array().unwrap().iter().filter(|sh| sh["kind"] == "chart").count(); + let before = charts(&tool(&mut s, "inspect_slide", json!({}))); + let r = call(&mut s, 2, "tools/call", json!({"name": "insert_chart", "arguments": {"type": "column", "data": [["x", "A"], ["q1", 1]]}})); + assert_eq!(r["result"]["isError"], true); + let msg = r["result"]["content"][0]["text"].as_str().unwrap(); + assert!(msg.contains("unknown argument \"data\"") && msg.contains("categories") && msg.contains("series"), "{msg}"); + assert_eq!(charts(&tool(&mut s, "inspect_slide", json!({}))), before, "a refused call inserts nothing"); + let r = call(&mut s, 3, "tools/call", json!({"name": "list_commands", "arguments": {"query": "theme", "limit": 1}})); + assert_eq!(r["result"]["isError"], true); + let msg = r["result"]["content"][0]["text"].as_str().unwrap(); + assert!(msg.contains("unknown arguments \"limit\", \"query\"") && msg.contains("filter"), "{msg}"); + // The command's own parameters stay inside `params`. + tool(&mut s, "run_command", json!({"command": "slide.new", "params": {"layout": "blank"}})); + let tools = call(&mut s, 4, "tools/list", json!({})); + let chart = tools["result"]["tools"].as_array().unwrap().iter().find(|t| t["name"] == "insert_chart").unwrap(); + assert_eq!(chart["inputSchema"]["additionalProperties"], false); +} + #[test] fn errors_are_reported_not_panics() { let mut s = Server::new(Box::new(Headless::new())); diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 1a457dda..29986a79 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -47,10 +47,10 @@ fn boolean(desc: &str) -> Value { json!({"type": "boolean", "description": desc}) } fn mods_schema() -> Value { - json!({"type": "object", "description": "Modifier keys (cmd = Command on macOS / Ctrl elsewhere; alt = Option)", "properties": {"shift": {"type": "boolean"}, "alt": {"type": "boolean"}, "cmd": {"type": "boolean"}}}) + json!({"type": "object", "additionalProperties": false, "description": "Modifier keys (cmd = Command on macOS / Ctrl elsewhere; alt = Option)", "properties": {"shift": {"type": "boolean"}, "alt": {"type": "boolean"}, "cmd": {"type": "boolean"}}}) } fn obj(props: Value, required: &[&str]) -> Value { - let mut o = json!({"type": "object", "properties": props}); + let mut o = json!({"type": "object", "properties": props, "additionalProperties": false}); if !required.is_empty() { o["required"] = json!(required); } @@ -294,8 +294,37 @@ fn wrap(r: Result) -> ToolResult { } } +/// Keys other than the tool's schema properties are refused, so a misspelled argument does not +/// succeed with the tool's defaults. Keys inside a free object such as `run_command`'s `params` +/// belong to the command and are left for the command to read. +fn unknown_argument_error(name: &str, args: &Value) -> Option { + let defs = tool_definitions(); + let def = defs.iter().find(|t| t.get("name").and_then(Value::as_str) == Some(name))?; + let mut allowed: Vec<&str> = def + .get("inputSchema") + .and_then(|s| s.get("properties")) + .and_then(Value::as_object) + .map(|p| p.keys().map(String::as_str).collect()) + .unwrap_or_default(); + allowed.sort_unstable(); + let obj = args.as_object()?; + let mut unknown: Vec<&str> = obj.keys().map(String::as_str).filter(|k| !allowed.contains(k)).collect(); + unknown.sort_unstable(); + let first = unknown.first()?; + let expected = if allowed.is_empty() { "none".to_string() } else { allowed.join(", ") }; + let (what, list) = if unknown.len() == 1 { + ("unknown argument", format!("{first:?}")) + } else { + ("unknown arguments", unknown.iter().map(|k| format!("{k:?}")).collect::>().join(", ")) + }; + Some(format!("{name}: {what} {list} (expected: {expected})")) +} + pub fn call_tool(b: &mut dyn Backend, name: &str, a: &Value) -> ToolResult { let a = if a.is_object() { a.clone() } else { json!({}) }; + if let Some(msg) = unknown_argument_error(name, &a) { + return ToolResult::error(msg); + } match name { "list_commands" => wrap(b.call("engine.commands", json!({})).map(|v| { let f = a.get("filter").and_then(Value::as_str).unwrap_or("").to_lowercase(); diff --git a/docs/mcp.md b/docs/mcp.md index f7e24c7e..07284e70 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -30,6 +30,8 @@ deck. | `screenshot` | The whole window, UI included (connected mode) | | `call_app` | Any [control protocol](control-protocol.md) method (connected mode) | +A tool refuses an argument that is not in its schema (`insert_chart: unknown argument "data" (expected: categories, series, title, type)`). Keys inside `run_command`'s `params` are that command's parameters. + Useful commands for building decks: `slide.new {layout, title, body}`, `shape.insert {preset, rect, text}`, `shape.connect {from, to, preset}`, `shape.freeform {points, closed, smooth}`, `shape.merge {op, ids}`, `format.*`, `design.theme`, `transition.set`, `animation.set`,