mirror of
https://github.com/openai/codex.git
synced 2026-05-19 02:33:10 +00:00
fix: default unknown tool schemas to empty schemas (#22380)
## Why
Some tool providers, especially MCP servers and dynamic tool sources,
can supply schema nodes that omit `type` and have no recognized JSON
Schema shape hints. Previously, `sanitize_json_schema` filled those
unknown nodes in as `string`, which made the schema parseable but
invented a scalar constraint that the provider did not specify. For
description-only fields, that could incorrectly steer tool arguments
away from the provider's actual accepted shape.
The Responses API accepts permissive empty schemas such as `{}` at
nested property positions, so Codex should preserve that permissive
meaning instead of coercing unknown schema nodes into a misleading
scalar type.
## What Changed
- Changed the no-hints fallback in `codex-rs/tools/src/json_schema.rs`
to clear unrecognized object schema nodes to `{}`.
- Empty schemas now remain `{}` rather than becoming `type: "string"`.
- Description-only or otherwise metadata-only nested property schemas
now become `{}` while surrounding object/array/string/number inference
still applies when recognized hints are present.
- Updated `codex-tools` and `codex-core` tests to cover top-level empty
schemas, nested empty schemas, metadata-only malformed schemas, dynamic
tools, and MCP tool specs.
## Verification
- `cargo test -p codex-tools`
- `cargo test -p codex-core
test_mcp_tool_property_missing_type_defaults_to_empty_schema`
- Manually verified the real Responses API behavior for both
empty-schema positions:
- Top-level function `parameters: {}` is accepted and echoed back as
`{"type":"object","properties":{}}`; when forced to call the tool,
Responses emitted empty object arguments: `"arguments": "{}"`.
- Nested property schema `{}` is accepted and preserved as `{}`; when
forced to call a tool with `metadata.extra`, Responses emitted
`"arguments": "{\"metadata\":{\"extra\":\"codex schema sanitizer
behavior\"}}"`.
This commit is contained in:
@@ -907,7 +907,7 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_mcp_tool_property_missing_type_defaults_to_string() {
|
||||
async fn test_mcp_tool_property_missing_type_defaults_to_empty_schema() {
|
||||
let config = test_config().await;
|
||||
let model_info = construct_model_info_offline("gpt-5.4", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
@@ -950,10 +950,7 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() {
|
||||
name: "search".to_string(),
|
||||
parameters: JsonSchema::object(
|
||||
/*properties*/
|
||||
BTreeMap::from([(
|
||||
"query".to_string(),
|
||||
JsonSchema::string(Some("search query".to_string())),
|
||||
)]),
|
||||
BTreeMap::from([("query".to_string(), JsonSchema::default())]),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
|
||||
@@ -27,10 +27,7 @@ fn parse_dynamic_tool_sanitizes_input_schema() {
|
||||
name: "lookup_ticket".to_string(),
|
||||
description: "Fetch a ticket".to_string(),
|
||||
input_schema: JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"id".to_string(),
|
||||
JsonSchema::string(Some("Ticket identifier".to_string()),),
|
||||
)]),
|
||||
BTreeMap::from([("id".to_string(), JsonSchema::default(),)]),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
|
||||
@@ -166,6 +166,7 @@ pub fn parse_tool_input_schema(input_schema: &JsonValue) -> Result<JsonSchema, s
|
||||
/// - Collapses `const` into single-value `enum`.
|
||||
/// - Fills required child fields for object/array schema types, including
|
||||
/// nullable unions, with permissive defaults when absent.
|
||||
/// - Coerces object schemas with no recognized schema hints into `{}`.
|
||||
fn sanitize_json_schema(value: &mut JsonValue) {
|
||||
match value {
|
||||
JsonValue::Bool(_) => {
|
||||
@@ -228,7 +229,8 @@ fn sanitize_json_schema(value: &mut JsonValue) {
|
||||
{
|
||||
schema_types.push(JsonSchemaPrimitiveType::Number);
|
||||
} else {
|
||||
schema_types.push(JsonSchemaPrimitiveType::String);
|
||||
map.clear();
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -34,7 +34,8 @@ fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() {
|
||||
//
|
||||
// Expected normalization behavior:
|
||||
// - `properties` implies an object schema when `type` is omitted.
|
||||
// - The child property keeps its description and defaults to a string type.
|
||||
// - The child property has no recognized schema hints, so it is coerced to
|
||||
// an empty permissive schema.
|
||||
let schema = parse_tool_input_schema(&serde_json::json!({
|
||||
"properties": {
|
||||
"query": {"description": "search query"}
|
||||
@@ -45,16 +46,33 @@ fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() {
|
||||
assert_eq!(
|
||||
schema,
|
||||
JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"query".to_string(),
|
||||
JsonSchema::string(Some("search query".to_string())),
|
||||
)]),
|
||||
BTreeMap::from([("query".to_string(), JsonSchema::default())]),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_tool_input_schema_coerces_unrecognized_object_schema_to_empty_schema() {
|
||||
// Example schema shape:
|
||||
// {
|
||||
// "description": "Ticket identifier",
|
||||
// "title": "Ticket ID"
|
||||
// }
|
||||
//
|
||||
// Expected normalization behavior:
|
||||
// - Object schemas with no recognized schema hints are treated as
|
||||
// malformed and coerced to the empty permissive schema.
|
||||
let schema = parse_tool_input_schema(&serde_json::json!({
|
||||
"description": "Ticket identifier",
|
||||
"title": "Ticket ID"
|
||||
}))
|
||||
.expect("parse schema");
|
||||
|
||||
assert_eq!(schema, JsonSchema::default());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_tool_input_schema_preserves_integer_and_defaults_array_items() {
|
||||
// Example schema shape:
|
||||
@@ -250,16 +268,62 @@ fn parse_tool_input_schema_infers_string_from_enum_const_and_format_keywords() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_tool_input_schema_defaults_empty_schema_to_string() {
|
||||
fn parse_tool_input_schema_preserves_empty_schema() {
|
||||
// Example schema shape:
|
||||
// {}
|
||||
//
|
||||
// Expected normalization behavior:
|
||||
// - With no structural hints at all, the normalizer falls back to a
|
||||
// permissive string schema.
|
||||
// - An empty JSON Schema is already a valid permissive schema, so it stays
|
||||
// empty rather than being rewritten as an object schema.
|
||||
let schema = parse_tool_input_schema(&serde_json::json!({})).expect("parse schema");
|
||||
|
||||
assert_eq!(schema, JsonSchema::string(/*description*/ None));
|
||||
assert_eq!(schema, JsonSchema::default());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_tool_input_schema_preserves_nested_empty_schema() {
|
||||
// Example schema shape:
|
||||
// {
|
||||
// "type": "object",
|
||||
// "properties": {
|
||||
// "metadata": {
|
||||
// "properties": {
|
||||
// "extra": {}
|
||||
// }
|
||||
// }
|
||||
// }
|
||||
// }
|
||||
//
|
||||
// Expected normalization behavior:
|
||||
// - The sanitizer recurses through nested object properties.
|
||||
// - The innermost `extra` field is an empty JSON Schema and stays empty.
|
||||
let schema = parse_tool_input_schema(&serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"metadata": {
|
||||
"properties": {
|
||||
"extra": {}
|
||||
}
|
||||
}
|
||||
}
|
||||
}))
|
||||
.expect("parse schema");
|
||||
|
||||
assert_eq!(
|
||||
schema,
|
||||
JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"metadata".to_string(),
|
||||
JsonSchema::object(
|
||||
BTreeMap::from([("extra".to_string(), JsonSchema::default())]),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None,
|
||||
)
|
||||
)]),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None,
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user