Merge PR #375 from NikitkaRa: MCP tools refuse an argument that is not in their schema (#371)

# Conflicts:
#	crates/mcp/src/tests.rs

crates/mcp/src/tests.rs

@@ -120,6 +120,28 @@ fn invalid_id_never_falls_back_to_selection() {
     tool(&mut s, "run_command", json!({"command": "shape.inspect", "params": {"id": title_id}}));
 }
 
+#[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()));

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<Value, String>) -> 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<String> {
+    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::<Vec<_>>().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();

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`,