diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index ec880c6a..89322d08 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -710,6 +710,7 @@ fn ensure_valid_json_schema(schema: &mut Value) { if let Some(properties) = params_obj.get_mut("properties") { if let Some(properties_obj) = properties.as_object_mut() { for (_key, prop) in properties_obj.iter_mut() { + normalize_nullable(prop); if prop.is_object() && prop.get("type").and_then(|t| t.as_str()) == Some("object") { @@ -722,6 +723,61 @@ fn ensure_valid_json_schema(schema: &mut Value) { } } +/// Normalizes nullable type representations that some providers (e.g. Vertex Gemini via Bifrost) +/// don't support: +/// - `"type": ["integer", "null"]` → `"type": "integer"` (drops the null variant) +/// - `"anyOf": [T, {"type": "null"}]` → T (unwraps to the non-null schema) +/// +/// Optional-ness is already conveyed by the field being absent from `required`. +fn normalize_nullable(schema: &mut Value) { + let Some(obj) = schema.as_object_mut() else { + return; + }; + + // Handle type: ["T", "null"] array form (schemars 1.x style for nullable primitives) + if let Some(type_val) = obj.get("type").cloned() { + if let Some(types) = type_val.as_array() { + let non_null: Vec<&Value> = types + .iter() + .filter(|t| t.as_str() != Some("null")) + .collect(); + if non_null.len() == 1 { + let scalar = non_null[0].clone(); + obj.insert("type".to_string(), scalar); + return; + } + } + } + + // Handle anyOf: [T, {type: "null"}] form — merge the non-null variant's fields + // into the current object (preserving sibling keys like "description" or "default") + // rather than replacing the whole schema. + if let Some(any_of) = obj.remove("anyOf") { + if let Some(variants) = any_of.as_array() { + if variants.len() == 2 { + let is_null = |v: &Value| v.get("type").and_then(|t| t.as_str()) == Some("null"); + let non_null = if is_null(&variants[0]) { + Some(&variants[1]) + } else if is_null(&variants[1]) { + Some(&variants[0]) + } else { + None + }; + if let Some(replacement) = non_null { + if let Some(replacement_obj) = replacement.as_object() { + for (k, v) in replacement_obj { + obj.entry(k.clone()).or_insert(v.clone()); + } + return; + } + } + } + } + // Put it back if we couldn't simplify + obj.insert("anyOf".to_string(), any_of); + } +} + fn strip_data_prefix(line: &str) -> Option<&str> { // SSE spec allows both "data: value" and "data:value" (space after colon is optional) line.strip_prefix("data: ") @@ -1139,6 +1195,85 @@ mod tests { let mut tools = vec![original_schema.clone()]; validate_tool_schemas(&mut tools); assert_eq!(tools[0], original_schema); + + // Test case 4: anyOf nullable is unwrapped, preserving sibling metadata + let mut tools = vec![json!({ + "type": "function", + "function": { + "name": "shell", + "description": "run shell", + "parameters": { + "type": "object", + "properties": { + "command": { "type": "string" }, + "timeout_secs": { + "description": "timeout in seconds", + "anyOf": [ + { "type": "integer", "format": "uint64", "minimum": 0 }, + { "type": "null" } + ] + } + }, + "required": ["command"] + } + } + })]; + validate_tool_schemas(&mut tools); + let timeout_schema = &tools[0]["function"]["parameters"]["properties"]["timeout_secs"]; + assert_eq!(timeout_schema["type"], "integer"); + assert_eq!(timeout_schema["format"], "uint64"); + assert_eq!(timeout_schema["description"], "timeout in seconds"); + assert!(timeout_schema.get("anyOf").is_none()); + + // Test case 4b: type array form (schemars 1.x style for nullable primitives) + let mut tools = vec![json!({ + "type": "function", + "function": { + "name": "shell", + "description": "run shell", + "parameters": { + "type": "object", + "properties": { + "command": { "type": "string" }, + "timeout_secs": { + "type": ["integer", "null"], + "format": "uint64", + "minimum": 0 + } + }, + "required": ["command"] + } + } + })]; + validate_tool_schemas(&mut tools); + let timeout_schema = &tools[0]["function"]["parameters"]["properties"]["timeout_secs"]; + assert_eq!(timeout_schema["type"], "integer"); + assert!(!timeout_schema["type"].is_array()); + + // Test case 5: Verify the actual ShellParams schema is compatible (no anyOf for timeout_secs) + use crate::agents::platform_extensions::developer::shell::ShellParams; + use schemars::schema_for; + let schema_value = serde_json::to_value(schema_for!(ShellParams)).unwrap(); + let schema_obj = schema_value.as_object().unwrap().clone(); + let tool = rmcp::model::Tool::new("shell", "run shell", schema_obj); + let mut tools = vec![json!({ + "type": "function", + "function": { + "name": tool.name, + "description": tool.description, + "parameters": tool.input_schema, + } + })]; + validate_tool_schemas(&mut tools); + let timeout = &tools[0]["function"]["parameters"]["properties"]["timeout_secs"]; + assert!( + timeout.get("anyOf").is_none(), + "timeout_secs should not have anyOf after validation, got: {timeout}" + ); + assert_eq!( + timeout["type"], "integer", + "timeout_secs should have type=integer" + ); } const OPENAI_TOOL_USE_RESPONSE: &str = r#"{