From 840012822f6ba6e000f342b0cf789276a160fc65 Mon Sep 17 00:00:00 2001
From: Jake <jake@corbel.consulting>
Date: Wed, 22 Apr 2026 09:14:42 +0100
Subject: [PATCH] fix(cr-2026-04-22): list_transcripts tool returns -32602 on
 malformed params
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

MAJOR from the 2026-04-22 review (crates/mcp/src/lib.rs:188-195):
the handler called serde_json::from_value(args).unwrap_or_default(),
so a request like { "limit": "twenty" } silently became the default
limit of 20. Every other tool handler in this file map_errs to
-32602 Invalid arguments; this one was the outlier.

Switches to the same map_err pattern. Empty params still
deserialise cleanly to Args::default (via #[serde(default)] on the
Option<i64> field), so callers that send no args are unaffected —
only genuinely malformed shapes now error.

Regression test: tools/call with list_transcripts and a
string-typed limit must return code -32602 with an "Invalid
arguments" message.
---
 crates/mcp/src/lib.rs | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs
index e82b65c..66a8d26 100644
--- a/crates/mcp/src/lib.rs
+++ b/crates/mcp/src/lib.rs
@@ -191,7 +191,13 @@ async fn list_transcripts_tool(pool: &SqlitePool, args: Value) -> Result<Value,
         #[serde(default)]
         limit: Option<i64>,
     }
-    let args: Args = serde_json::from_value(args).unwrap_or_default();
+    // Every other tool handler returns -32602 on malformed args;
+    // previously this one silently fell back to defaults, hiding
+    // client bugs (2026-04-22 review MAJOR). An empty Value
+    // deserialises cleanly into Args::default, so the "no args"
+    // path still works — only genuinely malformed payloads error.
+    let args: Args = serde_json::from_value(args)
+        .map_err(|e| error(-32602, format!("Invalid arguments: {e}")))?;
     let limit = args.limit.unwrap_or(20).clamp(1, 200);
 
     let rows = kon_storage::list_transcripts(pool, limit)
@@ -420,6 +426,32 @@ mod tests {
         assert!(err.message.contains("expected value"));
     }
 
+    #[tokio::test]
+    async fn list_transcripts_rejects_malformed_params_with_invalid_arguments() {
+        // Regression for the 2026-04-22 review MAJOR: previously the
+        // handler did `from_value(args).unwrap_or_default()`, so
+        // `{"limit": "not-a-number"}` silently became `limit = 20`.
+        // Every other handler returns -32602 on shape mismatch; this
+        // one must now do the same.
+        let request = json!({
+            "jsonrpc": "2.0",
+            "id": 99,
+            "method": "tools/call",
+            "params": {
+                "name": "list_transcripts",
+                "arguments": { "limit": "twenty" },
+            },
+        });
+
+        let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap();
+        let response = handle_message(&pool, request).await.expect("has response");
+
+        assert!(response.result.is_none());
+        let err = response.error.expect("expected error");
+        assert_eq!(err.code, -32602, "invalid arguments must surface as -32602");
+        assert!(err.message.contains("Invalid arguments"));
+    }
+
     #[tokio::test]
     async fn unknown_method_returns_method_not_found_error() {
         let request = json!({