From d9448530f8ffb112ef419fe089bb4f80370ee0b7 Mon Sep 17 00:00:00 2001 From: Wendy Tang Date: Fri, 7 Feb 2025 16:51:13 -0800 Subject: [PATCH 1/9] validate tool schemas --- crates/goose/src/providers/formats/openai.rs | 99 +++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-) diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index 528c3f232..780f1c21c 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -253,6 +253,36 @@ pub fn get_usage(data: &Value) -> Result { Ok(Usage::new(input_tokens, output_tokens, total_tokens)) } +/// Validates and fixes tool schemas to ensure they have proper parameter structure. +/// If parameters exists, ensures it has properties and required fields, or removes parameters entirely. +pub fn validate_tool_schemas(tools: &mut [Value]) { + for tool in tools.iter_mut() { + if let Some(function) = tool.get_mut("function") { + if let Some(parameters) = function.get_mut("parameters") { + // If parameters exists but doesn't have properties and required, add them + if parameters.is_object() { + let params_obj = parameters.as_object_mut().unwrap(); + + // Ensure type field exists + if !params_obj.contains_key("type") { + params_obj.insert("type".to_string(), json!("object")); + } + + // Ensure properties field exists + if !params_obj.contains_key("properties") { + params_obj.insert("properties".to_string(), json!({})); + } + + // Ensure required field exists + if !params_obj.contains_key("required") { + params_obj.insert("required".to_string(), json!([])); + } + } + } + } + } +} + pub fn create_request( model_config: &ModelConfig, system: &str, @@ -275,12 +305,15 @@ pub fn create_request( }); let messages_spec = format_messages(messages, image_format); - let tools_spec = if !tools.is_empty() { + let mut tools_spec = if !tools.is_empty() { format_tools(tools)? } else { vec![] }; + // Validate tool schemas + validate_tool_schemas(&mut tools_spec); + let mut messages_array = vec![system_message]; messages_array.extend(messages_spec); @@ -326,6 +359,70 @@ mod tests { use mcp_core::content::Content; use serde_json::json; + #[test] + fn test_validate_tool_schemas() { + // Test case 1: Empty parameters object + let mut tools = vec![json!({ + "type": "function", + "function": { + "name": "test_func", + "description": "test description", + "parameters": { + "type": "object" + } + } + })]; + + validate_tool_schemas(&mut tools); + + let params = tools[0]["function"]["parameters"].as_object().unwrap(); + assert!(params.contains_key("properties")); + assert!(params.contains_key("required")); + assert_eq!(params["type"], "object"); + assert_eq!(params["properties"], json!({})); + assert_eq!(params["required"], json!([])); + + // Test case 2: Missing type field + let mut tools = vec![json!({ + "type": "function", + "function": { + "name": "test_func", + "description": "test description", + "parameters": { + "properties": {} + } + } + })]; + + validate_tool_schemas(&mut tools); + + let params = tools[0]["function"]["parameters"].as_object().unwrap(); + assert_eq!(params["type"], "object"); + + // Test case 3: Complete valid schema should remain unchanged + let original_schema = json!({ + "type": "function", + "function": { + "name": "test_func", + "description": "test description", + "parameters": { + "type": "object", + "properties": { + "location": { + "type": "string", + "description": "City and country" + } + }, + "required": ["location"] + } + } + }); + + let mut tools = vec![original_schema.clone()]; + validate_tool_schemas(&mut tools); + assert_eq!(tools[0], original_schema); + } + const OPENAI_TOOL_USE_RESPONSE: &str = r#"{ "choices": [{ "role": "assistant", From 420ffcbb3f948ad0898606c84a34a3368d7e4bb7 Mon Sep 17 00:00:00 2001 From: Wendy Tang Date: Fri, 7 Feb 2025 16:52:55 -0800 Subject: [PATCH 2/9] fmt --- crates/goose/src/providers/formats/openai.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index 780f1c21c..96585f08c 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -374,7 +374,7 @@ mod tests { })]; validate_tool_schemas(&mut tools); - + let params = tools[0]["function"]["parameters"].as_object().unwrap(); assert!(params.contains_key("properties")); assert!(params.contains_key("required")); @@ -395,7 +395,7 @@ mod tests { })]; validate_tool_schemas(&mut tools); - + let params = tools[0]["function"]["parameters"].as_object().unwrap(); assert_eq!(params["type"], "object"); From 7f8e77f6e634a51b5fc448903f605b42d77e00e7 Mon Sep 17 00:00:00 2001 From: angiejones Date: Mon, 10 Feb 2025 15:45:34 -0600 Subject: [PATCH 3/9] removing limitation notice in docs --- documentation/docs/tutorials/jetbrains-mcp.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/documentation/docs/tutorials/jetbrains-mcp.md b/documentation/docs/tutorials/jetbrains-mcp.md index 166b894e6..7c4c33f19 100644 --- a/documentation/docs/tutorials/jetbrains-mcp.md +++ b/documentation/docs/tutorials/jetbrains-mcp.md @@ -10,10 +10,6 @@ The JetBrains extension is designed to work within your IDE. Goose can accomplis This tutorial covers how to enable and use the JetBrains MCP Server as a built-in Goose extension to integrate with any JetBrains IDE. -:::warning Known Limitation -The JetBrains extension [does not work](https://github.com/block/goose/issues/933) with OpenAI models (e.g. gpt-4o). -::: - ## Configuration 1. Add the [MCP Server plugin](https://plugins.jetbrains.com/plugin/26071-mcp-server) to your IDE. From f909fd5141bdb4cc57c70899022b265f3927b79a Mon Sep 17 00:00:00 2001 From: Wendy Tang Date: Mon, 10 Feb 2025 15:07:32 -0800 Subject: [PATCH 4/9] check json schema for properties --- crates/goose/src/providers/formats/openai.rs | 54 +++++++++++++++----- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index 96585f08c..28b36f56c 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -254,28 +254,54 @@ pub fn get_usage(data: &Value) -> Result { } /// Validates and fixes tool schemas to ensure they have proper parameter structure. -/// If parameters exists, ensures it has properties and required fields, or removes parameters entirely. +/// If parameters exist, ensures they have properties and required fields, or removes parameters entirely. pub fn validate_tool_schemas(tools: &mut [Value]) { for tool in tools.iter_mut() { if let Some(function) = tool.get_mut("function") { if let Some(parameters) = function.get_mut("parameters") { - // If parameters exists but doesn't have properties and required, add them if parameters.is_object() { - let params_obj = parameters.as_object_mut().unwrap(); + ensure_valid_json_schema(parameters); + } + } + } + } +} - // Ensure type field exists - if !params_obj.contains_key("type") { - params_obj.insert("type".to_string(), json!("object")); - } +/// Ensures that the given JSON value follows the expected JSON Schema structure. +fn ensure_valid_json_schema(schema: &mut Value) { + if let Some(params_obj) = schema.as_object_mut() { + // Check if this is meant to be an object type schema + let is_object_type = params_obj.get("type") + .and_then(|t| t.as_str()) + .map_or(true, |t| t == "object"); // Default to true if no type is specified + + // Only apply full schema validation to object types + if is_object_type { + // Ensure properties field exists and is an object + if !params_obj.contains_key("properties") { + params_obj.insert("properties".to_string(), json!({})); + } - // Ensure properties field exists - if !params_obj.contains_key("properties") { - params_obj.insert("properties".to_string(), json!({})); - } + // Ensure required field exists and is an array + if !params_obj.contains_key("required") { + params_obj.insert("required".to_string(), json!([])); + } - // Ensure required field exists - if !params_obj.contains_key("required") { - params_obj.insert("required".to_string(), json!([])); + // Ensure type field exists + if !params_obj.contains_key("type") { + params_obj.insert("type".to_string(), json!("object")); + } + + // Recursively validate properties if it exists + 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() { + if prop.is_object() && prop.get("type") + .and_then(|t| t.as_str()) + .map_or(false, |t| t == "object") + { + ensure_valid_json_schema(prop); + } } } } From 0e33ffa3f4a4eb67174e5117284ca0601375ffed Mon Sep 17 00:00:00 2001 From: Wendy Tang Date: Mon, 10 Feb 2025 15:27:38 -0800 Subject: [PATCH 5/9] fmt --- crates/goose/src/providers/formats/openai.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index 28b36f56c..ddeafe775 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -271,7 +271,8 @@ pub fn validate_tool_schemas(tools: &mut [Value]) { fn ensure_valid_json_schema(schema: &mut Value) { if let Some(params_obj) = schema.as_object_mut() { // Check if this is meant to be an object type schema - let is_object_type = params_obj.get("type") + let is_object_type = params_obj + .get("type") .and_then(|t| t.as_str()) .map_or(true, |t| t == "object"); // Default to true if no type is specified @@ -296,9 +297,11 @@ 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() { - if prop.is_object() && prop.get("type") - .and_then(|t| t.as_str()) - .map_or(false, |t| t == "object") + if prop.is_object() + && prop + .get("type") + .and_then(|t| t.as_str()) + .map_or(false, |t| t == "object") { ensure_valid_json_schema(prop); } From b9c14287a7d16217a7901f64caafd4a2e259037e Mon Sep 17 00:00:00 2001 From: Wendy Tang Date: Mon, 10 Feb 2025 15:41:33 -0800 Subject: [PATCH 6/9] commit changes --- crates/goose/src/providers/formats/openai.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index ddeafe775..cbf65d3e1 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -298,10 +298,9 @@ fn ensure_valid_json_schema(schema: &mut Value) { if let Some(properties_obj) = properties.as_object_mut() { for (_key, prop) in properties_obj.iter_mut() { if prop.is_object() - && prop + && (prop .get("type") - .and_then(|t| t.as_str()) - .map_or(false, |t| t == "object") + .and_then(|t| t.as_str()) == Some("object")) { ensure_valid_json_schema(prop); } From 1f6c787c342dc8313dfe194a750df6d97cb1d23a Mon Sep 17 00:00:00 2001 From: Wendy Tang Date: Mon, 10 Feb 2025 15:41:48 -0800 Subject: [PATCH 7/9] fmt --- crates/goose/src/providers/formats/openai.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index cbf65d3e1..e99f8b698 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -298,9 +298,7 @@ fn ensure_valid_json_schema(schema: &mut Value) { if let Some(properties_obj) = properties.as_object_mut() { for (_key, prop) in properties_obj.iter_mut() { if prop.is_object() - && (prop - .get("type") - .and_then(|t| t.as_str()) == Some("object")) + && (prop.get("type").and_then(|t| t.as_str()) == Some("object")) { ensure_valid_json_schema(prop); } From 2fc22834d50e555b934b000ccd51c10aee519122 Mon Sep 17 00:00:00 2001 From: Wendy Tang Date: Mon, 10 Feb 2025 17:16:16 -0800 Subject: [PATCH 8/9] or_insert_with --- crates/goose/src/providers/formats/openai.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index e99f8b698..daf79e52e 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -278,20 +278,10 @@ fn ensure_valid_json_schema(schema: &mut Value) { // Only apply full schema validation to object types if is_object_type { - // Ensure properties field exists and is an object - if !params_obj.contains_key("properties") { - params_obj.insert("properties".to_string(), json!({})); - } - - // Ensure required field exists and is an array - if !params_obj.contains_key("required") { - params_obj.insert("required".to_string(), json!([])); - } - - // Ensure type field exists - if !params_obj.contains_key("type") { - params_obj.insert("type".to_string(), json!("object")); - } + // Ensure required fields exist with default values + params_obj.entry("properties").or_insert_with(|| json!({})); + params_obj.entry("required").or_insert_with(|| json!([])); + params_obj.entry("type").or_insert_with(|| json!("object")); // Recursively validate properties if it exists if let Some(properties) = params_obj.get_mut("properties") { From d85f59824963685adf5fbebc87cb0b37e8053cbb Mon Sep 17 00:00:00 2001 From: Wendy Tang Date: Tue, 11 Feb 2025 23:18:56 -0800 Subject: [PATCH 9/9] simplify test --- crates/goose/src/providers/formats/openai.rs | 33 ++++++++++++++------ 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index daf79e52e..20b5aaaae 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -288,7 +288,10 @@ fn ensure_valid_json_schema(schema: &mut Value) { if let Some(properties_obj) = properties.as_object_mut() { for (_key, prop) in properties_obj.iter_mut() { if prop.is_object() - && (prop.get("type").and_then(|t| t.as_str()) == Some("object")) + && prop + .get("type") + .and_then(|t| t.as_str()) + .map_or(false, |t| t == "object") { ensure_valid_json_schema(prop); } @@ -378,7 +381,8 @@ mod tests { #[test] fn test_validate_tool_schemas() { // Test case 1: Empty parameters object - let mut tools = vec![json!({ + // Input JSON with an incomplete parameters object + let mut actual = vec![json!({ "type": "function", "function": { "name": "test_func", @@ -389,14 +393,25 @@ mod tests { } })]; - validate_tool_schemas(&mut tools); + // Run the function to validate and update schemas + validate_tool_schemas(&mut actual); - let params = tools[0]["function"]["parameters"].as_object().unwrap(); - assert!(params.contains_key("properties")); - assert!(params.contains_key("required")); - assert_eq!(params["type"], "object"); - assert_eq!(params["properties"], json!({})); - assert_eq!(params["required"], json!([])); + // Expected JSON after validation + let expected = vec![json!({ + "type": "function", + "function": { + "name": "test_func", + "description": "test description", + "parameters": { + "type": "object", + "properties": {}, + "required": [] + } + } + })]; + + // Compare entire JSON structures instead of individual fields + assert_eq!(actual, expected); // Test case 2: Missing type field let mut tools = vec![json!({