From e8f8ec06e33c2cc0822a8922c952641dde323cf8 Mon Sep 17 00:00:00 2001 From: Nick Stebbings <47646783+nick-stebbings@users.noreply.github.com> Date: Wed, 11 Mar 2026 07:08:01 +1300 Subject: [PATCH] fix(mcp): strip top-level null params before forwarding to MCP servers (#795) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(llm): per-provider unsupported parameter filtering (#749, #728) (#809) Add declarative `unsupported_params` field to provider definitions in providers.json. Parameters listed are stripped from requests before sending, preventing 400 errors from providers that reject them (e.g. gpt-5 family and kimi-k2.5 rejecting custom temperature values). - Add `unsupported_params` to ProviderDefinition and RegistryProviderConfig - Propagate from registry through config resolution - Generic strip helpers handle temperature, max_tokens, stop_sequences - Apply filtering in RigAdapter and AnthropicOAuthProvider - Mark openai and tinfoil providers as unsupporting temperature - Update openai default model to gpt-5-mini Co-authored-by: Claude Opus 4.6 * fix(mcp): strip top-level null params before forwarding to MCP servers LLMs frequently emit `"field": null` for optional parameters in tool calls. Many MCP servers reject explicit nulls for fields that should simply be absent — e.g. Notion returns 400 for `"sort": null` in a search call, expecting the field to be omitted entirely. Strip top-level null keys from the params object before calling `call_tool()`. Only top-level keys are stripped; nested nulls are preserved since they may be semantically meaningful. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Illia Polosukhin Co-authored-by: Claude Opus 4.6 --- src/tools/mcp/client.rs | 59 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/src/tools/mcp/client.rs b/src/tools/mcp/client.rs index cd74d572..61c9d5c7 100644 --- a/src/tools/mcp/client.rs +++ b/src/tools/mcp/client.rs @@ -490,6 +490,12 @@ impl Tool for McpToolWrapper { _ctx: &JobContext, ) -> Result { let start = std::time::Instant::now(); + + // Strip top-level null values before forwarding — LLMs often emit + // `"field": null` for optional params, but many MCP servers reject + // explicit nulls for fields that should simply be absent. + let params = strip_top_level_nulls(params); + let result = self.client.call_tool(&self.tool.name, params).await?; let content: String = result .content @@ -516,9 +522,22 @@ impl Tool for McpToolWrapper { } } -/// Sanitize an HTTP error response body for safe display. +/// Remove top-level keys whose value is JSON null from an object. /// -/// Detects full HTML error pages (containing ` serde_json::Value { + match value { + serde_json::Value::Object(map) => { + let filtered = map.into_iter().filter(|(_, v)| !v.is_null()).collect(); + serde_json::Value::Object(filtered) + } + other => other, + } +} + #[cfg(test)] mod tests { use super::*; @@ -806,4 +825,40 @@ mod tests { let mock_non_http = MockTransport::new(false, vec![]); assert!(!mock_non_http.supports_http_features()); } + + #[test] + fn test_strip_top_level_nulls_removes_null_fields() { + let input = serde_json::json!({ + "query": "search term", + "sort": null, + "filter": null, + "page_size": 10 + }); + let result = strip_top_level_nulls(input); + let obj = result.as_object().unwrap(); + assert_eq!(obj.len(), 2); + assert_eq!(obj["query"], "search term"); + assert_eq!(obj["page_size"], 10); + assert!(!obj.contains_key("sort")); + assert!(!obj.contains_key("filter")); + } + + #[test] + fn test_strip_top_level_nulls_preserves_non_objects() { + let input = serde_json::json!("just a string"); + let result = strip_top_level_nulls(input.clone()); + assert_eq!(result, input); + } + + #[test] + fn test_strip_top_level_nulls_preserves_nested_nulls() { + let input = serde_json::json!({ + "outer": { "inner": null }, + "top_null": null + }); + let result = strip_top_level_nulls(input); + let obj = result.as_object().unwrap(); + assert_eq!(obj.len(), 1); + assert!(obj["outer"]["inner"].is_null()); + } }