Merge pull request #146 from salignatmoandal/fix/mcp-batch-keep-going-iserror

MCP batch: surface failures when keep_going is set (#145)

crates/automation/src/mcp.rs

@@ -127,11 +127,20 @@ impl<B: Backend> Server<B> {
             }
             "batch" => {
                 let keep = args.get("keep_going").and_then(Value::as_bool).unwrap_or(false);
-                let calls = args.get("calls").and_then(Value::as_array).cloned().unwrap_or_default();
+                let Some(calls) = args.get("calls").and_then(Value::as_array) else {
+                    return error_result("`calls` must be an array");
+                };
                 let mut out = Vec::new();
                 let mut failed = false;
                 for c in calls.iter().take(1000) {
-                    let cmd = c.get("command").and_then(Value::as_str).unwrap_or("");
+                    let Some(cmd) = c.get("command").and_then(Value::as_str).map(str::trim).filter(|s| !s.is_empty()) else {
+                        out.push(json!({"command": "", "ok": false, "error": "`command` required"}));
+                        failed = true;
+                        if !keep {
+                            break;
+                        }
+                        continue;
+                    };
                     let r = self.exec(cmd, c.get("params").cloned().unwrap_or(json!({})));
                     match r {
                         Ok(v) => out.push(json!({"command": cmd, "ok": true, "result": v})),
@@ -144,7 +153,9 @@ impl<B: Backend> Server<B> {
                         }
                     }
                 }
-                if failed && !keep { Err(serde_json::to_string(&out).unwrap_or_default()) } else { Ok(json!(out)) }
+                // Any failure sets isError so agents that only check that flag notice;
+                // keep_going still runs the rest and the payload lists every call.
+                if failed { Err(serde_json::to_string(&out).unwrap_or_default()) } else { Ok(json!(out)) }
             }
             "inspect_session" => self.backend.call("session.inspect", json!({"detail": a("detail")})),
             "new_session" => self.exec("session.new", args.clone()),

crates/automation/tests/agent_tasks.rs

@@ -135,3 +135,39 @@ fn errors_are_reported_not_panics() {
     let r = rpc(&mut s, 2, "tools/call", json!({"name": "screenshot", "arguments": {}}));
     assert_eq!(r["result"]["isError"], true);
 }
+
+#[test]
+fn batch_keep_going_still_sets_is_error_on_partial_failure() {
+    let mut s = Server::new(Headless::new(Engine::default()));
+    let r = rpc(
+        &mut s,
+        1,
+        "tools/call",
+        json!({"name": "batch", "arguments": {"keep_going": true, "calls": [
+            {"command": "track.new", "params": {"count": 1, "name": "Kept"}},
+            {"command": "mix.volume", "params": {"track": "Nope", "db": -6}},
+            {"command": "track.new", "params": {"count": 1, "name": "After"}},
+        ]}}),
+    );
+    assert_eq!(r["result"]["isError"], true, "{r}");
+    let text = r["result"]["content"][0]["text"].as_str().unwrap();
+    let results: Value = serde_json::from_str(text).unwrap();
+    let arr = results.as_array().unwrap();
+    assert_eq!(arr.len(), 3, "{results}");
+    assert_eq!(arr[0]["ok"], true);
+    assert_eq!(arr[1]["ok"], false);
+    assert_eq!(arr[2]["ok"], true);
+    let sess = session(&mut s);
+    assert!(sess["tracks"].as_array().unwrap().iter().any(|t| t["name"] == "Kept"));
+    assert!(sess["tracks"].as_array().unwrap().iter().any(|t| t["name"] == "After"));
+}
+
+#[test]
+fn batch_rejects_non_array_calls() {
+    let mut s = Server::new(Headless::new(Engine::default()));
+    let r = rpc(&mut s, 1, "tools/call", json!({"name": "batch", "arguments": {"calls": {"command": "track.new"}}}));
+    assert_eq!(r["result"]["isError"], true, "{r}");
+    let text = r["result"]["content"][0]["text"].as_str().unwrap();
+    assert!(text.contains("`calls` must be an array"), "{text}");
+    assert_eq!(session(&mut s)["tracks"].as_array().unwrap().len(), 0);
+}

docs/mcp.md

@@ -22,7 +22,7 @@ claude mcp add soundcraft -- soundcraft-cli mcp --demo
 |---|---|
 | `list_commands` | Discover every command (id, label, menu path, shortcut, params doc, enabled). |
 | `execute` | Run one command with JSON params. |
-| `batch` | Run several commands in order (`keep_going` optional). |
+| `batch` | Run several commands in order (`keep_going` optional). Any failed call makes the tool result `isError: true`; with `keep_going` the remaining calls still run and every result is listed. |
 | `inspect_session` | Verify work: tracks, clips, mixer, plugins, sends, automation, markers, selection; each `sources[]` entry includes `loaded` (decoded audio in the pool). |
 | `new_session`, `open_session`, `save_session` | Session files (`.scraft`, audio in `Audio Files/`). `open_session` returns `missing`: the media files that could not be loaded (empty when all are found). |
 | `import_audio` | WAV, AIFF, FLAC, MP3, OGG, AAC/M4A, ALAC, CAF onto a new or existing track. |