From 5a6e90599434137b677b7224d995932183844646 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 21 May 2026 16:52:36 -0700 Subject: [PATCH 1/4] Fix auto-review permission profile override (#23956) ## Summary The auto-review runtime sync path was assigning a raw `PermissionProfile` into `runtime_permission_profile_override`, whose field now expects `RuntimePermissionProfileOverride`. That broke the TUI Bazel build. This changes the assignment to store `RuntimePermissionProfileOverride::from_config(&self.config)`, matching the other runtime override paths and preserving the active profile and network metadata with the permission profile. --- codex-rs/tui/src/app/config_persistence.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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; From 0cec50814895fe257be9b325eca8243648455f8f Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Thu, 21 May 2026 17:32:14 -0700 Subject: [PATCH 2/4] feat: support local refs and defs in tool input schemas (#23357) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Why Some connector tool input schemas use local JSON Schema references and definition tables to avoid duplicating large nested shapes. Codex previously lowered these schemas into the supported subset in a way that could discard `$ref`-only schema objects and lose the corresponding definitions, which made non-strict tool registration less faithful than the original connector schema. This keeps the existing minimal-lowering policy: Codex still does not raw-pass through arbitrary JSON Schema, but it now preserves local reference structure that fits the Responses-compatible subset and prunes definition entries that cannot be reached by following `$ref`s from the root schema after sanitization, including refs found transitively inside other reachable definitions. The pruning matters because Responses parses definition tables even when entries are unused, so keeping dead definitions wastes prompt tokens. # What changed - Added `$ref`, `$defs`, and legacy `definitions` fields to the tool `JsonSchema` representation. - Updated `parse_tool_input_schema` lowering so `$ref`-only schema objects survive sanitization instead of becoming `{}`. - Sanitized definition tables recursively and dropped malformed definition tables so non-strict registration degrades gracefully. - Added reachability pruning for root definition tables by starting from refs outside definition tables, then following refs inside reachable definitions. - Added JSON Pointer decoding for local definition refs such as `#/$defs/Foo~1Bar`. # Verification ran local golden-schema probes against representative connector schemas to validate behavior on real generated schemas: | Golden schema | Before bytes | After bytes | `$defs` before -> after | `$ref` before -> after | Result | |---|---:|---:|---:|---:|---| | `google_calendar/create_space` | 7111 | 4526 | 7 -> 7 | 7 -> 7 | all definitions preserved because all are reachable | | `figma/apply_file_variable_changes` | 4609 | 999 | 8 -> 5 | 8 -> 5 | unused defs pruned after unsupported `oneOf` shapes lower away | | `snowflake/list_catalog_integrations` | 1380 | 404 | 3 -> 0 | 0 -> 0 | all defs pruned because none are referenced | | `dropbox/create_shared_link` | 8894 | 1836 | 14 -> 4 | 9 -> 4 | only defs reachable from the root schema after sanitization are retained, including transitively through other retained defs | Token increase across golden schema due to this change: Screenshot 2026-05-19 at 1 47 04 PM --- MODULE.bazel.lock | 1 + codex-rs/Cargo.lock | 8 + codex-rs/Cargo.toml | 1 + codex-rs/tools/Cargo.toml | 2 + codex-rs/tools/src/json_schema.rs | 221 +++++++++- codex-rs/tools/src/json_schema_tests.rs | 535 ++++++++++++++++++++++++ 6 files changed, 767 insertions(+), 1 deletion(-) 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/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..83733ffb7b 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,7 @@ 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); let schema: JsonSchema = serde_json::from_value(input_schema)?; if matches!( schema.schema_type, @@ -159,10 +170,55 @@ pub fn parse_tool_input_schema(input_schema: &JsonValue) -> Result, + 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); + } + } + } + } +} + /// 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 +256,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 +266,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 +300,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 +339,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..90ddcc4529 100644 --- a/codex-rs/tools/src/json_schema_tests.rs +++ b/codex-rs/tools/src/json_schema_tests.rs @@ -848,3 +848,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() + } + ); +} From 7e802b22f13e2714efd2fb2a6e396319958d6506 Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Thu, 21 May 2026 18:11:47 -0700 Subject: [PATCH 3/4] Expose conversation history to extension tools (#23963) ## Why Extension tools that need conversation context should be able to read it from the live tool invocation instead of reaching into thread persistence themselves. ## What changed - Add a `ConversationHistory` snapshot to extension `ToolCall`s and populate it from the current raw in-memory response history. - Expose all history items at this boundary so each extension can filter and bound the subset it needs before consuming or forwarding it. - Cover the adapter and registry dispatch paths and update existing extension tests that construct `ToolCall` literals. ## Test plan - `cargo test -p codex-tools` - `cargo test -p codex-extension-api` - `cargo test -p codex-goal-extension` - `cargo test -p codex-memories-extension` - `cargo test -p codex-core passes_turn_fields_to_extension_call` - `cargo test -p codex-core extension_tool_executors_are_model_visible_and_dispatchable` --- codex-rs/core/src/context_manager/history.rs | 5 ++++ .../src/tools/handlers/extension_tools.rs | 25 +++++++++++++++++-- codex-rs/core/src/tools/router_tests.rs | 14 +++++++++++ codex-rs/ext/extension-api/src/lib.rs | 1 + .../ext/goal/tests/goal_extension_backend.rs | 1 + codex-rs/ext/memories/src/tests.rs | 4 +++ codex-rs/tools/src/lib.rs | 1 + codex-rs/tools/src/tool_call.rs | 21 ++++++++++++++++ 8 files changed, 70 insertions(+), 2 deletions(-) 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 764e66f0ce..470b32beae 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -1,5 +1,6 @@ use std::sync::Arc; +use codex_tools::ConversationHistory; use codex_tools::ToolCall as ExtensionToolCall; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -53,7 +54,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 } } @@ -86,12 +87,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(), } } @@ -108,6 +112,8 @@ fn extension_tool_hook_input(arguments: &str) -> Value { 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; @@ -236,6 +242,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(), @@ -261,6 +278,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/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, } From 464ab40dfa1fd5058ea52512c29f38d2e4f6b204 Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Thu, 21 May 2026 18:26:17 -0700 Subject: [PATCH 4/4] feat: best-effort compact large tool schemas (#23904) ## Why The `dev/cc/ref-def` branch preserves richer JSON Schema detail for connector tools, including `$defs` and nested shapes. That improves fidelity, but it pushes the largest connector schemas well past the intended tool-schema budget. This PR adds a best-effort compaction pass for unusually large tool input schemas so the p99 and max tails stay small while ordinary schemas are left alone. ## What Changed - Added best-effort large-schema compaction in `codex-rs/tools/src/json_schema.rs` after schema sanitization and definition pruning. - Compaction runs as a waterfall only while the compact JSON budget proxy is exceeded: 1. Strip schema `description` metadata. 2. Drop root `$defs` / `definitions`. 3. Collapse deep nested complex schema objects to `{}`. - Kept top-level argument names and immediate schema shape where possible. ## Corpus Results Scope: 2,025 schemas under `golden_schemas`, all parsed successfully. Token count is `o200k_base` over compact JSON from `parse_tool_input_schema`. | Percentile | Before `origin/main` `4dbca61e20` | After branch `dev/cc/ref-def` `f9bf071758` | After this PR | |---|---:|---:|---:| | p0 | 9 | 9 | 9 | | p10 | 59 | 63 | 63 | | p25 | 81 | 86 | 86 | | p50 | 114 | 127 | 125 | | p75 | 174 | 205 | 202 | | p90 | 295 | 335 | 322 | | p95 | 391 | 526 | 422 | | p99 | 794 | 1,303 | 689 | | max | 2,836 | 3,337 | 887 | After this PR, `0 / 2,025` schemas are over 1k tokens. ### Compaction Savings These are cumulative waterfall stages over the same corpus. Later passes only run for schemas that are still over the compact JSON budget proxy. | Stage | Total tokens | Step savings | Schemas changed by step | |---|---:|---:|---:| | No compaction | 391,862 | - | - | | Strip schema `description` metadata | 350,961 | 40,901 | 66 | | Drop root `$defs` / `definitions` | 340,683 | 10,278 | 13 | | Collapse deep complex schemas to `{}` | 335,875 | 4,808 | 6 | --- codex-rs/tools/src/json_schema.rs | 166 ++++++++++ codex-rs/tools/src/json_schema_tests.rs | 387 ++++++++++++++++++++++++ 2 files changed, 553 insertions(+) diff --git a/codex-rs/tools/src/json_schema.rs b/codex-rs/tools/src/json_schema.rs index 83733ffb7b..02936d52d1 100644 --- a/codex-rs/tools/src/json_schema.rs +++ b/codex-rs/tools/src/json_schema.rs @@ -160,6 +160,7 @@ pub fn parse_tool_input_schema(input_schema: &JsonValue) -> Result 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, @@ -214,6 +256,130 @@ fn for_each_schema_child( } } +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. diff --git a/codex-rs/tools/src/json_schema_tests.rs b/codex-rs/tools/src/json_schema_tests.rs index 90ddcc4529..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: