mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
Validate strict Responses API tool schemas
This commit is contained in:
@@ -10,6 +10,7 @@ use super::X_OPENAI_SUBAGENT_HEADER;
|
||||
use crate::AttestationContext;
|
||||
use crate::AttestationProvider;
|
||||
use crate::GenerateAttestationFuture;
|
||||
use crate::client_common::Prompt;
|
||||
use codex_api::ApiError;
|
||||
use codex_api::ResponseEvent;
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
@@ -23,6 +24,7 @@ use codex_model_provider_info::create_oss_provider_with_base_url;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_protocol::SessionId;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
@@ -36,6 +38,9 @@ use codex_rollout_trace::RawTraceEventPayload;
|
||||
use codex_rollout_trace::RolloutTrace;
|
||||
use codex_rollout_trace::TraceWriter;
|
||||
use codex_rollout_trace::replay_bundle;
|
||||
use codex_tools::JsonSchema;
|
||||
use codex_tools::ResponsesApiTool;
|
||||
use codex_tools::ToolSpec;
|
||||
use futures::StreamExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
@@ -340,6 +345,44 @@ async fn summarize_memories_returns_empty_for_empty_input() {
|
||||
assert_eq!(output.len(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_responses_request_rejects_invalid_strict_tool_schema() -> anyhow::Result<()> {
|
||||
let client = test_model_client(SessionSource::Cli);
|
||||
let provider = create_oss_provider_with_base_url("https://example.com/v1", WireApi::Responses)
|
||||
.to_api_provider(/*auth_mode*/ None)?;
|
||||
let prompt = Prompt {
|
||||
tools: vec![ToolSpec::Function(ResponsesApiTool {
|
||||
name: "lookup_order".to_string(),
|
||||
description: "Look up an order".to_string(),
|
||||
strict: true,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::any_of(
|
||||
vec![JsonSchema::string(/*description*/ None)],
|
||||
/*description*/ None,
|
||||
),
|
||||
output_schema: None,
|
||||
})],
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let err = client
|
||||
.build_responses_request(
|
||||
&provider,
|
||||
&prompt,
|
||||
&test_model_info(),
|
||||
/*effort*/ None,
|
||||
ReasoningSummaryConfig::None,
|
||||
/*service_tier*/ None,
|
||||
)
|
||||
.expect_err("invalid strict tool schema should fail request building");
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"strict tool parameters must be an object schema"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn dropped_response_stream_traces_cancelled_partial_output() -> anyhow::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
use crate::AdditionalProperties;
|
||||
use crate::JsonSchema;
|
||||
use crate::JsonSchemaPrimitiveType;
|
||||
use crate::JsonSchemaType;
|
||||
use crate::ToolDefinition;
|
||||
use crate::ToolName;
|
||||
use crate::parse_dynamic_tool;
|
||||
@@ -26,9 +29,10 @@ pub struct FreeformToolFormat {
|
||||
pub struct ResponsesApiTool {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
/// TODO: Validation. When strict is set to true, the JSON schema,
|
||||
/// `required` and `additional_properties` must be present. All fields in
|
||||
/// `properties` must be present in `required`.
|
||||
/// Whether Responses API strict JSON Schema validation should be enforced.
|
||||
///
|
||||
/// When true, request construction fails unless the schema uses the
|
||||
/// Responses API strict subset.
|
||||
pub strict: bool,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub defer_loading: Option<bool>,
|
||||
@@ -37,6 +41,98 @@ pub struct ResponsesApiTool {
|
||||
pub output_schema: Option<Value>,
|
||||
}
|
||||
|
||||
impl ResponsesApiTool {
|
||||
pub(crate) fn validate_for_responses_api(&self) -> Result<(), serde_json::Error> {
|
||||
if !self.strict {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if !matches!(
|
||||
self.parameters.schema_type,
|
||||
Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object))
|
||||
) {
|
||||
return Err(strict_schema_error(
|
||||
"strict tool parameters must be an object schema",
|
||||
));
|
||||
}
|
||||
|
||||
validate_strict_schema(&self.parameters).map_err(strict_schema_error)
|
||||
}
|
||||
}
|
||||
|
||||
fn strict_schema_error(message: impl Into<String>) -> serde_json::Error {
|
||||
serde_json::Error::io(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
message.into(),
|
||||
))
|
||||
}
|
||||
|
||||
fn validate_strict_schema(schema: &JsonSchema) -> Result<(), String> {
|
||||
if schema_allows_object(schema) {
|
||||
let required = schema
|
||||
.required
|
||||
.as_ref()
|
||||
.ok_or_else(|| "strict object schemas must include `required`".to_string())?;
|
||||
|
||||
if !matches!(
|
||||
schema.additional_properties,
|
||||
Some(AdditionalProperties::Boolean(false))
|
||||
) {
|
||||
return Err(
|
||||
"strict object schemas must set `additionalProperties` to false".to_string(),
|
||||
);
|
||||
}
|
||||
|
||||
if let Some(properties) = &schema.properties {
|
||||
for name in properties.keys() {
|
||||
if !required.contains(name) {
|
||||
return Err(format!(
|
||||
"strict object schemas must list every property in `required`; missing `{name}`"
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(properties) = &schema.properties {
|
||||
for schema in properties.values() {
|
||||
validate_strict_schema(schema)?;
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(items) = &schema.items {
|
||||
validate_strict_schema(items)?;
|
||||
}
|
||||
|
||||
if let Some(any_of) = &schema.any_of {
|
||||
for schema in any_of {
|
||||
validate_strict_schema(schema)?;
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(defs) = &schema.defs {
|
||||
for schema in defs.values() {
|
||||
validate_strict_schema(schema)?;
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(definitions) = &schema.definitions {
|
||||
for schema in definitions.values() {
|
||||
validate_strict_schema(schema)?;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn schema_allows_object(schema: &JsonSchema) -> bool {
|
||||
match &schema.schema_type {
|
||||
Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) => true,
|
||||
Some(JsonSchemaType::Multiple(types)) => types.contains(&JsonSchemaPrimitiveType::Object),
|
||||
Some(JsonSchemaType::Single(_)) | None => false,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
#[serde(tag = "type")]
|
||||
#[allow(clippy::large_enum_variant)]
|
||||
|
||||
@@ -167,3 +167,128 @@ fn loadable_tool_spec_namespace_serializes_with_deferred_child_tools() {
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_responses_api_tool_serializes_valid_schema() {
|
||||
let tool = strict_tool(JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"order_id".to_string(),
|
||||
JsonSchema::string(/*description*/ None),
|
||||
)]),
|
||||
Some(vec!["order_id".to_string()]),
|
||||
Some(false.into()),
|
||||
));
|
||||
|
||||
tool.validate_for_responses_api()
|
||||
.expect("strict schema should validate");
|
||||
|
||||
assert_eq!(
|
||||
serde_json::to_value(tool).expect("serialize strict tool"),
|
||||
json!({
|
||||
"name": "lookup_order",
|
||||
"description": "Look up an order",
|
||||
"strict": true,
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"order_id": { "type": "string" }
|
||||
},
|
||||
"required": ["order_id"],
|
||||
"additionalProperties": false,
|
||||
},
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_responses_api_tool_rejects_invalid_schemas() {
|
||||
let cases = [
|
||||
(
|
||||
"non-object root",
|
||||
JsonSchema::any_of(
|
||||
vec![JsonSchema::string(/*description*/ None)],
|
||||
/*description*/ None,
|
||||
),
|
||||
"strict tool parameters must be an object schema",
|
||||
),
|
||||
(
|
||||
"missing required",
|
||||
JsonSchema::object(BTreeMap::new(), /*required*/ None, Some(false.into())),
|
||||
"strict object schemas must include `required`",
|
||||
),
|
||||
(
|
||||
"missing additionalProperties",
|
||||
JsonSchema::object(
|
||||
BTreeMap::new(),
|
||||
Some(Vec::new()),
|
||||
/*additional_properties*/ None,
|
||||
),
|
||||
"strict object schemas must set `additionalProperties` to false",
|
||||
),
|
||||
(
|
||||
"true additionalProperties",
|
||||
JsonSchema::object(BTreeMap::new(), Some(Vec::new()), Some(true.into())),
|
||||
"strict object schemas must set `additionalProperties` to false",
|
||||
),
|
||||
(
|
||||
"schema additionalProperties",
|
||||
JsonSchema::object(
|
||||
BTreeMap::new(),
|
||||
Some(Vec::new()),
|
||||
Some(JsonSchema::string(/*description*/ None).into()),
|
||||
),
|
||||
"strict object schemas must set `additionalProperties` to false",
|
||||
),
|
||||
(
|
||||
"property omitted from required",
|
||||
JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"order_id".to_string(),
|
||||
JsonSchema::string(/*description*/ None),
|
||||
)]),
|
||||
Some(Vec::new()),
|
||||
Some(false.into()),
|
||||
),
|
||||
"strict object schemas must list every property in `required`; missing `order_id`",
|
||||
),
|
||||
(
|
||||
"nested object missing required",
|
||||
JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"customer".to_string(),
|
||||
JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"name".to_string(),
|
||||
JsonSchema::string(/*description*/ None),
|
||||
)]),
|
||||
/*required*/ None,
|
||||
Some(false.into()),
|
||||
),
|
||||
)]),
|
||||
Some(vec!["customer".to_string()]),
|
||||
Some(false.into()),
|
||||
),
|
||||
"strict object schemas must include `required`",
|
||||
),
|
||||
];
|
||||
|
||||
for (name, parameters, expected) in cases {
|
||||
let err = match strict_tool(parameters).validate_for_responses_api() {
|
||||
Ok(()) => panic!("{name} should fail validation"),
|
||||
Err(err) => err,
|
||||
};
|
||||
|
||||
assert_eq!(err.to_string(), expected, "{name}");
|
||||
}
|
||||
}
|
||||
|
||||
fn strict_tool(parameters: JsonSchema) -> ResponsesApiTool {
|
||||
ResponsesApiTool {
|
||||
name: "lookup_order".to_string(),
|
||||
description: "Look up an order".to_string(),
|
||||
strict: true,
|
||||
defer_loading: None,
|
||||
parameters,
|
||||
output_schema: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ use crate::FreeformTool;
|
||||
use crate::JsonSchema;
|
||||
use crate::LoadableToolSpec;
|
||||
use crate::ResponsesApiNamespace;
|
||||
use crate::ResponsesApiNamespaceTool;
|
||||
use crate::ResponsesApiTool;
|
||||
use codex_protocol::config_types::WebSearchContextSize;
|
||||
use codex_protocol::config_types::WebSearchFilters as ConfigWebSearchFilters;
|
||||
@@ -78,14 +79,26 @@ impl From<LoadableToolSpec> for ToolSpec {
|
||||
pub fn create_tools_json_for_responses_api(
|
||||
tools: &[ToolSpec],
|
||||
) -> Result<Vec<Value>, serde_json::Error> {
|
||||
let mut tools_json = Vec::new();
|
||||
tools
|
||||
.iter()
|
||||
.map(|tool| {
|
||||
validate_tool_for_responses_api(tool)?;
|
||||
serde_json::to_value(tool)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
for tool in tools {
|
||||
let json = serde_json::to_value(tool)?;
|
||||
tools_json.push(json);
|
||||
fn validate_tool_for_responses_api(tool: &ToolSpec) -> Result<(), serde_json::Error> {
|
||||
match tool {
|
||||
ToolSpec::Function(tool) => tool.validate_for_responses_api(),
|
||||
ToolSpec::Namespace(namespace) => namespace.tools.iter().try_for_each(|tool| match tool {
|
||||
ResponsesApiNamespaceTool::Function(tool) => tool.validate_for_responses_api(),
|
||||
}),
|
||||
ToolSpec::ToolSearch { .. }
|
||||
| ToolSpec::ImageGeneration { .. }
|
||||
| ToolSpec::WebSearch { .. }
|
||||
| ToolSpec::Freeform(_) => Ok(()),
|
||||
}
|
||||
|
||||
Ok(tools_json)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
|
||||
@@ -149,6 +149,28 @@ fn create_tools_json_for_responses_api_includes_top_level_name() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_tools_json_for_responses_api_rejects_invalid_strict_schema() {
|
||||
let err = create_tools_json_for_responses_api(&[ToolSpec::Function(ResponsesApiTool {
|
||||
name: "demo".to_string(),
|
||||
description: "A demo tool".to_string(),
|
||||
strict: true,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(
|
||||
BTreeMap::from([("foo".to_string(), JsonSchema::string(/*description*/ None))]),
|
||||
Some(Vec::new()),
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: None,
|
||||
})])
|
||||
.expect_err("invalid strict schema should be rejected");
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"strict object schemas must list every property in `required`; missing `foo`"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn namespace_tool_spec_serializes_expected_wire_shape() {
|
||||
assert_eq!(
|
||||
|
||||
Reference in New Issue
Block a user