mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 15:40:18 +00:00
fix(mcp): strip top-level null params before forwarding to MCP servers (#795)
* 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 <[email protected]> * 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 <[email protected]> --------- Co-authored-by: Illia Polosukhin <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Illia Polosukhin
Claude Opus 4.6
parent
c566faf28f
commit
e8f8ec06e3
+57
-2
@@ -490,6 +490,12 @@ impl Tool for McpToolWrapper {
|
||||
_ctx: &JobContext,
|
||||
) -> Result<ToolOutput, ToolError> {
|
||||
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 `<html` or `<!DOCTYPE`) and
|
||||
/// LLMs frequently emit `"field": null` for optional parameters. Many MCP
|
||||
/// servers (e.g. Notion) treat an explicit `null` as an invalid value for
|
||||
/// optional fields that should simply be absent. Stripping these before
|
||||
/// forwarding avoids 400-class rejections from strict servers.
|
||||
fn strip_top_level_nulls(value: serde_json::Value) -> 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());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user