Skip to content

Commit 139fb54

Browse files
committed
fix(openai): normalize tool schemas for provider requests
Signed-off-by: Istvan Benedek <istvan.benedek.dev@gmail.com>
1 parent b1eff5f commit 139fb54

File tree

3 files changed

+1054
-29
lines changed

3 files changed

+1054
-29
lines changed

crates/goose/src/providers/chatgpt_codex.rs

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::model::ModelConfig;
44
use crate::providers::api_client::AuthProvider;
55
use crate::providers::base::{ConfigKey, MessageStream, Provider, ProviderDef, ProviderMetadata};
66
use crate::providers::errors::ProviderError;
7+
use crate::providers::formats::openai::validate_tool_schemas;
78
use crate::providers::formats::openai_responses::responses_api_to_streaming_message;
89
use crate::providers::openai_compatible::handle_status_openai_compat;
910
use crate::providers::retry::ProviderRetry;
@@ -268,7 +269,7 @@ fn create_codex_request(
268269
.ok_or_else(|| anyhow!("Codex payload must be a JSON object"))?;
269270

270271
if !tools.is_empty() {
271-
let tools_spec: Vec<Value> = tools
272+
let mut tools_spec: Vec<Value> = tools
272273
.iter()
273274
.map(|tool| {
274275
json!({
@@ -280,6 +281,8 @@ fn create_codex_request(
280281
})
281282
.collect();
282283

284+
validate_tool_schemas(&mut tools_spec);
285+
283286
payload_obj.insert("tools".to_string(), json!(tools_spec));
284287
payload_obj.insert("tool_choice".to_string(), json!("auto"));
285288
payload_obj.insert("parallel_tool_calls".to_string(), json!(true));
@@ -1015,7 +1018,7 @@ mod tests {
10151018
use crate::conversation::message::Message;
10161019
use goose_test_support::TEST_IMAGE_B64;
10171020
use jsonwebtoken::{Algorithm, EncodingKey, Header};
1018-
use rmcp::model::{CallToolRequestParams, CallToolResult, Content, ErrorCode, ErrorData};
1021+
use rmcp::model::{CallToolRequestParams, CallToolResult, Content, ErrorCode, ErrorData, Tool};
10191022
use rmcp::object;
10201023
use test_case::test_case;
10211024
use wiremock::matchers::{body_string_contains, method, path};
@@ -1042,6 +1045,17 @@ mod tests {
10421045
.unwrap_or_default()
10431046
}
10441047

1048+
fn schema_contains_key(value: &Value, needle: &str) -> bool {
1049+
match value {
1050+
Value::Object(map) => {
1051+
map.contains_key(needle)
1052+
|| map.values().any(|child| schema_contains_key(child, needle))
1053+
}
1054+
Value::Array(items) => items.iter().any(|child| schema_contains_key(child, needle)),
1055+
_ => false,
1056+
}
1057+
}
1058+
10451059
#[test_case(
10461060
vec![
10471061
Message::user().with_text("user text"),
@@ -1312,4 +1326,42 @@ mod tests {
13121326
let instructions = payload["instructions"].as_str().unwrap();
13131327
assert_eq!(instructions, "system prompt");
13141328
}
1329+
1330+
#[test]
1331+
fn test_codex_request_sanitizes_tool_schema() {
1332+
let model = ModelConfig::new("gpt-5.4").unwrap();
1333+
let tool = Tool::new(
1334+
"render_treemap",
1335+
"Render a treemap",
1336+
object!({
1337+
"$defs": {
1338+
"TreemapNode": {
1339+
"type": "object",
1340+
"properties": {
1341+
"name": { "type": "string" },
1342+
"children": {
1343+
"type": ["array", "null"],
1344+
"items": { "$ref": "#/$defs/TreemapNode" }
1345+
}
1346+
},
1347+
"required": ["name"],
1348+
"additionalProperties": false
1349+
}
1350+
},
1351+
"$ref": "#/$defs/TreemapNode"
1352+
}),
1353+
);
1354+
1355+
let payload = create_codex_request(&model, "system prompt", &[], &[tool]).unwrap();
1356+
let parameters = &payload["tools"][0]["parameters"];
1357+
1358+
assert!(!schema_contains_key(parameters, "$defs"));
1359+
assert!(!schema_contains_key(parameters, "$ref"));
1360+
assert!(!schema_contains_key(parameters, "anyOf"));
1361+
assert_eq!(parameters["type"], "object");
1362+
assert_eq!(
1363+
parameters["properties"]["children"]["items"]["type"],
1364+
"object"
1365+
);
1366+
}
13151367
}

0 commit comments

Comments
 (0)