diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 1e10a5696..b15827997 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -9138,6 +9138,24 @@ mod tests { validate_dynamic_tools(&tools).expect("valid schema"); } + #[test] + fn validate_dynamic_tools_accepts_nullable_field_schema() { + let tools = vec![ApiDynamicToolSpec { + name: "my_tool".to_string(), + description: "test".to_string(), + input_schema: json!({ + "type": "object", + "properties": { + "query": {"type": ["string", "null"]} + }, + "required": ["query"], + "additionalProperties": false + }), + defer_loading: false, + }]; + validate_dynamic_tools(&tools).expect("valid schema"); + } + #[test] fn config_load_error_marks_cloud_requirements_failures_for_relogin() { let err = std::io::Error::other(CloudRequirementsLoadError::new( diff --git a/codex-rs/core/src/tools/context_tests.rs b/codex-rs/core/src/tools/context_tests.rs index 07fcfda98..2f310f443 100644 --- a/codex-rs/core/src/tools/context_tests.rs +++ b/codex-rs/core/src/tools/context_tests.rs @@ -147,11 +147,11 @@ fn tool_search_payloads_roundtrip_as_tool_search_outputs() { description: String::new(), strict: false, defer_loading: Some(true), - parameters: codex_tools::JsonSchema::Object { - properties: Default::default(), - required: None, - additional_properties: None, - }, + parameters: codex_tools::JsonSchema::object( + /*properties*/ Default::default(), + /*required*/ None, + /*additional_properties*/ None, + ), output_schema: None, }, )], diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 13fa4930d..023c513a4 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -832,16 +832,15 @@ fn test_mcp_tool_property_missing_type_defaults_to_string() { tool.spec, ToolSpec::Function(ResponsesApiTool { name: "dash/search".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object( + /*properties*/ + BTreeMap::from([( "query".to_string(), - JsonSchema::String { - description: Some("search query".to_string()) - } + JsonSchema::string(Some("search query".to_string())), )]), - required: None, - additional_properties: None, - }, + /*required*/ None, + /*additional_properties*/ None + ), description: "Search docs".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), @@ -851,7 +850,7 @@ fn test_mcp_tool_property_missing_type_defaults_to_string() { } #[test] -fn test_mcp_tool_integer_normalized_to_number() { +fn test_mcp_tool_preserves_integer_schema() { let config = test_config(); let model_info = construct_model_info_offline("gpt-5-codex", &config); let mut features = Features::with_defaults(); @@ -890,14 +889,15 @@ fn test_mcp_tool_integer_normalized_to_number() { tool.spec, ToolSpec::Function(ResponsesApiTool { name: "dash/paginate".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object( + /*properties*/ + BTreeMap::from([( "page".to_string(), - JsonSchema::Number { description: None } + JsonSchema::integer(/*description*/ None), )]), - required: None, - additional_properties: None, - }, + /*required*/ None, + /*additional_properties*/ None + ), description: "Pagination".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), @@ -947,17 +947,18 @@ fn test_mcp_tool_array_without_items_gets_default_string_items() { tool.spec, ToolSpec::Function(ResponsesApiTool { name: "dash/tags".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object( + /*properties*/ + BTreeMap::from([( "tags".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: None - } + JsonSchema::array( + JsonSchema::string(/*description*/ None), + /*description*/ None, + ), )]), - required: None, - additional_properties: None, - }, + /*required*/ None, + /*additional_properties*/ None + ), description: "Tags".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), @@ -1008,14 +1009,21 @@ fn test_mcp_tool_anyof_defaults_to_string() { tool.spec, ToolSpec::Function(ResponsesApiTool { name: "dash/value".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object( + /*properties*/ + BTreeMap::from([( "value".to_string(), - JsonSchema::String { description: None } + JsonSchema::any_of( + vec![ + JsonSchema::string(/*description*/ None), + JsonSchema::number(/*description*/ None), + ], + /*description*/ None, + ), )]), - required: None, - additional_properties: None, - }, + /*required*/ None, + /*additional_properties*/ None + ), description: "AnyOf Value".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), @@ -1082,50 +1090,51 @@ fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { tool.spec, ToolSpec::Function(ResponsesApiTool { name: "test_server/do_something_cool".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object( + /*properties*/ + BTreeMap::from([ ( "string_argument".to_string(), - JsonSchema::String { description: None } + JsonSchema::string(/*description*/ None), ), ( "number_argument".to_string(), - JsonSchema::Number { description: None } + JsonSchema::number(/*description*/ None), ), ( "object_argument".to_string(), - JsonSchema::Object { - properties: BTreeMap::from([ + JsonSchema::object( + BTreeMap::from([ ( "string_property".to_string(), - JsonSchema::String { description: None } + JsonSchema::string(/*description*/ None), ), ( "number_property".to_string(), - JsonSchema::Number { description: None } + JsonSchema::number(/*description*/ None), ), ]), - required: Some(vec![ + Some(vec![ "string_property".to_string(), "number_property".to_string(), ]), - additional_properties: Some( - JsonSchema::Object { - properties: BTreeMap::from([( + Some( + JsonSchema::object( + BTreeMap::from([( "addtl_prop".to_string(), - JsonSchema::String { description: None } - ),]), - required: Some(vec!["addtl_prop".to_string(),]), - additional_properties: Some(false.into()), - } - .into() + JsonSchema::string(/*description*/ None), + )]), + Some(vec!["addtl_prop".to_string()]), + Some(false.into()), + ) + .into(), ), - }, + ), ), ]), - required: None, - additional_properties: None, - }, + /*required*/ None, + /*additional_properties*/ None + ), description: "Do something cool".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), diff --git a/codex-rs/tools/src/agent_job_tool.rs b/codex-rs/tools/src/agent_job_tool.rs index 7966487a2..bcdec5dde 100644 --- a/codex-rs/tools/src/agent_job_tool.rs +++ b/codex-rs/tools/src/agent_job_tool.rs @@ -7,64 +7,48 @@ pub fn create_spawn_agents_on_csv_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "csv_path".to_string(), - JsonSchema::String { - description: Some("Path to the CSV file containing input rows.".to_string()), - }, + JsonSchema::string(Some("Path to the CSV file containing input rows.".to_string())), ), ( "instruction".to_string(), - JsonSchema::String { - description: Some( - "Instruction template to apply to each CSV row. Use {column_name} placeholders to inject values from the row." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Instruction template to apply to each CSV row. Use {column_name} placeholders to inject values from the row." + .to_string(), + )), ), ( "id_column".to_string(), - JsonSchema::String { - description: Some("Optional column name to use as stable item id.".to_string()), - }, + JsonSchema::string(Some( + "Optional column name to use as stable item id.".to_string(), + )), ), ( "output_csv_path".to_string(), - JsonSchema::String { - description: Some("Optional output CSV path for exported results.".to_string()), - }, + JsonSchema::string(Some("Optional output CSV path for exported results.".to_string())), ), ( "max_concurrency".to_string(), - JsonSchema::Number { - description: Some( - "Maximum concurrent workers for this job. Defaults to 16 and is capped by config." - .to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum concurrent workers for this job. Defaults to 16 and is capped by config." + .to_string(), + )), ), ( "max_workers".to_string(), - JsonSchema::Number { - description: Some( - "Alias for max_concurrency. Set to 1 to run sequentially.".to_string(), - ), - }, + JsonSchema::number(Some( + "Alias for max_concurrency. Set to 1 to run sequentially.".to_string(), + )), ), ( "max_runtime_seconds".to_string(), - JsonSchema::Number { - description: Some( - "Maximum runtime per worker before it is failed. Defaults to 1800 seconds." - .to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum runtime per worker before it is failed. Defaults to 1800 seconds." + .to_string(), + )), ), ( "output_schema".to_string(), - JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + JsonSchema::object(BTreeMap::new(), /*required*/ None, /*additional_properties*/ None), ), ]); @@ -74,11 +58,7 @@ pub fn create_spawn_agents_on_csv_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["csv_path".to_string(), "instruction".to_string()]), - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, Some(vec!["csv_path".to_string(), "instruction".to_string()]), Some(false.into())), output_schema: None, }) } @@ -87,32 +67,22 @@ pub fn create_report_agent_job_result_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "job_id".to_string(), - JsonSchema::String { - description: Some("Identifier of the job.".to_string()), - }, + JsonSchema::string(Some("Identifier of the job.".to_string())), ), ( "item_id".to_string(), - JsonSchema::String { - description: Some("Identifier of the job item.".to_string()), - }, + JsonSchema::string(Some("Identifier of the job item.".to_string())), ), ( "result".to_string(), - JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + JsonSchema::object(BTreeMap::new(), /*required*/ None, /*additional_properties*/ None), ), ( "stop".to_string(), - JsonSchema::Boolean { - description: Some( - "Optional. When true, cancels the remaining job items after this result is recorded." - .to_string(), - ), - }, + JsonSchema::boolean(Some( + "Optional. When true, cancels the remaining job items after this result is recorded." + .to_string(), + )), ), ]); @@ -123,15 +93,11 @@ pub fn create_report_agent_job_result_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec![ + parameters: JsonSchema::object(properties, Some(vec![ "job_id".to_string(), "item_id".to_string(), "result".to_string(), - ]), - additional_properties: Some(false.into()), - }, + ]), Some(false.into())), output_schema: None, }) } diff --git a/codex-rs/tools/src/agent_job_tool_tests.rs b/codex-rs/tools/src/agent_job_tool_tests.rs index 3c92ec76c..95f865977 100644 --- a/codex-rs/tools/src/agent_job_tool_tests.rs +++ b/codex-rs/tools/src/agent_job_tool_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::JsonSchema; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -12,73 +13,61 @@ fn spawn_agents_on_csv_tool_requires_csv_and_instruction() { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "csv_path".to_string(), - JsonSchema::String { - description: Some("Path to the CSV file containing input rows.".to_string()), - }, + JsonSchema::string(Some( + "Path to the CSV file containing input rows.".to_string(), + )), ), ( "instruction".to_string(), - JsonSchema::String { - description: Some( - "Instruction template to apply to each CSV row. Use {column_name} placeholders to inject values from the row." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Instruction template to apply to each CSV row. Use {column_name} placeholders to inject values from the row." + .to_string(), + )), ), ( "id_column".to_string(), - JsonSchema::String { - description: Some("Optional column name to use as stable item id.".to_string()), - }, + JsonSchema::string(Some( + "Optional column name to use as stable item id.".to_string(), + )), ), ( "output_csv_path".to_string(), - JsonSchema::String { - description: Some("Optional output CSV path for exported results.".to_string()), - }, + JsonSchema::string(Some( + "Optional output CSV path for exported results.".to_string(), + )), ), ( "max_concurrency".to_string(), - JsonSchema::Number { - description: Some( - "Maximum concurrent workers for this job. Defaults to 16 and is capped by config." - .to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum concurrent workers for this job. Defaults to 16 and is capped by config." + .to_string(), + )), ), ( "max_workers".to_string(), - JsonSchema::Number { - description: Some( - "Alias for max_concurrency. Set to 1 to run sequentially.".to_string(), - ), - }, + JsonSchema::number(Some( + "Alias for max_concurrency. Set to 1 to run sequentially.".to_string(), + )), ), ( "max_runtime_seconds".to_string(), - JsonSchema::Number { - description: Some( - "Maximum runtime per worker before it is failed. Defaults to 1800 seconds." - .to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum runtime per worker before it is failed. Defaults to 1800 seconds." + .to_string(), + )), ), ( "output_schema".to_string(), - JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None, + ), ), - ]), - required: Some(vec!["csv_path".to_string(), "instruction".to_string()]), - additional_properties: Some(false.into()), - }, + ]), Some(vec!["csv_path".to_string(), "instruction".to_string()]), Some(false.into())), output_schema: None, }) ); @@ -95,45 +84,35 @@ fn report_agent_job_result_tool_requires_result_payload() { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "job_id".to_string(), - JsonSchema::String { - description: Some("Identifier of the job.".to_string()), - }, + JsonSchema::string(Some("Identifier of the job.".to_string())), ), ( "item_id".to_string(), - JsonSchema::String { - description: Some("Identifier of the job item.".to_string()), - }, + JsonSchema::string(Some("Identifier of the job item.".to_string())), ), ( "result".to_string(), - JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None, + ), ), ( "stop".to_string(), - JsonSchema::Boolean { - description: Some( - "Optional. When true, cancels the remaining job items after this result is recorded." - .to_string(), - ), - }, + JsonSchema::boolean(Some( + "Optional. When true, cancels the remaining job items after this result is recorded." + .to_string(), + )), ), - ]), - required: Some(vec![ + ]), Some(vec![ "job_id".to_string(), "item_id".to_string(), "result".to_string(), - ]), - additional_properties: Some(false.into()), - }, + ]), Some(false.into())), output_schema: None, }) ); diff --git a/codex-rs/tools/src/agent_tool.rs b/codex-rs/tools/src/agent_tool.rs index 1a1eeb19d..cf7bc25b9 100644 --- a/codex-rs/tools/src/agent_tool.rs +++ b/codex-rs/tools/src/agent_tool.rs @@ -38,11 +38,7 @@ pub fn create_spawn_agent_tool_v1(options: SpawnAgentToolOptions<'_>) -> ToolSpe ), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: None, - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, /*required*/ None, Some(false.into())), output_schema: Some(spawn_agent_output_schema_v1()), }) } @@ -61,12 +57,10 @@ pub fn create_spawn_agent_tool_v2(options: SpawnAgentToolOptions<'_>) -> ToolSpe } properties.insert( "task_name".to_string(), - JsonSchema::String { - description: Some( - "Task name for the new agent. Use lowercase letters, digits, and underscores." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Task name for the new agent. Use lowercase letters, digits, and underscores." + .to_string(), + )), ); ToolSpec::Function(ResponsesApiTool { @@ -77,11 +71,11 @@ pub fn create_spawn_agent_tool_v2(options: SpawnAgentToolOptions<'_>) -> ToolSpe ), strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["task_name".to_string(), "message".to_string()]), - additional_properties: Some(false.into()), - }, + 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, )), @@ -92,28 +86,22 @@ pub fn create_send_input_tool_v1() -> ToolSpec { let properties = BTreeMap::from([ ( "target".to_string(), - JsonSchema::String { - description: Some("Agent id to message (from spawn_agent).".to_string()), - }, + JsonSchema::string(Some("Agent id to message (from spawn_agent).".to_string())), ), ( "message".to_string(), - JsonSchema::String { - description: Some( - "Legacy plain-text message to send to the agent. Use either message or items." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Legacy plain-text message to send to the agent. Use either message or items." + .to_string(), + )), ), ("items".to_string(), create_collab_input_items_schema()), ( "interrupt".to_string(), - JsonSchema::Boolean { - description: Some( - "When true, stop the agent's current task and handle this immediately. When false (default), queue this message." - .to_string(), - ), - }, + JsonSchema::boolean(Some( + "When true, stop the agent's current task and handle this immediately. When false (default), queue this message." + .to_string(), + )), ), ]); @@ -123,11 +111,7 @@ pub fn create_send_input_tool_v1() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["target".to_string()]), - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, Some(vec!["target".to_string()]), Some(false.into())), output_schema: Some(send_input_output_schema()), }) } @@ -136,17 +120,15 @@ pub fn create_send_message_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "target".to_string(), - JsonSchema::String { - description: Some( - "Agent id or canonical task name to message (from spawn_agent).".to_string(), - ), - }, + JsonSchema::string(Some( + "Agent id or canonical task name to message (from spawn_agent).".to_string(), + )), ), ( "message".to_string(), - JsonSchema::String { - description: Some("Message text to queue on the target agent.".to_string()), - }, + JsonSchema::string(Some( + "Message text to queue on the target agent.".to_string(), + )), ), ]); @@ -156,11 +138,7 @@ pub fn create_send_message_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["target".to_string(), "message".to_string()]), - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, Some(vec!["target".to_string(), "message".to_string()]), Some(false.into())), output_schema: None, }) } @@ -169,26 +147,22 @@ pub fn create_followup_task_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "target".to_string(), - JsonSchema::String { - description: Some( - "Agent id or canonical task name to message (from spawn_agent).".to_string(), - ), - }, + JsonSchema::string(Some( + "Agent id or canonical task name to message (from spawn_agent).".to_string(), + )), ), ( "message".to_string(), - JsonSchema::String { - description: Some("Message text to send to the target agent.".to_string()), - }, + JsonSchema::string(Some( + "Message text to send to the target agent.".to_string(), + )), ), ( "interrupt".to_string(), - JsonSchema::Boolean { - description: Some( - "When true, stop the agent's current task and handle this immediately. When false (default), queue this message." - .to_string(), - ), - }, + JsonSchema::boolean(Some( + "When true, stop the agent's current task and handle this immediately. When false (default), queue this message." + .to_string(), + )), ), ]); @@ -198,11 +172,7 @@ pub fn create_followup_task_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["target".to_string(), "message".to_string()]), - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, Some(vec!["target".to_string(), "message".to_string()]), Some(false.into())), output_schema: None, }) } @@ -210,9 +180,7 @@ pub fn create_followup_task_tool() -> ToolSpec { pub fn create_resume_agent_tool() -> ToolSpec { let properties = BTreeMap::from([( "id".to_string(), - JsonSchema::String { - description: Some("Agent id to resume.".to_string()), - }, + JsonSchema::string(Some("Agent id to resume.".to_string())), )]); ToolSpec::Function(ResponsesApiTool { @@ -222,11 +190,7 @@ pub fn create_resume_agent_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["id".to_string()]), - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, Some(vec!["id".to_string()]), Some(false.into())), output_schema: Some(resume_agent_output_schema()), }) } @@ -258,12 +222,10 @@ pub fn create_wait_agent_tool_v2(options: WaitAgentTimeoutOptions) -> ToolSpec { pub fn create_list_agents_tool() -> ToolSpec { let properties = BTreeMap::from([( "path_prefix".to_string(), - JsonSchema::String { - description: Some( - "Optional task-path prefix. Accepts the same relative or absolute task-path syntax as other MultiAgentV2 agent targets." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Optional task-path prefix. Accepts the same relative or absolute task-path syntax as other MultiAgentV2 agent targets." + .to_string(), + )), )]); ToolSpec::Function(ResponsesApiTool { @@ -273,11 +235,7 @@ pub fn create_list_agents_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: None, - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, /*required*/ None, Some(false.into())), output_schema: Some(list_agents_output_schema()), }) } @@ -285,9 +243,7 @@ pub fn create_list_agents_tool() -> ToolSpec { pub fn create_close_agent_tool_v1() -> ToolSpec { let properties = BTreeMap::from([( "target".to_string(), - JsonSchema::String { - description: Some("Agent id to close (from spawn_agent).".to_string()), - }, + JsonSchema::string(Some("Agent id to close (from spawn_agent).".to_string())), )]); ToolSpec::Function(ResponsesApiTool { @@ -295,11 +251,7 @@ pub fn create_close_agent_tool_v1() -> ToolSpec { description: "Close an agent and any open descendants when they are no longer needed, and return the target agent's previous status before shutdown was requested. Don't keep agents open for too long if they are not needed anymore.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["target".to_string()]), - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, Some(vec!["target".to_string()]), Some(false.into())), output_schema: Some(close_agent_output_schema()), }) } @@ -307,11 +259,9 @@ pub fn create_close_agent_tool_v1() -> ToolSpec { pub fn create_close_agent_tool_v2() -> ToolSpec { let properties = BTreeMap::from([( "target".to_string(), - JsonSchema::String { - description: Some( - "Agent id or canonical task name to close (from spawn_agent).".to_string(), - ), - }, + JsonSchema::string(Some( + "Agent id or canonical task name to close (from spawn_agent).".to_string(), + )), )]); ToolSpec::Function(ResponsesApiTool { @@ -319,11 +269,7 @@ pub fn create_close_agent_tool_v2() -> ToolSpec { description: "Close an agent and any open descendants when they are no longer needed, and return the target agent's previous status before shutdown was requested. Don't keep agents open for too long if they are not needed anymore.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["target".to_string()]), - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, Some(vec!["target".to_string()]), Some(false.into())), output_schema: Some(close_agent_output_schema()), }) } @@ -522,98 +468,71 @@ fn create_collab_input_items_schema() -> JsonSchema { let properties = BTreeMap::from([ ( "type".to_string(), - JsonSchema::String { - description: Some( - "Input item type: text, image, local_image, skill, or mention.".to_string(), - ), - }, + JsonSchema::string(Some( + "Input item type: text, image, local_image, skill, or mention.".to_string(), + )), ), ( "text".to_string(), - JsonSchema::String { - description: Some("Text content when type is text.".to_string()), - }, + JsonSchema::string(Some("Text content when type is text.".to_string())), ), ( "image_url".to_string(), - JsonSchema::String { - description: Some("Image URL when type is image.".to_string()), - }, + JsonSchema::string(Some("Image URL when type is image.".to_string())), ), ( "path".to_string(), - JsonSchema::String { - description: Some( - "Path when type is local_image/skill, or structured mention target such as app:// or plugin://@ when type is mention." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Path when type is local_image/skill, or structured mention target such as app:// or plugin://@ when type is mention." + .to_string(), + )), ), ( "name".to_string(), - JsonSchema::String { - description: Some("Display name when type is skill or mention.".to_string()), - }, + JsonSchema::string(Some("Display name when type is skill or mention.".to_string())), ), ]); - JsonSchema::Array { - items: Box::new(JsonSchema::Object { - properties, - required: None, - additional_properties: Some(false.into()), - }), - description: Some( + JsonSchema::array(JsonSchema::object(properties, /*required*/ None, Some(false.into())), Some( "Structured input items. Use this to pass explicit mentions (for example app:// connector paths)." .to_string(), - ), - } + )) } fn spawn_agent_common_properties_v1(agent_type_description: &str) -> BTreeMap { BTreeMap::from([ ( "message".to_string(), - JsonSchema::String { - description: Some( - "Initial plain-text task for the new agent. Use either message or items." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Initial plain-text task for the new agent. Use either message or items." + .to_string(), + )), ), ("items".to_string(), create_collab_input_items_schema()), ( "agent_type".to_string(), - JsonSchema::String { - description: Some(agent_type_description.to_string()), - }, + JsonSchema::string(Some(agent_type_description.to_string())), ), ( "fork_context".to_string(), - JsonSchema::Boolean { - description: Some( - "When true, fork the current thread history into the new agent before sending the initial prompt. This must be used when you want the new agent to have exactly the same context as you." - .to_string(), - ), - }, + JsonSchema::boolean(Some( + "When true, fork the current thread history into the new agent before sending the initial prompt. This must be used when you want the new agent to have exactly the same context as you." + .to_string(), + )), ), ( "model".to_string(), - JsonSchema::String { - description: Some( - "Optional model override for the new agent. Replaces the inherited model." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Optional model override for the new agent. Replaces the inherited model." + .to_string(), + )), ), ( "reasoning_effort".to_string(), - JsonSchema::String { - description: Some( - "Optional reasoning effort override for the new agent. Replaces the inherited reasoning effort." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Optional reasoning effort override for the new agent. Replaces the inherited reasoning effort." + .to_string(), + )), ), ]) } @@ -622,42 +541,32 @@ fn spawn_agent_common_properties_v2(agent_type_description: &str) -> BTreeMap JsonSchema let properties = BTreeMap::from([ ( "targets".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some( + JsonSchema::array( + JsonSchema::string(/*description*/ None), + Some( "Agent ids to wait on. Pass multiple ids to wait for whichever finishes first." .to_string(), ), - }, + ), ), ( "timeout_ms".to_string(), - JsonSchema::Number { - description: Some(format!( - "Optional timeout in milliseconds. Defaults to {}, min {}, max {}. Prefer longer waits (minutes) to avoid busy polling.", - options.default_timeout_ms, options.min_timeout_ms, options.max_timeout_ms, - )), - }, + JsonSchema::number(Some(format!( + "Optional timeout in milliseconds. Defaults to {}, min {}, max {}. Prefer longer waits (minutes) to avoid busy polling.", + options.default_timeout_ms, options.min_timeout_ms, options.max_timeout_ms, + ))), ), ]); - JsonSchema::Object { + JsonSchema::object( properties, - required: Some(vec!["targets".to_string()]), - additional_properties: Some(false.into()), - } + Some(vec!["targets".to_string()]), + Some(false.into()), + ) } fn wait_agent_tool_parameters_v2(options: WaitAgentTimeoutOptions) -> JsonSchema { let properties = BTreeMap::from([( "timeout_ms".to_string(), - JsonSchema::Number { - description: Some(format!( - "Optional timeout in milliseconds. Defaults to {}, min {}, max {}. Prefer longer waits (minutes) to avoid busy polling.", - options.default_timeout_ms, options.min_timeout_ms, options.max_timeout_ms, - )), - }, + JsonSchema::number(Some(format!( + "Optional timeout in milliseconds. Defaults to {}, min {}, max {}. Prefer longer waits (minutes) to avoid busy polling.", + options.default_timeout_ms, options.min_timeout_ms, options.max_timeout_ms, + ))), )]); - JsonSchema::Object { - properties, - required: None, - additional_properties: Some(false.into()), - } + JsonSchema::object(properties, /*required*/ None, Some(false.into())) } #[cfg(test)] diff --git a/codex-rs/tools/src/agent_tool_tests.rs b/codex-rs/tools/src/agent_tool_tests.rs index 2ebfb605b..165f01153 100644 --- a/codex-rs/tools/src/agent_tool_tests.rs +++ b/codex-rs/tools/src/agent_tool_tests.rs @@ -1,4 +1,6 @@ use super::*; +use crate::JsonSchemaPrimitiveType; +use crate::JsonSchemaType; use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; @@ -47,14 +49,14 @@ fn spawn_agent_tool_v2_requires_task_name_and_lists_visible_models() { else { panic!("spawn_agent should be a function tool"); }; - let JsonSchema::Object { - properties, - required, - .. - } = parameters - else { - panic!("spawn_agent should use object params"); - }; + assert_eq!( + parameters.schema_type, + Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) + ); + let properties = parameters + .properties + .as_ref() + .expect("spawn_agent should use object params"); assert!(description.contains("visible display (`visible-model`)")); assert!(!description.contains("hidden display (`hidden-model`)")); assert!(properties.contains_key("task_name")); @@ -64,13 +66,11 @@ fn spawn_agent_tool_v2_requires_task_name_and_lists_visible_models() { assert!(!properties.contains_key("fork_context")); assert_eq!( properties.get("agent_type"), - Some(&JsonSchema::String { - description: Some("role help".to_string()), - }) + Some(&JsonSchema::string(Some("role help".to_string()))) ); assert_eq!( - required, - Some(vec!["task_name".to_string(), "message".to_string()]) + parameters.required.as_ref(), + Some(&vec!["task_name".to_string(), "message".to_string()]) ); assert_eq!( output_schema.expect("spawn_agent output schema")["required"], @@ -89,9 +89,14 @@ fn spawn_agent_tool_v1_keeps_legacy_fork_context_field() { let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = tool else { panic!("spawn_agent should be a function tool"); }; - let JsonSchema::Object { properties, .. } = parameters else { - panic!("spawn_agent should use object params"); - }; + assert_eq!( + parameters.schema_type, + Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) + ); + let properties = parameters + .properties + .as_ref() + .expect("spawn_agent should use object params"); assert!(properties.contains_key("fork_context")); assert!(!properties.contains_key("fork_turns")); @@ -107,21 +112,21 @@ fn send_message_tool_requires_message_and_has_no_output_schema() { else { panic!("send_message should be a function tool"); }; - let JsonSchema::Object { - properties, - required, - .. - } = parameters - else { - panic!("send_message should use object params"); - }; + assert_eq!( + parameters.schema_type, + Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) + ); + let properties = parameters + .properties + .as_ref() + .expect("send_message should use object params"); assert!(properties.contains_key("target")); assert!(properties.contains_key("message")); assert!(!properties.contains_key("interrupt")); assert!(!properties.contains_key("items")); assert_eq!( - required, - Some(vec!["target".to_string(), "message".to_string()]) + parameters.required.as_ref(), + Some(&vec!["target".to_string(), "message".to_string()]) ); assert_eq!(output_schema, None); } @@ -136,21 +141,21 @@ fn followup_task_tool_requires_message_and_has_no_output_schema() { else { panic!("followup_task should be a function tool"); }; - let JsonSchema::Object { - properties, - required, - .. - } = parameters - else { - panic!("followup_task should use object params"); - }; + assert_eq!( + parameters.schema_type, + Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) + ); + let properties = parameters + .properties + .as_ref() + .expect("followup_task should use object params"); assert!(properties.contains_key("target")); assert!(properties.contains_key("message")); assert!(properties.contains_key("interrupt")); assert!(!properties.contains_key("items")); assert_eq!( - required, - Some(vec!["target".to_string(), "message".to_string()]) + parameters.required.as_ref(), + Some(&vec!["target".to_string(), "message".to_string()]) ); assert_eq!(output_schema, None); } @@ -169,17 +174,17 @@ fn wait_agent_tool_v2_uses_timeout_only_summary_output() { else { panic!("wait_agent should be a function tool"); }; - let JsonSchema::Object { - properties, - required, - .. - } = parameters - else { - panic!("wait_agent should use object params"); - }; + assert_eq!( + parameters.schema_type, + Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) + ); + let properties = parameters + .properties + .as_ref() + .expect("wait_agent should use object params"); assert!(!properties.contains_key("targets")); assert!(properties.contains_key("timeout_ms")); - assert_eq!(required, None); + assert_eq!(parameters.required.as_ref(), None); assert_eq!( output_schema.expect("wait output schema")["properties"]["message"]["description"], json!("Brief wait summary without the agent's final content.") @@ -196,9 +201,14 @@ fn list_agents_tool_includes_path_prefix_and_agent_fields() { else { panic!("list_agents should be a function tool"); }; - let JsonSchema::Object { properties, .. } = parameters else { - panic!("list_agents should use object params"); - }; + assert_eq!( + parameters.schema_type, + Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) + ); + let properties = parameters + .properties + .as_ref() + .expect("list_agents should use object params"); assert!(properties.contains_key("path_prefix")); assert_eq!( output_schema.expect("list_agents output schema")["properties"]["agents"]["items"]["required"], diff --git a/codex-rs/tools/src/apply_patch_tool.rs b/codex-rs/tools/src/apply_patch_tool.rs index 6360eef5b..469bb5236 100644 --- a/codex-rs/tools/src/apply_patch_tool.rs +++ b/codex-rs/tools/src/apply_patch_tool.rs @@ -102,9 +102,9 @@ pub fn create_apply_patch_freeform_tool() -> ToolSpec { pub fn create_apply_patch_json_tool() -> ToolSpec { let properties = BTreeMap::from([( "input".to_string(), - JsonSchema::String { - description: Some("The entire contents of the apply_patch command".to_string()), - }, + JsonSchema::string(Some( + "The entire contents of the apply_patch command".to_string(), + )), )]); ToolSpec::Function(ResponsesApiTool { @@ -112,11 +112,11 @@ pub fn create_apply_patch_json_tool() -> ToolSpec { description: APPLY_PATCH_JSON_TOOL_DESCRIPTION.to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["input".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["input".to_string()]), + Some(false.into()), + ), output_schema: None, }) } diff --git a/codex-rs/tools/src/apply_patch_tool_tests.rs b/codex-rs/tools/src/apply_patch_tool_tests.rs index 5b3f1117c..c12859458 100644 --- a/codex-rs/tools/src/apply_patch_tool_tests.rs +++ b/codex-rs/tools/src/apply_patch_tool_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::JsonSchema; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -29,18 +30,16 @@ fn create_apply_patch_json_tool_matches_expected_spec() { description: APPLY_PATCH_JSON_TOOL_DESCRIPTION.to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object( + BTreeMap::from([( "input".to_string(), - JsonSchema::String { - description: Some( - "The entire contents of the apply_patch command".to_string(), - ), - }, + JsonSchema::string(Some( + "The entire contents of the apply_patch command".to_string(), + ),), )]), - required: Some(vec!["input".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["input".to_string()]), + Some(false.into()) + ), output_schema: None, }) ); diff --git a/codex-rs/tools/src/code_mode.rs b/codex-rs/tools/src/code_mode.rs index c90c3f7bc..07e236ebd 100644 --- a/codex-rs/tools/src/code_mode.rs +++ b/codex-rs/tools/src/code_mode.rs @@ -53,32 +53,26 @@ pub fn create_wait_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "cell_id".to_string(), - JsonSchema::String { - description: Some("Identifier of the running exec cell.".to_string()), - }, + JsonSchema::string(Some("Identifier of the running exec cell.".to_string())), ), ( "yield_time_ms".to_string(), - JsonSchema::Number { - description: Some( - "How long to wait (in milliseconds) for more output before yielding again." - .to_string(), - ), - }, + JsonSchema::number(Some( + "How long to wait (in milliseconds) for more output before yielding again." + .to_string(), + )), ), ( "max_tokens".to_string(), - JsonSchema::Number { - description: Some( - "Maximum number of output tokens to return for this wait call.".to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum number of output tokens to return for this wait call.".to_string(), + )), ), ( "terminate".to_string(), - JsonSchema::Boolean { - description: Some("Whether to terminate the running exec cell.".to_string()), - }, + JsonSchema::boolean(Some( + "Whether to terminate the running exec cell.".to_string(), + )), ), ]); @@ -90,11 +84,11 @@ pub fn create_wait_tool() -> ToolSpec { codex_code_mode::build_wait_tool_description().trim() ), strict: false, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["cell_id".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["cell_id".to_string()]), + Some(false.into()), + ), output_schema: None, defer_loading: None, }) diff --git a/codex-rs/tools/src/code_mode_tests.rs b/codex-rs/tools/src/code_mode_tests.rs index a7089e543..39bdd1f91 100644 --- a/codex-rs/tools/src/code_mode_tests.rs +++ b/codex-rs/tools/src/code_mode_tests.rs @@ -20,14 +20,10 @@ fn augment_tool_spec_for_code_mode_augments_function_tools() { description: "Look up an order".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object(BTreeMap::from([( "order_id".to_string(), - JsonSchema::String { description: None }, - )]), - required: Some(vec!["order_id".to_string()]), - additional_properties: Some(AdditionalProperties::Boolean(false)), - }, + JsonSchema::string(/*description*/ None), + )]), Some(vec!["order_id".to_string()]), Some(AdditionalProperties::Boolean(false))), output_schema: Some(json!({ "type": "object", "properties": { @@ -41,14 +37,10 @@ fn augment_tool_spec_for_code_mode_augments_function_tools() { description: "Look up an order\n\nexec tool declaration:\n```ts\ndeclare const tools: { lookup_order(args: { order_id: string; }): Promise<{ ok: boolean; }>; };\n```".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object(BTreeMap::from([( "order_id".to_string(), - JsonSchema::String { description: None }, - )]), - required: Some(vec!["order_id".to_string()]), - additional_properties: Some(AdditionalProperties::Boolean(false)), - }, + JsonSchema::string(/*description*/ None), + )]), Some(vec!["order_id".to_string()]), Some(AdditionalProperties::Boolean(false))), output_schema: Some(json!({ "type": "object", "properties": { @@ -114,11 +106,11 @@ fn tool_spec_to_code_mode_tool_definition_skips_unsupported_variants() { tool_spec_to_code_mode_tool_definition(&ToolSpec::ToolSearch { execution: "sync".to_string(), description: "Search".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None + ), }), None ); @@ -137,44 +129,32 @@ fn create_wait_tool_matches_expected_spec() { ), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "cell_id".to_string(), - JsonSchema::String { - description: Some("Identifier of the running exec cell.".to_string()), - }, + JsonSchema::string(Some("Identifier of the running exec cell.".to_string()),), ), ( "max_tokens".to_string(), - JsonSchema::Number { - description: Some( + JsonSchema::number(Some( "Maximum number of output tokens to return for this wait call." .to_string(), - ), - }, + ),), ), ( "terminate".to_string(), - JsonSchema::Boolean { - description: Some( + JsonSchema::boolean(Some( "Whether to terminate the running exec cell.".to_string(), - ), - }, + ),), ), ( "yield_time_ms".to_string(), - JsonSchema::Number { - description: Some( + JsonSchema::number(Some( "How long to wait (in milliseconds) for more output before yielding again." .to_string(), - ), - }, + ),), ), - ]), - required: Some(vec!["cell_id".to_string()]), - additional_properties: Some(false.into()), - }, + ]), Some(vec!["cell_id".to_string()]), Some(false.into())), output_schema: None, }) ); diff --git a/codex-rs/tools/src/dynamic_tool_tests.rs b/codex-rs/tools/src/dynamic_tool_tests.rs index 2f4de34c2..42afd4627 100644 --- a/codex-rs/tools/src/dynamic_tool_tests.rs +++ b/codex-rs/tools/src/dynamic_tool_tests.rs @@ -25,16 +25,14 @@ fn parse_dynamic_tool_sanitizes_input_schema() { ToolDefinition { name: "lookup_ticket".to_string(), description: "Fetch a ticket".to_string(), - input_schema: JsonSchema::Object { - properties: BTreeMap::from([( + input_schema: JsonSchema::object( + BTreeMap::from([( "id".to_string(), - JsonSchema::String { - description: Some("Ticket identifier".to_string()), - }, + JsonSchema::string(Some("Ticket identifier".to_string()),), )]), - required: None, - additional_properties: None, - }, + /*required*/ None, + /*additional_properties*/ None + ), output_schema: None, defer_loading: false, } @@ -58,11 +56,11 @@ fn parse_dynamic_tool_preserves_defer_loading() { ToolDefinition { name: "lookup_ticket".to_string(), description: "Fetch a ticket".to_string(), - input_schema: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + input_schema: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: None, defer_loading: true, } diff --git a/codex-rs/tools/src/js_repl_tool.rs b/codex-rs/tools/src/js_repl_tool.rs index d4d74c65e..60089e0fb 100644 --- a/codex-rs/tools/src/js_repl_tool.rs +++ b/codex-rs/tools/src/js_repl_tool.rs @@ -45,11 +45,7 @@ pub fn create_js_repl_reset_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(BTreeMap::new(), /*required*/ None, Some(false.into())), output_schema: None, }) } diff --git a/codex-rs/tools/src/js_repl_tool_tests.rs b/codex-rs/tools/src/js_repl_tool_tests.rs index cef10fa2a..7d6d63f17 100644 --- a/codex-rs/tools/src/js_repl_tool_tests.rs +++ b/codex-rs/tools/src/js_repl_tool_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::JsonSchema; use crate::ToolSpec; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -29,11 +30,11 @@ fn js_repl_reset_tool_matches_expected_spec() { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + Some(false.into()) + ), output_schema: None, }) ); diff --git a/codex-rs/tools/src/json_schema.rs b/codex-rs/tools/src/json_schema.rs index 727a085af..22a641491 100644 --- a/codex-rs/tools/src/json_schema.rs +++ b/codex-rs/tools/src/json_schema.rs @@ -4,40 +4,125 @@ use serde_json::Value as JsonValue; use serde_json::json; use std::collections::BTreeMap; -/// Generic JSON-Schema subset needed for our tool definitions. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -#[serde(tag = "type", rename_all = "lowercase")] -pub enum JsonSchema { - Boolean { - #[serde(skip_serializing_if = "Option::is_none")] - description: Option, - }, - String { - #[serde(skip_serializing_if = "Option::is_none")] - description: Option, - }, - /// MCP schema allows "number" | "integer" for Number. - #[serde(alias = "integer")] - Number { - #[serde(skip_serializing_if = "Option::is_none")] - description: Option, - }, - Array { - items: Box, +/// Primitive JSON Schema type names we support in tool definitions. +/// +/// This mirrors the OpenAI Structured Outputs subset for JSON Schema `type`: +/// string, number, boolean, integer, object, array, and null. +/// Keywords such as `enum`, `const`, and `anyOf` are modeled separately. +/// See . +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum JsonSchemaPrimitiveType { + String, + Number, + Boolean, + Integer, + Object, + Array, + Null, +} - #[serde(skip_serializing_if = "Option::is_none")] - description: Option, - }, - Object { +/// JSON Schema `type` supports either a single type name or a union of names. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(untagged)] +pub enum JsonSchemaType { + Single(JsonSchemaPrimitiveType), + Multiple(Vec), +} + +/// Generic JSON-Schema subset needed for our tool definitions. +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +pub struct JsonSchema { + #[serde(rename = "type", skip_serializing_if = "Option::is_none")] + pub schema_type: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub description: Option, + #[serde(rename = "enum", skip_serializing_if = "Option::is_none")] + pub enum_values: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub items: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub properties: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub required: Option>, + #[serde( + rename = "additionalProperties", + skip_serializing_if = "Option::is_none" + )] + pub additional_properties: Option, + #[serde(rename = "anyOf", skip_serializing_if = "Option::is_none")] + pub any_of: Option>, +} + +impl JsonSchema { + /// Construct a scalar/object/array schema with a single JSON Schema type. + fn typed(schema_type: JsonSchemaPrimitiveType, description: Option) -> Self { + Self { + schema_type: Some(JsonSchemaType::Single(schema_type)), + description, + ..Default::default() + } + } + + pub fn any_of(variants: Vec, description: Option) -> Self { + Self { + description, + any_of: Some(variants), + ..Default::default() + } + } + + pub fn boolean(description: Option) -> Self { + Self::typed(JsonSchemaPrimitiveType::Boolean, description) + } + + pub fn string(description: Option) -> Self { + Self::typed(JsonSchemaPrimitiveType::String, description) + } + + pub fn number(description: Option) -> Self { + Self::typed(JsonSchemaPrimitiveType::Number, description) + } + + pub fn integer(description: Option) -> Self { + Self::typed(JsonSchemaPrimitiveType::Integer, description) + } + + pub fn null(description: Option) -> Self { + Self::typed(JsonSchemaPrimitiveType::Null, description) + } + + pub fn string_enum(values: Vec, description: Option) -> Self { + Self { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::String)), + description, + enum_values: Some(values), + ..Default::default() + } + } + + pub fn array(items: JsonSchema, description: Option) -> Self { + Self { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Array)), + description, + items: Some(Box::new(items)), + ..Default::default() + } + } + + pub fn object( properties: BTreeMap, - #[serde(skip_serializing_if = "Option::is_none")] required: Option>, - #[serde( - rename = "additionalProperties", - skip_serializing_if = "Option::is_none" - )] additional_properties: Option, - }, + ) -> Self { + Self { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + properties: Some(properties), + required, + additional_properties, + ..Default::default() + } + } } /// Whether additional properties are allowed, and if so, any required schema. @@ -64,16 +149,23 @@ impl From for AdditionalProperties { pub fn parse_tool_input_schema(input_schema: &JsonValue) -> Result { let mut input_schema = input_schema.clone(); sanitize_json_schema(&mut input_schema); - serde_json::from_value::(input_schema) + let schema: JsonSchema = serde_json::from_value(input_schema)?; + if matches!( + schema.schema_type, + Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Null)) + ) { + return Err(singleton_null_schema_error()); + } + Ok(schema) } /// Sanitize a JSON Schema (as serde_json::Value) so it can fit our limited -/// JsonSchema enum. This function: -/// - Ensures every schema object has a "type". If missing, infers it from -/// common keywords (properties => object, items => array, enum/const/format => string) -/// and otherwise defaults to "string". -/// - Fills required child fields (e.g. array items, object properties) with -/// permissive defaults when absent. +/// schema representation. This function: +/// - Ensures every typed schema object has a `"type"` when required. +/// - Preserves explicit `anyOf`. +/// - Collapses `const` into single-value `enum`. +/// - Fills required child fields for object/array schema types, including +/// nullable unions, with permissive defaults when absent. fn sanitize_json_schema(value: &mut JsonValue) { match value { JsonValue::Bool(_) => { @@ -96,81 +188,153 @@ fn sanitize_json_schema(value: &mut JsonValue) { if let Some(items) = map.get_mut("items") { sanitize_json_schema(items); } - for combiner in ["oneOf", "anyOf", "allOf", "prefixItems"] { - if let Some(value) = map.get_mut(combiner) { - sanitize_json_schema(value); - } - } - - let mut schema_type = map - .get("type") - .and_then(|value| value.as_str()) - .map(str::to_string); - - if schema_type.is_none() - && let Some(JsonValue::Array(types)) = map.get("type") + if let Some(additional_properties) = map.get_mut("additionalProperties") + && !matches!(additional_properties, JsonValue::Bool(_)) { - for candidate in types { - if let Some(candidate_type) = candidate.as_str() - && matches!( - candidate_type, - "object" | "array" | "string" | "number" | "integer" | "boolean" - ) - { - schema_type = Some(candidate_type.to_string()); - break; - } - } + sanitize_json_schema(additional_properties); + } + if let Some(value) = map.get_mut("prefixItems") { + sanitize_json_schema(value); + } + if let Some(value) = map.get_mut("anyOf") { + sanitize_json_schema(value); } - if schema_type.is_none() { + if let Some(const_value) = map.remove("const") { + map.insert("enum".to_string(), JsonValue::Array(vec![const_value])); + } + + let mut schema_types = normalized_schema_types(map); + + if schema_types.is_empty() && map.contains_key("anyOf") { + return; + } + + if schema_types.is_empty() { if map.contains_key("properties") || map.contains_key("required") || map.contains_key("additionalProperties") { - schema_type = Some("object".to_string()); + schema_types.push(JsonSchemaPrimitiveType::Object); } else if map.contains_key("items") || map.contains_key("prefixItems") { - schema_type = Some("array".to_string()); - } else if map.contains_key("enum") - || map.contains_key("const") - || map.contains_key("format") - { - schema_type = Some("string".to_string()); + schema_types.push(JsonSchemaPrimitiveType::Array); + } else if map.contains_key("enum") || map.contains_key("format") { + schema_types.push(JsonSchemaPrimitiveType::String); } else if map.contains_key("minimum") || map.contains_key("maximum") || map.contains_key("exclusiveMinimum") || map.contains_key("exclusiveMaximum") || map.contains_key("multipleOf") { - schema_type = Some("number".to_string()); + schema_types.push(JsonSchemaPrimitiveType::Number); + } else { + schema_types.push(JsonSchemaPrimitiveType::String); } } - let schema_type = schema_type.unwrap_or_else(|| "string".to_string()); - map.insert("type".to_string(), JsonValue::String(schema_type.clone())); - - if schema_type == "object" { - if !map.contains_key("properties") { - map.insert( - "properties".to_string(), - JsonValue::Object(serde_json::Map::new()), - ); - } - if let Some(additional_properties) = map.get_mut("additionalProperties") - && !matches!(additional_properties, JsonValue::Bool(_)) - { - sanitize_json_schema(additional_properties); - } - } - - if schema_type == "array" && !map.contains_key("items") { - map.insert("items".to_string(), json!({ "type": "string" })); - } + write_schema_types(map, &schema_types); + ensure_default_children_for_schema_types(map, &schema_types); } _ => {} } } +fn ensure_default_children_for_schema_types( + map: &mut serde_json::Map, + schema_types: &[JsonSchemaPrimitiveType], +) { + if schema_types.contains(&JsonSchemaPrimitiveType::Object) && !map.contains_key("properties") { + map.insert( + "properties".to_string(), + JsonValue::Object(serde_json::Map::new()), + ); + } + + if schema_types.contains(&JsonSchemaPrimitiveType::Array) && !map.contains_key("items") { + map.insert("items".to_string(), json!({ "type": "string" })); + } +} + +fn normalized_schema_types( + map: &serde_json::Map, +) -> Vec { + let Some(schema_type) = map.get("type") else { + return Vec::new(); + }; + + match schema_type { + JsonValue::String(schema_type) => schema_type_from_str(schema_type).into_iter().collect(), + JsonValue::Array(schema_types) => schema_types + .iter() + .filter_map(JsonValue::as_str) + .filter_map(schema_type_from_str) + .collect(), + _ => Vec::new(), + } +} + +fn write_schema_types( + map: &mut serde_json::Map, + schema_types: &[JsonSchemaPrimitiveType], +) { + match schema_types { + [] => { + map.remove("type"); + } + [schema_type] => { + map.insert( + "type".to_string(), + JsonValue::String(schema_type_name(*schema_type).to_string()), + ); + } + _ => { + map.insert( + "type".to_string(), + JsonValue::Array( + schema_types + .iter() + .map(|schema_type| { + JsonValue::String(schema_type_name(*schema_type).to_string()) + }) + .collect(), + ), + ); + } + } +} + +fn schema_type_from_str(schema_type: &str) -> Option { + match schema_type { + "string" => Some(JsonSchemaPrimitiveType::String), + "number" => Some(JsonSchemaPrimitiveType::Number), + "boolean" => Some(JsonSchemaPrimitiveType::Boolean), + "integer" => Some(JsonSchemaPrimitiveType::Integer), + "object" => Some(JsonSchemaPrimitiveType::Object), + "array" => Some(JsonSchemaPrimitiveType::Array), + "null" => Some(JsonSchemaPrimitiveType::Null), + _ => None, + } +} + +fn schema_type_name(schema_type: JsonSchemaPrimitiveType) -> &'static str { + match schema_type { + JsonSchemaPrimitiveType::String => "string", + JsonSchemaPrimitiveType::Number => "number", + JsonSchemaPrimitiveType::Boolean => "boolean", + JsonSchemaPrimitiveType::Integer => "integer", + JsonSchemaPrimitiveType::Object => "object", + JsonSchemaPrimitiveType::Array => "array", + JsonSchemaPrimitiveType::Null => "null", + } +} + +fn singleton_null_schema_error() -> serde_json::Error { + serde_json::Error::io(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "tool input schema must not be a singleton null type", + )) +} + #[cfg(test)] #[path = "json_schema_tests.rs"] mod tests; diff --git a/codex-rs/tools/src/json_schema_tests.rs b/codex-rs/tools/src/json_schema_tests.rs index d4c829c0e..3f13df763 100644 --- a/codex-rs/tools/src/json_schema_tests.rs +++ b/codex-rs/tools/src/json_schema_tests.rs @@ -1,5 +1,7 @@ use super::AdditionalProperties; use super::JsonSchema; +use super::JsonSchemaPrimitiveType; +use super::JsonSchemaType; use super::parse_tool_input_schema; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -18,7 +20,7 @@ fn parse_tool_input_schema_coerces_boolean_schemas() { // semantics directly. let schema = parse_tool_input_schema(&serde_json::json!(true)).expect("parse schema"); - assert_eq!(schema, JsonSchema::String { description: None }); + assert_eq!(schema, JsonSchema::string(/*description*/ None)); } #[test] @@ -42,21 +44,19 @@ fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() { assert_eq!( schema, - JsonSchema::Object { - properties: BTreeMap::from([( + JsonSchema::object( + BTreeMap::from([( "query".to_string(), - JsonSchema::String { - description: Some("search query".to_string()), - }, + JsonSchema::string(Some("search query".to_string())), )]), - required: None, - additional_properties: None, - } + /*required*/ None, + /*additional_properties*/ None + ) ); } #[test] -fn parse_tool_input_schema_normalizes_integer_and_missing_array_items() { +fn parse_tool_input_schema_preserves_integer_and_defaults_array_items() { // Example schema shape: // { // "type": "object", @@ -67,8 +67,7 @@ fn parse_tool_input_schema_normalizes_integer_and_missing_array_items() { // } // // Expected normalization behavior: - // - `"integer"` is accepted by the baseline model through the legacy - // number/integer alias. + // - `"integer"` is preserved distinctly from `"number"`. // - Arrays missing `items` receive a permissive string `items` schema. let schema = parse_tool_input_schema(&serde_json::json!({ "type": "object", @@ -81,20 +80,23 @@ fn parse_tool_input_schema_normalizes_integer_and_missing_array_items() { assert_eq!( schema, - JsonSchema::Object { - properties: BTreeMap::from([ - ("page".to_string(), JsonSchema::Number { description: None }), + JsonSchema::object( + BTreeMap::from([ + ( + "page".to_string(), + JsonSchema::integer(/*description*/ None), + ), ( "tags".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: None, - }, + JsonSchema::array( + JsonSchema::string(/*description*/ None), + /*description*/ None, + ) ), ]), - required: None, - additional_properties: None, - } + /*required*/ None, + /*additional_properties*/ None + ) ); } @@ -118,9 +120,7 @@ fn parse_tool_input_schema_sanitizes_additional_properties_schema() { // // Expected normalization behavior: // - `additionalProperties` schema objects are recursively sanitized. - // - The nested schema is normalized into the baseline object form. - // - In the baseline model, the nested `anyOf` degrades to a plain string - // field because richer combiners are not preserved. + // - The nested schema is normalized into the current object/anyOf form. let schema = parse_tool_input_schema(&serde_json::json!({ "type": "object", "additionalProperties": { @@ -134,20 +134,24 @@ fn parse_tool_input_schema_sanitizes_additional_properties_schema() { assert_eq!( schema, - JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: Some(AdditionalProperties::Schema(Box::new( - JsonSchema::Object { - properties: BTreeMap::from([( - "value".to_string(), - JsonSchema::String { description: None }, - )]), - required: Some(vec!["value".to_string()]), - additional_properties: None, - }, - ))), - } + JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + Some(AdditionalProperties::Schema(Box::new(JsonSchema::object( + BTreeMap::from([( + "value".to_string(), + JsonSchema::any_of( + vec![ + JsonSchema::string(/*description*/ None), + JsonSchema::number(/*description*/ None), + ], + /*description*/ None, + ), + )]), + Some(vec!["value".to_string()]), + /*additional_properties*/ None, + )))) + ) ); } @@ -168,11 +172,7 @@ fn parse_tool_input_schema_infers_object_shape_from_boolean_additional_propertie assert_eq!( schema, - JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: Some(false.into()), - } + JsonSchema::object(BTreeMap::new(), /*required*/ None, Some(false.into())) ); } @@ -191,7 +191,7 @@ fn parse_tool_input_schema_infers_number_from_numeric_keywords() { })) .expect("parse schema"); - assert_eq!(schema, JsonSchema::Number { description: None }); + assert_eq!(schema, JsonSchema::number(/*description*/ None)); } #[test] @@ -209,7 +209,7 @@ fn parse_tool_input_schema_infers_number_from_multiple_of() { })) .expect("parse schema"); - assert_eq!(schema, JsonSchema::Number { description: None }); + assert_eq!(schema, JsonSchema::number(/*description*/ None)); } #[test] @@ -220,7 +220,8 @@ fn parse_tool_input_schema_infers_string_from_enum_const_and_format_keywords() { // { "format": "date-time" } // // Expected normalization behavior: - // - Each of these keywords implies a string schema when `type` is omitted. + // - `enum` and `const` normalize into explicit string-enum schemas. + // - `format` still falls back to a plain string schema. let enum_schema = parse_tool_input_schema(&serde_json::json!({ "enum": ["fast", "safe"] })) @@ -234,9 +235,18 @@ fn parse_tool_input_schema_infers_string_from_enum_const_and_format_keywords() { })) .expect("parse format schema"); - assert_eq!(enum_schema, JsonSchema::String { description: None }); - assert_eq!(const_schema, JsonSchema::String { description: None }); - assert_eq!(format_schema, JsonSchema::String { description: None }); + assert_eq!( + enum_schema, + JsonSchema::string_enum( + vec![serde_json::json!("fast"), serde_json::json!("safe")], + /*description*/ None, + ) + ); + assert_eq!( + const_schema, + JsonSchema::string_enum(vec![serde_json::json!("file")], /*description*/ None) + ); + assert_eq!(format_schema, JsonSchema::string(/*description*/ None)); } #[test] @@ -245,11 +255,11 @@ fn parse_tool_input_schema_defaults_empty_schema_to_string() { // {} // // Expected normalization behavior: - // - With no structural hints at all, the baseline normalizer falls back to - // a permissive string schema. + // - With no structural hints at all, the normalizer falls back to a + // permissive string schema. let schema = parse_tool_input_schema(&serde_json::json!({})).expect("parse schema"); - assert_eq!(schema, JsonSchema::String { description: None }); + assert_eq!(schema, JsonSchema::string(/*description*/ None)); } #[test] @@ -263,8 +273,8 @@ fn parse_tool_input_schema_infers_array_from_prefix_items() { // // Expected normalization behavior: // - `prefixItems` implies an array schema when `type` is omitted. - // - The baseline model still stores the normalized result as a regular - // array schema with string items. + // - The normalized result is stored as a regular array schema with string + // items. let schema = parse_tool_input_schema(&serde_json::json!({ "prefixItems": [ {"type": "string"} @@ -274,10 +284,10 @@ fn parse_tool_input_schema_infers_array_from_prefix_items() { assert_eq!( schema, - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: None, - } + JsonSchema::array( + JsonSchema::string(/*description*/ None), + /*description*/ None, + ) ); } @@ -309,18 +319,14 @@ fn parse_tool_input_schema_preserves_boolean_additional_properties_on_inferred_o assert_eq!( schema, - JsonSchema::Object { - properties: BTreeMap::from([( + JsonSchema::object( + BTreeMap::from([( "metadata".to_string(), - JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: Some(AdditionalProperties::Boolean(true)), - }, + JsonSchema::object(BTreeMap::new(), /*required*/ None, Some(true.into())), )]), - required: None, - additional_properties: None, - } + /*required*/ None, + /*additional_properties*/ None + ) ); } @@ -347,22 +353,202 @@ fn parse_tool_input_schema_infers_object_shape_from_schema_additional_properties assert_eq!( schema, - JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: Some(AdditionalProperties::Schema(Box::new( - JsonSchema::String { description: None }, - ))), + JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + Some(JsonSchema::string(/*description*/ None).into()) + ) + ); +} + +#[test] +fn parse_tool_input_schema_rewrites_const_to_single_value_enum() { + // Example schema shape: + // { + // "const": "tagged" + // } + // + // Expected normalization behavior: + // - `const` is rewritten through the sanitizer's `map.remove("const")` + // path into an equivalent single-value string enum schema. + let schema = parse_tool_input_schema(&serde_json::json!({ + "const": "tagged" + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema::string_enum(vec![serde_json::json!("tagged")], /*description*/ None) + ); +} + +#[test] +fn parse_tool_input_schema_rejects_singleton_null_type() { + let err = parse_tool_input_schema(&serde_json::json!({ + "type": "null" + })) + .expect_err("singleton null should be rejected"); + + assert!( + err.to_string() + .contains("tool input schema must not be a singleton null type"), + "unexpected error: {err}" + ); +} + +#[test] +fn parse_tool_input_schema_fills_default_properties_for_nullable_object_union() { + // Example schema shape: + // { + // "type": ["object", "null"] + // } + // + // Expected normalization behavior: + // - The full union is preserved. + // - Object members of the union still receive default `properties`. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": ["object", "null"] + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Multiple(vec![ + JsonSchemaPrimitiveType::Object, + JsonSchemaPrimitiveType::Null, + ])), + properties: Some(BTreeMap::new()), + ..Default::default() + } + ); +} + +#[test] +fn parse_tool_input_schema_fills_default_items_for_nullable_array_union() { + // Example schema shape: + // { + // "type": ["array", "null"] + // } + // + // Expected normalization behavior: + // - The full union is preserved. + // - Array members of the union still receive default `items`. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": ["array", "null"] + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Multiple(vec![ + JsonSchemaPrimitiveType::Array, + JsonSchemaPrimitiveType::Null, + ])), + items: Some(Box::new(JsonSchema::string(/*description*/ None))), + ..Default::default() } ); } // Schemas that should be preserved for Responses API compatibility rather than -// being rewritten into a different shape. These currently fail on the baseline -// normalizer and are the intended signal for the new JsonSchema work. +// being rewritten into a different shape. + +#[test] +fn parse_tool_input_schema_preserves_nested_nullable_any_of_shape() { + // Example schema shape: + // { + // "type": "object", + // "properties": { + // "open": { + // "anyOf": [ + // { + // "type": "array", + // "items": { + // "type": "object", + // "properties": { + // "ref_id": { "type": "string" }, + // "lineno": { "anyOf": [{ "type": "integer" }, { "type": "null" }] } + // }, + // "required": ["ref_id"], + // "additionalProperties": false + // } + // }, + // { "type": "null" } + // ] + // } + // } + // } + // + // Expected normalization behavior: + // - Nested nullable `anyOf` shapes are preserved all the way down. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "open": { + "anyOf": [ + { + "type": "array", + "items": { + "type": "object", + "properties": { + "ref_id": {"type": "string"}, + "lineno": {"anyOf": [{"type": "integer"}, {"type": "null"}]} + }, + "required": ["ref_id"], + "additionalProperties": false + } + }, + {"type": "null"} + ] + } + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema::object( + BTreeMap::from([( + "open".to_string(), + JsonSchema::any_of( + vec![ + JsonSchema::array( + JsonSchema::object( + BTreeMap::from([ + ( + "lineno".to_string(), + JsonSchema::any_of( + vec![ + JsonSchema::integer(/*description*/ None), + JsonSchema::null(/*description*/ None), + ], + /*description*/ None, + ), + ), + ( + "ref_id".to_string(), + JsonSchema::string(/*description*/ None), + ), + ]), + Some(vec!["ref_id".to_string()]), + Some(false.into()), + ), + /*description*/ None, + ), + JsonSchema::null(/*description*/ None), + ], + /*description*/ None, + ), + ),]), + /*required*/ None, + /*additional_properties*/ None + ) + ); +} #[test] -#[ignore = "Expected to pass after the new JsonSchema preserves nullable type unions"] fn parse_tool_input_schema_preserves_nested_nullable_type_union() { // Example schema shape: // { @@ -395,23 +581,25 @@ fn parse_tool_input_schema_preserves_nested_nullable_type_union() { assert_eq!( schema, - JsonSchema::Object { - properties: BTreeMap::from([( + JsonSchema::object( + BTreeMap::from([( "nickname".to_string(), - serde_json::from_value(serde_json::json!({ - "type": ["string", "null"], - "description": "Optional nickname" - })) - .expect("nested nullable schema"), + JsonSchema { + schema_type: Some(JsonSchemaType::Multiple(vec![ + JsonSchemaPrimitiveType::String, + JsonSchemaPrimitiveType::Null, + ])), + description: Some("Optional nickname".to_string()), + ..Default::default() + }, )]), - required: Some(vec!["nickname".to_string()]), - additional_properties: Some(false.into()), - } + Some(vec!["nickname".to_string()]), + Some(false.into()), + ) ); } #[test] -#[ignore = "Expected to pass after the new JsonSchema preserves nested anyOf schemas"] fn parse_tool_input_schema_preserves_nested_any_of_property() { // Example schema shape: // { @@ -444,19 +632,155 @@ fn parse_tool_input_schema_preserves_nested_any_of_property() { assert_eq!( schema, - JsonSchema::Object { - properties: BTreeMap::from([( + JsonSchema::object( + BTreeMap::from([( "query".to_string(), - serde_json::from_value(serde_json::json!({ - "anyOf": [ - { "type": "string" }, - { "type": "number" } - ] - })) - .expect("nested anyOf schema"), + JsonSchema::any_of( + vec![ + JsonSchema::string(/*description*/ None), + JsonSchema::number(/*description*/ None), + ], + /*description*/ None, + ), )]), - required: None, - additional_properties: None, + /*required*/ None, + /*additional_properties*/ None + ) + ); +} + +#[test] +fn parse_tool_input_schema_preserves_type_unions_without_rewriting_to_any_of() { + // Example schema shape: + // { + // "type": ["string", "null"], + // "description": "optional string" + // } + // + // Expected normalization behavior: + // - Explicit type unions are preserved as unions rather than rewritten to + // `anyOf`. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": ["string", "null"], + "description": "optional string" + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Multiple(vec![ + JsonSchemaPrimitiveType::String, + JsonSchemaPrimitiveType::Null, + ])), + description: Some("optional string".to_string()), + ..Default::default() } ); } + +#[test] +fn parse_tool_input_schema_preserves_explicit_enum_type_union() { + // Example schema shape: + // { + // "type": ["string", "null"], + // "enum": ["short", "medium", "long"], + // "description": "optional response length" + // } + // + // Expected normalization behavior: + // - The explicit string/null union is preserved alongside the enum values. + let schema = super::parse_tool_input_schema(&serde_json::json!({ + "type": ["string", "null"], + "enum": ["short", "medium", "long"], + "description": "optional response length" + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Multiple(vec![ + JsonSchemaPrimitiveType::String, + JsonSchemaPrimitiveType::Null, + ])), + description: Some("optional response length".to_string()), + enum_values: Some(vec![ + serde_json::json!("short"), + serde_json::json!("medium"), + serde_json::json!("long"), + ]), + ..Default::default() + } + ); +} + +#[test] +fn parse_tool_input_schema_preserves_string_enum_constraints() { + // Example schema shape: + // { + // "type": "object", + // "properties": { + // "response_length": { "type": "enum", "enum": ["short", "medium", "long"] }, + // "kind": { "type": "const", "const": "tagged" }, + // "scope": { "type": "enum", "enum": ["one", "two"] } + // } + // } + // + // Expected normalization behavior: + // - Legacy `type: "enum"` and `type: "const"` inputs are normalized into + // the current string-enum representation. + let schema = super::parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "response_length": { + "type": "enum", + "enum": ["short", "medium", "long"] + }, + "kind": { + "type": "const", + "const": "tagged" + }, + "scope": { + "type": "enum", + "enum": ["one", "two"] + } + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema::object( + BTreeMap::from([ + ( + "kind".to_string(), + JsonSchema::string_enum( + vec![serde_json::json!("tagged")], + /*description*/ None, + ), + ), + ( + "response_length".to_string(), + JsonSchema::string_enum( + vec![ + serde_json::json!("short"), + serde_json::json!("medium"), + serde_json::json!("long"), + ], + /*description*/ None, + ), + ), + ( + "scope".to_string(), + JsonSchema::string_enum( + vec![serde_json::json!("one"), serde_json::json!("two")], + /*description*/ None, + ), + ), + ]), + /*required*/ None, + /*additional_properties*/ None + ) + ); +} diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 16e84700b..4fa90783f 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -55,6 +55,8 @@ pub use js_repl_tool::create_js_repl_reset_tool; pub use js_repl_tool::create_js_repl_tool; pub use json_schema::AdditionalProperties; pub use json_schema::JsonSchema; +pub use json_schema::JsonSchemaPrimitiveType; +pub use json_schema::JsonSchemaType; pub use json_schema::parse_tool_input_schema; pub use local_tool::CommandToolOptions; pub use local_tool::ShellToolOptions; diff --git a/codex-rs/tools/src/local_tool.rs b/codex-rs/tools/src/local_tool.rs index 8779ac263..3e369ab1e 100644 --- a/codex-rs/tools/src/local_tool.rs +++ b/codex-rs/tools/src/local_tool.rs @@ -20,62 +20,47 @@ pub fn create_exec_command_tool(options: CommandToolOptions) -> ToolSpec { let mut properties = BTreeMap::from([ ( "cmd".to_string(), - JsonSchema::String { - description: Some("Shell command to execute.".to_string()), - }, + JsonSchema::string(Some("Shell command to execute.".to_string())), ), ( "workdir".to_string(), - JsonSchema::String { - description: Some( - "Optional working directory to run the command in; defaults to the turn cwd." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Optional working directory to run the command in; defaults to the turn cwd." + .to_string(), + )), ), ( "shell".to_string(), - JsonSchema::String { - description: Some( - "Shell binary to launch. Defaults to the user's default shell.".to_string(), - ), - }, + JsonSchema::string(Some( + "Shell binary to launch. Defaults to the user's default shell.".to_string(), + )), ), ( "tty".to_string(), - JsonSchema::Boolean { - description: Some( - "Whether to allocate a TTY for the command. Defaults to false (plain pipes); set to true to open a PTY and access TTY process." - .to_string(), - ), - }, + JsonSchema::boolean(Some( + "Whether to allocate a TTY for the command. Defaults to false (plain pipes); set to true to open a PTY and access TTY process." + .to_string(), + )), ), ( "yield_time_ms".to_string(), - JsonSchema::Number { - description: Some( - "How long to wait (in milliseconds) for output before yielding.".to_string(), - ), - }, + JsonSchema::number(Some( + "How long to wait (in milliseconds) for output before yielding.".to_string(), + )), ), ( "max_output_tokens".to_string(), - JsonSchema::Number { - description: Some( - "Maximum number of tokens to return. Excess output will be truncated." - .to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum number of tokens to return. Excess output will be truncated.".to_string(), + )), ), ]); if options.allow_login_shell { properties.insert( "login".to_string(), - JsonSchema::Boolean { - description: Some( - "Whether to run the shell with -l/-i semantics. Defaults to true.".to_string(), - ), - }, + JsonSchema::boolean(Some( + "Whether to run the shell with -l/-i semantics. Defaults to true.".to_string(), + )), ); } properties.extend(create_approval_parameters( @@ -95,11 +80,11 @@ pub fn create_exec_command_tool(options: CommandToolOptions) -> ToolSpec { }, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["cmd".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["cmd".to_string()]), + Some(false.into()), + ), output_schema: Some(unified_exec_output_schema()), }) } @@ -108,32 +93,27 @@ pub fn create_write_stdin_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "session_id".to_string(), - JsonSchema::Number { - description: Some("Identifier of the running unified exec session.".to_string()), - }, + JsonSchema::number(Some( + "Identifier of the running unified exec session.".to_string(), + )), ), ( "chars".to_string(), - JsonSchema::String { - description: Some("Bytes to write to stdin (may be empty to poll).".to_string()), - }, + JsonSchema::string(Some( + "Bytes to write to stdin (may be empty to poll).".to_string(), + )), ), ( "yield_time_ms".to_string(), - JsonSchema::Number { - description: Some( - "How long to wait (in milliseconds) for output before yielding.".to_string(), - ), - }, + JsonSchema::number(Some( + "How long to wait (in milliseconds) for output before yielding.".to_string(), + )), ), ( "max_output_tokens".to_string(), - JsonSchema::Number { - description: Some( - "Maximum number of tokens to return. Excess output will be truncated." - .to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum number of tokens to return. Excess output will be truncated.".to_string(), + )), ), ]); @@ -144,11 +124,11 @@ pub fn create_write_stdin_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["session_id".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["session_id".to_string()]), + Some(false.into()), + ), output_schema: Some(unified_exec_output_schema()), }) } @@ -157,22 +137,22 @@ pub fn create_shell_tool(options: ShellToolOptions) -> ToolSpec { let mut properties = BTreeMap::from([ ( "command".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some("The command to execute".to_string()), - }, + JsonSchema::array( + JsonSchema::string(/*description*/ None), + Some("The command to execute".to_string()), + ), ), ( "workdir".to_string(), - JsonSchema::String { - description: Some("The working directory to execute the command in".to_string()), - }, + JsonSchema::string(Some( + "The working directory to execute the command in".to_string(), + )), ), ( "timeout_ms".to_string(), - JsonSchema::Number { - description: Some("The timeout for the command in milliseconds".to_string()), - }, + JsonSchema::number(Some( + "The timeout for the command in milliseconds".to_string(), + )), ), ]); properties.extend(create_approval_parameters( @@ -207,11 +187,11 @@ Examples of valid command strings: description, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["command".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["command".to_string()]), + Some(false.into()), + ), output_schema: None, }) } @@ -220,34 +200,30 @@ pub fn create_shell_command_tool(options: CommandToolOptions) -> ToolSpec { let mut properties = BTreeMap::from([ ( "command".to_string(), - JsonSchema::String { - description: Some( - "The shell script to execute in the user's default shell".to_string(), - ), - }, + JsonSchema::string(Some( + "The shell script to execute in the user's default shell".to_string(), + )), ), ( "workdir".to_string(), - JsonSchema::String { - description: Some("The working directory to execute the command in".to_string()), - }, + JsonSchema::string(Some( + "The working directory to execute the command in".to_string(), + )), ), ( "timeout_ms".to_string(), - JsonSchema::Number { - description: Some("The timeout for the command in milliseconds".to_string()), - }, + JsonSchema::number(Some( + "The timeout for the command in milliseconds".to_string(), + )), ), ]); if options.allow_login_shell { properties.insert( "login".to_string(), - JsonSchema::Boolean { - description: Some( - "Whether to run the shell with login shell semantics. Defaults to true." - .to_string(), - ), - }, + JsonSchema::boolean(Some( + "Whether to run the shell with login shell semantics. Defaults to true." + .to_string(), + )), ); } properties.extend(create_approval_parameters( @@ -281,11 +257,11 @@ Examples of valid command strings: description, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["command".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["command".to_string()]), + Some(false.into()), + ), output_schema: None, }) } @@ -294,12 +270,9 @@ pub fn create_request_permissions_tool(description: String) -> ToolSpec { let properties = BTreeMap::from([ ( "reason".to_string(), - JsonSchema::String { - description: Some( - "Optional short explanation for why additional permissions are needed." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Optional short explanation for why additional permissions are needed.".to_string(), + )), ), ("permissions".to_string(), permission_profile_schema()), ]); @@ -309,11 +282,11 @@ pub fn create_request_permissions_tool(description: String) -> ToolSpec { description, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["permissions".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["permissions".to_string()]), + Some(false.into()), + ), output_schema: None, }) } @@ -363,40 +336,33 @@ fn create_approval_parameters( let mut properties = BTreeMap::from([ ( "sandbox_permissions".to_string(), - JsonSchema::String { - description: Some( - if exec_permission_approvals_enabled { - "Sandbox permissions for the command. Use \"with_additional_permissions\" to request additional sandboxed filesystem or network permissions (preferred), or \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"." - } else { - "Sandbox permissions for the command. Set to \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"." - } - .to_string(), - ), - }, + JsonSchema::string(Some( + if exec_permission_approvals_enabled { + "Sandbox permissions for the command. Use \"with_additional_permissions\" to request additional sandboxed filesystem or network permissions (preferred), or \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"." + } else { + "Sandbox permissions for the command. Set to \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"." + } + .to_string(), + )), ), ( "justification".to_string(), - JsonSchema::String { - description: Some( - r#"Only set if sandbox_permissions is \"require_escalated\". + JsonSchema::string(Some( + r#"Only set if sandbox_permissions is \"require_escalated\". Request approval from the user to run this command outside the sandbox. Phrased as a simple question that summarizes the purpose of the command as it relates to the task at hand - e.g. 'Do you want to fetch and pull the latest version of this git branch?'"# .to_string(), - ), - }, + )), ), ( "prefix_rule".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some( + JsonSchema::array(JsonSchema::string(/*description*/ None), Some( r#"Only specify when sandbox_permissions is `require_escalated`. Suggest a prefix command pattern that will allow you to fulfill similar requests from the user in the future. Should be a short but reasonable prefix, e.g. [\"git\", \"pull\"] or [\"uv\", \"run\"] or [\"pytest\"]."#.to_string(), - ), - }, + )), ), ]); @@ -411,50 +377,48 @@ fn create_approval_parameters( } fn permission_profile_schema() -> JsonSchema { - JsonSchema::Object { - properties: BTreeMap::from([ + JsonSchema::object( + BTreeMap::from([ ("network".to_string(), network_permissions_schema()), ("file_system".to_string(), file_system_permissions_schema()), ]), - required: None, - additional_properties: Some(false.into()), - } + /*required*/ None, + Some(false.into()), + ) } fn network_permissions_schema() -> JsonSchema { - JsonSchema::Object { - properties: BTreeMap::from([( + JsonSchema::object( + BTreeMap::from([( "enabled".to_string(), - JsonSchema::Boolean { - description: Some("Set to true to request network access.".to_string()), - }, + JsonSchema::boolean(Some("Set to true to request network access.".to_string())), )]), - required: None, - additional_properties: Some(false.into()), - } + /*required*/ None, + Some(false.into()), + ) } fn file_system_permissions_schema() -> JsonSchema { - JsonSchema::Object { - properties: BTreeMap::from([ + JsonSchema::object( + BTreeMap::from([ ( "read".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some("Absolute paths to grant read access to.".to_string()), - }, + JsonSchema::array( + JsonSchema::string(/*description*/ None), + Some("Absolute paths to grant read access to.".to_string()), + ), ), ( "write".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some("Absolute paths to grant write access to.".to_string()), - }, + JsonSchema::array( + JsonSchema::string(/*description*/ None), + Some("Absolute paths to grant write access to.".to_string()), + ), ), ]), - required: None, - additional_properties: Some(false.into()), - } + /*required*/ None, + Some(false.into()), + ) } fn windows_destructive_filesystem_guidance() -> &'static str { diff --git a/codex-rs/tools/src/local_tool_tests.rs b/codex-rs/tools/src/local_tool_tests.rs index a8a914001..b751545b3 100644 --- a/codex-rs/tools/src/local_tool_tests.rs +++ b/codex-rs/tools/src/local_tool_tests.rs @@ -35,56 +35,42 @@ Examples of valid command strings: let properties = BTreeMap::from([ ( "command".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some("The command to execute".to_string()), - }, + JsonSchema::array(JsonSchema::string(/*description*/ None), Some("The command to execute".to_string())), ), ( "workdir".to_string(), - JsonSchema::String { - description: Some("The working directory to execute the command in".to_string()), - }, + JsonSchema::string(Some("The working directory to execute the command in".to_string())), ), ( "timeout_ms".to_string(), - JsonSchema::Number { - description: Some("The timeout for the command in milliseconds".to_string()), - }, + JsonSchema::number(Some("The timeout for the command in milliseconds".to_string())), ), ( "sandbox_permissions".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Sandbox permissions for the command. Set to \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"." .to_string(), - ), - }, + )), ), ( "justification".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( r#"Only set if sandbox_permissions is \"require_escalated\". Request approval from the user to run this command outside the sandbox. Phrased as a simple question that summarizes the purpose of the command as it relates to the task at hand - e.g. 'Do you want to fetch and pull the latest version of this git branch?'"# .to_string(), - ), - }, + )), ), ( "prefix_rule".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some( + JsonSchema::array(JsonSchema::string(/*description*/ None), Some( r#"Only specify when sandbox_permissions is `require_escalated`. Suggest a prefix command pattern that will allow you to fulfill similar requests from the user in the future. Should be a short but reasonable prefix, e.g. [\"git\", \"pull\"] or [\"uv\", \"run\"] or [\"pytest\"]."# .to_string(), - ), - }, + )), ), ]); @@ -95,11 +81,11 @@ Examples of valid command strings: description, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["command".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["command".to_string()]), + Some(false.into()) + ), output_schema: None, }) ); @@ -125,60 +111,46 @@ fn exec_command_tool_matches_expected_spec() { let mut properties = BTreeMap::from([ ( "cmd".to_string(), - JsonSchema::String { - description: Some("Shell command to execute.".to_string()), - }, + JsonSchema::string(Some("Shell command to execute.".to_string())), ), ( "workdir".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Optional working directory to run the command in; defaults to the turn cwd." .to_string(), - ), - }, + )), ), ( "shell".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Shell binary to launch. Defaults to the user's default shell.".to_string(), - ), - }, + )), ), ( "tty".to_string(), - JsonSchema::Boolean { - description: Some( + JsonSchema::boolean(Some( "Whether to allocate a TTY for the command. Defaults to false (plain pipes); set to true to open a PTY and access TTY process." .to_string(), - ), - }, + )), ), ( "yield_time_ms".to_string(), - JsonSchema::Number { - description: Some( + JsonSchema::number(Some( "How long to wait (in milliseconds) for output before yielding.".to_string(), - ), - }, + )), ), ( "max_output_tokens".to_string(), - JsonSchema::Number { - description: Some( + JsonSchema::number(Some( "Maximum number of tokens to return. Excess output will be truncated." .to_string(), - ), - }, + )), ), ( "login".to_string(), - JsonSchema::Boolean { - description: Some( + JsonSchema::boolean(Some( "Whether to run the shell with -l/-i semantics. Defaults to true.".to_string(), - ), - }, + )), ), ]); properties.extend(create_approval_parameters( @@ -192,11 +164,11 @@ fn exec_command_tool_matches_expected_spec() { description, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["cmd".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["cmd".to_string()]), + Some(false.into()) + ), output_schema: Some(unified_exec_output_schema()), }) ); @@ -209,32 +181,27 @@ fn write_stdin_tool_matches_expected_spec() { let properties = BTreeMap::from([ ( "session_id".to_string(), - JsonSchema::Number { - description: Some("Identifier of the running unified exec session.".to_string()), - }, + JsonSchema::number(Some( + "Identifier of the running unified exec session.".to_string(), + )), ), ( "chars".to_string(), - JsonSchema::String { - description: Some("Bytes to write to stdin (may be empty to poll).".to_string()), - }, + JsonSchema::string(Some( + "Bytes to write to stdin (may be empty to poll).".to_string(), + )), ), ( "yield_time_ms".to_string(), - JsonSchema::Number { - description: Some( - "How long to wait (in milliseconds) for output before yielding.".to_string(), - ), - }, + JsonSchema::number(Some( + "How long to wait (in milliseconds) for output before yielding.".to_string(), + )), ), ( "max_output_tokens".to_string(), - JsonSchema::Number { - description: Some( - "Maximum number of tokens to return. Excess output will be truncated." - .to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum number of tokens to return. Excess output will be truncated.".to_string(), + )), ), ]); @@ -247,11 +214,11 @@ fn write_stdin_tool_matches_expected_spec() { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["session_id".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["session_id".to_string()]), + Some(false.into()) + ), output_schema: Some(unified_exec_output_schema()), }) ); @@ -266,22 +233,22 @@ fn shell_tool_with_request_permission_includes_additional_permissions() { let mut properties = BTreeMap::from([ ( "command".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some("The command to execute".to_string()), - }, + JsonSchema::array( + JsonSchema::string(/*description*/ None), + Some("The command to execute".to_string()), + ), ), ( "workdir".to_string(), - JsonSchema::String { - description: Some("The working directory to execute the command in".to_string()), - }, + JsonSchema::string(Some( + "The working directory to execute the command in".to_string(), + )), ), ( "timeout_ms".to_string(), - JsonSchema::Number { - description: Some("The timeout for the command in milliseconds".to_string()), - }, + JsonSchema::number(Some( + "The timeout for the command in milliseconds".to_string(), + )), ), ]); properties.extend(create_approval_parameters( @@ -318,11 +285,11 @@ Examples of valid command strings: description, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["command".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["command".to_string()]), + Some(false.into()) + ), output_schema: None, }) ); @@ -336,12 +303,9 @@ fn request_permissions_tool_includes_full_permission_schema() { let properties = BTreeMap::from([ ( "reason".to_string(), - JsonSchema::String { - description: Some( - "Optional short explanation for why additional permissions are needed." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Optional short explanation for why additional permissions are needed.".to_string(), + )), ), ("permissions".to_string(), permission_profile_schema()), ]); @@ -353,11 +317,11 @@ fn request_permissions_tool_includes_full_permission_schema() { description: "Request extra permissions for this turn.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["permissions".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["permissions".to_string()]), + Some(false.into()) + ), output_schema: None, }) ); @@ -392,32 +356,28 @@ Examples of valid command strings: let mut properties = BTreeMap::from([ ( "command".to_string(), - JsonSchema::String { - description: Some( - "The shell script to execute in the user's default shell".to_string(), - ), - }, + JsonSchema::string(Some( + "The shell script to execute in the user's default shell".to_string(), + )), ), ( "workdir".to_string(), - JsonSchema::String { - description: Some("The working directory to execute the command in".to_string()), - }, + JsonSchema::string(Some( + "The working directory to execute the command in".to_string(), + )), ), ( "timeout_ms".to_string(), - JsonSchema::Number { - description: Some("The timeout for the command in milliseconds".to_string()), - }, + JsonSchema::number(Some( + "The timeout for the command in milliseconds".to_string(), + )), ), ( "login".to_string(), - JsonSchema::Boolean { - description: Some( - "Whether to run the shell with login shell semantics. Defaults to true." - .to_string(), - ), - }, + JsonSchema::boolean(Some( + "Whether to run the shell with login shell semantics. Defaults to true." + .to_string(), + )), ), ]); properties.extend(create_approval_parameters( @@ -431,11 +391,11 @@ Examples of valid command strings: description, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["command".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["command".to_string()]), + Some(false.into()) + ), output_schema: None, }) ); diff --git a/codex-rs/tools/src/mcp_resource_tool.rs b/codex-rs/tools/src/mcp_resource_tool.rs index 309d6d2f2..fd2e0ac2a 100644 --- a/codex-rs/tools/src/mcp_resource_tool.rs +++ b/codex-rs/tools/src/mcp_resource_tool.rs @@ -7,21 +7,17 @@ pub fn create_list_mcp_resources_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "server".to_string(), - JsonSchema::String { - description: Some( - "Optional MCP server name. When omitted, lists resources from every configured server." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Optional MCP server name. When omitted, lists resources from every configured server." + .to_string(), + )), ), ( "cursor".to_string(), - JsonSchema::String { - description: Some( - "Opaque cursor returned by a previous list_mcp_resources call for the same server." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Opaque cursor returned by a previous list_mcp_resources call for the same server." + .to_string(), + )), ), ]); @@ -30,11 +26,7 @@ pub fn create_list_mcp_resources_tool() -> ToolSpec { description: "Lists resources provided by MCP servers. Resources allow servers to share data that provides context to language models, such as files, database schemas, or application-specific information. Prefer resources over web search when possible.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: None, - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, /*required*/ None, Some(false.into())), output_schema: None, }) } @@ -43,21 +35,17 @@ pub fn create_list_mcp_resource_templates_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "server".to_string(), - JsonSchema::String { - description: Some( - "Optional MCP server name. When omitted, lists resource templates from all configured servers." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Optional MCP server name. When omitted, lists resource templates from all configured servers." + .to_string(), + )), ), ( "cursor".to_string(), - JsonSchema::String { - description: Some( - "Opaque cursor returned by a previous list_mcp_resource_templates call for the same server." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Opaque cursor returned by a previous list_mcp_resource_templates call for the same server." + .to_string(), + )), ), ]); @@ -66,11 +54,7 @@ pub fn create_list_mcp_resource_templates_tool() -> ToolSpec { description: "Lists resource templates provided by MCP servers. Parameterized resource templates allow servers to share data that takes parameters and provides context to language models, such as files, database schemas, or application-specific information. Prefer resource templates over web search when possible.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: None, - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, /*required*/ None, Some(false.into())), output_schema: None, }) } @@ -79,21 +63,17 @@ pub fn create_read_mcp_resource_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "server".to_string(), - JsonSchema::String { - description: Some( - "MCP server name exactly as configured. Must match the 'server' field returned by list_mcp_resources." - .to_string(), - ), - }, + JsonSchema::string(Some( + "MCP server name exactly as configured. Must match the 'server' field returned by list_mcp_resources." + .to_string(), + )), ), ( "uri".to_string(), - JsonSchema::String { - description: Some( - "Resource URI to read. Must be one of the URIs returned by list_mcp_resources." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Resource URI to read. Must be one of the URIs returned by list_mcp_resources." + .to_string(), + )), ), ]); @@ -104,11 +84,11 @@ pub fn create_read_mcp_resource_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["server".to_string(), "uri".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["server".to_string(), "uri".to_string()]), + Some(false.into()), + ), output_schema: None, }) } diff --git a/codex-rs/tools/src/mcp_resource_tool_tests.rs b/codex-rs/tools/src/mcp_resource_tool_tests.rs index 9af37700f..2c0d03ee5 100644 --- a/codex-rs/tools/src/mcp_resource_tool_tests.rs +++ b/codex-rs/tools/src/mcp_resource_tool_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::JsonSchema; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -11,30 +12,22 @@ fn list_mcp_resources_tool_matches_expected_spec() { description: "Lists resources provided by MCP servers. Resources allow servers to share data that provides context to language models, such as files, database schemas, or application-specific information. Prefer resources over web search when possible.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "server".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Optional MCP server name. When omitted, lists resources from every configured server." .to_string(), - ), - }, + ),), ), ( "cursor".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Opaque cursor returned by a previous list_mcp_resources call for the same server." .to_string(), - ), - }, + ),), ), - ]), - required: None, - additional_properties: Some(false.into()), - }, + ]), /*required*/ None, Some(false.into())), output_schema: None, }) ); @@ -49,30 +42,22 @@ fn list_mcp_resource_templates_tool_matches_expected_spec() { description: "Lists resource templates provided by MCP servers. Parameterized resource templates allow servers to share data that takes parameters and provides context to language models, such as files, database schemas, or application-specific information. Prefer resource templates over web search when possible.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "server".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Optional MCP server name. When omitted, lists resource templates from all configured servers." .to_string(), - ), - }, + ),), ), ( "cursor".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Opaque cursor returned by a previous list_mcp_resource_templates call for the same server." .to_string(), - ), - }, + ),), ), - ]), - required: None, - additional_properties: Some(false.into()), - }, + ]), /*required*/ None, Some(false.into())), output_schema: None, }) ); @@ -89,30 +74,22 @@ fn read_mcp_resource_tool_matches_expected_spec() { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "server".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "MCP server name exactly as configured. Must match the 'server' field returned by list_mcp_resources." .to_string(), - ), - }, + ),), ), ( "uri".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Resource URI to read. Must be one of the URIs returned by list_mcp_resources." .to_string(), - ), - }, + ),), ), - ]), - required: Some(vec!["server".to_string(), "uri".to_string()]), - additional_properties: Some(false.into()), - }, + ]), Some(vec!["server".to_string(), "uri".to_string()]), Some(false.into())), output_schema: None, }) ); diff --git a/codex-rs/tools/src/mcp_tool_tests.rs b/codex-rs/tools/src/mcp_tool_tests.rs index 90e2c9541..5a9452632 100644 --- a/codex-rs/tools/src/mcp_tool_tests.rs +++ b/codex-rs/tools/src/mcp_tool_tests.rs @@ -34,11 +34,11 @@ fn parse_mcp_tool_inserts_empty_properties() { ToolDefinition { name: "no_props".to_string(), description: "No properties".to_string(), - input_schema: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + input_schema: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), defer_loading: false, } @@ -72,11 +72,11 @@ fn parse_mcp_tool_preserves_top_level_output_schema() { ToolDefinition { name: "with_output".to_string(), description: "Has output schema".to_string(), - input_schema: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + input_schema: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({ "properties": { "result": { @@ -112,11 +112,11 @@ fn parse_mcp_tool_preserves_output_schema_without_inferred_type() { ToolDefinition { name: "with_enum_output".to_string(), description: "Has enum output schema".to_string(), - input_schema: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + input_schema: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({ "enum": ["ok", "error"] }))), diff --git a/codex-rs/tools/src/plan_tool.rs b/codex-rs/tools/src/plan_tool.rs index 89ddcb597..5041b5361 100644 --- a/codex-rs/tools/src/plan_tool.rs +++ b/codex-rs/tools/src/plan_tool.rs @@ -5,30 +5,28 @@ use std::collections::BTreeMap; pub fn create_update_plan_tool() -> ToolSpec { let plan_item_properties = BTreeMap::from([ - ("step".to_string(), JsonSchema::String { description: None }), + ("step".to_string(), JsonSchema::string(/*description*/ None)), ( "status".to_string(), - JsonSchema::String { - description: Some("One of: pending, in_progress, completed".to_string()), - }, + JsonSchema::string(Some("One of: pending, in_progress, completed".to_string())), ), ]); let properties = BTreeMap::from([ ( "explanation".to_string(), - JsonSchema::String { description: None }, + JsonSchema::string(/*description*/ None), ), ( "plan".to_string(), - JsonSchema::Array { - description: Some("The list of steps".to_string()), - items: Box::new(JsonSchema::Object { - properties: plan_item_properties, - required: Some(vec!["step".to_string(), "status".to_string()]), - additional_properties: Some(false.into()), - }), - }, + JsonSchema::array( + JsonSchema::object( + plan_item_properties, + Some(vec!["step".to_string(), "status".to_string()]), + Some(false.into()), + ), + Some("The list of steps".to_string()), + ), ), ]); @@ -41,11 +39,11 @@ At most one step can be in_progress at a time. .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["plan".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["plan".to_string()]), + Some(false.into()), + ), output_schema: None, }) } diff --git a/codex-rs/tools/src/request_user_input_tool.rs b/codex-rs/tools/src/request_user_input_tool.rs index 77c536738..ad0d4db55 100644 --- a/codex-rs/tools/src/request_user_input_tool.rs +++ b/codex-rs/tools/src/request_user_input_tool.rs @@ -12,71 +12,60 @@ pub fn create_request_user_input_tool(description: String) -> ToolSpec { let option_props = BTreeMap::from([ ( "label".to_string(), - JsonSchema::String { - description: Some("User-facing label (1-5 words).".to_string()), - }, + JsonSchema::string(Some("User-facing label (1-5 words).".to_string())), ), ( "description".to_string(), - JsonSchema::String { - description: Some( - "One short sentence explaining impact/tradeoff if selected.".to_string(), - ), - }, + JsonSchema::string(Some( + "One short sentence explaining impact/tradeoff if selected.".to_string(), + )), ), ]); - let options_schema = JsonSchema::Array { - description: Some( + let options_schema = JsonSchema::array(JsonSchema::object( + option_props, + Some(vec!["label".to_string(), "description".to_string()]), + Some(false.into()), + ), Some( "Provide 2-3 mutually exclusive choices. Put the recommended option first and suffix its label with \"(Recommended)\". Do not include an \"Other\" option in this list; the client will add a free-form \"Other\" option automatically." .to_string(), - ), - items: Box::new(JsonSchema::Object { - properties: option_props, - required: Some(vec!["label".to_string(), "description".to_string()]), - additional_properties: Some(false.into()), - }), - }; + )); let question_props = BTreeMap::from([ ( "id".to_string(), - JsonSchema::String { - description: Some( - "Stable identifier for mapping answers (snake_case).".to_string(), - ), - }, + JsonSchema::string(Some( + "Stable identifier for mapping answers (snake_case).".to_string(), + )), ), ( "header".to_string(), - JsonSchema::String { - description: Some( - "Short header label shown in the UI (12 or fewer chars).".to_string(), - ), - }, + JsonSchema::string(Some( + "Short header label shown in the UI (12 or fewer chars).".to_string(), + )), ), ( "question".to_string(), - JsonSchema::String { - description: Some("Single-sentence prompt shown to the user.".to_string()), - }, + JsonSchema::string(Some( + "Single-sentence prompt shown to the user.".to_string(), + )), ), ("options".to_string(), options_schema), ]); - let questions_schema = JsonSchema::Array { - description: Some("Questions to show the user. Prefer 1 and do not exceed 3".to_string()), - items: Box::new(JsonSchema::Object { - properties: question_props, - required: Some(vec![ + let questions_schema = JsonSchema::array( + JsonSchema::object( + question_props, + Some(vec![ "id".to_string(), "header".to_string(), "question".to_string(), "options".to_string(), ]), - additional_properties: Some(false.into()), - }), - }; + Some(false.into()), + ), + Some("Questions to show the user. Prefer 1 and do not exceed 3".to_string()), + ); let properties = BTreeMap::from([("questions".to_string(), questions_schema)]); @@ -85,11 +74,11 @@ pub fn create_request_user_input_tool(description: String) -> ToolSpec { description, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["questions".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["questions".to_string()]), + Some(false.into()), + ), output_schema: None, }) } diff --git a/codex-rs/tools/src/request_user_input_tool_tests.rs b/codex-rs/tools/src/request_user_input_tool_tests.rs index e7a305f86..68c2639e4 100644 --- a/codex-rs/tools/src/request_user_input_tool_tests.rs +++ b/codex-rs/tools/src/request_user_input_tool_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::JsonSchema; use codex_protocol::config_types::ModeKind; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -12,91 +13,77 @@ fn request_user_input_tool_includes_questions_schema() { description: "Ask the user to choose.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object(BTreeMap::from([( "questions".to_string(), - JsonSchema::Array { - description: Some( - "Questions to show the user. Prefer 1 and do not exceed 3".to_string(), - ), - items: Box::new(JsonSchema::Object { - properties: BTreeMap::from([ + JsonSchema::array( + JsonSchema::object( + BTreeMap::from([ ( "header".to_string(), - JsonSchema::String { - description: Some( - "Short header label shown in the UI (12 or fewer chars)." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Short header label shown in the UI (12 or fewer chars)." + .to_string(), + )), ), ( "id".to_string(), - JsonSchema::String { - description: Some( - "Stable identifier for mapping answers (snake_case)." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Stable identifier for mapping answers (snake_case)." + .to_string(), + )), ), ( "options".to_string(), - JsonSchema::Array { - description: Some( - "Provide 2-3 mutually exclusive choices. Put the recommended option first and suffix its label with \"(Recommended)\". Do not include an \"Other\" option in this list; the client will add a free-form \"Other\" option automatically." - .to_string(), - ), - items: Box::new(JsonSchema::Object { - properties: BTreeMap::from([ + JsonSchema::array( + JsonSchema::object( + BTreeMap::from([ ( "description".to_string(), - JsonSchema::String { - description: Some( - "One short sentence explaining impact/tradeoff if selected." - .to_string(), - ), - }, + JsonSchema::string(Some( + "One short sentence explaining impact/tradeoff if selected." + .to_string(), + )), ), ( "label".to_string(), - JsonSchema::String { - description: Some( - "User-facing label (1-5 words)." - .to_string(), - ), - }, + JsonSchema::string(Some( + "User-facing label (1-5 words)." + .to_string(), + )), ), ]), - required: Some(vec![ + Some(vec![ "label".to_string(), "description".to_string(), ]), - additional_properties: Some(false.into()), - }), - }, + Some(false.into()), + ), + Some( + "Provide 2-3 mutually exclusive choices. Put the recommended option first and suffix its label with \"(Recommended)\". Do not include an \"Other\" option in this list; the client will add a free-form \"Other\" option automatically." + .to_string(), + ), + ), ), ( "question".to_string(), - JsonSchema::String { - description: Some( - "Single-sentence prompt shown to the user.".to_string(), - ), - }, + JsonSchema::string(Some( + "Single-sentence prompt shown to the user.".to_string(), + )), ), ]), - required: Some(vec![ + Some(vec![ "id".to_string(), "header".to_string(), "question".to_string(), "options".to_string(), ]), - additional_properties: Some(false.into()), - }), - }, - )]), - required: Some(vec!["questions".to_string()]), - additional_properties: Some(false.into()), - }, + Some(false.into()), + ), + Some( + "Questions to show the user. Prefer 1 and do not exceed 3".to_string(), + ), + ), + )]), Some(vec!["questions".to_string()]), Some(false.into())), output_schema: None, }) ); diff --git a/codex-rs/tools/src/responses_api.rs b/codex-rs/tools/src/responses_api.rs index 6a8823582..50bb4f223 100644 --- a/codex-rs/tools/src/responses_api.rs +++ b/codex-rs/tools/src/responses_api.rs @@ -38,6 +38,7 @@ pub struct ResponsesApiTool { #[derive(Debug, Clone, Serialize, PartialEq)] #[serde(tag = "type")] +#[allow(clippy::large_enum_variant)] pub enum ToolSearchOutputTool { #[allow(dead_code)] #[serde(rename = "function")] diff --git a/codex-rs/tools/src/responses_api_tests.rs b/codex-rs/tools/src/responses_api_tests.rs index ee19f3728..c3ce4cec2 100644 --- a/codex-rs/tools/src/responses_api_tests.rs +++ b/codex-rs/tools/src/responses_api_tests.rs @@ -18,14 +18,14 @@ fn tool_definition_to_responses_api_tool_omits_false_defer_loading() { tool_definition_to_responses_api_tool(ToolDefinition { name: "lookup_order".to_string(), description: "Look up an order".to_string(), - input_schema: JsonSchema::Object { - properties: BTreeMap::from([( + input_schema: JsonSchema::object( + BTreeMap::from([( "order_id".to_string(), - JsonSchema::String { description: None }, + JsonSchema::string(/*description*/ None), )]), - required: Some(vec!["order_id".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["order_id".to_string()]), + Some(false.into()) + ), output_schema: Some(json!({"type": "object"})), defer_loading: false, }), @@ -34,14 +34,14 @@ fn tool_definition_to_responses_api_tool_omits_false_defer_loading() { description: "Look up an order".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object( + BTreeMap::from([( "order_id".to_string(), - JsonSchema::String { description: None }, + JsonSchema::string(/*description*/ None), )]), - required: Some(vec!["order_id".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["order_id".to_string()]), + Some(false.into()) + ), output_schema: Some(json!({"type": "object"})), } ); @@ -70,14 +70,14 @@ fn dynamic_tool_to_responses_api_tool_preserves_defer_loading() { description: "Look up an order".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object( + BTreeMap::from([( "order_id".to_string(), - JsonSchema::String { description: None }, + JsonSchema::string(/*description*/ None), )]), - required: Some(vec!["order_id".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["order_id".to_string()]), + Some(false.into()) + ), output_schema: None, } ); @@ -115,14 +115,10 @@ fn mcp_tool_to_deferred_responses_api_tool_sets_defer_loading() { description: "Look up an order".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object(BTreeMap::from([( "order_id".to_string(), - JsonSchema::String { description: None }, - )]), - required: Some(vec!["order_id".to_string()]), - additional_properties: Some(false.into()), - }, + JsonSchema::string(/*description*/ None), + )]), Some(vec!["order_id".to_string()]), Some(false.into())), output_schema: None, } ); @@ -138,11 +134,11 @@ fn tool_search_output_namespace_serializes_with_deferred_child_tools() { description: "Create a calendar event.".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::Object { - properties: Default::default(), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + Default::default(), + /*required*/ None, + /*additional_properties*/ None, + ), output_schema: None, })], }); diff --git a/codex-rs/tools/src/tool_definition_tests.rs b/codex-rs/tools/src/tool_definition_tests.rs index 4a3b3708b..0d4d97f5c 100644 --- a/codex-rs/tools/src/tool_definition_tests.rs +++ b/codex-rs/tools/src/tool_definition_tests.rs @@ -7,11 +7,11 @@ fn tool_definition() -> ToolDefinition { ToolDefinition { name: "lookup_order".to_string(), description: "Look up an order".to_string(), - input_schema: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + input_schema: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None, + ), output_schema: Some(serde_json::json!({ "type": "object", })), diff --git a/codex-rs/tools/src/tool_discovery.rs b/codex-rs/tools/src/tool_discovery.rs index 6b2bb3de2..703316020 100644 --- a/codex-rs/tools/src/tool_discovery.rs +++ b/codex-rs/tools/src/tool_discovery.rs @@ -147,17 +147,13 @@ pub fn create_tool_search_tool(app_tools: &[ToolSearchAppInfo], default_limit: u let properties = BTreeMap::from([ ( "query".to_string(), - JsonSchema::String { - description: Some("Search query for apps tools.".to_string()), - }, + JsonSchema::string(Some("Search query for apps tools.".to_string())), ), ( "limit".to_string(), - JsonSchema::Number { - description: Some(format!( - "Maximum number of tools to return (defaults to {default_limit})." - )), - }, + JsonSchema::number(Some(format!( + "Maximum number of tools to return (defaults to {default_limit})." + ))), ), ]); @@ -193,11 +189,11 @@ pub fn create_tool_search_tool(app_tools: &[ToolSearchAppInfo], default_limit: u ToolSpec::ToolSearch { execution: "client".to_string(), description, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec!["query".to_string()]), - additional_properties: Some(false.into()), - }, + Some(vec!["query".to_string()]), + Some(false.into()), + ), } } @@ -279,37 +275,29 @@ pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> Tool let properties = BTreeMap::from([ ( "tool_type".to_string(), - JsonSchema::String { - description: Some( - "Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"." + .to_string(), + )), ), ( "action_type".to_string(), - JsonSchema::String { - description: Some( - "Suggested action for the tool. Use \"install\" or \"enable\".".to_string(), - ), - }, + JsonSchema::string(Some( + "Suggested action for the tool. Use \"install\" or \"enable\".".to_string(), + )), ), ( "tool_id".to_string(), - JsonSchema::String { - description: Some(format!( - "Connector or plugin id to suggest. Must be one of: {discoverable_tool_ids}." - )), - }, + JsonSchema::string(Some(format!( + "Connector or plugin id to suggest. Must be one of: {discoverable_tool_ids}." + ))), ), ( "suggest_reason".to_string(), - JsonSchema::String { - description: Some( - "Concise one-line user-facing reason why this tool can help with the current request." - .to_string(), - ), - }, + JsonSchema::string(Some( + "Concise one-line user-facing reason why this tool can help with the current request." + .to_string(), + )), ), ]); @@ -323,16 +311,16 @@ pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> Tool description, strict: false, defer_loading: None, - parameters: JsonSchema::Object { + parameters: JsonSchema::object( properties, - required: Some(vec![ + Some(vec![ "tool_type".to_string(), "action_type".to_string(), "tool_id".to_string(), "suggest_reason".to_string(), ]), - additional_properties: Some(false.into()), - }, + Some(false.into()), + ), output_schema: None, }) } diff --git a/codex-rs/tools/src/tool_discovery_tests.rs b/codex-rs/tools/src/tool_discovery_tests.rs index 97756f565..ea5acf9aa 100644 --- a/codex-rs/tools/src/tool_discovery_tests.rs +++ b/codex-rs/tools/src/tool_discovery_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::JsonSchema; use codex_app_server_protocol::AppInfo; use pretty_assertions::assert_eq; use rmcp::model::JsonObject; @@ -50,27 +51,19 @@ fn create_tool_search_tool_deduplicates_and_renders_enabled_apps() { ToolSpec::ToolSearch { execution: "client".to_string(), description: "# Apps (Connectors) tool discovery\n\nSearches over apps/connectors tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to all the tools of the following apps/connectors:\n- Google Drive: Use Google Drive as the single entrypoint for Drive, Docs, Sheets, and Slides work.\n- Slack\nSome of the tools may not have been provided to you upfront, and you should use this tool (`tool_search`) to search for the required tools and load them for the apps mentioned above. For the apps mentioned above, always use `tool_search` instead of `list_mcp_resources` or `list_mcp_resource_templates` for tool discovery.".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "limit".to_string(), - JsonSchema::Number { - description: Some( + JsonSchema::number(Some( "Maximum number of tools to return (defaults to 8)." .to_string(), - ), - }, + ),), ), ( "query".to_string(), - JsonSchema::String { - description: Some("Search query for apps tools.".to_string()), - }, + JsonSchema::string(Some("Search query for apps tools.".to_string()),), ), - ]), - required: Some(vec!["query".to_string()]), - additional_properties: Some(false.into()), - }, + ]), Some(vec!["query".to_string()]), Some(false.into())), } ); } @@ -103,53 +96,41 @@ fn create_tool_suggest_tool_uses_plugin_summary_fallback() { description: "# Tool suggestion discovery\n\nSuggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin, when the user clearly wants a capability that is not currently available in the active `tools` list.\n\nUse this ONLY when:\n- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means.\n- For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.\n- For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.\n\nTool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.\n\nDiscoverable tools:\n- GitHub (id: `github`, type: plugin, action: install): skills; MCP servers: github-mcp; app connectors: github-app\n- Slack (id: `slack@openai-curated`, type: connector, action: install): No description provided.\n\nWorkflow:\n\n1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.\n2. Match the user's request against the discoverable tools list above. Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard.\n3. If one tool clearly fits, call `tool_suggest` with:\n - `tool_type`: `connector` or `plugin`\n - `action_type`: `install` or `enable`\n - `tool_id`: exact id from the discoverable tools list above\n - `suggest_reason`: concise one-line user-facing reason this tool can help with the current request\n4. After the suggestion flow completes:\n - if the user finished the install or enable flow, continue by searching again or using the newly available tool\n - if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks for it.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "action_type".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Suggested action for the tool. Use \"install\" or \"enable\"." .to_string(), - ), - }, + ),), ), ( "suggest_reason".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Concise one-line user-facing reason why this tool can help with the current request." .to_string(), - ), - }, + ),), ), ( "tool_id".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Connector or plugin id to suggest. Must be one of: slack@openai-curated, github." .to_string(), - ), - }, + ),), ), ( "tool_type".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"." .to_string(), - ), - }, + ),), ), - ]), - required: Some(vec![ + ]), Some(vec![ "tool_type".to_string(), "action_type".to_string(), "tool_id".to_string(), "suggest_reason".to_string(), - ]), - additional_properties: Some(false.into()), - }, + ]), Some(false.into())), output_schema: None, }) ); @@ -198,11 +179,11 @@ fn collect_tool_search_output_tools_groups_results_by_namespace() { description: "Create a calendar event.".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::Object { - properties: Default::default(), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + Default::default(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: None, }), ResponsesApiNamespaceTool::Function(ResponsesApiTool { @@ -210,11 +191,11 @@ fn collect_tool_search_output_tools_groups_results_by_namespace() { description: "List calendar events.".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::Object { - properties: Default::default(), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + Default::default(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: None, }), ], @@ -227,11 +208,11 @@ fn collect_tool_search_output_tools_groups_results_by_namespace() { description: "Read an email.".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::Object { - properties: Default::default(), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + Default::default(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: None, })], }), @@ -262,11 +243,11 @@ fn collect_tool_search_output_tools_falls_back_to_connector_name_description() { description: "Read multiple emails.".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::Object { - properties: Default::default(), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + Default::default(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: None, })], })], diff --git a/codex-rs/tools/src/tool_registry_plan_tests.rs b/codex-rs/tools/src/tool_registry_plan_tests.rs index d8d264e2e..1b687d054 100644 --- a/codex-rs/tools/src/tool_registry_plan_tests.rs +++ b/codex-rs/tools/src/tool_registry_plan_tests.rs @@ -5,6 +5,8 @@ use crate::DiscoverablePluginInfo; use crate::DiscoverableTool; use crate::FreeformTool; use crate::JsonSchema; +use crate::JsonSchemaPrimitiveType; +use crate::JsonSchemaType; use crate::ResponsesApiTool; use crate::ResponsesApiWebSearchFilters; use crate::ResponsesApiWebSearchUserLocation; @@ -172,9 +174,7 @@ fn test_build_specs_collab_tools_enabled() { let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &spawn_agent.spec else { panic!("spawn_agent should be a function tool"); }; - let JsonSchema::Object { properties, .. } = parameters else { - panic!("spawn_agent should use object params"); - }; + let (properties, _) = expect_object_schema(parameters); assert!(properties.contains_key("fork_context")); assert!(!properties.contains_key("fork_turns")); } @@ -223,21 +223,14 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { else { panic!("spawn_agent should be a function tool"); }; - let JsonSchema::Object { - properties, - required, - .. - } = parameters - else { - panic!("spawn_agent should use object params"); - }; + let (properties, required) = expect_object_schema(parameters); assert!(properties.contains_key("task_name")); assert!(properties.contains_key("message")); assert!(properties.contains_key("fork_turns")); assert!(!properties.contains_key("items")); assert!(!properties.contains_key("fork_context")); assert_eq!( - required.as_ref(), + required, Some(&vec!["task_name".to_string(), "message".to_string()]) ); let output_schema = output_schema @@ -255,20 +248,13 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { panic!("send_message should be a function tool"); }; assert_eq!(output_schema, &None); - let JsonSchema::Object { - properties, - required, - .. - } = parameters - else { - panic!("send_message should use object params"); - }; + let (properties, required) = expect_object_schema(parameters); assert!(properties.contains_key("target")); assert!(!properties.contains_key("interrupt")); assert!(properties.contains_key("message")); assert!(!properties.contains_key("items")); assert_eq!( - required.as_ref(), + required, Some(&vec!["target".to_string(), "message".to_string()]) ); @@ -282,19 +268,12 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { panic!("followup_task should be a function tool"); }; assert_eq!(output_schema, &None); - let JsonSchema::Object { - properties, - required, - .. - } = parameters - else { - panic!("followup_task should use object params"); - }; + let (properties, required) = expect_object_schema(parameters); assert!(properties.contains_key("target")); assert!(properties.contains_key("message")); assert!(!properties.contains_key("items")); assert_eq!( - required.as_ref(), + required, Some(&vec!["target".to_string(), "message".to_string()]) ); @@ -307,17 +286,10 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { else { panic!("wait_agent should be a function tool"); }; - let JsonSchema::Object { - properties, - required, - .. - } = parameters - else { - panic!("wait_agent should use object params"); - }; + let (properties, required) = expect_object_schema(parameters); assert!(!properties.contains_key("targets")); assert!(properties.contains_key("timeout_ms")); - assert_eq!(required, &None); + assert_eq!(required, None); let output_schema = output_schema .as_ref() .expect("wait_agent should define output schema"); @@ -335,16 +307,9 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { else { panic!("list_agents should be a function tool"); }; - let JsonSchema::Object { - properties, - required, - .. - } = parameters - else { - panic!("list_agents should use object params"); - }; + let (properties, required) = expect_object_schema(parameters); assert!(properties.contains_key("path_prefix")); - assert_eq!(required.as_ref(), None); + assert_eq!(required, None); let output_schema = output_schema .as_ref() .expect("list_agents should define output schema"); @@ -416,9 +381,7 @@ fn view_image_tool_omits_detail_without_original_detail_feature() { let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &view_image.spec else { panic!("view_image should be a function tool"); }; - let JsonSchema::Object { properties, .. } = parameters else { - panic!("view_image should use an object schema"); - }; + let (properties, _) = expect_object_schema(parameters); assert!(!properties.contains_key("detail")); } @@ -448,16 +411,13 @@ fn view_image_tool_includes_detail_with_original_detail_feature() { let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &view_image.spec else { panic!("view_image should be a function tool"); }; - let JsonSchema::Object { properties, .. } = parameters else { - panic!("view_image should use an object schema"); - }; + let (properties, _) = expect_object_schema(parameters); assert!(properties.contains_key("detail")); - let Some(JsonSchema::String { - description: Some(description), - }) = properties.get("detail") - else { - panic!("view_image detail should include a description"); - }; + let description = expect_string_description( + properties + .get("detail") + .expect("view_image detail should include a description"), + ); assert!(description.contains("only supported value is `original`")); assert!(description.contains("omit this field for default resized behavior")); } @@ -1118,40 +1078,40 @@ fn test_build_specs_mcp_tools_converted() { &tool.spec, &ToolSpec::Function(ResponsesApiTool { name: "test_server/do_something_cool".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object( + BTreeMap::from([ ( "string_argument".to_string(), - JsonSchema::String { description: None } + JsonSchema::string(/*description*/ None), ), ( "number_argument".to_string(), - JsonSchema::Number { description: None } + JsonSchema::number(/*description*/ None), ), ( "object_argument".to_string(), - JsonSchema::Object { - properties: BTreeMap::from([ + JsonSchema::object( + BTreeMap::from([ ( "string_property".to_string(), - JsonSchema::String { description: None } + JsonSchema::string(/*description*/ None), ), ( "number_property".to_string(), - JsonSchema::Number { description: None } + JsonSchema::number(/*description*/ None), ), ]), - required: Some(vec![ + Some(vec![ "string_property".to_string(), "number_property".to_string(), ]), - additional_properties: Some(false.into()), - }, + Some(false.into()), + ), ), ]), - required: None, - additional_properties: None, - }, + /*required*/ None, + /*additional_properties*/ None + ), description: "Do something cool".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), @@ -1524,11 +1484,9 @@ fn tool_suggest_description_lists_discoverable_tools() { assert!(description.contains("DO NOT explore or recommend tools that are not on this list.")); assert!(!description.contains("{{discoverable_tools}}")); assert!(!description.contains("tool_search fails to find a good match")); - let JsonSchema::Object { required, .. } = parameters else { - panic!("expected object parameters"); - }; + let (_, required) = expect_object_schema(parameters); assert_eq!( - required.as_ref(), + required, Some(&vec![ "tool_type".to_string(), "action_type".to_string(), @@ -1588,6 +1546,91 @@ fn code_mode_augments_mcp_tool_descriptions_with_namespaced_sample() { ); } +#[test] +fn code_mode_preserves_nullable_and_literal_mcp_input_shapes() { + let model_info = model_info(); + let mut features = Features::with_defaults(); + features.enable(Feature::CodeMode); + features.enable(Feature::UnifiedExec); + let available_models = Vec::new(); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + available_models: &available_models, + features: &features, + web_search_mode: Some(WebSearchMode::Cached), + session_source: SessionSource::Cli, + sandbox_policy: &SandboxPolicy::DangerFullAccess, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + }); + + let (tools, _) = build_specs( + &tools_config, + Some(HashMap::from([( + "mcp__sample__fn".to_string(), + mcp_tool( + "fn", + "Sample fn", + serde_json::json!({ + "type": "object", + "properties": { + "open": { + "anyOf": [ + { + "type": "array", + "items": { + "type": "object", + "properties": { + "ref_id": {"type": "string"}, + "lineno": {"anyOf": [{"type": "integer"}, {"type": "null"}]} + }, + "required": ["ref_id"], + "additionalProperties": false + } + }, + {"type": "null"} + ] + }, + "tagged_list": { + "anyOf": [ + { + "type": "array", + "items": { + "type": "object", + "properties": { + "kind": {"type": "const", "const": "tagged"}, + "variant": {"type": "enum", "enum": ["alpha", "beta"]}, + "scope": {"type": "enum", "enum": ["one", "two"]} + }, + "required": ["kind", "variant", "scope"] + } + }, + {"type": "null"} + ] + }, + "response_length": {"type": "enum", "enum": ["short", "medium", "long"]} + }, + "additionalProperties": false + }), + ), + )])), + /*app_tools*/ None, + &[], + ); + + let ToolSpec::Function(ResponsesApiTool { description, .. }) = + &find_tool(&tools, "mcp__sample__fn").spec + else { + panic!("expected function tool"); + }; + + assert!(description.contains( + r#"exec tool declaration: +```ts +declare const tools: { mcp__sample__fn(args: { open?: Array<{ lineno?: number | null; ref_id: string; }> | null; response_length?: "short" | "medium" | "long"; tagged_list?: Array<{ kind: "tagged"; scope: "one" | "two"; variant: "alpha" | "beta"; }> | null; }): Promise<{ _meta?: unknown; content: Array; isError?: boolean; structuredContent?: unknown; }>; }; +```"# + )); +} + #[test] fn code_mode_augments_builtin_tool_descriptions_with_typed_sample() { let model_info = model_info(); @@ -1884,30 +1927,46 @@ fn find_tool<'a>(tools: &'a [ConfiguredToolSpec], expected_name: &str) -> &'a Co .unwrap_or_else(|| panic!("expected tool {expected_name}")) } +fn expect_object_schema( + schema: &JsonSchema, +) -> (&BTreeMap, Option<&Vec>) { + assert_eq!( + schema.schema_type, + Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) + ); + let properties = schema + .properties + .as_ref() + .expect("expected object properties"); + (properties, schema.required.as_ref()) +} + +fn expect_string_description(schema: &JsonSchema) -> &str { + assert_eq!( + schema.schema_type, + Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::String)) + ); + schema.description.as_deref().expect("expected description") +} + fn strip_descriptions_schema(schema: &mut JsonSchema) { - match schema { - JsonSchema::Boolean { description } - | JsonSchema::String { description } - | JsonSchema::Number { description } => { - *description = None; - } - JsonSchema::Array { items, description } => { - strip_descriptions_schema(items); - *description = None; - } - JsonSchema::Object { - properties, - required: _, - additional_properties, - } => { - for value in properties.values_mut() { - strip_descriptions_schema(value); - } - if let Some(AdditionalProperties::Schema(schema)) = additional_properties { - strip_descriptions_schema(schema); - } + if let Some(variants) = &mut schema.any_of { + for variant in variants { + strip_descriptions_schema(variant); } } + if let Some(items) = &mut schema.items { + strip_descriptions_schema(items); + } + if let Some(properties) = &mut schema.properties { + for value in properties.values_mut() { + strip_descriptions_schema(value); + } + } + if let Some(AdditionalProperties::Schema(schema)) = &mut schema.additional_properties { + strip_descriptions_schema(schema); + } + schema.description = None; } fn strip_descriptions_tool(spec: &mut ToolSpec) { diff --git a/codex-rs/tools/src/tool_spec_tests.rs b/codex-rs/tools/src/tool_spec_tests.rs index c84ea94a3..c94f54e6b 100644 --- a/codex-rs/tools/src/tool_spec_tests.rs +++ b/codex-rs/tools/src/tool_spec_tests.rs @@ -24,11 +24,11 @@ fn tool_spec_name_covers_all_variants() { description: "Look up an order".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: None, }) .name(), @@ -38,11 +38,11 @@ fn tool_spec_name_covers_all_variants() { ToolSpec::ToolSearch { execution: "sync".to_string(), description: "Search for tools".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None + ), } .name(), "tool_search" @@ -90,11 +90,11 @@ fn configured_tool_spec_name_delegates_to_tool_spec() { description: "Look up an order".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::new(), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: None, }), /*supports_parallel_tool_calls*/ true, @@ -140,14 +140,11 @@ fn create_tools_json_for_responses_api_includes_top_level_name() { description: "A demo tool".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([( - "foo".to_string(), - JsonSchema::String { description: None }, - )]), - required: None, - additional_properties: None, - }, + parameters: JsonSchema::object( + BTreeMap::from([("foo".to_string(), JsonSchema::string(/*description*/ None),)]), + /*required*/ None, + /*additional_properties*/ None + ), output_schema: None, })]) .expect("serialize tools"), @@ -210,16 +207,14 @@ fn tool_search_tool_spec_serializes_expected_wire_shape() { serde_json::to_value(ToolSpec::ToolSearch { execution: "sync".to_string(), description: "Search app tools".to_string(), - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object( + BTreeMap::from([( "query".to_string(), - JsonSchema::String { - description: Some("Tool search query".to_string()), - }, + JsonSchema::string(Some("Tool search query".to_string()),), )]), - required: Some(vec!["query".to_string()]), - additional_properties: Some(AdditionalProperties::Boolean(false)), - }, + Some(vec!["query".to_string()]), + Some(AdditionalProperties::Boolean(false)) + ), }) .expect("serialize tool_search"), json!({ diff --git a/codex-rs/tools/src/utility_tool.rs b/codex-rs/tools/src/utility_tool.rs index dca9d312f..b0f93c972 100644 --- a/codex-rs/tools/src/utility_tool.rs +++ b/codex-rs/tools/src/utility_tool.rs @@ -7,31 +7,23 @@ pub fn create_list_dir_tool() -> ToolSpec { let properties = BTreeMap::from([ ( "dir_path".to_string(), - JsonSchema::String { - description: Some("Absolute path to the directory to list.".to_string()), - }, + JsonSchema::string(Some("Absolute path to the directory to list.".to_string())), ), ( "offset".to_string(), - JsonSchema::Number { - description: Some( - "The entry number to start listing from. Must be 1 or greater.".to_string(), - ), - }, + JsonSchema::number(Some( + "The entry number to start listing from. Must be 1 or greater.".to_string(), + )), ), ( "limit".to_string(), - JsonSchema::Number { - description: Some("The maximum number of entries to return.".to_string()), - }, + JsonSchema::number(Some("The maximum number of entries to return.".to_string())), ), ( "depth".to_string(), - JsonSchema::Number { - description: Some( - "The maximum directory depth to traverse. Must be 1 or greater.".to_string(), - ), - }, + JsonSchema::number(Some( + "The maximum directory depth to traverse. Must be 1 or greater.".to_string(), + )), ), ]); @@ -42,11 +34,7 @@ pub fn create_list_dir_tool() -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["dir_path".to_string()]), - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, Some(vec!["dir_path".to_string()]), Some(false.into())), output_schema: None, }) } @@ -55,54 +43,44 @@ pub fn create_test_sync_tool() -> ToolSpec { let barrier_properties = BTreeMap::from([ ( "id".to_string(), - JsonSchema::String { - description: Some( - "Identifier shared by concurrent calls that should rendezvous".to_string(), - ), - }, + JsonSchema::string(Some( + "Identifier shared by concurrent calls that should rendezvous".to_string(), + )), ), ( "participants".to_string(), - JsonSchema::Number { - description: Some( - "Number of tool calls that must arrive before the barrier opens".to_string(), - ), - }, + JsonSchema::number(Some( + "Number of tool calls that must arrive before the barrier opens".to_string(), + )), ), ( "timeout_ms".to_string(), - JsonSchema::Number { - description: Some( - "Maximum time in milliseconds to wait at the barrier".to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum time in milliseconds to wait at the barrier".to_string(), + )), ), ]); let properties = BTreeMap::from([ ( "sleep_before_ms".to_string(), - JsonSchema::Number { - description: Some( - "Optional delay in milliseconds before any other action".to_string(), - ), - }, + JsonSchema::number(Some( + "Optional delay in milliseconds before any other action".to_string(), + )), ), ( "sleep_after_ms".to_string(), - JsonSchema::Number { - description: Some( - "Optional delay in milliseconds after completing the barrier".to_string(), - ), - }, + JsonSchema::number(Some( + "Optional delay in milliseconds after completing the barrier".to_string(), + )), ), ( "barrier".to_string(), - JsonSchema::Object { - properties: barrier_properties, - required: Some(vec!["id".to_string(), "participants".to_string()]), - additional_properties: Some(false.into()), - }, + JsonSchema::object( + barrier_properties, + Some(vec!["id".to_string(), "participants".to_string()]), + Some(false.into()), + ), ), ]); @@ -111,11 +89,7 @@ pub fn create_test_sync_tool() -> ToolSpec { description: "Internal synchronization helper used by Codex integration tests.".to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: None, - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, /*required*/ None, Some(false.into())), output_schema: None, }) } diff --git a/codex-rs/tools/src/utility_tool_tests.rs b/codex-rs/tools/src/utility_tool_tests.rs index a8ea6777f..2984d02f4 100644 --- a/codex-rs/tools/src/utility_tool_tests.rs +++ b/codex-rs/tools/src/utility_tool_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::JsonSchema; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -13,46 +14,34 @@ fn list_dir_tool_matches_expected_spec() { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "depth".to_string(), - JsonSchema::Number { - description: Some( - "The maximum directory depth to traverse. Must be 1 or greater." - .to_string(), - ), - }, + JsonSchema::number(Some( + "The maximum directory depth to traverse. Must be 1 or greater." + .to_string(), + )), ), ( "dir_path".to_string(), - JsonSchema::String { - description: Some( - "Absolute path to the directory to list.".to_string(), - ), - }, + JsonSchema::string(Some( + "Absolute path to the directory to list.".to_string(), + )), ), ( "limit".to_string(), - JsonSchema::Number { - description: Some( - "The maximum number of entries to return.".to_string(), - ), - }, + JsonSchema::number(Some( + "The maximum number of entries to return.".to_string(), + )), ), ( "offset".to_string(), - JsonSchema::Number { - description: Some( - "The entry number to start listing from. Must be 1 or greater." - .to_string(), - ), - }, + JsonSchema::number(Some( + "The entry number to start listing from. Must be 1 or greater." + .to_string(), + )), ), - ]), - required: Some(vec!["dir_path".to_string()]), - additional_properties: Some(false.into()), - }, + ]), Some(vec!["dir_path".to_string()]), Some(false.into())), output_schema: None, }) ); @@ -68,69 +57,51 @@ fn test_sync_tool_matches_expected_spec() { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "barrier".to_string(), - JsonSchema::Object { - properties: BTreeMap::from([ + JsonSchema::object( + BTreeMap::from([ ( "id".to_string(), - JsonSchema::String { - description: Some( - "Identifier shared by concurrent calls that should rendezvous" - .to_string(), - ), - }, + JsonSchema::string(Some( + "Identifier shared by concurrent calls that should rendezvous" + .to_string(), + )), ), ( "participants".to_string(), - JsonSchema::Number { - description: Some( - "Number of tool calls that must arrive before the barrier opens" - .to_string(), - ), - }, + JsonSchema::number(Some( + "Number of tool calls that must arrive before the barrier opens" + .to_string(), + )), ), ( "timeout_ms".to_string(), - JsonSchema::Number { - description: Some( - "Maximum time in milliseconds to wait at the barrier" - .to_string(), - ), - }, + JsonSchema::number(Some( + "Maximum time in milliseconds to wait at the barrier" + .to_string(), + )), ), ]), - required: Some(vec![ - "id".to_string(), - "participants".to_string(), - ]), - additional_properties: Some(false.into()), - }, + Some(vec!["id".to_string(), "participants".to_string()]), + Some(false.into()), + ), ), ( "sleep_after_ms".to_string(), - JsonSchema::Number { - description: Some( - "Optional delay in milliseconds after completing the barrier" - .to_string(), - ), - }, + JsonSchema::number(Some( + "Optional delay in milliseconds after completing the barrier" + .to_string(), + )), ), ( "sleep_before_ms".to_string(), - JsonSchema::Number { - description: Some( - "Optional delay in milliseconds before any other action" - .to_string(), - ), - }, + JsonSchema::number(Some( + "Optional delay in milliseconds before any other action".to_string(), + )), ), - ]), - required: None, - additional_properties: Some(false.into()), - }, + ]), /*required*/ None, Some(false.into())), output_schema: None, }) ); diff --git a/codex-rs/tools/src/view_image.rs b/codex-rs/tools/src/view_image.rs index a6881674b..5d591a339 100644 --- a/codex-rs/tools/src/view_image.rs +++ b/codex-rs/tools/src/view_image.rs @@ -14,18 +14,14 @@ pub struct ViewImageToolOptions { pub fn create_view_image_tool(options: ViewImageToolOptions) -> ToolSpec { let mut properties = BTreeMap::from([( "path".to_string(), - JsonSchema::String { - description: Some("Local filesystem path to an image file".to_string()), - }, + JsonSchema::string(Some("Local filesystem path to an image file".to_string())), )]); if options.can_request_original_image_detail { properties.insert( "detail".to_string(), - JsonSchema::String { - description: Some( - "Optional detail override. The only supported value is `original`; omit this field for default resized behavior. Use `original` to preserve the file's original resolution instead of resizing to fit. This is important when high-fidelity image perception or precise localization is needed, especially for CUA agents.".to_string(), - ), - }, + JsonSchema::string(Some( + "Optional detail override. The only supported value is `original`; omit this field for default resized behavior. Use `original` to preserve the file's original resolution instead of resizing to fit. This is important when high-fidelity image perception or precise localization is needed, especially for CUA agents.".to_string(), + )), ); } @@ -35,11 +31,7 @@ pub fn create_view_image_tool(options: ViewImageToolOptions) -> ToolSpec { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["path".to_string()]), - additional_properties: Some(false.into()), - }, + parameters: JsonSchema::object(properties, Some(vec!["path".to_string()]), Some(false.into())), output_schema: Some(view_image_output_schema()), }) } diff --git a/codex-rs/tools/src/view_image_tests.rs b/codex-rs/tools/src/view_image_tests.rs index 75be84a6a..3d6782965 100644 --- a/codex-rs/tools/src/view_image_tests.rs +++ b/codex-rs/tools/src/view_image_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::JsonSchema; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -14,16 +15,10 @@ fn view_image_tool_omits_detail_without_original_detail_feature() { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([( + parameters: JsonSchema::object(BTreeMap::from([( "path".to_string(), - JsonSchema::String { - description: Some("Local filesystem path to an image file".to_string()), - }, - )]), - required: Some(vec!["path".to_string()]), - additional_properties: Some(false.into()), - }, + JsonSchema::string(Some("Local filesystem path to an image file".to_string()),), + )]), Some(vec!["path".to_string()]), Some(false.into())), output_schema: Some(view_image_output_schema()), }) ); @@ -41,26 +36,18 @@ fn view_image_tool_includes_detail_with_original_detail_feature() { .to_string(), strict: false, defer_loading: None, - parameters: JsonSchema::Object { - properties: BTreeMap::from([ + parameters: JsonSchema::object(BTreeMap::from([ ( "detail".to_string(), - JsonSchema::String { - description: Some( + JsonSchema::string(Some( "Optional detail override. The only supported value is `original`; omit this field for default resized behavior. Use `original` to preserve the file's original resolution instead of resizing to fit. This is important when high-fidelity image perception or precise localization is needed, especially for CUA agents.".to_string(), - ), - }, + ),), ), ( "path".to_string(), - JsonSchema::String { - description: Some("Local filesystem path to an image file".to_string()), - }, + JsonSchema::string(Some("Local filesystem path to an image file".to_string()),), ), - ]), - required: Some(vec!["path".to_string()]), - additional_properties: Some(false.into()), - }, + ]), Some(vec!["path".to_string()]), Some(false.into())), output_schema: Some(view_image_output_schema()), }) );