From a5bc45e847207baddeb77c184359f1fe98993d33 Mon Sep 17 00:00:00 2001 From: Jake Date: Wed, 22 Apr 2026 09:23:17 +0100 Subject: [PATCH] fix(cr-2026-04-22): list_transcripts accepts omitted arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression surfaced by the batch review: commit 8400128 switched list_transcripts from unwrap_or_default to map_err(-32602). This correctly errors on malformed payloads but also rejected the common case where a client omits the 'arguments' field entirely — which arrives as Value::Null, and serde_json::from_value does not deserialise Null into a struct. Short-circuits the Null case to Args::default() before attempting deserialisation. Genuine shape mismatches ("limit": "twenty") still return -32602 as the previous test asserts. New regression test: tools/call with list_transcripts and no arguments key must return a successful response. --- crates/mcp/src/lib.rs | 48 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index 66a8d26..34527ff 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -191,13 +191,19 @@ async fn list_transcripts_tool(pool: &SqlitePool, args: Value) -> Result, } - // 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}")))?; + // The `arguments` field in CallParams defaults to `Value::Null` + // when a client omits it entirely. `serde_json::from_value` does + // not accept Null as an empty object, so we short-circuit that + // case before deserialising — a missing `arguments` still falls + // back to defaults (the common case for list_transcripts), while + // a genuinely malformed payload returns -32602 per the Invalid + // arguments contract the other handlers use. + let args: Args = if args.is_null() { + Args::default() + } else { + 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) @@ -426,6 +432,34 @@ mod tests { assert!(err.message.contains("expected value")); } + #[tokio::test] + async fn list_transcripts_accepts_omitted_arguments() { + // Regression for the review-of-review: tools/call requests + // that omit `arguments` arrive with `Value::Null`. The + // malformed-params fix must not reject those — it is the + // common shape for an empty call, equivalent to defaults. + let request = json!({ + "jsonrpc": "2.0", + "id": 98, + "method": "tools/call", + "params": { + "name": "list_transcripts", + // `arguments` omitted + }, + }); + + let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); + kon_storage::migrations::run_migrations(&pool).await.unwrap(); + let response = handle_message(&pool, request).await.expect("has response"); + + assert!( + response.error.is_none(), + "omitted arguments must not error, got: {:?}", + response.error + ); + assert!(response.result.is_some()); + } + #[tokio::test] async fn list_transcripts_rejects_malformed_params_with_invalid_arguments() { // Regression for the 2026-04-22 review MAJOR: previously the