mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
chore: add JSON schema policy fixture coverage (#24152)
## Why Before changing the Codex Bridge JSON schema policy, add integration coverage around real connector-like MCP tool schemas. The existing unit tests cover individual sanitizer behaviors, but they do not make it easy to see whether full fixture schemas keep model-visible guidance, prune only unreachable definitions, drop unsupported JSON Schema fields, and stay within the Responses API schema budget. ## What Changed - Added `tools/tests/json_schema_policy_fixtures.rs`, which converts MCP tool fixtures through `mcp_tool_to_responses_api_tool` and validates the resulting Responses tool parameters. - Added connector-style fixtures for Slack, Google Calendar, Google Drive, Notion, and Microsoft Outlook Email under `tools/tests/fixtures/json_schema_policy/`. - Added fixture assertions for preserved guidance, pruned definitions, expected field drops after `JsonSchema` conversion, marker count baselines, and dangling local `$ref` prevention. - Added a real oversized golden Notion `create_page` input schema fixture to exercise the compaction path that strips descriptions, drops root `$defs`, rewrites local refs, and fits the compacted schema under the budget.
This commit is contained in:
committed by
GitHub
Unverified
parent
7924743c38
commit
10ac2781eb
Generated
+1
@@ -3770,6 +3770,7 @@ dependencies = [
|
||||
"codex-features",
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-cargo-bin",
|
||||
"codex-utils-output-truncation",
|
||||
"codex-utils-pty",
|
||||
"codex-utils-string",
|
||||
|
||||
@@ -31,6 +31,7 @@ tracing = { workspace = true }
|
||||
urlencoding = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
codex-utils-cargo-bin = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
|
||||
[lib]
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
{
|
||||
"source": "standard/google_calendar/tools",
|
||||
"tools": [
|
||||
{
|
||||
"name": "google_calendar_create_event",
|
||||
"description": "Create a calendar event.",
|
||||
"input_schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"event": {
|
||||
"$ref": "#/definitions/Event"
|
||||
},
|
||||
"notify_attendees": {
|
||||
"type": "boolean",
|
||||
"description": "Whether attendees should receive notifications."
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"event"
|
||||
],
|
||||
"definitions": {
|
||||
"Event": {
|
||||
"type": "object",
|
||||
"description": "Calendar event payload.",
|
||||
"properties": {
|
||||
"title": {
|
||||
"type": "string",
|
||||
"description": "Event title."
|
||||
},
|
||||
"start": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/DateTime"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
"DateTime": {
|
||||
"type": "object",
|
||||
"description": "Calendar date-time.",
|
||||
"properties": {
|
||||
"dateTime": {
|
||||
"type": "string",
|
||||
"format": "date-time",
|
||||
"description": "RFC3339 date-time."
|
||||
},
|
||||
"timeZone": {
|
||||
"type": "string",
|
||||
"enum": [
|
||||
"UTC",
|
||||
"America/Los_Angeles"
|
||||
],
|
||||
"description": "IANA time zone."
|
||||
}
|
||||
}
|
||||
},
|
||||
"UnusedCalendarResource": {
|
||||
"type": "string",
|
||||
"description": "Unreachable calendar resource."
|
||||
}
|
||||
}
|
||||
},
|
||||
"expected_preserved": [
|
||||
{
|
||||
"pointer": "/properties/event/$ref",
|
||||
"value": "#/definitions/Event"
|
||||
},
|
||||
{
|
||||
"pointer": "/definitions/DateTime/properties/timeZone/enum/1",
|
||||
"value": "America/Los_Angeles"
|
||||
}
|
||||
],
|
||||
"expected_pruned": [
|
||||
"/definitions/UnusedCalendarResource"
|
||||
],
|
||||
"expected_dropped_fields": [
|
||||
"/definitions/DateTime/properties/dateTime/format"
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,65 @@
|
||||
{
|
||||
"source": "standard/google_drive/tools",
|
||||
"tools": [
|
||||
{
|
||||
"name": "google_drive_copy_file",
|
||||
"description": "Copy a Google Drive file.",
|
||||
"input_schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"file": {
|
||||
"description": "File selector.",
|
||||
"oneOf": [
|
||||
{
|
||||
"type": "string",
|
||||
"description": "A Drive file ID."
|
||||
},
|
||||
{
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"shared_drive_id": {
|
||||
"type": "string",
|
||||
"description": "Shared drive identifier."
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
},
|
||||
"metadata": {
|
||||
"description": "Optional copy metadata.",
|
||||
"allOf": [
|
||||
{
|
||||
"type": "object",
|
||||
"description": "Base metadata.",
|
||||
"properties": {
|
||||
"source": {
|
||||
"type": "string",
|
||||
"description": "Metadata source."
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
},
|
||||
"title": {
|
||||
"type": "string",
|
||||
"description": "Copied file title."
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"file"
|
||||
]
|
||||
},
|
||||
"expected_preserved": [
|
||||
{
|
||||
"pointer": "/properties/title/description",
|
||||
"value": "Copied file title."
|
||||
}
|
||||
],
|
||||
"expected_pruned": [],
|
||||
"expected_dropped_fields": [
|
||||
"/properties/file/oneOf",
|
||||
"/properties/metadata/allOf"
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
+90
@@ -0,0 +1,90 @@
|
||||
{
|
||||
"source": "golden/microsoft_outlook_email/tools",
|
||||
"tools": [
|
||||
{
|
||||
"name": "microsoft_outlook_email_send",
|
||||
"description": "Send an Outlook email.",
|
||||
"input_schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"message_id": {
|
||||
"$ref": "#/$defs/Message/properties/id"
|
||||
},
|
||||
"attachment": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"$ref": "#/$defs/Attachment"
|
||||
}
|
||||
}
|
||||
},
|
||||
"$defs": {
|
||||
"Message": {
|
||||
"type": "object",
|
||||
"description": "Outlook message.",
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": "string",
|
||||
"description": "Message identifier."
|
||||
},
|
||||
"sender": {
|
||||
"$ref": "#/$defs/EmailAddress"
|
||||
},
|
||||
"importance": {
|
||||
"type": "string",
|
||||
"enum": [
|
||||
"low",
|
||||
"normal",
|
||||
"high"
|
||||
],
|
||||
"description": "Message importance."
|
||||
}
|
||||
}
|
||||
},
|
||||
"Attachment": {
|
||||
"type": "object",
|
||||
"description": "Email attachment.",
|
||||
"properties": {
|
||||
"name": {
|
||||
"type": "string",
|
||||
"description": "Attachment file name."
|
||||
}
|
||||
}
|
||||
},
|
||||
"EmailAddress": {
|
||||
"type": "object",
|
||||
"description": "Email address object.",
|
||||
"properties": {
|
||||
"address": {
|
||||
"type": "string",
|
||||
"description": "SMTP address."
|
||||
}
|
||||
}
|
||||
},
|
||||
"GiantUnusedPayload": {
|
||||
"type": "object",
|
||||
"description": "Representative unreachable Outlook payload.",
|
||||
"properties": {
|
||||
"opaque": {
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"expected_preserved": [
|
||||
{
|
||||
"pointer": "/properties/message_id/$ref",
|
||||
"value": "#/$defs/Message/properties/id"
|
||||
},
|
||||
{
|
||||
"pointer": "/$defs/Message/properties/importance/enum/2",
|
||||
"value": "high"
|
||||
}
|
||||
],
|
||||
"expected_pruned": [
|
||||
"/$defs/GiantUnusedPayload"
|
||||
],
|
||||
"expected_dropped_fields": []
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,72 @@
|
||||
{
|
||||
"source": "golden/notion/tools",
|
||||
"tools": [
|
||||
{
|
||||
"name": "notion_fetch_page",
|
||||
"description": "Fetch a Notion page.",
|
||||
"input_schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"page": {
|
||||
"$ref": "#/$defs/Page%20Ref"
|
||||
},
|
||||
"filter": {
|
||||
"type": "object",
|
||||
"additionalProperties": {
|
||||
"$ref": "#/$defs/Filter"
|
||||
}
|
||||
},
|
||||
"content_mode": {
|
||||
"description": "How much content to return.",
|
||||
"anyOf": [
|
||||
{
|
||||
"type": "string",
|
||||
"enum": [
|
||||
"summary",
|
||||
"full"
|
||||
]
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
"$defs": {
|
||||
"Page Ref": {
|
||||
"type": "string",
|
||||
"description": "Notion page ID or URL."
|
||||
},
|
||||
"Filter": {
|
||||
"type": "object",
|
||||
"description": "Filter object.",
|
||||
"properties": {
|
||||
"created_by": {
|
||||
"type": "string",
|
||||
"description": "Creator user ID."
|
||||
}
|
||||
}
|
||||
},
|
||||
"UnusedDatabase": {
|
||||
"type": "object",
|
||||
"description": "Unreachable Notion database schema."
|
||||
}
|
||||
}
|
||||
},
|
||||
"expected_preserved": [
|
||||
{
|
||||
"pointer": "/properties/page/$ref",
|
||||
"value": "#/$defs/Page%20Ref"
|
||||
},
|
||||
{
|
||||
"pointer": "/properties/content_mode/anyOf/0/enum/0",
|
||||
"value": "summary"
|
||||
}
|
||||
],
|
||||
"expected_pruned": [
|
||||
"/$defs/UnusedDatabase"
|
||||
],
|
||||
"expected_dropped_fields": []
|
||||
}
|
||||
]
|
||||
}
|
||||
Vendored
+1124
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,75 @@
|
||||
{
|
||||
"source": "standard/slack/tools",
|
||||
"tools": [
|
||||
{
|
||||
"name": "slack_schedule_message",
|
||||
"description": "Schedule a Slack message.",
|
||||
"input_schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"channel_id": {
|
||||
"type": "string",
|
||||
"description": "Slack channel ID."
|
||||
},
|
||||
"message": {
|
||||
"type": "string",
|
||||
"description": "Message text."
|
||||
},
|
||||
"post_at": {
|
||||
"type": "integer",
|
||||
"description": "Unix timestamp for delivery."
|
||||
},
|
||||
"thread_ts": {
|
||||
"description": "Optional parent thread timestamp.",
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/$defs/SlackTs"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"channel_id",
|
||||
"message",
|
||||
"post_at"
|
||||
],
|
||||
"additionalProperties": false,
|
||||
"$defs": {
|
||||
"SlackTs": {
|
||||
"type": "string",
|
||||
"description": "Slack timestamp string.",
|
||||
"pattern": "^[0-9]+[.][0-9]+$"
|
||||
},
|
||||
"UnusedPayload": {
|
||||
"type": "object",
|
||||
"description": "Large unreachable Slack payload.",
|
||||
"properties": {
|
||||
"debug": {
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"expected_preserved": [
|
||||
{
|
||||
"pointer": "/properties/thread_ts/anyOf/0/$ref",
|
||||
"value": "#/$defs/SlackTs"
|
||||
},
|
||||
{
|
||||
"pointer": "/$defs/SlackTs/description",
|
||||
"value": "Slack timestamp string."
|
||||
}
|
||||
],
|
||||
"expected_pruned": [
|
||||
"/$defs/UnusedPayload"
|
||||
],
|
||||
"expected_dropped_fields": [
|
||||
"/$defs/SlackTs/pattern"
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,224 @@
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::mcp_tool_to_responses_api_tool;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde::Deserialize;
|
||||
use serde::de::DeserializeOwned;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use std::fs;
|
||||
use std::sync::Arc;
|
||||
|
||||
const FIXTURE_PATHS: [&str; 5] = [
|
||||
"tests/fixtures/json_schema_policy/slack.json",
|
||||
"tests/fixtures/json_schema_policy/google_calendar.json",
|
||||
"tests/fixtures/json_schema_policy/google_drive.json",
|
||||
"tests/fixtures/json_schema_policy/notion.json",
|
||||
"tests/fixtures/json_schema_policy/microsoft_outlook_email.json",
|
||||
];
|
||||
const OVERSIZED_NOTION_CREATE_PAGE_SCHEMA_PATH: &str =
|
||||
"tests/fixtures/json_schema_policy/oversized_notion_create_page_input_schema.json";
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct FixtureFile {
|
||||
source: String,
|
||||
tools: Vec<FixtureTool>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct FixtureTool {
|
||||
name: String,
|
||||
description: String,
|
||||
input_schema: Value,
|
||||
#[serde(default)]
|
||||
expected_preserved: Vec<ExpectedValue>,
|
||||
#[serde(default)]
|
||||
expected_pruned: Vec<String>,
|
||||
#[serde(default)]
|
||||
expected_dropped_fields: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct ExpectedValue {
|
||||
pointer: String,
|
||||
value: Value,
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn json_schema_policy_fixtures_convert_to_responses_tools() {
|
||||
for fixture in FIXTURE_PATHS.into_iter().map(load_fixture::<FixtureFile>) {
|
||||
for fixture_tool in &fixture.tools {
|
||||
let responses_tool = convert_fixture_tool(&fixture, fixture_tool);
|
||||
let parameters = serde_json::to_value(&responses_tool.parameters)
|
||||
.expect("responses parameters should serialize");
|
||||
|
||||
let expected_fields = [
|
||||
(
|
||||
"preserve the tool name",
|
||||
json!(fixture_tool.name),
|
||||
json!(responses_tool.name),
|
||||
),
|
||||
(
|
||||
"preserve the tool description",
|
||||
json!(fixture_tool.description),
|
||||
json!(responses_tool.description),
|
||||
),
|
||||
(
|
||||
"remain a strict:false tool",
|
||||
json!(false),
|
||||
json!(responses_tool.strict),
|
||||
),
|
||||
(
|
||||
"produce object-shaped parameters",
|
||||
json!("object"),
|
||||
parameters.get("type").cloned().unwrap_or(Value::Null),
|
||||
),
|
||||
];
|
||||
|
||||
for (message, expected, actual) in expected_fields {
|
||||
assert_eq!(actual, expected, "{} should {message}", fixture_tool.name);
|
||||
}
|
||||
assert!(
|
||||
parameters.get("properties").is_some_and(Value::is_object),
|
||||
"{} should produce a parameters.properties object",
|
||||
fixture_tool.name
|
||||
);
|
||||
|
||||
for expected in &fixture_tool.expected_preserved {
|
||||
assert_eq!(
|
||||
parameters.pointer(&expected.pointer),
|
||||
Some(&expected.value),
|
||||
"{} should preserve {}",
|
||||
fixture_tool.name,
|
||||
expected.pointer
|
||||
);
|
||||
}
|
||||
|
||||
for pointer in &fixture_tool.expected_pruned {
|
||||
assert!(
|
||||
parameters.pointer(pointer).is_none(),
|
||||
"{} should prune unreachable definition {pointer}",
|
||||
fixture_tool.name
|
||||
);
|
||||
}
|
||||
|
||||
for pointer in &fixture_tool.expected_dropped_fields {
|
||||
assert!(
|
||||
fixture_tool.input_schema.pointer(pointer).is_some(),
|
||||
"{} fixture should contain expected dropped field {pointer}",
|
||||
fixture_tool.name
|
||||
);
|
||||
assert!(
|
||||
parameters.pointer(pointer).is_none(),
|
||||
"{} should drop field {pointer} after JsonSchema conversion",
|
||||
fixture_tool.name
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn json_schema_policy_oversized_golden_schema_triggers_compaction() {
|
||||
let fixture: FixtureFile = load_fixture(OVERSIZED_NOTION_CREATE_PAGE_SCHEMA_PATH);
|
||||
let fixture_tool = fixture
|
||||
.tools
|
||||
.first()
|
||||
.expect("oversized fixture should contain a tool");
|
||||
let input_bytes = compact_json_len(&fixture_tool.input_schema);
|
||||
|
||||
let responses_tool = convert_fixture_tool(&fixture, fixture_tool);
|
||||
let parameters =
|
||||
serde_json::to_value(&responses_tool.parameters).expect("responses parameters serialize");
|
||||
let output_bytes = compact_json_len(¶meters);
|
||||
|
||||
assert!(
|
||||
output_bytes < input_bytes,
|
||||
"compaction should reduce schema size from {input_bytes} bytes"
|
||||
);
|
||||
|
||||
let absent_pointers = [
|
||||
("/description", "drop root description"),
|
||||
("/properties/parent/description", "drop nested descriptions"),
|
||||
(
|
||||
"/$defs",
|
||||
"drop root definitions after stripping descriptions is insufficient",
|
||||
),
|
||||
];
|
||||
for (pointer, message) in absent_pointers {
|
||||
assert!(
|
||||
parameters.pointer(pointer).is_none(),
|
||||
"oversized schema should {message}"
|
||||
);
|
||||
}
|
||||
|
||||
let expected_values = [
|
||||
(
|
||||
"/properties/parent",
|
||||
json!({}),
|
||||
"rewrite local refs before dropping root definitions",
|
||||
),
|
||||
(
|
||||
"/properties/children/items",
|
||||
json!({}),
|
||||
"rewrite nested local refs before dropping root definitions",
|
||||
),
|
||||
(
|
||||
"/properties/markdown/type",
|
||||
json!("string"),
|
||||
"retain top-level argument shape",
|
||||
),
|
||||
(
|
||||
"/properties/properties/type",
|
||||
json!("object"),
|
||||
"retain object argument shape",
|
||||
),
|
||||
];
|
||||
for (pointer, expected, message) in expected_values {
|
||||
assert_eq!(
|
||||
parameters.pointer(pointer),
|
||||
Some(&expected),
|
||||
"oversized schema should {message}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn load_fixture<T: DeserializeOwned>(path: &str) -> T {
|
||||
let path = codex_utils_cargo_bin::find_resource!(path)
|
||||
.unwrap_or_else(|err| panic!("resolve fixture {path}: {err}"));
|
||||
let fixture = fs::read_to_string(&path)
|
||||
.unwrap_or_else(|err| panic!("read fixture {}: {err}", path.display()));
|
||||
serde_json::from_str(&fixture)
|
||||
.unwrap_or_else(|err| panic!("parse fixture {}: {err}", path.display()))
|
||||
}
|
||||
|
||||
fn convert_fixture_tool(
|
||||
fixture: &FixtureFile,
|
||||
fixture_tool: &FixtureTool,
|
||||
) -> codex_tools::ResponsesApiTool {
|
||||
let name = &fixture_tool.name;
|
||||
let input_schema = fixture_tool
|
||||
.input_schema
|
||||
.as_object()
|
||||
.unwrap_or_else(|| panic!("{name} input_schema should be an object"))
|
||||
.clone();
|
||||
let tool = rmcp::model::Tool {
|
||||
name: name.to_string().into(),
|
||||
title: None,
|
||||
description: Some(fixture_tool.description.clone().into()),
|
||||
input_schema: Arc::new(input_schema),
|
||||
output_schema: None,
|
||||
annotations: None,
|
||||
execution: None,
|
||||
icons: None,
|
||||
meta: None,
|
||||
};
|
||||
|
||||
mcp_tool_to_responses_api_tool(&ToolName::namespaced(&fixture.source, name), &tool)
|
||||
.unwrap_or_else(|err| panic!("convert {name} from {}: {err}", fixture.source))
|
||||
}
|
||||
|
||||
fn compact_json_len(value: &Value) -> usize {
|
||||
serde_json::to_vec(value)
|
||||
.unwrap_or_else(|err| panic!("serialize compact JSON: {err}"))
|
||||
.len()
|
||||
}
|
||||
Reference in New Issue
Block a user