diff --git a/codex-rs/core/src/tools/handlers/current_time.rs b/codex-rs/core/src/tools/handlers/current_time.rs index 4cd410595e..5d48896011 100644 --- a/codex-rs/core/src/tools/handlers/current_time.rs +++ b/codex-rs/core/src/tools/handlers/current_time.rs @@ -75,7 +75,7 @@ impl ToolExecutor for CurrentTimeHandler { }, "required": ["current_time"], "additionalProperties": false - })), + }).into()), })], }) } diff --git a/codex-rs/core/src/tools/handlers/get_context_remaining_spec.rs b/codex-rs/core/src/tools/handlers/get_context_remaining_spec.rs index 4ff54243f1..102dfbd138 100644 --- a/codex-rs/core/src/tools/handlers/get_context_remaining_spec.rs +++ b/codex-rs/core/src/tools/handlers/get_context_remaining_spec.rs @@ -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()), }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_spec.rs b/codex-rs/core/src/tools/handlers/multi_agents_spec.rs index 8d9d48ef3d..2e11d899b7 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_spec.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_spec.rs @@ -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(), + ), }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_spec_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_spec_tests.rs index e789a55da8..888a11376a 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_spec_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_spec_tests.rs @@ -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", diff --git a/codex-rs/core/src/tools/handlers/shell_spec.rs b/codex-rs/core/src/tools/handlers/shell_spec.rs index b39408a375..cabe62cbe8 100644 --- a/codex-rs/core/src/tools/handlers/shell_spec.rs +++ b/codex-rs/core/src/tools/handlers/shell_spec.rs @@ -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()), }) } diff --git a/codex-rs/core/src/tools/handlers/shell_spec_tests.rs b/codex-rs/core/src/tools/handlers/shell_spec_tests.rs index 000add0bbd..ee2886a159 100644 --- a/codex-rs/core/src/tools/handlers/shell_spec_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_spec_tests.rs @@ -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()), }) ); } diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index 37c0f3c823..0a026fbb6d 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -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) } diff --git a/codex-rs/core/src/tools/handlers/view_image_spec.rs b/codex-rs/core/src/tools/handlers/view_image_spec.rs index f28d9f8b1a..6107caf8f1 100644 --- a/codex-rs/core/src/tools/handlers/view_image_spec.rs +++ b/codex-rs/core/src/tools/handlers/view_image_spec.rs @@ -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()), }) } diff --git a/codex-rs/ext/memories/src/tools/mod.rs b/codex-rs/ext/memories/src/tools/mod.rs index 22cf0e295d..2852e3c6a7 100644 --- a/codex-rs/ext/memories/src/tools/mod.rs +++ b/codex-rs/ext/memories/src/tools/mod.rs @@ -67,7 +67,7 @@ pub(super) fn memory_function_tool( defer_loading: None, parameters: parse_tool_input_schema(&schema::input_schema_for::()) .unwrap_or_else(|err| panic!("generated input schema for {name} should parse: {err}")), - output_schema: Some(schema::output_schema_for::()), + output_schema: Some(schema::output_schema_for::().into()), }; ToolSpec::Namespace(ResponsesApiNamespace { diff --git a/codex-rs/ext/skills/src/tools/mod.rs b/codex-rs/ext/skills/src/tools/mod.rs index 22bac2bce9..49052667a2 100644 --- a/codex-rs/ext/skills/src/tools/mod.rs +++ b/codex-rs/ext/skills/src/tools/mod.rs @@ -304,7 +304,7 @@ fn skill_function_tool(name: &str, description: &s defer_loading: None, parameters: parse_tool_input_schema(&schema::input_schema_for::()) .unwrap_or_else(|err| panic!("generated input schema for {name} should parse: {err}")), - output_schema: Some(schema::output_schema_for::()), + output_schema: Some(schema::output_schema_for::().into()), }; ToolSpec::Namespace(ResponsesApiNamespace { diff --git a/codex-rs/tools/src/code_mode.rs b/codex-rs/tools/src/code_mode.rs index e5580ede13..31816b6b5b 100644 --- a/codex-rs/tools/src/code_mode.rs +++ b/codex-rs/tools/src/code_mode.rs @@ -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 { @@ -157,7 +161,7 @@ fn code_mode_tool_definitions_for_spec(spec: &ToolSpec) -> Vec { diff --git a/codex-rs/tools/src/code_mode_tests.rs b/codex-rs/tools/src/code_mode_tests.rs index 0e6eb7f694..3fe3d7575e 100644 --- a/codex-rs/tools/src/code_mode_tests.rs +++ b/codex-rs/tools/src/code_mode_tests.rs @@ -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() + ), }) ); } diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index db84442b17..820ee5413c 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -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; diff --git a/codex-rs/tools/src/mcp_tool.rs b/codex-rs/tools/src/mcp_tool.rs index 4ab24b9cd4..7e57d09c3a 100644 --- a/codex-rs/tools/src/mcp_tool.rs +++ b/codex-rs/tools/src/mcp_tool.rs @@ -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)] diff --git a/codex-rs/tools/src/mcp_tool_tests.rs b/codex-rs/tools/src/mcp_tool_tests.rs index 5f9b7b3264..e17b7a21ce 100644 --- a/codex-rs/tools/src/mcp_tool_tests.rs +++ b/codex-rs/tools/src/mcp_tool_tests.rs @@ -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, } ); diff --git a/codex-rs/tools/src/output_schema.rs b/codex-rs/tools/src/output_schema.rs new file mode 100644 index 0000000000..30f9cd4f34 --- /dev/null +++ b/codex-rs/tools/src/output_schema.rs @@ -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), + McpCallToolResult(Option>>), +} + +impl ToolOutputSchema { + pub(crate) fn from_mcp_output_schema( + structured_content: Option>>, + ) -> 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 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; diff --git a/codex-rs/tools/src/output_schema_tests.rs b/codex-rs/tools/src/output_schema_tests.rs new file mode 100644 index 0000000000..a98c59454c --- /dev/null +++ b/codex-rs/tools/src/output_schema_tests.rs @@ -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 = 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 + ); +} diff --git a/codex-rs/tools/src/responses_api.rs b/codex-rs/tools/src/responses_api.rs index e450dcf35f..07b622c226 100644 --- a/codex-rs/tools/src/responses_api.rs +++ b/codex-rs/tools/src/responses_api.rs @@ -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, pub parameters: JsonSchema, #[serde(skip)] - pub output_schema: Option, + pub output_schema: Option, } #[derive(Debug, Clone, Serialize, PartialEq)] diff --git a/codex-rs/tools/src/responses_api_tests.rs b/codex-rs/tools/src/responses_api_tests.rs index 43212060a3..6fccaee364 100644 --- a/codex-rs/tools/src/responses_api_tests.rs +++ b/codex-rs/tools/src/responses_api_tests.rs @@ -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()), } ); } diff --git a/codex-rs/tools/src/tool_definition.rs b/codex-rs/tools/src/tool_definition.rs index d3c5b0209b..1701239c76 100644 --- a/codex-rs/tools/src/tool_definition.rs +++ b/codex-rs/tools/src/tool_definition.rs @@ -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, + pub output_schema: Option, pub defer_loading: bool, } diff --git a/codex-rs/tools/src/tool_definition_tests.rs b/codex-rs/tools/src/tool_definition_tests.rs index 0d4d97f5ce..93f06c08cc 100644 --- a/codex-rs/tools/src/tool_definition_tests.rs +++ b/codex-rs/tools/src/tool_definition_tests.rs @@ -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, } } diff --git a/codex-rs/tools/src/tool_search_tests.rs b/codex-rs/tools/src/tool_search_tests.rs index aed34a6e3b..5c35e9c327 100644 --- a/codex-rs/tools/src/tool_search_tests.rs +++ b/codex-rs/tools/src/tool_search_tests.rs @@ -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(),