mirror of
https://github.com/openai/codex.git
synced 2026-10-30 19:18:49 +00:00
Share tool output schemas and defer MCP envelope construction (#45439)
## Why MCP tool parsing eagerly cloned structured output schemas and built full call-result envelopes. Cloning tool definitions also copied their output-schema JSON, even before a consumer needed it. ## What changed - Introduce `ToolOutputSchema` with immutable `Arc` storage so tool definitions share output schemas when cloned. - Retain MCP structured output schemas and materialize the call-result envelope only when JSON is requested. - Update code-mode consumers and schema mutation sites to materialize JSON explicitly, reusing uniquely owned storage when possible. - Move structured content into the MCP envelope without an extra clone, preserving property order. ## Testing Add tests for JSON preservation, mutation isolation, equality between lazy and materialized schemas, reuse of uniquely owned storage, and equivalent code-mode definitions. GitOrigin-RevId: e98ba4c2f0efedc99f7cbc7bba206cc63a3bd8f4
This commit is contained in:
@@ -75,7 +75,7 @@ impl ToolExecutor<ToolInvocation> for CurrentTimeHandler {
|
||||
},
|
||||
"required": ["current_time"],
|
||||
"additionalProperties": false
|
||||
})),
|
||||
}).into()),
|
||||
})],
|
||||
})
|
||||
}
|
||||
|
||||
@@ -14,7 +14,7 @@ pub fn create_get_context_remaining_tool() -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(BTreeMap::new(), /*required*/ None, Some(false.into())),
|
||||
output_schema: Some(get_context_remaining_output_schema()),
|
||||
output_schema: Some(get_context_remaining_output_schema().into()),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -92,7 +92,7 @@ pub fn create_spawn_agent_tool_v1(options: SpawnAgentToolOptions) -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(properties, /*required*/ None, Some(false.into())),
|
||||
output_schema: Some(spawn_agent_output_schema_v1()),
|
||||
output_schema: Some(spawn_agent_output_schema_v1().into()),
|
||||
})],
|
||||
})
|
||||
}
|
||||
@@ -134,9 +134,9 @@ pub fn create_spawn_agent_tool_v2(options: SpawnAgentToolOptions) -> ToolSpec {
|
||||
Some(vec!["task_name".to_string(), "message".to_string()]),
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: Some(spawn_agent_output_schema_v2(
|
||||
options.hide_agent_type_model_reasoning,
|
||||
)),
|
||||
output_schema: Some(
|
||||
spawn_agent_output_schema_v2(options.hide_agent_type_model_reasoning).into(),
|
||||
),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -173,7 +173,7 @@ pub fn create_send_input_tool_v1() -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(properties, Some(vec!["target".to_string()]), Some(false.into())),
|
||||
output_schema: Some(send_input_output_schema()),
|
||||
output_schema: Some(send_input_output_schema().into()),
|
||||
})],
|
||||
})
|
||||
}
|
||||
@@ -256,7 +256,7 @@ pub fn create_resume_agent_tool() -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(properties, Some(vec!["id".to_string()]), Some(false.into())),
|
||||
output_schema: Some(resume_agent_output_schema()),
|
||||
output_schema: Some(resume_agent_output_schema().into()),
|
||||
})],
|
||||
})
|
||||
}
|
||||
@@ -272,7 +272,7 @@ pub fn create_wait_agent_tool_v1(options: WaitAgentTimeoutOptions) -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: wait_agent_tool_parameters_v1(options),
|
||||
output_schema: Some(wait_output_schema_v1()),
|
||||
output_schema: Some(wait_output_schema_v1().into()),
|
||||
})],
|
||||
})
|
||||
}
|
||||
@@ -285,7 +285,7 @@ pub fn create_wait_agent_tool_v2(options: WaitAgentTimeoutOptions) -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: wait_agent_tool_parameters_v2(options),
|
||||
output_schema: Some(wait_output_schema_v2()),
|
||||
output_schema: Some(wait_output_schema_v2().into()),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -306,7 +306,7 @@ pub fn create_list_agents_tool() -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(properties, /*required*/ None, Some(false.into())),
|
||||
output_schema: Some(list_agents_output_schema()),
|
||||
output_schema: Some(list_agents_output_schema().into()),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -325,9 +325,12 @@ pub fn create_close_agent_tool_v1() -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(properties, Some(vec!["target".to_string()]), Some(false.into())),
|
||||
output_schema: Some(agent_previous_status_output_schema(
|
||||
"The agent status observed before shutdown was requested.",
|
||||
)),
|
||||
output_schema: Some(
|
||||
agent_previous_status_output_schema(
|
||||
"The agent status observed before shutdown was requested.",
|
||||
)
|
||||
.into(),
|
||||
),
|
||||
})],
|
||||
})
|
||||
}
|
||||
@@ -346,9 +349,12 @@ pub fn create_interrupt_agent_tool_v2() -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(properties, Some(vec!["target".to_string()]), Some(false.into())),
|
||||
output_schema: Some(agent_previous_status_output_schema(
|
||||
"The agent status observed before the interrupt request was handled.",
|
||||
)),
|
||||
output_schema: Some(
|
||||
agent_previous_status_output_schema(
|
||||
"The agent status observed before the interrupt request was handled.",
|
||||
)
|
||||
.into(),
|
||||
),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -122,7 +122,7 @@ fn spawn_agent_tool_v2_requires_task_name_and_lists_visible_models() {
|
||||
Some(&vec!["task_name".to_string(), "message".to_string()])
|
||||
);
|
||||
assert_eq!(
|
||||
output_schema.expect("spawn_agent output schema")["required"],
|
||||
output_schema.expect("spawn_agent output schema").to_value()["required"],
|
||||
json!(["task_name", "nickname"])
|
||||
);
|
||||
}
|
||||
@@ -416,7 +416,7 @@ fn wait_agent_tool_v2_uses_timeout_only_summary_output() {
|
||||
);
|
||||
assert_eq!(parameters.required.as_ref(), None);
|
||||
assert_eq!(
|
||||
output_schema.expect("wait output schema")["properties"]["message"]["description"],
|
||||
output_schema.expect("wait output schema").to_value()["properties"]["message"]["description"],
|
||||
json!(
|
||||
"Brief wait summary without the agent's final content, including any timeout adjustment."
|
||||
)
|
||||
@@ -449,7 +449,8 @@ fn list_agents_tool_includes_path_prefix_and_agent_fields() {
|
||||
Some("Task-path prefix filter without a trailing slash. Omit to list all live agents.")
|
||||
);
|
||||
assert_eq!(
|
||||
output_schema.expect("list_agents output schema")["properties"]["agents"]["items"]["required"],
|
||||
output_schema.expect("list_agents output schema").to_value()["properties"]["agents"]["items"]
|
||||
["required"],
|
||||
json!(["agent_name", "agent_status"])
|
||||
);
|
||||
}
|
||||
@@ -462,8 +463,8 @@ fn list_agents_tool_status_schema_includes_interrupted() {
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
output_schema.expect("list_agents output schema")["properties"]["agents"]["items"]["properties"]
|
||||
["agent_status"]["allOf"][0]["oneOf"][0]["enum"],
|
||||
output_schema.expect("list_agents output schema").to_value()["properties"]["agents"]["items"]
|
||||
["properties"]["agent_status"]["allOf"][0]["oneOf"][0]["enum"],
|
||||
json!([
|
||||
"pending_init",
|
||||
"running",
|
||||
|
||||
@@ -110,7 +110,7 @@ pub(crate) fn create_exec_command_tool_with_environment_id(
|
||||
Some(vec!["cmd".to_string()]),
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: Some(unified_exec_output_schema()),
|
||||
output_schema: Some(unified_exec_output_schema().into()),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -154,7 +154,7 @@ pub fn create_write_stdin_tool() -> ToolSpec {
|
||||
Some(vec!["session_id".to_string()]),
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: Some(unified_exec_output_schema()),
|
||||
output_schema: Some(unified_exec_output_schema().into()),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -93,7 +93,7 @@ fn exec_command_tool_matches_expected_spec() {
|
||||
Some(vec!["cmd".to_string()]),
|
||||
Some(false.into())
|
||||
),
|
||||
output_schema: Some(unified_exec_output_schema()),
|
||||
output_schema: Some(unified_exec_output_schema().into()),
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -159,7 +159,7 @@ fn write_stdin_tool_matches_expected_spec() {
|
||||
Some(vec!["session_id".to_string()]),
|
||||
Some(false.into())
|
||||
),
|
||||
output_schema: Some(unified_exec_output_schema()),
|
||||
output_schema: Some(unified_exec_output_schema().into()),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -501,14 +501,16 @@ fn one_shot_exec_command_spec(spec: ToolSpec) -> ToolSpec {
|
||||
"Maximum command runtime. Defaults to 10000 ms.".to_string(),
|
||||
)),
|
||||
);
|
||||
if let Some(output_properties) = spec
|
||||
.output_schema
|
||||
.as_mut()
|
||||
.and_then(|schema| schema.get_mut("properties"))
|
||||
.and_then(serde_json::Value::as_object_mut)
|
||||
{
|
||||
output_properties.remove("session_id");
|
||||
}
|
||||
spec.output_schema = spec.output_schema.map(|schema| {
|
||||
let mut schema = schema.into_value();
|
||||
if let Some(output_properties) = schema
|
||||
.get_mut("properties")
|
||||
.and_then(serde_json::Value::as_object_mut)
|
||||
{
|
||||
output_properties.remove("session_id");
|
||||
}
|
||||
schema.into()
|
||||
});
|
||||
ToolSpec::Function(spec)
|
||||
}
|
||||
|
||||
|
||||
@@ -46,7 +46,7 @@ pub fn create_view_image_tool(options: ViewImageToolOptions) -> ToolSpec {
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(properties, Some(vec!["path".to_string()]), Some(false.into())),
|
||||
output_schema: Some(view_image_output_schema(options)),
|
||||
output_schema: Some(view_image_output_schema(options).into()),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -67,7 +67,7 @@ pub(super) fn memory_function_tool<I: JsonSchema, O: JsonSchema>(
|
||||
defer_loading: None,
|
||||
parameters: parse_tool_input_schema(&schema::input_schema_for::<I>())
|
||||
.unwrap_or_else(|err| panic!("generated input schema for {name} should parse: {err}")),
|
||||
output_schema: Some(schema::output_schema_for::<O>()),
|
||||
output_schema: Some(schema::output_schema_for::<O>().into()),
|
||||
};
|
||||
|
||||
ToolSpec::Namespace(ResponsesApiNamespace {
|
||||
|
||||
@@ -304,7 +304,7 @@ fn skill_function_tool<I: JsonSchema, O: JsonSchema>(name: &str, description: &s
|
||||
defer_loading: None,
|
||||
parameters: parse_tool_input_schema(&schema::input_schema_for::<I>())
|
||||
.unwrap_or_else(|err| panic!("generated input schema for {name} should parse: {err}")),
|
||||
output_schema: Some(schema::output_schema_for::<O>()),
|
||||
output_schema: Some(schema::output_schema_for::<O>().into()),
|
||||
};
|
||||
|
||||
ToolSpec::Namespace(ResponsesApiNamespace {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use crate::ResponsesApiNamespaceTool;
|
||||
use crate::ToolName;
|
||||
use crate::ToolOutputSchema;
|
||||
use crate::ToolSpec;
|
||||
use codex_code_mode::CodeModeToolKind;
|
||||
use codex_code_mode::ToolDefinition as CodeModeToolDefinition;
|
||||
@@ -37,7 +38,10 @@ pub fn augment_tool_spec_for_code_mode(spec: ToolSpec) -> ToolSpec {
|
||||
description: tool.description.clone(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: serde_json::to_value(&tool.parameters).ok(),
|
||||
output_schema: tool.output_schema.clone(),
|
||||
output_schema: tool
|
||||
.output_schema
|
||||
.as_ref()
|
||||
.map(ToolOutputSchema::to_value),
|
||||
};
|
||||
tool.description =
|
||||
codex_code_mode::augment_tool_definition(definition).description;
|
||||
@@ -131,7 +135,7 @@ fn code_mode_tool_definitions_for_spec(spec: &ToolSpec) -> Vec<CodeModeToolDefin
|
||||
description: tool.description.clone(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: serde_json::to_value(&tool.parameters).ok(),
|
||||
output_schema: tool.output_schema.clone(),
|
||||
output_schema: tool.output_schema.as_ref().map(ToolOutputSchema::to_value),
|
||||
}]
|
||||
}
|
||||
ToolSpec::Freeform(tool) => {
|
||||
@@ -157,7 +161,7 @@ fn code_mode_tool_definitions_for_spec(spec: &ToolSpec) -> Vec<CodeModeToolDefin
|
||||
description: tool.description.clone(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: serde_json::to_value(&tool.parameters).ok(),
|
||||
output_schema: tool.output_schema.clone(),
|
||||
output_schema: tool.output_schema.as_ref().map(ToolOutputSchema::to_value),
|
||||
}
|
||||
}
|
||||
ResponsesApiNamespaceTool::Custom(tool) => {
|
||||
|
||||
@@ -14,6 +14,27 @@ use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
#[test]
|
||||
fn code_mode_materializes_mcp_output_schemas() {
|
||||
let output = json!({"type": "object", "properties": {"ok": {"type": "boolean"}}});
|
||||
let mut tool = rmcp::model::Tool::new(
|
||||
"lookup_order",
|
||||
"Look up an order",
|
||||
std::sync::Arc::new(rmcp::model::object(json!({"type": "object"}))),
|
||||
);
|
||||
tool.output_schema = Some(std::sync::Arc::new(rmcp::model::object(output.clone())));
|
||||
let parsed =
|
||||
crate::mcp_tool_to_responses_api_tool(&ToolName::plain("lookup_order"), &tool).unwrap();
|
||||
let eager = ResponsesApiTool {
|
||||
output_schema: Some(crate::mcp_call_tool_result_output_schema(output).into()),
|
||||
..parsed.clone()
|
||||
};
|
||||
assert_eq!(
|
||||
super::collect_code_mode_tool_definitions([&ToolSpec::Function(parsed)]),
|
||||
super::collect_code_mode_tool_definitions([&ToolSpec::Function(eager)]),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_mode_tool_names_do_not_prefix_the_default_namespace() {
|
||||
for tool_name in [
|
||||
@@ -45,13 +66,16 @@ fn augment_tool_spec_for_code_mode_augments_function_tools() {
|
||||
Some(vec!["order_id".to_string()]),
|
||||
Some(AdditionalProperties::Boolean(false))
|
||||
),
|
||||
output_schema: Some(json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"ok": {"type": "boolean"}
|
||||
},
|
||||
"required": ["ok"],
|
||||
})),
|
||||
output_schema: Some(
|
||||
json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"ok": {"type": "boolean"}
|
||||
},
|
||||
"required": ["ok"],
|
||||
})
|
||||
.into()
|
||||
),
|
||||
})),
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "lookup_order".to_string(),
|
||||
@@ -72,13 +96,16 @@ declare const tools: { lookup_order(args: { order_id: string; }): Promise<{ ok:
|
||||
Some(vec!["order_id".to_string()]),
|
||||
Some(AdditionalProperties::Boolean(false))
|
||||
),
|
||||
output_schema: Some(json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"ok": {"type": "boolean"}
|
||||
},
|
||||
"required": ["ok"],
|
||||
})),
|
||||
output_schema: Some(
|
||||
json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"ok": {"type": "boolean"}
|
||||
},
|
||||
"required": ["ok"],
|
||||
})
|
||||
.into()
|
||||
),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ mod function_call_error;
|
||||
mod image_detail;
|
||||
mod json_schema;
|
||||
mod mcp_tool;
|
||||
mod output_schema;
|
||||
mod request_plugin_install;
|
||||
mod response_history;
|
||||
mod responses_api;
|
||||
@@ -40,6 +41,7 @@ pub use json_schema::parse_tool_input_schema_without_compaction;
|
||||
pub use mcp_tool::mcp_call_tool_result_output_schema;
|
||||
pub use mcp_tool::parse_agent_plugin_mcp_tool;
|
||||
pub use mcp_tool::parse_mcp_tool;
|
||||
pub use output_schema::ToolOutputSchema;
|
||||
pub use request_plugin_install::REQUEST_PLUGIN_INSTALL_APPROVAL_KIND_VALUE;
|
||||
pub use request_plugin_install::REQUEST_PLUGIN_INSTALL_PERSIST_ALWAYS_VALUE;
|
||||
pub use request_plugin_install::REQUEST_PLUGIN_INSTALL_PERSIST_KEY;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use crate::ToolDefinition;
|
||||
use crate::ToolOutputSchema;
|
||||
use crate::parse_tool_input_schema;
|
||||
use codex_utils_string::take_bytes_at_char_boundary;
|
||||
use serde_json::Value as JsonValue;
|
||||
@@ -35,12 +36,6 @@ fn parse_mcp_tool_with_description_limit(
|
||||
}
|
||||
|
||||
let input_schema = parse_tool_input_schema(&serialized_input_schema)?;
|
||||
let structured_content_schema = tool
|
||||
.output_schema
|
||||
.as_ref()
|
||||
.map(|output_schema| serde_json::Value::Object(output_schema.as_ref().clone()))
|
||||
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new()));
|
||||
|
||||
Ok(ToolDefinition {
|
||||
name: tool.name.to_string(),
|
||||
description: tool
|
||||
@@ -52,15 +47,15 @@ fn parse_mcp_tool_with_description_limit(
|
||||
})
|
||||
.unwrap_or_default(),
|
||||
input_schema,
|
||||
output_schema: Some(mcp_call_tool_result_output_schema(
|
||||
structured_content_schema,
|
||||
output_schema: Some(ToolOutputSchema::from_mcp_output_schema(
|
||||
tool.output_schema.clone(),
|
||||
)),
|
||||
defer_loading: false,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn mcp_call_tool_result_output_schema(structured_content_schema: JsonValue) -> JsonValue {
|
||||
json!({
|
||||
let mut schema = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"content": {
|
||||
@@ -69,7 +64,7 @@ pub fn mcp_call_tool_result_output_schema(structured_content_schema: JsonValue)
|
||||
"type": "object"
|
||||
}
|
||||
},
|
||||
"structuredContent": structured_content_schema,
|
||||
"structuredContent": null,
|
||||
"isError": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -79,7 +74,11 @@ pub fn mcp_call_tool_result_output_schema(structured_content_schema: JsonValue)
|
||||
},
|
||||
"required": ["content"],
|
||||
"additionalProperties": false
|
||||
})
|
||||
});
|
||||
// Replacing the placeholder keeps the existing property order while moving
|
||||
// the supplied schema instead of cloning it through json!.
|
||||
schema["properties"]["structuredContent"] = structured_content_schema;
|
||||
schema
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -34,7 +34,7 @@ fn parse_mcp_tool_inserts_empty_properties() {
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))),
|
||||
output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({})).into()),
|
||||
defer_loading: false,
|
||||
}
|
||||
);
|
||||
@@ -106,16 +106,19 @@ fn parse_mcp_tool_preserves_top_level_output_schema() {
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({
|
||||
"properties": {
|
||||
"result": {
|
||||
"properties": {
|
||||
"nested": {}
|
||||
output_schema: Some(
|
||||
mcp_call_tool_result_output_schema(serde_json::json!({
|
||||
"properties": {
|
||||
"result": {
|
||||
"properties": {
|
||||
"nested": {}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"required": ["result"]
|
||||
}))),
|
||||
},
|
||||
"required": ["result"]
|
||||
}))
|
||||
.into()
|
||||
),
|
||||
defer_loading: false,
|
||||
}
|
||||
);
|
||||
@@ -146,9 +149,12 @@ fn parse_mcp_tool_preserves_output_schema_without_inferred_type() {
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({
|
||||
"enum": ["ok", "error"]
|
||||
}))),
|
||||
output_schema: Some(
|
||||
mcp_call_tool_result_output_schema(serde_json::json!({
|
||||
"enum": ["ok", "error"]
|
||||
}))
|
||||
.into()
|
||||
),
|
||||
defer_loading: false,
|
||||
}
|
||||
);
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
//! Shares immutable output schema storage and materializes private JSON for consumers.
|
||||
|
||||
use crate::mcp_call_tool_result_output_schema;
|
||||
use serde_json::Map;
|
||||
use serde_json::Value;
|
||||
use std::sync::Arc;
|
||||
|
||||
/// Immutable output schema that shares an MCP tool's structured result schema
|
||||
/// until a consumer needs the complete call-result envelope.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ToolOutputSchema(OutputSchemaStorage);
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
enum OutputSchemaStorage {
|
||||
Json(Arc<Value>),
|
||||
McpCallToolResult(Option<Arc<Map<String, Value>>>),
|
||||
}
|
||||
|
||||
impl ToolOutputSchema {
|
||||
pub(crate) fn from_mcp_output_schema(
|
||||
structured_content: Option<Arc<Map<String, Value>>>,
|
||||
) -> Self {
|
||||
Self(OutputSchemaStorage::McpCallToolResult(structured_content))
|
||||
}
|
||||
|
||||
/// Materialize the schema for consumers that traverse JSON values.
|
||||
pub fn to_value(&self) -> Value {
|
||||
match &self.0 {
|
||||
OutputSchemaStorage::Json(schema) => schema.as_ref().clone(),
|
||||
OutputSchemaStorage::McpCallToolResult(structured_content) => {
|
||||
mcp_call_tool_result_output_schema(Value::Object(
|
||||
structured_content.as_deref().cloned().unwrap_or_default(),
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Consume the schema, reusing uniquely owned JSON and cloning shared storage.
|
||||
pub fn into_value(self) -> Value {
|
||||
match self.0 {
|
||||
OutputSchemaStorage::Json(schema) => Arc::unwrap_or_clone(schema),
|
||||
OutputSchemaStorage::McpCallToolResult(structured_content) => {
|
||||
mcp_call_tool_result_output_schema(Value::Object(
|
||||
structured_content
|
||||
.map(Arc::unwrap_or_clone)
|
||||
.unwrap_or_default(),
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<Value> for ToolOutputSchema {
|
||||
fn from(value: Value) -> Self {
|
||||
Self(OutputSchemaStorage::Json(Arc::new(value)))
|
||||
}
|
||||
}
|
||||
|
||||
impl PartialEq for ToolOutputSchema {
|
||||
fn eq(&self, other: &Self) -> bool {
|
||||
match (&self.0, &other.0) {
|
||||
(OutputSchemaStorage::Json(left), OutputSchemaStorage::Json(right)) => left == right,
|
||||
(
|
||||
OutputSchemaStorage::McpCallToolResult(left),
|
||||
OutputSchemaStorage::McpCallToolResult(right),
|
||||
) => match (left, right) {
|
||||
(Some(left), Some(right)) => left == right,
|
||||
(None, None) => true,
|
||||
(Some(schema), None) | (None, Some(schema)) => schema.is_empty(),
|
||||
},
|
||||
(OutputSchemaStorage::Json(left), OutputSchemaStorage::McpCallToolResult(_)) => {
|
||||
left.as_ref() == &other.to_value()
|
||||
}
|
||||
(OutputSchemaStorage::McpCallToolResult(_), OutputSchemaStorage::Json(right)) => {
|
||||
&self.to_value() == right.as_ref()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "output_schema_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,69 @@
|
||||
//! Verify materialization preserves schemas, isolates mutations, and respects equality.
|
||||
|
||||
use super::ToolOutputSchema;
|
||||
use pretty_assertions::assert_eq;
|
||||
use pretty_assertions::assert_ne;
|
||||
use serde_json::Value;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[test]
|
||||
fn materializing_mcp_output_preserves_json_and_keeps_mutations_private() {
|
||||
let raw: serde_json::Map<String, Value> = serde_json::from_str(
|
||||
r#"{"properties":{"z":{"enum":[9007199254740993,1.0]},"a":{"type":"string"}},"required":["z","a"]}"#,
|
||||
)
|
||||
.unwrap();
|
||||
let raw = Arc::new(raw);
|
||||
let schema = ToolOutputSchema::from_mcp_output_schema(Some(Arc::clone(&raw)));
|
||||
let mut materialized = schema.to_value();
|
||||
assert_eq!(
|
||||
serde_json::to_string(&materialized["properties"]["structuredContent"]).unwrap(),
|
||||
serde_json::to_string(&raw).unwrap(),
|
||||
);
|
||||
materialized["properties"]["structuredContent"] = Value::Null;
|
||||
assert_eq!(
|
||||
schema.to_value()["properties"]["structuredContent"],
|
||||
Value::Object(raw.as_ref().clone()),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn materialized_and_lazy_schemas_compare_in_both_directions() {
|
||||
let populated = serde_json::from_value(serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {"result": {"type": "string"}},
|
||||
}))
|
||||
.unwrap();
|
||||
for structured_content in [None, Some(Arc::default()), Some(Arc::new(populated))] {
|
||||
let lazy = ToolOutputSchema::from_mcp_output_schema(structured_content);
|
||||
let materialized = ToolOutputSchema::from(lazy.to_value());
|
||||
assert_eq!(materialized, lazy);
|
||||
assert_eq!(lazy, materialized);
|
||||
|
||||
let mut different = lazy.to_value();
|
||||
different["properties"]["structuredContent"] = serde_json::json!({"type": "null"});
|
||||
let different = ToolOutputSchema::from(different);
|
||||
assert_ne!(different, lazy);
|
||||
assert_ne!(lazy, different);
|
||||
|
||||
assert_eq!(lazy.clone().into_value(), materialized.clone().into_value());
|
||||
assert_eq!(lazy.into_value(), materialized.into_value());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn consuming_unique_json_schema_reuses_storage() {
|
||||
let value = serde_json::json!({"description": "An output schema"});
|
||||
let description_ptr = value["description"].as_str().unwrap().as_ptr();
|
||||
let schema = ToolOutputSchema::from(value);
|
||||
|
||||
let value = schema.into_value();
|
||||
|
||||
assert_eq!(
|
||||
value,
|
||||
serde_json::json!({"description": "An output schema"})
|
||||
);
|
||||
assert_eq!(
|
||||
value["description"].as_str().unwrap().as_ptr(),
|
||||
description_ptr
|
||||
);
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
use crate::JsonSchema;
|
||||
use crate::ToolDefinition;
|
||||
use crate::ToolName;
|
||||
use crate::ToolOutputSchema;
|
||||
use crate::parse_agent_plugin_mcp_tool;
|
||||
use crate::parse_dynamic_tool;
|
||||
use crate::parse_mcp_tool;
|
||||
@@ -8,7 +9,6 @@ use codex_protocol::DEFAULT_FUNCTION_NAMESPACE;
|
||||
use codex_protocol::dynamic_tools::DynamicToolFunctionSpec;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value;
|
||||
|
||||
const MAX_SERIALIZED_MCP_TOOL_BYTES: usize = 8_000;
|
||||
|
||||
@@ -40,7 +40,7 @@ pub struct ResponsesApiTool {
|
||||
pub defer_loading: Option<bool>,
|
||||
pub parameters: JsonSchema,
|
||||
#[serde(skip)]
|
||||
pub output_schema: Option<Value>,
|
||||
pub output_schema: Option<ToolOutputSchema>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
|
||||
@@ -58,7 +58,7 @@ fn tool_definition_to_responses_api_tool_omits_false_defer_loading() {
|
||||
Some(vec!["order_id".to_string()]),
|
||||
Some(false.into())
|
||||
),
|
||||
output_schema: Some(json!({"type": "object"})),
|
||||
output_schema: Some(json!({"type": "object"}).into()),
|
||||
defer_loading: false,
|
||||
}),
|
||||
ResponsesApiTool {
|
||||
@@ -74,7 +74,7 @@ fn tool_definition_to_responses_api_tool_omits_false_defer_loading() {
|
||||
Some(vec!["order_id".to_string()]),
|
||||
Some(false.into())
|
||||
),
|
||||
output_schema: Some(json!({"type": "object"})),
|
||||
output_schema: Some(json!({"type": "object"}).into()),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
use crate::JsonSchema;
|
||||
use serde_json::Value as JsonValue;
|
||||
use crate::ToolOutputSchema;
|
||||
|
||||
/// Tool metadata and schemas that downstream crates can adapt into higher-level
|
||||
/// tool specs.
|
||||
@@ -8,7 +8,7 @@ pub struct ToolDefinition {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
pub input_schema: JsonSchema,
|
||||
pub output_schema: Option<JsonValue>,
|
||||
pub output_schema: Option<ToolOutputSchema>,
|
||||
pub defer_loading: bool,
|
||||
}
|
||||
|
||||
|
||||
@@ -12,9 +12,12 @@ fn tool_definition() -> ToolDefinition {
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None,
|
||||
),
|
||||
output_schema: Some(serde_json::json!({
|
||||
"type": "object",
|
||||
})),
|
||||
output_schema: Some(
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
})
|
||||
.into(),
|
||||
),
|
||||
defer_loading: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,7 +14,7 @@ fn top_level_function_search_results_use_the_default_namespace() {
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None,
|
||||
),
|
||||
output_schema: Some(serde_json::json!({ "type": "object" })),
|
||||
output_schema: Some(serde_json::json!({ "type": "object" }).into()),
|
||||
};
|
||||
let search_info = ToolSearchInfo::from_tool_spec(
|
||||
ToolSpec::Function(function_tool.clone()),
|
||||
@@ -130,7 +130,7 @@ fn mixed_namespaced_function_and_custom_tools_are_searchable() {
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None,
|
||||
),
|
||||
output_schema: Some(serde_json::json!({"type": "object"})),
|
||||
output_schema: Some(serde_json::json!({"type": "object"}).into()),
|
||||
};
|
||||
let custom_tool = crate::FreeformTool {
|
||||
name: "apply_patch".to_string(),
|
||||
|
||||
Reference in New Issue
Block a user