diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 02d7c76a91..1392b5a568 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -1150,6 +1150,7 @@ "jni_0.21.1": "{\"dependencies\":[{\"name\":\"cesu8\",\"req\":\"^1.1.0\"},{\"name\":\"cfg-if\",\"req\":\"^1.0.0\"},{\"name\":\"combine\",\"req\":\"^4.1.0\"},{\"name\":\"java-locator\",\"optional\":true,\"req\":\"^0.1\"},{\"name\":\"jni-sys\",\"req\":\"^0.3.0\"},{\"name\":\"libloading\",\"optional\":true,\"req\":\"^0.7\"},{\"name\":\"log\",\"req\":\"^0.4.4\"},{\"name\":\"thiserror\",\"req\":\"^1.0.20\"},{\"kind\":\"dev\",\"name\":\"assert_matches\",\"req\":\"^1.5.0\"},{\"kind\":\"dev\",\"name\":\"lazy_static\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"rusty-fork\",\"req\":\"^0.3.0\"},{\"kind\":\"build\",\"name\":\"walkdir\",\"req\":\"^2\"},{\"features\":[\"Win32_Globalization\"],\"name\":\"windows-sys\",\"req\":\"^0.45.0\",\"target\":\"cfg(windows)\"},{\"kind\":\"dev\",\"name\":\"bytemuck\",\"req\":\"^1.13.0\",\"target\":\"cfg(windows)\"}],\"features\":{\"default\":[],\"invocation\":[\"java-locator\",\"libloading\"]}}", "jobserver_0.1.34": "{\"dependencies\":[{\"features\":[\"std\"],\"name\":\"getrandom\",\"req\":\"^0.3.2\",\"target\":\"cfg(windows)\"},{\"name\":\"libc\",\"req\":\"^0.2.171\",\"target\":\"cfg(unix)\"},{\"features\":[\"fs\"],\"kind\":\"dev\",\"name\":\"nix\",\"req\":\"^0.28.0\",\"target\":\"cfg(unix)\"},{\"kind\":\"dev\",\"name\":\"tempfile\",\"req\":\"^3.10.1\"}],\"features\":{}}", "js-sys_0.3.85": "{\"dependencies\":[{\"default_features\":false,\"name\":\"once_cell\",\"req\":\"^1.12\"},{\"default_features\":false,\"name\":\"wasm-bindgen\",\"req\":\"=0.2.108\"}],\"features\":{\"default\":[\"std\"],\"std\":[\"wasm-bindgen/std\"]}}", + "jsonptr_0.7.1": "{\"dependencies\":[{\"features\":[\"fancy\"],\"name\":\"miette\",\"optional\":true,\"req\":\"^7.4.0\"},{\"kind\":\"dev\",\"name\":\"quickcheck\",\"req\":\"^1.0.3\"},{\"kind\":\"dev\",\"name\":\"quickcheck_macros\",\"req\":\"^1.0.0\"},{\"features\":[\"alloc\"],\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0.203\"},{\"features\":[\"alloc\"],\"name\":\"serde_json\",\"optional\":true,\"req\":\"^1.0.119\"},{\"name\":\"syn\",\"optional\":true,\"req\":\"^1.0.109\",\"target\":\"cfg(any())\"},{\"name\":\"toml\",\"optional\":true,\"req\":\"^0.8\"}],\"features\":{\"assign\":[],\"default\":[\"std\",\"serde\",\"json\",\"resolve\",\"assign\",\"delete\"],\"delete\":[\"resolve\"],\"json\":[\"dep:serde_json\",\"serde\"],\"miette\":[\"dep:miette\",\"std\"],\"resolve\":[],\"std\":[\"serde/std\",\"serde_json?/std\"],\"toml\":[\"dep:toml\",\"serde\",\"std\"]}}", "jsonwebtoken_9.3.1": "{\"dependencies\":[{\"name\":\"base64\",\"req\":\"^0.22\"},{\"default_features\":false,\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.4\",\"target\":\"cfg(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\"))))\"},{\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.4\",\"target\":\"cfg(not(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\")))))\"},{\"name\":\"js-sys\",\"req\":\"^0.3\",\"target\":\"cfg(target_arch = \\\"wasm32\\\")\"},{\"name\":\"pem\",\"optional\":true,\"req\":\"^3\"},{\"features\":[\"std\"],\"name\":\"ring\",\"req\":\"^0.17.4\",\"target\":\"cfg(not(target_arch = \\\"wasm32\\\"))\"},{\"features\":[\"std\",\"wasm32_unknown_unknown_js\"],\"name\":\"ring\",\"req\":\"^0.17.4\",\"target\":\"cfg(target_arch = \\\"wasm32\\\")\"},{\"features\":[\"derive\"],\"name\":\"serde\",\"req\":\"^1.0\"},{\"name\":\"serde_json\",\"req\":\"^1.0\"},{\"name\":\"simple_asn1\",\"optional\":true,\"req\":\"^0.6\"},{\"features\":[\"wasm-bindgen\"],\"kind\":\"dev\",\"name\":\"time\",\"req\":\"^0.3\",\"target\":\"cfg(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\"))))\"},{\"kind\":\"dev\",\"name\":\"time\",\"req\":\"^0.3\",\"target\":\"cfg(not(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\")))))\"},{\"kind\":\"dev\",\"name\":\"wasm-bindgen-test\",\"req\":\"^0.3.1\"}],\"features\":{\"default\":[\"use_pem\"],\"use_pem\":[\"pem\",\"simple_asn1\"]}}", "keyring_3.6.3": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"base64\",\"req\":\"^0.22\"},{\"name\":\"byteorder\",\"optional\":true,\"req\":\"^1.2\",\"target\":\"cfg(target_os = \\\"windows\\\")\"},{\"features\":[\"derive\",\"wrap_help\"],\"kind\":\"dev\",\"name\":\"clap\",\"req\":\"^4\"},{\"name\":\"dbus-secret-service\",\"optional\":true,\"req\":\"^4.0.0-rc.1\",\"target\":\"cfg(target_os = \\\"openbsd\\\")\"},{\"name\":\"dbus-secret-service\",\"optional\":true,\"req\":\"^4.0.0-rc.2\",\"target\":\"cfg(target_os = \\\"linux\\\")\"},{\"name\":\"dbus-secret-service\",\"optional\":true,\"req\":\"^4.0.1\",\"target\":\"cfg(target_os = \\\"freebsd\\\")\"},{\"kind\":\"dev\",\"name\":\"doc-comment\",\"req\":\"^0.3\"},{\"kind\":\"dev\",\"name\":\"env_logger\",\"req\":\"^0.11.5\"},{\"kind\":\"dev\",\"name\":\"fastrand\",\"req\":\"^2\"},{\"features\":[\"std\"],\"name\":\"linux-keyutils\",\"optional\":true,\"req\":\"^0.2\",\"target\":\"cfg(target_os = \\\"linux\\\")\"},{\"name\":\"log\",\"req\":\"^0.4.22\"},{\"name\":\"openssl\",\"optional\":true,\"req\":\"^0.10.66\"},{\"kind\":\"dev\",\"name\":\"rpassword\",\"req\":\"^7\"},{\"kind\":\"dev\",\"name\":\"rprompt\",\"req\":\"^2\"},{\"name\":\"secret-service\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"freebsd\\\")\"},{\"name\":\"secret-service\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"linux\\\")\"},{\"name\":\"secret-service\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"openbsd\\\")\"},{\"name\":\"security-framework\",\"optional\":true,\"req\":\"^2\",\"target\":\"cfg(target_os = \\\"ios\\\")\"},{\"name\":\"security-framework\",\"optional\":true,\"req\":\"^3\",\"target\":\"cfg(target_os = \\\"macos\\\")\"},{\"kind\":\"dev\",\"name\":\"whoami\",\"req\":\"^1.5\"},{\"features\":[\"Win32_Foundation\",\"Win32_Security_Credentials\"],\"name\":\"windows-sys\",\"optional\":true,\"req\":\"^0.60\",\"target\":\"cfg(target_os = \\\"windows\\\")\"},{\"name\":\"zbus\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"freebsd\\\")\"},{\"name\":\"zbus\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"linux\\\")\"},{\"name\":\"zbus\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"openbsd\\\")\"},{\"name\":\"zeroize\",\"req\":\"^1.8.1\",\"target\":\"cfg(target_os = \\\"windows\\\")\"}],\"features\":{\"apple-native\":[\"dep:security-framework\"],\"async-io\":[\"zbus?/async-io\"],\"async-secret-service\":[\"dep:secret-service\",\"dep:zbus\"],\"crypto-openssl\":[\"dbus-secret-service?/crypto-openssl\",\"secret-service?/crypto-openssl\"],\"crypto-rust\":[\"dbus-secret-service?/crypto-rust\",\"secret-service?/crypto-rust\"],\"linux-native\":[\"dep:linux-keyutils\"],\"linux-native-async-persistent\":[\"linux-native\",\"async-secret-service\"],\"linux-native-sync-persistent\":[\"linux-native\",\"sync-secret-service\"],\"sync-secret-service\":[\"dep:dbus-secret-service\"],\"tokio\":[\"zbus?/tokio\"],\"vendored\":[\"dbus-secret-service?/vendored\",\"openssl?/vendored\"],\"windows-native\":[\"dep:windows-sys\",\"dep:byteorder\"]}}", "kqueue-sys_1.0.4": "{\"dependencies\":[{\"name\":\"bitflags\",\"req\":\"^1.2.1\"},{\"name\":\"libc\",\"req\":\"^0.2.74\"}],\"features\":{}}", diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 7c9955323d..4acaeb7c27 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3773,12 +3773,14 @@ dependencies = [ "codex-utils-output-truncation", "codex-utils-pty", "codex-utils-string", + "jsonptr", "pretty_assertions", "rmcp", "serde", "serde_json", "thiserror 2.0.18", "tracing", + "urlencoding", ] [[package]] @@ -8125,6 +8127,12 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "jsonptr" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a5a3cc660ba5d72bce0b3bb295bf20847ccbb40fd423f3f05b61273672e561fe" + [[package]] name = "jsonwebtoken" version = "9.3.1" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index ec38a87cff..df169504f0 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -301,6 +301,7 @@ indexmap = "2.12.0" insta = "1.46.3" inventory = "0.3.19" itertools = "0.14.0" +jsonptr = { version = "0.7.1", default-features = false } jsonwebtoken = "9.3.1" keyring = { version = "3.6", default-features = false } landlock = "0.4.4" diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 6a05535488..e5f6e4132d 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -126,6 +126,11 @@ impl ContextManager { &self.items } + /// Returns raw items in the history and consumes the snapshot. + pub(crate) fn into_raw_items(self) -> Vec { + self.items + } + pub(crate) fn history_version(&self) -> u64 { self.history_version } diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index 32ec1b85b0..ac130eced7 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -1,14 +1,16 @@ use std::sync::Arc; +use codex_tools::ConversationHistory; +use codex_tools::ToolCall as ExtensionToolCall; +use codex_tools::ToolName; +use codex_tools::ToolSpec; + use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use codex_tools::ToolCall as ExtensionToolCall; -use codex_tools::ToolName; -use codex_tools::ToolSpec; pub(crate) struct ExtensionToolAdapter(Arc>); @@ -47,7 +49,7 @@ impl ToolExecutor for ExtensionToolAdapter { &self, invocation: ToolInvocation, ) -> Result, FunctionCallError> { - self.0.handle(to_extension_call(&invocation)).await + self.0.handle(to_extension_call(&invocation).await).await } } @@ -57,12 +59,15 @@ impl CoreToolRuntime for ExtensionToolAdapter { } } -fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { +async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { + let conversation_history = + ConversationHistory::new(invocation.session.clone_history().await.into_raw_items()); ExtensionToolCall { turn_id: invocation.turn.sub_id.clone(), call_id: invocation.call_id.clone(), tool_name: invocation.tool_name.clone(), truncation_policy: invocation.turn.truncation_policy, + conversation_history, payload: invocation.payload.clone(), } } @@ -71,6 +76,8 @@ fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { mod tests { use std::sync::Arc; + use codex_protocol::models::ContentItem; + use codex_protocol::models::ResponseItem; use pretty_assertions::assert_eq; use serde_json::json; use tokio::sync::Mutex; @@ -199,6 +206,17 @@ mod tests { let (session, turn) = crate::session::tests::make_session_and_context().await; let turn_id = turn.sub_id.clone(); let truncation_policy = turn.truncation_policy; + let history_item = ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "extension history".to_string(), + }], + phase: None, + }; + session + .record_into_history(std::slice::from_ref(&history_item), &turn) + .await; let invocation = ToolInvocation { session: session.into(), turn: turn.into(), @@ -224,6 +242,10 @@ mod tests { codex_tools::ToolName::plain("extension_echo") ); assert_eq!(captured_call.truncation_policy, truncation_policy); + assert_eq!( + captured_call.conversation_history.items(), + std::slice::from_ref(&history_item) + ); match captured_call.payload { ToolPayload::Function { arguments } => { assert_eq!(arguments, json!({ "message": "hello" }).to_string()); diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index 375faba248..9d685e53b6 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -11,6 +11,7 @@ use codex_extension_api::ResponsesApiTool; use codex_extension_api::ToolCall as ExtensionToolCall; use codex_extension_api::ToolExecutor; use codex_protocol::dynamic_tools::DynamicToolSpec; +use codex_protocol::models::ContentItem; use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; @@ -81,6 +82,7 @@ impl ToolExecutor for ExtensionEchoExecutor { Ok(Box::new(codex_tools::JsonToolOutput::new(json!({ "arguments": arguments, "callId": call.call_id, + "conversationHistory": call.conversation_history.items(), "ok": true, })))) } @@ -327,6 +329,17 @@ fn mcp_tool_info( async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow::Result<()> { let (mut session, turn) = make_session_and_context().await; session.services.extensions = extension_tool_test_registry(); + let history_item = ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "extension history".to_string(), + }], + phase: None, + }; + session + .record_into_history(std::slice::from_ref(&history_item), &turn) + .await; let router = ToolRouter::from_turn_context( &turn, @@ -384,6 +397,7 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow json!({ "arguments": { "message": "hello" }, "callId": "call-extension", + "conversationHistory": [history_item], "ok": true, }) ); diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index 06cbc6f889..a7c5c87e52 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -10,6 +10,7 @@ pub use capabilities::NoopExtensionEventSink; pub use capabilities::NoopResponseItemInjector; pub use capabilities::ResponseItemInjectionFuture; pub use capabilities::ResponseItemInjector; +pub use codex_tools::ConversationHistory; pub use codex_tools::FunctionCallError; pub use codex_tools::JsonToolOutput; pub use codex_tools::ResponsesApiTool; diff --git a/codex-rs/ext/goal/tests/goal_extension_backend.rs b/codex-rs/ext/goal/tests/goal_extension_backend.rs index 28e55064fd..cdeacbebe6 100644 --- a/codex-rs/ext/goal/tests/goal_extension_backend.rs +++ b/codex-rs/ext/goal/tests/goal_extension_backend.rs @@ -625,6 +625,7 @@ fn tool_call(tool_name: &str, call_id: &str, arguments: serde_json::Value) -> To call_id: call_id.to_string(), tool_name: codex_extension_api::ToolName::plain(tool_name), truncation_policy: TruncationPolicy::Bytes(1024), + conversation_history: codex_extension_api::ConversationHistory::default(), payload: ToolPayload::Function { arguments: arguments.to_string(), }, diff --git a/codex-rs/ext/memories/src/tests.rs b/codex-rs/ext/memories/src/tests.rs index e88d7d8db9..c2e90e6520 100644 --- a/codex-rs/ext/memories/src/tests.rs +++ b/codex-rs/ext/memories/src/tests.rs @@ -139,6 +139,7 @@ async fn read_tool_reads_memory_file() { call_id: "call-1".to_string(), tool_name: memory_tool_name(crate::READ_TOOL_NAME), truncation_policy: TruncationPolicy::Bytes(1024), + conversation_history: codex_extension_api::ConversationHistory::default(), payload: payload.clone(), }) .await @@ -183,6 +184,7 @@ async fn search_tool_accepts_multiple_queries() { call_id: "call-1".to_string(), tool_name: memory_tool_name(crate::SEARCH_TOOL_NAME), truncation_policy: TruncationPolicy::Bytes(1024), + conversation_history: codex_extension_api::ConversationHistory::default(), payload: payload.clone(), }) .await @@ -253,6 +255,7 @@ async fn search_tool_accepts_windowed_all_match_mode() { call_id: "call-1".to_string(), tool_name: memory_tool_name(crate::SEARCH_TOOL_NAME), truncation_policy: TruncationPolicy::Bytes(1024), + conversation_history: codex_extension_api::ConversationHistory::default(), payload: payload.clone(), }) .await @@ -303,6 +306,7 @@ async fn search_tool_rejects_legacy_single_query() { call_id: "call-1".to_string(), tool_name: memory_tool_name(crate::SEARCH_TOOL_NAME), truncation_policy: TruncationPolicy::Bytes(1024), + conversation_history: codex_extension_api::ConversationHistory::default(), payload, }) .await; diff --git a/codex-rs/tools/Cargo.toml b/codex-rs/tools/Cargo.toml index 334ce79580..7d02f1bf36 100644 --- a/codex-rs/tools/Cargo.toml +++ b/codex-rs/tools/Cargo.toml @@ -17,6 +17,7 @@ codex-utils-absolute-path = { workspace = true } codex-utils-output-truncation = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-string = { workspace = true } +jsonptr = { workspace = true } rmcp = { workspace = true, default-features = false, features = [ "base64", "macros", @@ -27,6 +28,7 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } +urlencoding = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/tools/src/json_schema.rs b/codex-rs/tools/src/json_schema.rs index ecd880cc7a..02936d52d1 100644 --- a/codex-rs/tools/src/json_schema.rs +++ b/codex-rs/tools/src/json_schema.rs @@ -3,6 +3,10 @@ use serde::Serialize; use serde_json::Value as JsonValue; use serde_json::json; use std::collections::BTreeMap; +use std::collections::BTreeSet; + +const DEFINITION_TABLE_KEYS: [&str; 2] = ["$defs", "definitions"]; +const SCHEMA_CHILD_KEYS: [&str; 2] = ["items", "anyOf"]; /// Primitive JSON Schema type names we support in tool definitions. /// @@ -33,6 +37,8 @@ pub enum JsonSchemaType { /// Generic JSON-Schema subset needed for our tool definitions. #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] pub struct JsonSchema { + #[serde(rename = "$ref", skip_serializing_if = "Option::is_none")] + pub schema_ref: Option, #[serde(rename = "type", skip_serializing_if = "Option::is_none")] pub schema_type: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -52,6 +58,10 @@ pub struct JsonSchema { pub additional_properties: Option, #[serde(rename = "anyOf", skip_serializing_if = "Option::is_none")] pub any_of: Option>, + #[serde(rename = "$defs", skip_serializing_if = "Option::is_none")] + pub defs: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub definitions: Option>, } impl JsonSchema { @@ -149,6 +159,8 @@ 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); + prune_unreachable_definitions(&mut input_schema); + compact_large_tool_schema(&mut input_schema); let schema: JsonSchema = serde_json::from_value(input_schema)?; if matches!( schema.schema_type, @@ -159,10 +171,220 @@ pub fn parse_tool_input_schema(input_schema: &JsonValue) -> Result bool { + compact_normalized_schema_len(value) <= MAX_COMPACT_TOOL_SCHEMA_BYTES +} + +fn compact_normalized_schema_len(value: &JsonValue) -> usize { + serde_json::from_value::(value.clone()) + .and_then(|schema| serde_json::to_vec(&schema)) + .map(|json| json.len()) + .unwrap_or(0) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum DefinitionTraversal { + Include, + Skip, +} + +fn for_each_schema_child( + map: &serde_json::Map, + definition_traversal: DefinitionTraversal, + visitor: &mut impl FnMut(&JsonValue), +) { + if let Some(properties) = map.get("properties") + && let Some(properties_map) = properties.as_object() + { + for value in properties_map.values() { + visitor(value); + } + } + + for key in SCHEMA_CHILD_KEYS { + if let Some(value) = map.get(key) { + visitor(value); + } + } + + if let Some(additional_properties) = map.get("additionalProperties") + && !matches!(additional_properties, JsonValue::Bool(_)) + { + visitor(additional_properties); + } + + if definition_traversal == DefinitionTraversal::Include { + for key in DEFINITION_TABLE_KEYS { + if let Some(definitions) = map.get(key) + && let Some(definitions_map) = definitions.as_object() + { + for value in definitions_map.values() { + visitor(value); + } + } + } + } +} + +fn strip_schema_descriptions(value: &mut JsonValue) { + match value { + JsonValue::Array(values) => { + for value in values { + strip_schema_descriptions(value); + } + } + JsonValue::Object(map) => { + map.remove("description"); + for_each_schema_child_mut(map, DefinitionTraversal::Include, &mut |value| { + strip_schema_descriptions(value); + }); + } + _ => {} + } +} + +fn for_each_schema_child_mut( + map: &mut serde_json::Map, + definition_traversal: DefinitionTraversal, + visitor: &mut impl FnMut(&mut JsonValue), +) { + if let Some(properties) = map.get_mut("properties") + && let Some(properties_map) = properties.as_object_mut() + { + for value in properties_map.values_mut() { + visitor(value); + } + } + + for key in SCHEMA_CHILD_KEYS { + if let Some(value) = map.get_mut(key) { + visitor(value); + } + } + + if let Some(additional_properties) = map.get_mut("additionalProperties") + && !matches!(additional_properties, JsonValue::Bool(_)) + { + visitor(additional_properties); + } + + if definition_traversal == DefinitionTraversal::Include { + for key in DEFINITION_TABLE_KEYS { + if let Some(definitions) = map.get_mut(key) + && let Some(definitions_map) = definitions.as_object_mut() + { + for value in definitions_map.values_mut() { + visitor(value); + } + } + } + } +} + +/// Replace local definition refs with empty schemas before dropping root +/// definition tables, so downstream behavior does not depend on how a schema +/// parser handles refs to missing definitions. +fn drop_schema_definitions(value: &mut JsonValue) { + rewrite_definition_refs_to_empty_schemas(value); + + let JsonValue::Object(map) = value else { + return; + }; + + for key in DEFINITION_TABLE_KEYS { + map.remove(key); + } +} + +fn rewrite_definition_refs_to_empty_schemas(value: &mut JsonValue) { + match value { + JsonValue::Array(values) => { + for value in values { + rewrite_definition_refs_to_empty_schemas(value); + } + } + JsonValue::Object(map) => { + if map + .get("$ref") + .and_then(JsonValue::as_str) + .and_then(parse_local_definition_ref) + .is_some() + { + *value = json!({}); + return; + } + + for_each_schema_child_mut(map, DefinitionTraversal::Skip, &mut |value| { + rewrite_definition_refs_to_empty_schemas(value); + }); + } + _ => {} + } +} + +fn collapse_deep_schema_objects(value: &mut JsonValue, depth: usize) { + match value { + JsonValue::Array(values) => { + for value in values { + collapse_deep_schema_objects(value, depth); + } + } + JsonValue::Object(map) => { + if depth >= MAX_COMPACT_TOOL_SCHEMA_DEPTH && is_complex_schema_object(map) { + *value = json!({}); + return; + } + + for_each_schema_child_mut(map, DefinitionTraversal::Skip, &mut |value| { + collapse_deep_schema_objects(value, depth + 1); + }); + } + _ => {} + } +} + +fn is_complex_schema_object(map: &serde_json::Map) -> bool { + SCHEMA_CHILD_KEYS.iter().any(|key| map.contains_key(*key)) + || map.contains_key("properties") + || map.contains_key("additionalProperties") + || map.contains_key("$ref") +} + /// Sanitize a JSON Schema (as serde_json::Value) so it can fit our limited /// schema representation. This function: /// - Ensures every typed schema object has a `"type"` when required. /// - Preserves explicit `anyOf`. +/// - Preserves `$ref` and reachable local `$defs` / `definitions`. /// - Collapses `const` into single-value `enum`. /// - Fills required child fields for object/array schema types, including /// nullable unions, with permissive defaults when absent. @@ -200,6 +422,9 @@ fn sanitize_json_schema(value: &mut JsonValue) { if let Some(value) = map.get_mut("anyOf") { sanitize_json_schema(value); } + for table in DEFINITION_TABLE_KEYS { + sanitize_schema_table(map, table); + } if let Some(const_value) = map.remove("const") { map.insert("enum".to_string(), JsonValue::Array(vec![const_value])); @@ -207,7 +432,7 @@ fn sanitize_json_schema(value: &mut JsonValue) { let mut schema_types = normalized_schema_types(map); - if schema_types.is_empty() && map.contains_key("anyOf") { + if schema_types.is_empty() && (map.contains_key("$ref") || map.contains_key("anyOf")) { return; } @@ -241,6 +466,29 @@ fn sanitize_json_schema(value: &mut JsonValue) { } } +/// Sanitize a schema definition table before deserializing into `JsonSchema`. +/// +/// Definition tables must be objects. Codex keeps valid definition tables and +/// recursively applies the same compatibility lowering used for inline schemas, +/// but drops malformed tables so `strict: false` tool registration degrades +/// gracefully instead of failing on an unreachable or invalid definition table. +fn sanitize_schema_table(map: &mut serde_json::Map, key: &str) { + let should_remove = match map.get_mut(key) { + Some(JsonValue::Object(definitions)) => { + for definition in definitions.values_mut() { + sanitize_json_schema(definition); + } + false + } + Some(_) => true, + None => false, + }; + + if should_remove { + map.remove(key); + } +} + fn ensure_default_children_for_schema_types( map: &mut serde_json::Map, schema_types: &[JsonSchemaPrimitiveType], @@ -257,6 +505,143 @@ fn ensure_default_children_for_schema_types( } } +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +struct DefinitionPointer { + table: &'static str, + name: String, +} + +/// Prune unused root definition entries to avoid sending tokens for definitions +/// the tool schema never references. +fn prune_unreachable_definitions(value: &mut JsonValue) { + let reachable = collect_reachable_definitions(value); + let JsonValue::Object(map) = value else { + return; + }; + + for table in DEFINITION_TABLE_KEYS { + prune_schema_table(map, table, &reachable); + } +} + +fn prune_schema_table( + map: &mut serde_json::Map, + table: &'static str, + reachable: &BTreeSet, +) { + let Some(JsonValue::Object(definitions)) = map.get_mut(table) else { + return; + }; + + definitions.retain(|name, _| { + reachable.contains(&DefinitionPointer { + table, + name: name.clone(), + }) + }); + + if definitions.is_empty() { + map.remove(table); + } +} + +fn collect_reachable_definitions(value: &JsonValue) -> BTreeSet { + let mut reachable = BTreeSet::new(); + let mut pending = Vec::new(); + + collect_refs_outside_definitions(value, &mut pending); + + while let Some(pointer) = pending.pop() { + if !reachable.insert(pointer.clone()) { + continue; + } + + if let Some(definition) = definition_for_pointer(value, &pointer) { + collect_refs(definition, &mut pending); + } + } + + reachable +} + +fn collect_refs_outside_definitions(value: &JsonValue, refs: &mut Vec) { + match value { + JsonValue::Array(values) => { + for value in values { + collect_refs_outside_definitions(value, refs); + } + } + JsonValue::Object(map) => { + collect_ref_from_map(map, refs); + for_each_schema_child(map, DefinitionTraversal::Skip, &mut |value| { + collect_refs_outside_definitions(value, refs); + }); + } + _ => {} + } +} + +fn collect_refs(value: &JsonValue, refs: &mut Vec) { + match value { + JsonValue::Array(values) => { + for value in values { + collect_refs(value, refs); + } + } + JsonValue::Object(map) => { + collect_ref_from_map(map, refs); + for value in map.values() { + collect_refs(value, refs); + } + } + _ => {} + } +} + +fn collect_ref_from_map( + map: &serde_json::Map, + refs: &mut Vec, +) { + if let Some(JsonValue::String(schema_ref)) = map.get("$ref") + && let Some(pointer) = parse_local_definition_ref(schema_ref) + { + refs.push(pointer); + } +} + +fn definition_for_pointer<'a>( + value: &'a JsonValue, + pointer: &DefinitionPointer, +) -> Option<&'a JsonValue> { + let JsonValue::Object(map) = value else { + return None; + }; + + map.get(pointer.table) + .and_then(JsonValue::as_object) + .and_then(|definitions| definitions.get(&pointer.name)) +} + +fn parse_local_definition_ref(schema_ref: &str) -> Option { + let fragment = schema_ref.strip_prefix('#')?; + let pointer = urlencoding::decode(fragment).ok()?; + let pointer = jsonptr::Pointer::parse(pointer.as_ref()).ok()?; + + let (table_token, pointer) = pointer.split_front()?; + let table = table_token.decoded(); + let table = DEFINITION_TABLE_KEYS + .into_iter() + .find(|candidate| table.as_ref() == *candidate)?; + + // Responses API non-strict mode accepts nested local refs such as + // `#/$defs/User/properties/name`, so keep the parent definition reachable. + let (name, _) = pointer.split_front()?; + Some(DefinitionPointer { + table, + name: name.decoded().into_owned(), + }) +} + fn normalized_schema_types( map: &serde_json::Map, ) -> Vec { diff --git a/codex-rs/tools/src/json_schema_tests.rs b/codex-rs/tools/src/json_schema_tests.rs index 5daaf048d0..52b30138c1 100644 --- a/codex-rs/tools/src/json_schema_tests.rs +++ b/codex-rs/tools/src/json_schema_tests.rs @@ -779,6 +779,393 @@ fn parse_tool_input_schema_preserves_explicit_enum_type_union() { ); } +fn many_string_properties(count: usize) -> serde_json::Map { + (0..count) + .map(|index| { + ( + format!("field_{index:03}"), + serde_json::json!({ "type": "string" }), + ) + }) + .collect() +} + +#[test] +fn parse_large_tool_input_schema_stops_after_descriptions_when_under_budget() { + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "description": "x".repeat(4_500), + "properties": { + "metadata": { + "$ref": "#/$defs/metadata" + } + }, + "$defs": { + "metadata": { + "type": "string", + "description": "Metadata value" + } + } + })) + .expect("parse schema"); + + assert_eq!( + serde_json::to_value(schema).expect("serialize schema"), + serde_json::json!({ + "type": "object", + "properties": { + "metadata": { + "$ref": "#/$defs/metadata" + } + }, + "$defs": { + "metadata": { + "type": "string" + } + } + }) + ); +} + +#[test] +fn parse_large_tool_input_schema_ignores_dropped_metadata_for_budget() { + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "event": { + "type": "object", + "title": "Calendar event", + "properties": { + "recurrence": { + "type": "object", + "examples": [ + { + "payload": "x".repeat(4_500) + } + ], + "properties": { + "pattern": { + "type": "string", + "title": "Recurrence pattern" + } + } + } + } + } + } + })) + .expect("parse schema"); + + assert_eq!( + serde_json::to_value(schema).expect("serialize schema"), + serde_json::json!({ + "type": "object", + "properties": { + "event": { + "type": "object", + "properties": { + "recurrence": { + "type": "object", + "properties": { + "pattern": { + "type": "string" + } + } + } + } + } + } + }) + ); +} + +#[test] +fn parse_large_tool_input_schema_stops_after_dropping_root_definitions_when_under_budget() { + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "description": "x".repeat(4_500), + "properties": { + "event": { + "type": "object", + "description": "Calendar event", + "properties": { + "recurrence": { + "type": "object", + "description": "Recurrence settings", + "properties": { + "pattern": { + "type": "string", + "description": "Recurrence pattern" + } + } + } + } + }, + "metadata": { + "$ref": "#/$defs/metadata" + } + }, + "$defs": { + "metadata": { + "type": "object", + "description": "metadata object", + "properties": many_string_properties(/*count*/ 300) + } + } + })) + .expect("parse schema"); + + assert_eq!( + serde_json::to_value(schema).expect("serialize schema"), + serde_json::json!({ + "type": "object", + "properties": { + "event": { + "type": "object", + "properties": { + "recurrence": { + "type": "object", + "properties": { + "pattern": { + "type": "string" + } + } + } + } + }, + "metadata": {} + } + }) + ); +} + +#[test] +fn parse_large_tool_input_schema_strips_descriptions_without_removing_description_property() { + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "description": "x".repeat(4_500), + "properties": { + "description": { + "type": "string", + "description": "User-facing description value" + }, + "metadata": { + "type": "object", + "description": "Metadata object", + "properties": { + "label": { + "type": "string", + "description": "Metadata label" + } + } + }, + "tags": { + "type": "array", + "description": "Tag list", + "items": { + "type": "string", + "description": "Tag value" + } + }, + "extras": { + "type": "object", + "additionalProperties": { + "type": "string", + "description": "Extra value" + } + }, + "choice": { + "description": "Choice value", + "anyOf": [ + { + "type": "string", + "description": "String choice" + }, + { + "type": "number", + "description": "Number choice" + } + ] + } + } + })) + .expect("parse schema"); + + assert_eq!( + serde_json::to_value(schema).expect("serialize schema"), + serde_json::json!({ + "type": "object", + "properties": { + "choice": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "number" + } + ] + }, + "description": { + "type": "string" + }, + "extras": { + "type": "object", + "properties": {}, + "additionalProperties": { + "type": "string" + } + }, + "metadata": { + "type": "object", + "properties": { + "label": { + "type": "string" + } + } + }, + "tags": { + "type": "array", + "items": { + "type": "string" + } + } + } + }) + ); +} + +#[test] +fn parse_large_tool_input_schema_preserves_object_enum_literal_descriptions() { + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "description": "x".repeat(4_500), + "properties": { + "choice": { + "enum": [ + { + "description": "first literal", + "id": 1 + }, + { + "description": "second literal", + "id": 2 + } + ] + } + } + })) + .expect("parse schema"); + + assert_eq!( + serde_json::to_value(schema).expect("serialize schema"), + serde_json::json!({ + "type": "object", + "properties": { + "choice": { + "type": "string", + "enum": [ + { + "description": "first literal", + "id": 1 + }, + { + "description": "second literal", + "id": 2 + } + ] + } + } + }) + ); +} + +#[test] +fn collapse_deep_schema_objects_traverses_schema_children() { + let mut schema = serde_json::json!({ + "type": "object", + "properties": { + "object_parent": { + "type": "object", + "properties": { + "complex": { + "type": "object", + "properties": { + "leaf": { "type": "string" } + } + }, + "scalar": { + "type": "string" + } + } + }, + "array_parent": { + "type": "array", + "items": { + "type": "object", + "properties": { + "leaf": { "type": "string" } + } + } + }, + "map_parent": { + "type": "object", + "additionalProperties": { + "type": "object", + "properties": { + "leaf": { "type": "string" } + } + } + }, + "union_parent": { + "anyOf": [ + { + "type": "object", + "properties": { + "leaf": { "type": "string" } + } + }, + { "type": "string" } + ] + } + } + }); + + super::collapse_deep_schema_objects(&mut schema, /*depth*/ 0); + + assert_eq!( + schema, + serde_json::json!({ + "type": "object", + "properties": { + "object_parent": { + "type": "object", + "properties": { + "complex": {}, + "scalar": { + "type": "string" + } + } + }, + "array_parent": { + "type": "array", + "items": {} + }, + "map_parent": { + "type": "object", + "additionalProperties": {} + }, + "union_parent": { + "anyOf": [ + {}, + { "type": "string" } + ] + } + } + }) + ); +} + #[test] fn parse_tool_input_schema_preserves_string_enum_constraints() { // Example schema shape: @@ -848,3 +1235,538 @@ fn parse_tool_input_schema_preserves_string_enum_constraints() { ) ); } + +#[test] +fn parse_tool_input_schema_preserves_refs_and_prunes_unreachable_defs() { + // Example schema shape: + // { + // "type": "object", + // "properties": { "user": { "$ref": "#/$defs/User" } }, + // "$defs": { + // "User": { "type": "object", "properties": { "name": { "type": "string" } } }, + // "Unused": { "type": "string" } + // } + // } + // + // Expected normalization behavior: + // - Local `$ref` is preserved as a schema hint. + // - Reachable `$defs` entries stay attached to the root schema. + // - Unreachable `$defs` entries are pruned. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "user": {"$ref": "#/$defs/User"} + }, + "$defs": { + "User": { + "type": "object", + "properties": { + "name": {"type": "string"} + } + }, + "Unused": {"type": "string"} + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + properties: Some(BTreeMap::from([( + "user".to_string(), + JsonSchema { + schema_ref: Some("#/$defs/User".to_string()), + ..Default::default() + }, + )])), + defs: Some(BTreeMap::from([( + "User".to_string(), + JsonSchema::object( + BTreeMap::from([( + "name".to_string(), + JsonSchema::string(/*description*/ None), + )]), + /*required*/ None, + /*additional_properties*/ None, + ), + )])), + ..Default::default() + } + ); +} + +#[test] +fn parse_tool_input_schema_preserves_refs_from_properties_named_def_tables() { + // Example schema shape: + // { + // "type": "object", + // "properties": { + // "$defs": { "$ref": "#/$defs/User" } + // }, + // "$defs": { "User": { "type": "string" }, "Unused": { "type": "boolean" } } + // } + // + // Expected normalization behavior: + // - A property named like the `$defs` keyword is treated as a user field + // while traversing `properties`. + // - Refs from that property schema still mark root definitions reachable. + // - Unreferenced root definitions are still pruned. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "$defs": {"$ref": "#/$defs/User"} + }, + "$defs": { + "User": {"type": "string"}, + "Unused": {"type": "boolean"} + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + properties: Some(BTreeMap::from([( + "$defs".to_string(), + JsonSchema { + schema_ref: Some("#/$defs/User".to_string()), + ..Default::default() + }, + )])), + defs: Some(BTreeMap::from([( + "User".to_string(), + JsonSchema::string(/*description*/ None), + )])), + ..Default::default() + } + ); +} + +#[test] +fn parse_tool_input_schema_collects_refs_from_schema_child_keywords() { + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "items_holder": { + "type": "array", + "items": {"$ref": "#/$defs/Item"} + }, + "map_holder": { + "type": "object", + "additionalProperties": {"$ref": "#/$defs/Extra"} + }, + "choice": { + "anyOf": [ + {"$ref": "#/$defs/Choice"}, + {"type": "string"} + ] + } + }, + "$defs": { + "Choice": {"type": "boolean"}, + "Extra": {"type": "number"}, + "Item": {"type": "string"}, + "Unused": {"type": "null"} + } + })) + .expect("parse schema"); + + assert_eq!( + serde_json::to_value(schema).expect("serialize schema"), + serde_json::json!({ + "type": "object", + "properties": { + "choice": { + "anyOf": [ + {"$ref": "#/$defs/Choice"}, + {"type": "string"} + ] + }, + "items_holder": { + "type": "array", + "items": {"$ref": "#/$defs/Item"} + }, + "map_holder": { + "type": "object", + "properties": {}, + "additionalProperties": {"$ref": "#/$defs/Extra"} + } + }, + "$defs": { + "Choice": {"type": "boolean"}, + "Extra": {"type": "number"}, + "Item": {"type": "string"} + } + }) + ); +} + +#[test] +fn parse_tool_input_schema_handles_cyclic_local_refs() { + // Example schema shape: + // { + // "type": "object", + // "properties": { "node": { "$ref": "#/$defs/Node" } }, + // "$defs": { + // "Node": { + // "type": "object", + // "properties": { "next": { "$ref": "#/$defs/Node" } } + // } + // } + // } + // + // Expected normalization behavior: + // - Recursive refs are preserved. + // - Pruning traversal terminates after visiting each local target once. + // - Responses API handles this recursive local-ref shape correctly. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "node": {"$ref": "#/$defs/Node"} + }, + "$defs": { + "Node": { + "type": "object", + "properties": { + "next": {"$ref": "#/$defs/Node"} + } + } + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + properties: Some(BTreeMap::from([( + "node".to_string(), + JsonSchema { + schema_ref: Some("#/$defs/Node".to_string()), + ..Default::default() + }, + )])), + defs: Some(BTreeMap::from([( + "Node".to_string(), + JsonSchema::object( + BTreeMap::from([( + "next".to_string(), + JsonSchema { + schema_ref: Some("#/$defs/Node".to_string()), + ..Default::default() + }, + )]), + /*required*/ None, + /*additional_properties*/ None, + ), + )])), + ..Default::default() + } + ); +} + +#[test] +fn parse_tool_input_schema_preserves_legacy_definitions() { + // Example schema shape: + // { + // "type": "object", + // "properties": { "user": { "$ref": "#/definitions/User" } }, + // "definitions": { + // "User": { "type": "object", "properties": { "profile": { "$ref": "#/definitions/Profile" } } }, + // "Profile": { "type": "object", "properties": { "name": { "type": "string" } } } + // } + // } + // + // Expected normalization behavior: + // - Codex preserves legacy `definitions`. + // - Reachability follows refs through the legacy definition table. + // - Unreachable legacy definition entries are pruned. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "user": {"$ref": "#/definitions/User"} + }, + "definitions": { + "User": { + "type": "object", + "properties": { + "profile": {"$ref": "#/definitions/Profile"} + } + }, + "Profile": { + "type": "object", + "properties": { + "name": {"type": "string"} + } + }, + "Unused": {"type": "string"} + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + properties: Some(BTreeMap::from([( + "user".to_string(), + JsonSchema { + schema_ref: Some("#/definitions/User".to_string()), + ..Default::default() + }, + )])), + definitions: Some(BTreeMap::from([ + ( + "Profile".to_string(), + JsonSchema::object( + BTreeMap::from([( + "name".to_string(), + JsonSchema::string(/*description*/ None), + )]), + /*required*/ None, + /*additional_properties*/ None, + ), + ), + ( + "User".to_string(), + JsonSchema::object( + BTreeMap::from([( + "profile".to_string(), + JsonSchema { + schema_ref: Some("#/definitions/Profile".to_string()), + ..Default::default() + }, + )]), + /*required*/ None, + /*additional_properties*/ None, + ), + ), + ])), + ..Default::default() + } + ); +} + +#[test] +fn parse_tool_input_schema_preserves_unresolved_and_external_refs() { + // Example schema shape: + // { + // "type": "object", + // "properties": { + // "missing": { "$ref": "#/$defs/Missing" }, + // "remote": { "$ref": "https://example.com/schema.json" } + // }, + // "$defs": { "Unused": { "type": "string" } } + // } + // + // Expected normalization behavior: + // - Unresolved local refs and external refs are preserved. + // - Unreachable local definitions are still pruned. + // - Responses API handles these refs correctly during downstream validation. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "missing": {"$ref": "#/$defs/Missing"}, + "remote": {"$ref": "https://example.com/schema.json"} + }, + "$defs": { + "Unused": {"type": "string"} + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + properties: Some(BTreeMap::from([ + ( + "missing".to_string(), + JsonSchema { + schema_ref: Some("#/$defs/Missing".to_string()), + ..Default::default() + }, + ), + ( + "remote".to_string(), + JsonSchema { + schema_ref: Some("https://example.com/schema.json".to_string()), + ..Default::default() + }, + ), + ])), + ..Default::default() + } + ); +} + +#[test] +fn parse_tool_input_schema_preserves_nested_defs_ref_parent() { + // Example schema shape: + // { + // "type": "object", + // "properties": { "name": { "$ref": "#/$defs/User/properties/name" } }, + // "$defs": { + // "User": { "type": "object", "properties": { "name": { "type": "string" } } }, + // "name": { "type": "string" }, + // "Unused": { "type": "boolean" } + // } + // } + // + // Expected normalization behavior: + // - The nested JSON Pointer ref remains unchanged. + // - The parent root definition is retained so the local ref does not dangle. + // - Unreferenced root definitions are still pruned. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "name": {"$ref": "#/$defs/User/properties/name"} + }, + "$defs": { + "User": { + "type": "object", + "properties": { + "name": {"type": "string"} + } + }, + "name": {"type": "string"}, + "Unused": {"type": "boolean"} + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + properties: Some(BTreeMap::from([( + "name".to_string(), + JsonSchema { + schema_ref: Some("#/$defs/User/properties/name".to_string()), + ..Default::default() + }, + )])), + defs: Some(BTreeMap::from([( + "User".to_string(), + JsonSchema::object( + BTreeMap::from([( + "name".to_string(), + JsonSchema::string(/*description*/ None), + )]), + /*required*/ None, + /*additional_properties*/ None, + ), + )])), + ..Default::default() + } + ); +} + +#[test] +fn parse_tool_input_schema_preserves_percent_encoded_definition_refs() { + // Example schema shape: + // { + // "type": "object", + // "properties": { + // "user": { "$ref": "#/$defs/User%20Name" }, + // "profile": { "$ref": "#/%24defs/Profile%7E0Name" } + // }, + // "$defs": { + // "User Name": { "type": "string" }, + // "Profile~Name": { "type": "string" }, + // "Unused": { "type": "boolean" } + // } + // } + // + // Expected normalization behavior: + // - URI fragment percent encoding is decoded before JSON Pointer `~` + // escaping, per RFC 6901 section 6. + // - The original `$ref` strings are preserved, but their definition + // targets are recognized as reachable and retained. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "user": {"$ref": "#/$defs/User%20Name"}, + "profile": {"$ref": "#/%24defs/Profile%7E0Name"} + }, + "$defs": { + "User Name": {"type": "string"}, + "Profile~Name": {"type": "string"}, + "Unused": {"type": "boolean"} + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + properties: Some(BTreeMap::from([ + ( + "profile".to_string(), + JsonSchema { + schema_ref: Some("#/%24defs/Profile%7E0Name".to_string()), + ..Default::default() + }, + ), + ( + "user".to_string(), + JsonSchema { + schema_ref: Some("#/$defs/User%20Name".to_string()), + ..Default::default() + }, + ), + ])), + defs: Some(BTreeMap::from([ + ( + "Profile~Name".to_string(), + JsonSchema::string(/*description*/ None), + ), + ( + "User Name".to_string(), + JsonSchema::string(/*description*/ None), + ), + ])), + ..Default::default() + } + ); +} + +#[test] +fn parse_tool_input_schema_drops_malformed_definition_tables() { + // Example schema shape: + // { + // "type": "object", + // "properties": { "user": { "$ref": "#/$defs/User" } }, + // "$defs": ["not", "an", "object"] + // } + // + // Expected normalization behavior: + // - Malformed `$defs` tables are dropped instead of rejecting the schema. + // - The unresolved local ref remains visible to the model. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "user": {"$ref": "#/$defs/User"} + }, + "$defs": ["not", "an", "object"] + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + properties: Some(BTreeMap::from([( + "user".to_string(), + JsonSchema { + schema_ref: Some("#/$defs/User".to_string()), + ..Default::default() + }, + )])), + ..Default::default() + } + ); +} diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 7b64776dca..c141bfb37a 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -57,6 +57,7 @@ pub use responses_api::dynamic_tool_to_responses_api_tool; pub use responses_api::mcp_tool_to_deferred_responses_api_tool; pub use responses_api::mcp_tool_to_responses_api_tool; pub use responses_api::tool_definition_to_responses_api_tool; +pub use tool_call::ConversationHistory; pub use tool_call::ToolCall; pub use tool_config::ShellCommandBackendConfig; pub use tool_config::ToolEnvironmentMode; diff --git a/codex-rs/tools/src/tool_call.rs b/codex-rs/tools/src/tool_call.rs index f92c92f979..32d428648f 100644 --- a/codex-rs/tools/src/tool_call.rs +++ b/codex-rs/tools/src/tool_call.rs @@ -1,7 +1,27 @@ use crate::FunctionCallError; use crate::ToolName; use crate::ToolPayload; +use codex_protocol::models::ResponseItem; use codex_utils_output_truncation::TruncationPolicy; +use std::sync::Arc; + +/// Raw response history snapshot available when an extension tool is invoked. +#[derive(Clone, Debug, Default)] +pub struct ConversationHistory { + items: Arc<[ResponseItem]>, +} + +impl ConversationHistory { + pub fn new(items: Vec) -> Self { + Self { + items: items.into(), + } + } + + pub fn items(&self) -> &[ResponseItem] { + &self.items + } +} // TODO: this is temporary and will disappear in the next PR (as we make codex-extension-api generic on Invocation. #[derive(Clone, Debug)] @@ -10,6 +30,7 @@ pub struct ToolCall { pub call_id: String, pub tool_name: ToolName, pub truncation_policy: TruncationPolicy, + pub conversation_history: ConversationHistory, pub payload: ToolPayload, } diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 4baea7fb04..1cba3c85a0 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -910,7 +910,8 @@ impl App { return; } - self.runtime_permission_profile_override = Some(permission_profile); + self.runtime_permission_profile_override = + Some(RuntimePermissionProfileOverride::from_config(&self.config)); self.sync_active_thread_permission_settings_to_cached_session() .await;