fix(cr-2026-04-22): list_transcripts accepts omitted arguments
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.
This commit is contained in:
@@ -191,13 +191,19 @@ async fn list_transcripts_tool(pool: &SqlitePool, args: Value) -> Result<Value,
|
|||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
limit: Option<i64>,
|
limit: Option<i64>,
|
||||||
}
|
}
|
||||||
// Every other tool handler returns -32602 on malformed args;
|
// The `arguments` field in CallParams defaults to `Value::Null`
|
||||||
// previously this one silently fell back to defaults, hiding
|
// when a client omits it entirely. `serde_json::from_value` does
|
||||||
// client bugs (2026-04-22 review MAJOR). An empty Value
|
// not accept Null as an empty object, so we short-circuit that
|
||||||
// deserialises cleanly into Args::default, so the "no args"
|
// case before deserialising — a missing `arguments` still falls
|
||||||
// path still works — only genuinely malformed payloads error.
|
// back to defaults (the common case for list_transcripts), while
|
||||||
let args: Args = serde_json::from_value(args)
|
// a genuinely malformed payload returns -32602 per the Invalid
|
||||||
.map_err(|e| error(-32602, format!("Invalid arguments: {e}")))?;
|
// 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 limit = args.limit.unwrap_or(20).clamp(1, 200);
|
||||||
|
|
||||||
let rows = kon_storage::list_transcripts(pool, limit)
|
let rows = kon_storage::list_transcripts(pool, limit)
|
||||||
@@ -426,6 +432,34 @@ mod tests {
|
|||||||
assert!(err.message.contains("expected value"));
|
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]
|
#[tokio::test]
|
||||||
async fn list_transcripts_rejects_malformed_params_with_invalid_arguments() {
|
async fn list_transcripts_rejects_malformed_params_with_invalid_arguments() {
|
||||||
// Regression for the 2026-04-22 review MAJOR: previously the
|
// Regression for the 2026-04-22 review MAJOR: previously the
|
||||||
|
|||||||
Reference in New Issue
Block a user