From 91aa64569c19d54599ee5a23a020806e933baf11 Mon Sep 17 00:00:00 2001 From: asto Date: Thu, 5 Nov 2026 13:20:04 +0800 Subject: [PATCH 01/11] fix(client): ship Anthropic input_schema verbatim instead of flattening the root The Messages adapter unconditionally ran the Responses-style root flatten (strip oneOf/anyOf/allOf/enum/not at the root, merge branch properties) on every tool's input_schema, commented as stripping shapes "which Anthropic rejects". The 2026-11-05 document check finds no such rejection: the tool-use guide defines input_schema as "A JSON Schema object defining the expected parameters for the tool" (platform.claude.com/docs/en/agents-and-tools/tool-use/ implement-tool-use) with no root-composition restriction, the structured-outputs strict-restrictions list names none either, and DeepSeek's Anthropic-compatible line documents input_schema as fully supported. The rewrite had no document basis and silently altered declared tool semantics, so the unconditional call is removed: input_schema reaches the wire exactly as declared, the synthesized constraint-note plumbing goes with it, and the provider-neutral registry sanitizer upstream is unchanged. strict passthrough is untouched. One shared builder change covers all four Messages lanes (Anthropic, DeepseekAnthropic, MinimaxAnthropic, Modelstudio Anthropic). Signed-off-by: asto --- crates/tui/src/client/anthropic.rs | 96 +++++++++++++++++++++++++----- 1 file changed, 80 insertions(+), 16 deletions(-) diff --git a/crates/tui/src/client/anthropic.rs b/crates/tui/src/client/anthropic.rs index 68e8b9460f..dddb48d864 100644 --- a/crates/tui/src/client/anthropic.rs +++ b/crates/tui/src/client/anthropic.rs @@ -31,7 +31,6 @@ use crate::config::{ApiProvider, wire_model_for_provider_route}; use crate::llm_client::StreamEventBox; use crate::logging; use crate::models::{ContentBlock, MessageRequest, MessageResponse, StreamEvent, Usage}; -use crate::tools::schema_sanitize; use super::prepared::WireDialect; use super::role_placement::{RolePlacement, role_placement}; @@ -88,23 +87,27 @@ impl DeepSeekClient { tools .iter() .map(|tool| { - // Sanitize the tool's input_schema the same way the - // OpenAI Responses adapter does: strip top-level - // oneOf/anyOf/allOf (which Anthropic rejects), merge - // alternative properties into the root, and surface - // the dropped constraint as a description note so the - // model still knows which parameters are expected. - let mut schema = tool.input_schema.clone(); - let constraint_note = schema_sanitize::sanitize_for_responses(&mut schema); - let description = match constraint_note { - Some(note) if tool.description.trim().is_empty() => note, - Some(note) => format!("{}\n\n{}", tool.description.trim(), note), - None => tool.description.clone(), - }; + // input_schema ships exactly as the tool declared it. + // The tool-use documents define the field simply as + // "A JSON Schema object defining the expected + // parameters for the tool" + // (platform.claude.com/docs/en/agents-and-tools/ + // tool-use/implement-tool-use, fetched 2026-11-05) — + // no root-composition restriction — and the + // structured-outputs strict-restrictions list names + // none either; DeepSeek's Anthropic-compatible line + // documents input_schema as fully supported. The + // earlier unconditional Responses-style root flatten + // (strip oneOf/anyOf/allOf/enum/not at the root, + // merge branch properties into the object) carried no + // document basis and rewrote declared tool semantics, + // so it is gone. The provider-neutral registry + // sanitizer that runs upstream of this adapter is + // unchanged. let mut value = json!({ "name": tool.name, - "description": description, - "input_schema": schema, + "description": tool.description, + "input_schema": tool.input_schema, }); if let Some(strict) = tool.strict { value["strict"] = json!(strict); @@ -1232,6 +1235,67 @@ mod tests { DeepSeekClient::new(&config).expect("Model Studio Messages client constructs") } + #[test] + fn anthropic_wire_ships_input_schema_verbatim_without_root_flattening() { + // The tool-use documents define input_schema simply as "A JSON Schema + // object defining the expected parameters for the tool" + // (platform.claude.com/docs/en/agents-and-tools/tool-use/ + // implement-tool-use, fetched 2026-11-05) with no root-composition + // restriction; the structured-outputs strict-restrictions list names + // none either, and DeepSeek's Anthropic-compatible line documents + // input_schema as fully supported. The schema therefore ships exactly + // as the tool declared it — the earlier unconditional Responses-style + // root flatten rewrote semantics the documents never forbade. The + // provider-neutral registry sanitizer upstream of this adapter is + // unchanged. One builder serves every Messages lane (Anthropic, + // DeepSeek, MiniMax, Model Studio), so this pins all of them. + let client = test_client(); + let mut request = request_with("claude-sonnet-4-6", None, None, None); + request.tools = Some(vec![Tool { + tool_type: None, + name: "apply_patch".to_string(), + description: "Patch files".to_string(), + input_schema: json!({ + "oneOf": [ + { + "type": "object", + "properties": {"patch": {"type": "string"}}, + "required": ["patch"] + }, + { + "type": "object", + "properties": { + "changes": {"type": "array", "items": {"type": "string"}} + }, + "required": ["changes"] + } + ] + }), + allowed_callers: None, + defer_loading: None, + input_examples: None, + strict: None, + cache_control: None, + }]); + + let body = client.build_anthropic_body(&request, true); + + let schema = &body["tools"][0]["input_schema"]; + assert!( + schema.get("oneOf").is_some(), + "a root union the documents do not forbid must reach the wire verbatim: {schema}" + ); + assert_eq!( + schema["oneOf"][0]["required"], + json!(["patch"]), + "branches keep their own required groups: {schema}" + ); + assert_eq!( + body["tools"][0]["description"], "Patch files", + "no constraint note is synthesized for a schema that ships as declared: {body}" + ); + } + #[test] fn sampling_params_dropped_for_every_model_from_opus_4_7_on() { // The sampling restriction blankets every model released after From 48daada87783f846d55482ffc3f1bd685abc48dc Mon Sep 17 00:00:00 2001 From: asto Date: Fri, 6 Nov 2026 18:35:38 +0800 Subject: [PATCH 02/11] fix(client): narrow the schema flatten to the recorded Anthropic rejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ship input_schema verbatim on every Messages lane except the one whose validator is recorded rejecting root compositions. The genuine Anthropic endpoint has returned HTTP 400 "input_schema does not support oneOf, allOf, or anyOf at the top level" since at least 2025-07 (f8e757322 quotes a live failure; CHANGELOG 0.9.1; anthropics/claude-code#4886 and #30212 report the identical server-side error, neither marked fixed), so ApiProvider::Anthropic now narrows exactly those three root keywords via sanitize_for_anthropic_messages: branch properties merge into the root, the dropped exactly-one / at-least-one constraint becomes a description note, and everything the record does not name — root enum/not, dependentRequired/dependentSchemas, the declared root type, non-object roots — ships verbatim. DeepSeek-Anthropic, MiniMax- Anthropic, and Model Studio Anthropic lanes keep the verbatim shipping: DeepSeek documents input_schema as fully supported and the others document no restrictions (fetched 2026-11-05). For apply_patch and finance, whose root unions triggered the old flatten, the Anthropic-lane wire bytes are identical to the previous release line; schemas without root compositions change only where the old flatten deleted unrecorded keywords. Pins: the three non-Anthropic lanes ship a root oneOf verbatim with no note; the Anthropic lane strips the union, merges branch properties, notes an empty description, and preserves enum/not/dependent keywords and a non-object enum root byte-identically. Module-level pins cover the sanitizer directly. Signed-off-by: asto --- crates/tui/src/client/anthropic.rs | 232 +++++++++++++++++++----- crates/tui/src/tools/schema_sanitize.rs | 123 +++++++++++++ 2 files changed, 309 insertions(+), 46 deletions(-) diff --git a/crates/tui/src/client/anthropic.rs b/crates/tui/src/client/anthropic.rs index dddb48d864..1e59a8c560 100644 --- a/crates/tui/src/client/anthropic.rs +++ b/crates/tui/src/client/anthropic.rs @@ -31,6 +31,7 @@ use crate::config::{ApiProvider, wire_model_for_provider_route}; use crate::llm_client::StreamEventBox; use crate::logging; use crate::models::{ContentBlock, MessageRequest, MessageResponse, StreamEvent, Usage}; +use crate::tools::schema_sanitize; use super::prepared::WireDialect; use super::role_placement::{RolePlacement, role_placement}; @@ -83,31 +84,48 @@ impl DeepSeekClient { if let Some(tools) = request.tools.as_ref() && !tools.is_empty() { + // Every Messages lane ships input_schema exactly as the tool + // declared it, with one recorded exception: the genuine Anthropic + // endpoint has returned HTTP 400 "input_schema does not support + // oneOf, allOf, or anyOf at the top level" since at least + // 2025-07 (f8e757322 quotes a live failure; CHANGELOG 0.9.1; + // anthropics/claude-code#4886/#30212), so that lane narrows just + // those three root keywords — merging branch properties and + // noting the dropped constraint — while root enum/not, dependent + // keywords, and the declared root type stay verbatim. The other + // lanes ship verbatim: DeepSeek's Anthropic-compatible line + // marks input_schema "Fully Supported" + // (api-docs.deepseek.com/guides/anthropic_api, fetched + // 2026-11-05), the Anthropic tool-use guide and its + // structured-outputs restrictions list name no root-composition + // ban, and MiniMax / Model Studio document none. MCP tool + // schemas carry no registry sanitization upstream of this + // adapter, so on the Anthropic lane this pass is their only + // normalization; the native-registry sanitizer is unchanged. + let narrow_root_compositions = matches!(self.api_provider, ApiProvider::Anthropic); body["tools"] = json!( tools .iter() .map(|tool| { - // input_schema ships exactly as the tool declared it. - // The tool-use documents define the field simply as - // "A JSON Schema object defining the expected - // parameters for the tool" - // (platform.claude.com/docs/en/agents-and-tools/ - // tool-use/implement-tool-use, fetched 2026-11-05) — - // no root-composition restriction — and the - // structured-outputs strict-restrictions list names - // none either; DeepSeek's Anthropic-compatible line - // documents input_schema as fully supported. The - // earlier unconditional Responses-style root flatten - // (strip oneOf/anyOf/allOf/enum/not at the root, - // merge branch properties into the object) carried no - // document basis and rewrote declared tool semantics, - // so it is gone. The provider-neutral registry - // sanitizer that runs upstream of this adapter is - // unchanged. + let (description, schema) = if narrow_root_compositions { + let mut schema = tool.input_schema.clone(); + let constraint_note = + schema_sanitize::sanitize_for_anthropic_messages(&mut schema); + let description = match constraint_note { + Some(note) if tool.description.trim().is_empty() => note, + Some(note) => { + format!("{}\n\n{}", tool.description.trim(), note) + } + None => tool.description.clone(), + }; + (description, schema) + } else { + (tool.description.clone(), tool.input_schema.clone()) + }; let mut value = json!({ "name": tool.name, - "description": tool.description, - "input_schema": tool.input_schema, + "description": description, + "input_schema": schema, }); if let Some(strict) = tool.strict { value["strict"] = json!(strict); @@ -1235,26 +1253,11 @@ mod tests { DeepSeekClient::new(&config).expect("Model Studio Messages client constructs") } - #[test] - fn anthropic_wire_ships_input_schema_verbatim_without_root_flattening() { - // The tool-use documents define input_schema simply as "A JSON Schema - // object defining the expected parameters for the tool" - // (platform.claude.com/docs/en/agents-and-tools/tool-use/ - // implement-tool-use, fetched 2026-11-05) with no root-composition - // restriction; the structured-outputs strict-restrictions list names - // none either, and DeepSeek's Anthropic-compatible line documents - // input_schema as fully supported. The schema therefore ships exactly - // as the tool declared it — the earlier unconditional Responses-style - // root flatten rewrote semantics the documents never forbade. The - // provider-neutral registry sanitizer upstream of this adapter is - // unchanged. One builder serves every Messages lane (Anthropic, - // DeepSeek, MiniMax, Model Studio), so this pins all of them. - let client = test_client(); - let mut request = request_with("claude-sonnet-4-6", None, None, None); - request.tools = Some(vec![Tool { + fn root_union_tool(name: &str, description: &str) -> Tool { + Tool { tool_type: None, - name: "apply_patch".to_string(), - description: "Patch files".to_string(), + name: name.to_string(), + description: description.to_string(), input_schema: json!({ "oneOf": [ { @@ -1276,23 +1279,160 @@ mod tests { input_examples: None, strict: None, cache_control: None, - }]); + } + } + + #[test] + fn non_anthropic_messages_lanes_ship_input_schema_verbatim_without_root_flattening() { + // The tool-use documents define input_schema simply as "A JSON Schema + // object defining the expected parameters for the tool" + // (platform.claude.com/docs/en/agents-and-tools/tool-use/ + // implement-tool-use, fetched 2026-11-05) with no root-composition + // restriction; the structured-outputs strict-restrictions list names + // none either, and DeepSeek's Anthropic-compatible line documents + // input_schema as fully supported. No record rejects root-level + // enum/not anywhere. The recorded Anthropic 400 (f8e757322; + // CHANGELOG 0.9.1; anthropics/claude-code#4886/#30212) is specific + // to the genuine Anthropic validator, so every other Messages lane + // ships the schema exactly as declared — one shared builder serves + // DeepSeek, MiniMax, and Model Studio, and this pins all three. + for client in [ + deepseek_test_client(crate::config::DEFAULT_DEEPSEEK_ANTHROPIC_BASE_URL), + minimax_test_client(), + modelstudio_test_client(crate::config::MODELSTUDIO_TOKEN_PLAN_ANTHROPIC_BASE_URL), + ] { + let mut request = request_with("claude-sonnet-4-6", None, None, None); + request.tools = Some(vec![root_union_tool("apply_patch", "Patch files")]); + + let body = client.build_anthropic_body(&request, true); + + let schema = &body["tools"][0]["input_schema"]; + assert!( + schema.get("oneOf").is_some(), + "a root union no record forbids must reach the wire verbatim: {schema}" + ); + assert_eq!( + schema["oneOf"][0]["required"], + json!(["patch"]), + "branches keep their own required groups: {schema}" + ); + assert_eq!( + body["tools"][0]["description"], "Patch files", + "no constraint note is synthesized for a schema that ships as declared: {body}" + ); + } + } + + #[test] + fn anthropic_lane_narrows_the_recorded_root_composition_keywords() { + // The genuine Anthropic endpoint is recorded rejecting root-level + // oneOf/anyOf/allOf with HTTP 400 "input_schema does not support + // oneOf, allOf, or anyOf at the top level" (f8e757322, 2026-07-11; + // CHANGELOG 0.9.1; anthropics/claude-code#4886/#30212), so this lane + // strips exactly those keywords, merges branch properties into the + // root, and surfaces the dropped constraint as a description note. + let client = test_client(); + let mut request = request_with("claude-sonnet-4-6", None, None, None); + request.tools = Some(vec![ + root_union_tool("apply_patch", "Patch files"), + root_union_tool("apply_changes", ""), + ]); let body = client.build_anthropic_body(&request, true); let schema = &body["tools"][0]["input_schema"]; assert!( - schema.get("oneOf").is_some(), - "a root union the documents do not forbid must reach the wire verbatim: {schema}" + schema.get("oneOf").is_none(), + "the recorded rejection bars a root oneOf on this lane: {schema}" + ); + assert!( + schema["properties"].get("patch").is_some() + && schema["properties"].get("changes").is_some(), + "branch properties are merged into the root so the model keeps its vocabulary: {schema}" + ); + assert!( + body["tools"][0]["description"] + .as_str() + .unwrap() + .starts_with("Patch files\n\nExactly one of these parameter groups"), + "the dropped exactly-one constraint is surfaced as a note: {}", + body["tools"][0]["description"] + ); + assert_eq!( + body["tools"][1]["description"], + "Exactly one of these parameter groups must be provided: `changes` | `patch`.", + "an empty description is replaced by the note alone: {body}" + ); + } + + #[test] + fn anthropic_lane_keeps_unrecorded_keywords_verbatim() { + // The recorded 400 names only top-level oneOf/allOf/anyOf. The old + // Responses-style flatten also stripped root enum/not, dependent + // keywords, and non-object roots with no record behind them; this + // pins that the narrowed pass preserves all of them. + let client = test_client(); + let mut request = request_with("claude-sonnet-4-6", None, None, None); + request.tools = Some(vec![Tool { + tool_type: None, + name: "pick_mode".to_string(), + description: "Pick a mode".to_string(), + input_schema: json!({ + "type": "object", + "properties": {"mode": {"type": "string"}}, + "not": {"type": "integer"}, + "dependentRequired": {"mode": ["confirm"]}, + "dependentSchemas": {"mode": {"required": ["confirm"]}} + }), + allowed_callers: None, + defer_loading: None, + input_examples: None, + strict: None, + cache_control: None, + }]); + let enum_root = Tool { + tool_type: None, + name: "style".to_string(), + description: "Style".to_string(), + input_schema: json!({ + "type": "string", + "enum": ["compact", "full"] + }), + allowed_callers: None, + defer_loading: None, + input_examples: None, + strict: None, + cache_control: None, + }; + request.tools.as_mut().unwrap().push(enum_root); + + let body = client.build_anthropic_body(&request, true); + + let schema = &body["tools"][0]["input_schema"]; + assert_eq!(schema["type"], "object", "root type stays as declared"); + assert_eq!( + schema["dependentRequired"], + json!({"mode": ["confirm"]}), + "dependent keywords are not in any record and stay verbatim: {schema}" + ); + assert!(schema.get("dependentSchemas").is_some()); + assert_eq!( + schema["not"], + json!({"type": "integer"}), + "root not is not in any record and stays verbatim: {schema}" ); assert_eq!( - schema["oneOf"][0]["required"], - json!(["patch"]), - "branches keep their own required groups: {schema}" + body["tools"][0]["description"], "Pick a mode", + "no note is synthesized without a dropped root composition: {body}" ); + let enum_schema = &body["tools"][1]["input_schema"]; assert_eq!( - body["tools"][0]["description"], "Patch files", - "no constraint note is synthesized for a schema that ships as declared: {body}" + enum_schema, + &json!({ + "type": "string", + "enum": ["compact", "full"] + }), + "a non-object root with enum ships byte-identically: {enum_schema}" ); } diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index bdc0bda33b..4e6bdd70a5 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -123,6 +123,42 @@ pub fn sanitize_for_responses(schema: &mut Value) -> Option { combine_constraint_notes(constraint_note, dependent_note) } +/// Narrow the root-composition keywords the Anthropic Messages validator is +/// recorded rejecting, and leave every other keyword untouched. +/// +/// The genuine Anthropic endpoint has returned HTTP 400 "input_schema does +/// not support oneOf, allOf, or anyOf at the top level" for tool schemas +/// carrying a root-level composition since at least 2025-07: commit +/// `f8e757322` quotes a live failure, CHANGELOG 0.9.1 records the same +/// rationale, and anthropics/claude-code#4886 / #30212 report the identical +/// server-side error. Unlike [`sanitize_for_responses`], this pass strips +/// only those three root keywords: root `enum`/`not`, +/// `dependentRequired`/`dependentSchemas`, the declared root type, and +/// non-object roots all ship verbatim because no record rejects them. +/// Root composition branch properties are merged into the object and the +/// dropped exactly-one / at-least-one constraint is surfaced as a +/// description note so the model still knows which parameter groups apply. +/// +/// Returns the same style of description note as [`sanitize_for_responses`]. +pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { + let constraint_note = schema + .as_object() + .and_then(root_composition_constraint_note); + + sanitize(schema); + + let Some(obj) = schema.as_object_mut() else { + return constraint_note; + }; + + merge_root_composition_properties(obj); + obj.remove("oneOf"); + obj.remove("anyOf"); + obj.remove("allOf"); + prune_dangling_required(schema); + constraint_note +} + fn strict_schema_supported(schema: &Value) -> bool { let mut normalized = schema.clone(); sanitize(&mut normalized); @@ -1033,6 +1069,93 @@ mod tests { assert_eq!(note, None); } + #[test] + fn anthropic_messages_sanitize_strips_only_recorded_root_keywords() { + // The recorded 400 names only top-level oneOf/allOf/anyOf + // (f8e757322; CHANGELOG 0.9.1; anthropics/claude-code#4886/#30212). + // Root enum/not, dependent keywords, and the declared root type are + // not in any record, so the narrowed pass preserves all of them. + let mut schema = json!({ + "type": "object", + "properties": { + "mode": {"type": "string"} + }, + "not": {"type": "integer"}, + "dependentRequired": {"mode": ["confirm"]}, + "dependentSchemas": {"mode": {"required": ["confirm"]}}, + "oneOf": [ + {"required": ["patch"]}, + {"required": ["changes"]} + ] + }); + + let note = sanitize_for_anthropic_messages(&mut schema); + + assert_eq!(schema["type"], "object"); + assert!(schema.get("oneOf").is_none()); + assert!(schema.get("anyOf").is_none()); + assert!(schema.get("allOf").is_none()); + assert_eq!(schema["not"], json!({"type": "integer"})); + assert_eq!(schema["dependentRequired"], json!({"mode": ["confirm"]})); + assert!(schema.get("dependentSchemas").is_some()); + assert_eq!( + note.as_deref(), + Some("Exactly one of these parameter groups must be provided: `changes` | `patch`.") + ); + } + + #[test] + fn anthropic_messages_sanitize_keeps_root_enum_and_non_object_roots() { + let mut enum_root = json!({ + "type": "string", + "enum": ["compact", "full"] + }); + + let note = sanitize_for_anthropic_messages(&mut enum_root); + + assert_eq!( + enum_root, + json!({ + "type": "string", + "enum": ["compact", "full"] + }) + ); + assert_eq!(note, None); + } + + #[test] + fn anthropic_messages_sanitize_merges_branch_properties_and_notes_groups() { + let mut schema = json!({ + "anyOf": [ + { + "type": "object", + "properties": { + "patch": {"type": "string"} + }, + "required": ["patch"] + }, + { + "type": "object", + "properties": { + "changes": {"type": "array", "items": {"type": "string"}} + }, + "required": ["changes"] + } + ] + }); + + let note = sanitize_for_anthropic_messages(&mut schema); + + assert!(schema.get("anyOf").is_none()); + assert!(schema["properties"].get("patch").is_some()); + assert!(schema["properties"].get("changes").is_some()); + assert!(schema.get("type").is_none()); + assert_eq!( + note.as_deref(), + Some("At least one of these parameter groups must be provided: `changes` | `patch`.") + ); + } + #[test] fn xai_sanitize_flattens_apply_patch_root_one_of() { // The exact shape that produced the live 400: From 698c41e3fdce6f16b3f2c3becae536eed4c31491 Mon Sep 17 00:00:00 2001 From: asto Date: Fri, 6 Nov 2026 22:26:06 +0800 Subject: [PATCH 03/11] fix(client): pin the sanitize anchor and gate narrowing by endpoint Fresh-review follow-ups on the narrowed Anthropic input_schema pass: - Pin the shared sanitize() anchor inside sanitize_for_anthropic_messages: it is load-bearing (the previous release line collapsed root nullable unions through it) and deleting it left every new test green while a root anyOf:[X, null] would ship the recorded-rejection union shape. - Key the narrowing gate on the first-party endpoint, not provider identity alone: a custom wire=anthropic provider pointed at api.anthropic.com now gets the same recorded-rejection protection the thinking gates apply via is_native_anthropic_base_url, and a gateway base URL still ships verbatim; both pinned at the adapter level. - Correct the CHANGELOG receipt (0.9.0, not 0.9.1) and carve the shared registry normalization out of the ships-verbatim doc claims. Signed-off-by: asto --- crates/tui/src/client/anthropic.rs | 84 ++++++++++++++++++++++--- crates/tui/src/tools/schema_sanitize.rs | 59 ++++++++++++++++- 2 files changed, 133 insertions(+), 10 deletions(-) diff --git a/crates/tui/src/client/anthropic.rs b/crates/tui/src/client/anthropic.rs index 1e59a8c560..2d99435856 100644 --- a/crates/tui/src/client/anthropic.rs +++ b/crates/tui/src/client/anthropic.rs @@ -88,21 +88,26 @@ impl DeepSeekClient { // declared it, with one recorded exception: the genuine Anthropic // endpoint has returned HTTP 400 "input_schema does not support // oneOf, allOf, or anyOf at the top level" since at least - // 2025-07 (f8e757322 quotes a live failure; CHANGELOG 0.9.1; - // anthropics/claude-code#4886/#30212), so that lane narrows just - // those three root keywords — merging branch properties and - // noting the dropped constraint — while root enum/not, dependent - // keywords, and the declared root type stay verbatim. The other - // lanes ship verbatim: DeepSeek's Anthropic-compatible line - // marks input_schema "Fully Supported" + // 2025-07 (f8e757322 quotes a live failure; CHANGELOG 0.9.0; + // anthropics/claude-code#4886/#30212), so the first-party lane — + // ApiProvider::Anthropic or a base URL on api.anthropic.com, the + // same endpoint predicate the thinking gates below use — narrows + // just those three root keywords, merging branch properties and + // noting the dropped constraint; root enum/not, dependent + // keywords, and the declared root type stay verbatim, and the + // shared registry normalization (nullable-union collapse, + // bare-object properties injection, dangling-required pruning) + // still applies there. The other lanes ship verbatim: DeepSeek's + // Anthropic-compatible line marks input_schema "Fully Supported" // (api-docs.deepseek.com/guides/anthropic_api, fetched // 2026-11-05), the Anthropic tool-use guide and its // structured-outputs restrictions list name no root-composition // ban, and MiniMax / Model Studio document none. MCP tool // schemas carry no registry sanitization upstream of this - // adapter, so on the Anthropic lane this pass is their only + // adapter, so on the narrowing lane this pass is their only // normalization; the native-registry sanitizer is unchanged. - let narrow_root_compositions = matches!(self.api_provider, ApiProvider::Anthropic); + let narrow_root_compositions = matches!(self.api_provider, ApiProvider::Anthropic) + || is_native_anthropic_base_url(&self.base_url); body["tools"] = json!( tools .iter() @@ -1436,6 +1441,67 @@ mod tests { ); } + fn custom_wire_test_client(name: &str, base_url: &str) -> DeepSeekClient { + let _ = rustls::crypto::ring::default_provider().install_default(); + let mut providers = crate::config::ProvidersConfig::default(); + providers.custom.insert( + name.to_string(), + crate::config::ProviderConfig { + api_key: Some("test-key".to_string()), + base_url: Some(base_url.to_string()), + wire: Some("anthropic".to_string()), + ..Default::default() + }, + ); + let config = crate::config::Config { + provider: Some(name.to_string()), + providers: Some(providers), + ..Default::default() + }; + DeepSeekClient::new(&config).expect("custom anthropic-wire client constructs") + } + + #[test] + fn custom_anthropic_wire_clients_narrow_by_first_party_endpoint() { + // The recorded 400 comes from the api.anthropic.com validator, not + // from a provider identity: a custom `wire = "anthropic"` provider + // pointed at a first-party base URL gets the same narrowing the + // thinking gates apply (is_native_anthropic_base_url), while one + // pointed at a third-party gateway ships the schema verbatim. + let native = custom_wire_test_client("byok-native", "https://api.anthropic.com"); + let mut request = request_with("claude-sonnet-4-6", None, None, None); + request.tools = Some(vec![root_union_tool("apply_patch", "Patch files")]); + + let body = native.build_anthropic_body(&request, true); + + let schema = &body["tools"][0]["input_schema"]; + assert!( + schema.get("oneOf").is_none(), + "a first-party base URL must narrow the recorded rejection: {schema}" + ); + assert_eq!( + body["tools"][0]["description"], + "Patch files\n\nExactly one of these parameter groups must be provided: `changes` | `patch`.", + "the dropped constraint is surfaced on the first-party lane: {body}" + ); + + let gateway = custom_wire_test_client("byok-gateway", "https://relay.example.com/v1"); + let mut request = request_with("claude-sonnet-4-6", None, None, None); + request.tools = Some(vec![root_union_tool("apply_patch", "Patch files")]); + + let body = gateway.build_anthropic_body(&request, true); + + let schema = &body["tools"][0]["input_schema"]; + assert!( + schema.get("oneOf").is_some(), + "a third-party gateway with no record must ship the union verbatim: {schema}" + ); + assert_eq!( + body["tools"][0]["description"], "Patch files", + "no note is synthesized on a verbatim lane: {body}" + ); + } + #[test] fn sampling_params_dropped_for_every_model_from_opus_4_7_on() { // The sampling restriction blankets every model released after diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index 4e6bdd70a5..d701037cee 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -129,7 +129,7 @@ pub fn sanitize_for_responses(schema: &mut Value) -> Option { /// The genuine Anthropic endpoint has returned HTTP 400 "input_schema does /// not support oneOf, allOf, or anyOf at the top level" for tool schemas /// carrying a root-level composition since at least 2025-07: commit -/// `f8e757322` quotes a live failure, CHANGELOG 0.9.1 records the same +/// `f8e757322` quotes a live failure, CHANGELOG 0.9.0 records the same /// rationale, and anthropics/claude-code#4886 / #30212 report the identical /// server-side error. Unlike [`sanitize_for_responses`], this pass strips /// only those three root keywords: root `enum`/`not`, @@ -139,6 +139,11 @@ pub fn sanitize_for_responses(schema: &mut Value) -> Option { /// dropped exactly-one / at-least-one constraint is surfaced as a /// description note so the model still knows which parameter groups apply. /// +/// The shared [`sanitize`] normalization still applies first, so a root +/// nullable union can collapse (rewriting the root `type`), a bare object +/// gains `properties`, and dangling `required` entries are pruned — only +/// the composition-keyword scope is narrower than [`sanitize_for_responses`]. +/// /// Returns the same style of description note as [`sanitize_for_responses`]. pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { let constraint_note = schema @@ -1156,6 +1161,58 @@ mod tests { ); } + #[test] + fn anthropic_messages_sanitize_keeps_the_shared_registry_normalization() { + // The narrowed pass still runs the shared registry normalization + // (sanitize): the previous release line collapsed root nullable + // unions through it, and without that anchor a root + // `anyOf:[X, null]` would ship the recorded-rejection union shape + // verbatim. Bare-object properties injection and dangling-required + // pruning ride the same call. + let mut nullable = json!({ + "anyOf": [ + { + "type": "object", + "properties": {"q": {"type": "string"}} + }, + {"type": "null"} + ] + }); + let note = sanitize_for_anthropic_messages(&mut nullable); + assert_eq!( + nullable, + json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "nullable": true + }), + "a root nullable union must collapse instead of shipping the recorded shape: {nullable}" + ); + assert_eq!(note, None); + + let mut bare = json!({"type": "object"}); + let note = sanitize_for_anthropic_messages(&mut bare); + assert_eq!( + bare, + json!({"type": "object", "properties": {}}), + "a bare object root keeps its properties injection: {bare}" + ); + assert_eq!(note, None); + + let mut dangling = json!({ + "type": "object", + "properties": {"a": {"type": "string"}}, + "required": ["a", "ghost"] + }); + let note = sanitize_for_anthropic_messages(&mut dangling); + assert_eq!( + dangling["required"], + json!(["a"]), + "required naming absent properties is pruned on this lane too: {dangling}" + ); + assert_eq!(note, None); + } + #[test] fn xai_sanitize_flattens_apply_patch_root_one_of() { // The exact shape that produced the live 400: From 78da100653b30203dad36755ba343f027525162e Mon Sep 17 00:00:00 2001 From: asto Date: Sat, 7 Nov 2026 02:01:44 +0800 Subject: [PATCH 04/11] fix(client): replace non-schema roots and pin the post-merge prune Round-3 fresh-review follow-ups on the narrowed Anthropic input_schema pass, each found by a dedicated review lane: - Replace a root that is not a JSON object at all (malformed MCP or server output) with an empty object, mirroring sanitize_for_responses: the API defines input_schema as a JSON Schema object, and shipping the raw value verbatim re-exposed the whole-request HTTP 400 failure class this pass exists to prevent. Declared non-object schemas (a string-enum root) still ship verbatim. - Restore type:"object" when stripping a root composition leaves a populated object schema whose root declared no type: the previous release line always forced the type, and the merge otherwise ships a typeless schema that invites non-object arguments. - Pin the explicit post-merge prune (a single-branch oneOf spreads its required groups after sanitize's own prune ran), add the missing root-allOf strip/merge/note fixture, pin the no-injection negative, and make the dependent-keyword fixtures well-formed with exact assertions instead of is_some() checks. - Correct the remaining stale CHANGELOG 0.9.1 citations (the rationale is recorded under 0.9.0) and document the replacement and type restore in the module doc, the pass doc comment, and the adapter call-site comment. Signed-off-by: asto --- crates/tui/src/client/anthropic.rs | 39 ++++-- crates/tui/src/tools/schema_sanitize.rs | 163 ++++++++++++++++++++++-- 2 files changed, 183 insertions(+), 19 deletions(-) diff --git a/crates/tui/src/client/anthropic.rs b/crates/tui/src/client/anthropic.rs index 2d99435856..4d938ece5a 100644 --- a/crates/tui/src/client/anthropic.rs +++ b/crates/tui/src/client/anthropic.rs @@ -93,11 +93,18 @@ impl DeepSeekClient { // ApiProvider::Anthropic or a base URL on api.anthropic.com, the // same endpoint predicate the thinking gates below use — narrows // just those three root keywords, merging branch properties and - // noting the dropped constraint; root enum/not, dependent - // keywords, and the declared root type stay verbatim, and the - // shared registry normalization (nullable-union collapse, - // bare-object properties injection, dangling-required pruning) - // still applies there. The other lanes ship verbatim: DeepSeek's + // noting the dropped constraint; the object type is restored when + // the merge leaves a populated schema whose root declared none, a + // root that is not a JSON object at all is replaced by an empty + // object (the API defines input_schema as a JSON Schema object), + // and root enum/not, dependent keywords, and the declared root + // type stay verbatim, with the shared registry normalization + // (nullable-union collapse, bare-object properties injection, + // dangling-required pruning) still applied there. Provider + // identity stays in the gate beside the endpoint because + // ANTHROPIC_BASE_URL overrides commonly front the genuine + // validator through a proxy. The other lanes ship verbatim: + // DeepSeek's // Anthropic-compatible line marks input_schema "Fully Supported" // (api-docs.deepseek.com/guides/anthropic_api, fetched // 2026-11-05), the Anthropic tool-use guide and its @@ -1297,7 +1304,7 @@ mod tests { // none either, and DeepSeek's Anthropic-compatible line documents // input_schema as fully supported. No record rejects root-level // enum/not anywhere. The recorded Anthropic 400 (f8e757322; - // CHANGELOG 0.9.1; anthropics/claude-code#4886/#30212) is specific + // CHANGELOG 0.9.0; anthropics/claude-code#4886/#30212) is specific // to the genuine Anthropic validator, so every other Messages lane // ships the schema exactly as declared — one shared builder serves // DeepSeek, MiniMax, and Model Studio, and this pins all three. @@ -1333,7 +1340,7 @@ mod tests { // The genuine Anthropic endpoint is recorded rejecting root-level // oneOf/anyOf/allOf with HTTP 400 "input_schema does not support // oneOf, allOf, or anyOf at the top level" (f8e757322, 2026-07-11; - // CHANGELOG 0.9.1; anthropics/claude-code#4886/#30212), so this lane + // CHANGELOG 0.9.0; anthropics/claude-code#4886/#30212), so this lane // strips exactly those keywords, merges branch properties into the // root, and surfaces the dropped constraint as a description note. let client = test_client(); @@ -1387,7 +1394,12 @@ mod tests { "properties": {"mode": {"type": "string"}}, "not": {"type": "integer"}, "dependentRequired": {"mode": ["confirm"]}, - "dependentSchemas": {"mode": {"required": ["confirm"]}} + "dependentSchemas": { + "mode": { + "properties": {"confirm": {"type": "boolean"}}, + "required": ["confirm"] + } + } }), allowed_callers: None, defer_loading: None, @@ -1420,7 +1432,16 @@ mod tests { json!({"mode": ["confirm"]}), "dependent keywords are not in any record and stay verbatim: {schema}" ); - assert!(schema.get("dependentSchemas").is_some()); + assert_eq!( + schema["dependentSchemas"], + json!({ + "mode": { + "properties": {"confirm": {"type": "boolean"}}, + "required": ["confirm"] + } + }), + "a well-formed dependent schema ships byte-identically: {schema}" + ); assert_eq!( schema["not"], json!({"type": "integer"}), diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index d701037cee..45b0e65359 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -9,7 +9,9 @@ //! The default sanitizer runs in-place on every schema returned by //! `ToolRegistry::tools_for_api()` before the registry hands them off. //! Provider-specific helpers below add stricter DeepSeek and OpenAI Responses -//! compatibility passes where their request shapes need it. +//! compatibility passes where their request shapes need it, and a narrower +//! Anthropic Messages pass that strips only the root composition keywords +//! that validator is recorded rejecting. use std::collections::HashSet; @@ -133,16 +135,23 @@ pub fn sanitize_for_responses(schema: &mut Value) -> Option { /// rationale, and anthropics/claude-code#4886 / #30212 report the identical /// server-side error. Unlike [`sanitize_for_responses`], this pass strips /// only those three root keywords: root `enum`/`not`, -/// `dependentRequired`/`dependentSchemas`, the declared root type, and -/// non-object roots all ship verbatim because no record rejects them. -/// Root composition branch properties are merged into the object and the -/// dropped exactly-one / at-least-one constraint is surfaced as a -/// description note so the model still knows which parameter groups apply. +/// `dependentRequired`/`dependentSchemas`, and the declared root type ship +/// verbatim because no record rejects them. Root composition branch +/// properties are merged into the object and the dropped exactly-one / +/// at-least-one constraint is surfaced as a description note so the model +/// still knows which parameter groups apply; where that merge leaves a +/// populated object schema whose root declared no `type`, the object type +/// is restored — the previous release line always forced it, and the +/// model-facing contract otherwise loses its object shape. /// /// The shared [`sanitize`] normalization still applies first, so a root /// nullable union can collapse (rewriting the root `type`), a bare object /// gains `properties`, and dangling `required` entries are pruned — only /// the composition-keyword scope is narrower than [`sanitize_for_responses`]. +/// A root that is not a JSON object at all (malformed server or MCP output) +/// is replaced by an empty object exactly as in [`sanitize_for_responses`]: +/// the API defines `input_schema` as a JSON Schema object, and a declared +/// non-object schema such as a string-enum root still ships verbatim. /// /// Returns the same style of description note as [`sanitize_for_responses`]. pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { @@ -152,14 +161,28 @@ pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { sanitize(schema); + if !schema.is_object() { + *schema = Value::Object(Map::new()); + } + let Some(obj) = schema.as_object_mut() else { return constraint_note; }; + let strips_composition = ["oneOf", "anyOf", "allOf"] + .iter() + .any(|key| obj.contains_key(*key)); merge_root_composition_properties(obj); obj.remove("oneOf"); obj.remove("anyOf"); obj.remove("allOf"); + let merged_properties = obj + .get("properties") + .and_then(Value::as_object) + .is_some_and(|properties| !properties.is_empty()); + if strips_composition && !obj.contains_key("type") && merged_properties { + obj.insert("type".into(), Value::String("object".to_string())); + } prune_dangling_required(schema); constraint_note } @@ -1077,7 +1100,7 @@ mod tests { #[test] fn anthropic_messages_sanitize_strips_only_recorded_root_keywords() { // The recorded 400 names only top-level oneOf/allOf/anyOf - // (f8e757322; CHANGELOG 0.9.1; anthropics/claude-code#4886/#30212). + // (f8e757322; CHANGELOG 0.9.0; anthropics/claude-code#4886/#30212). // Root enum/not, dependent keywords, and the declared root type are // not in any record, so the narrowed pass preserves all of them. let mut schema = json!({ @@ -1087,7 +1110,12 @@ mod tests { }, "not": {"type": "integer"}, "dependentRequired": {"mode": ["confirm"]}, - "dependentSchemas": {"mode": {"required": ["confirm"]}}, + "dependentSchemas": { + "mode": { + "properties": {"confirm": {"type": "boolean"}}, + "required": ["confirm"] + } + }, "oneOf": [ {"required": ["patch"]}, {"required": ["changes"]} @@ -1102,7 +1130,16 @@ mod tests { assert!(schema.get("allOf").is_none()); assert_eq!(schema["not"], json!({"type": "integer"})); assert_eq!(schema["dependentRequired"], json!({"mode": ["confirm"]})); - assert!(schema.get("dependentSchemas").is_some()); + assert_eq!( + schema["dependentSchemas"], + json!({ + "mode": { + "properties": {"confirm": {"type": "boolean"}}, + "required": ["confirm"] + } + }), + "a well-formed dependent schema ships byte-identically: {schema}" + ); assert_eq!( note.as_deref(), Some("Exactly one of these parameter groups must be provided: `changes` | `patch`.") @@ -1154,13 +1191,119 @@ mod tests { assert!(schema.get("anyOf").is_none()); assert!(schema["properties"].get("patch").is_some()); assert!(schema["properties"].get("changes").is_some()); - assert!(schema.get("type").is_none()); + assert_eq!( + schema["type"], "object", + "the merge leaves a populated object schema whose root declared no type, so the object type is restored: {schema}" + ); assert_eq!( note.as_deref(), Some("At least one of these parameter groups must be provided: `changes` | `patch`.") ); } + #[test] + fn anthropic_messages_sanitize_does_not_inject_type_without_a_root_composition() { + // Type restoration is scoped to stripped compositions: a declared + // typeless root with nothing to strip ships exactly as declared. + let mut schema = json!({"properties": {"x": {"type": "string"}}}); + + let note = sanitize_for_anthropic_messages(&mut schema); + + assert_eq!( + schema, + json!({"properties": {"x": {"type": "string"}}}), + "a typeless root without a composition must not gain a type: {schema}" + ); + assert_eq!(note, None); + } + + #[test] + fn anthropic_messages_sanitize_replaces_non_schema_roots_with_empty_objects() { + // The API defines input_schema as a JSON Schema object; a root that + // is an array or scalar (malformed server or MCP output) cannot be + // one, so the pass replaces it with an empty object exactly as the + // Responses pass does instead of shipping the raw value to the + // validator. A DECLARED non-object schema (string-enum root) is an + // object and keeps shipping verbatim. + let mut array_root = json!([{"type": "string"}]); + + let note = sanitize_for_anthropic_messages(&mut array_root); + + assert_eq!( + array_root, + json!({}), + "an array root is replaced, not shipped verbatim: {array_root}" + ); + assert_eq!(note, None); + + let mut scalar_root = json!("browse"); + + let note = sanitize_for_anthropic_messages(&mut scalar_root); + + assert_eq!( + scalar_root, + json!({}), + "a scalar root is replaced, not shipped verbatim: {scalar_root}" + ); + assert_eq!(note, None); + } + + #[test] + fn anthropic_messages_sanitize_pins_the_post_merge_required_prune() { + // A single-branch oneOf collapses inside sanitize(), which spreads + // the branch required groups to the root AFTER sanitize's own + // dangling-required prune has run; only the explicit post-merge + // prune removes the ghost entry. + let mut schema = json!({ + "oneOf": [{ + "type": "object", + "properties": {"a": {"type": "string"}}, + "required": ["a", "ghost"] + }] + }); + + let note = sanitize_for_anthropic_messages(&mut schema); + + assert_eq!( + schema["required"], + json!(["a"]), + "the explicit post-merge prune removes required entries spread after sanitize's prune: {schema}" + ); + assert_eq!(note, None); + } + + #[test] + fn anthropic_messages_sanitize_strips_root_all_of_and_notes_groups() { + // The recorded 400 names allOf alongside oneOf/anyOf; this pins the + // strip, the merge, and the All-prefix note for a root allOf. + let mut schema = json!({ + "allOf": [ + { + "type": "object", + "properties": {"path": {"type": "string"}}, + "required": ["path"] + }, + { + "type": "object", + "properties": {"url": {"type": "string"}}, + "required": ["url"] + } + ] + }); + + let note = sanitize_for_anthropic_messages(&mut schema); + + assert!(schema.get("allOf").is_none()); + assert_eq!(schema["type"], "object"); + assert!( + schema["properties"].get("path").is_some() && schema["properties"].get("url").is_some() + ); + assert_eq!( + note.as_deref(), + Some("All of these parameter groups must be provided: `path` | `url`.") + ); + } + #[test] fn anthropic_messages_sanitize_keeps_the_shared_registry_normalization() { // The narrowed pass still runs the shared registry normalization From fd2d309e06a1dfd84e0742d4a18e67ded79cc6c8 Mon Sep 17 00:00:00 2001 From: asto Date: Sat, 7 Nov 2026 17:12:01 +0800 Subject: [PATCH 05/11] fix(client): restore the object floor for property-less compositions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-4 review fix wave: - A root composition whose branches carry no properties (an allOf of $refs, required-only or boolean branches) merged nothing, and type restoration required non-empty merged properties, so the narrowing lane shipped a constraint-free typeless schema with usually no note — the model-facing contract lost its object shape, which the pass's own doc names as the reason restoration exists. The object type is now restored for every stripped composition, and when the merge contributes no properties the dropped constraint is surfaced as a generic keyword-specific note. Pinned (allOf-of-$refs and boolean branches). - Two pin gaps closed: the gate's identity disjunct (an Anthropic-identity client at a proxy base URL asserted to narrow) and the root-only strip scope (a nested oneOf under properties asserted to survive). - Pins for the remaining round-4 findings: verbatim lanes ship a schema the registry would normalize byte-identically (no normalization on the else branch), root properties win over same-named branch properties, the note keyword precedence is pinned, Openmodel joins the verbatim lane loop, and apply_patch/finance byte-identity with the previous release line is now a committed test. Signed-off-by: asto --- crates/tui/src/client/anthropic.rs | 73 ++++++- crates/tui/src/tools/schema_sanitize.rs | 253 +++++++++++++++++++++++- 2 files changed, 315 insertions(+), 11 deletions(-) diff --git a/crates/tui/src/client/anthropic.rs b/crates/tui/src/client/anthropic.rs index 4d938ece5a..e8af845ad7 100644 --- a/crates/tui/src/client/anthropic.rs +++ b/crates/tui/src/client/anthropic.rs @@ -93,8 +93,9 @@ impl DeepSeekClient { // ApiProvider::Anthropic or a base URL on api.anthropic.com, the // same endpoint predicate the thinking gates below use — narrows // just those three root keywords, merging branch properties and - // noting the dropped constraint; the object type is restored when - // the merge leaves a populated schema whose root declared none, a + // noting the dropped constraint (a generic note when the branches + // carry no properties); the object type is restored whenever a + // stripped composition leaves a typeless root, a // root that is not a JSON object at all is replaced by an empty // object (the API defines input_schema as a JSON Schema object), // and root enum/not, dependent keywords, and the declared root @@ -1265,6 +1266,23 @@ mod tests { DeepSeekClient::new(&config).expect("Model Studio Messages client constructs") } + fn openmodel_test_client() -> DeepSeekClient { + let _ = rustls::crypto::ring::default_provider().install_default(); + let config = crate::config::Config { + provider: Some("openmodel".to_string()), + providers: Some(crate::config::ProvidersConfig { + openmodel: crate::config::ProviderConfig { + api_key: Some("test-key".to_string()), + base_url: Some("https://api.openmodel.ai".to_string()), + ..Default::default() + }, + ..Default::default() + }), + ..Default::default() + }; + DeepSeekClient::new(&config).expect("Openmodel Messages client constructs") + } + fn root_union_tool(name: &str, description: &str) -> Tool { Tool { tool_type: None, @@ -1307,11 +1325,13 @@ mod tests { // CHANGELOG 0.9.0; anthropics/claude-code#4886/#30212) is specific // to the genuine Anthropic validator, so every other Messages lane // ships the schema exactly as declared — one shared builder serves - // DeepSeek, MiniMax, and Model Studio, and this pins all three. + // DeepSeek, MiniMax, Model Studio, and Openmodel, and this pins all + // four. for client in [ deepseek_test_client(crate::config::DEFAULT_DEEPSEEK_ANTHROPIC_BASE_URL), minimax_test_client(), modelstudio_test_client(crate::config::MODELSTUDIO_TOKEN_PLAN_ANTHROPIC_BASE_URL), + openmodel_test_client(), ] { let mut request = request_with("claude-sonnet-4-6", None, None, None); request.tools = Some(vec![root_union_tool("apply_patch", "Patch files")]); @@ -1335,6 +1355,53 @@ mod tests { } } + #[test] + fn non_anthropic_messages_lane_ships_a_schema_the_registry_would_normalize() { + // "Verbatim" means exactly as declared, not merely un-flattened: a + // bare object is a shape the shared sanitize() normalization would + // mutate (bare-object properties injection), so byte identity here + // proves the verbatim branch runs no normalization at all. + let client = deepseek_test_client(crate::config::DEFAULT_DEEPSEEK_ANTHROPIC_BASE_URL); + let mut tool = root_union_tool("mcp_bare", "MCP tool"); + tool.input_schema = json!({"type": "object"}); + let mut request = request_with("claude-sonnet-4-6", None, None, None); + request.tools = Some(vec![tool]); + + let body = client.build_anthropic_body(&request, true); + + assert_eq!( + body["tools"][0]["input_schema"], + json!({"type": "object"}), + "the bare object must ship without the properties injection: {body}" + ); + } + + #[test] + fn anthropic_identity_narrows_even_on_a_proxy_base_url() { + // Provider identity stays in the gate because ANTHROPIC_BASE_URL-style + // overrides commonly front the genuine validator through a proxy: a + // native-identity client at a non-anthropic.com base URL must still + // narrow, which is the only behavior that distinguishes the identity + // disjunct from the endpoint clause. + let client = anthropic_test_client(Some("https://proxy.example.org")); + let mut request = request_with("claude-sonnet-4-6", None, None, None); + request.tools = Some(vec![root_union_tool("apply_patch", "Patch files")]); + + let body = client.build_anthropic_body(&request, true); + + let schema = &body["tools"][0]["input_schema"]; + assert!( + schema.get("oneOf").is_none(), + "the Anthropic identity alone must narrow on a proxy base URL: {schema}" + ); + assert_eq!( + body["tools"][0]["description"], + "Patch files\n\nExactly one of these parameter groups must be provided: `changes` | `patch`.", + "the dropped constraint is still surfaced through the proxy: {}", + body["tools"][0]["description"] + ); + } + #[test] fn anthropic_lane_narrows_the_recorded_root_composition_keywords() { // The genuine Anthropic endpoint is recorded rejecting root-level diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index 45b0e65359..10cd5c739a 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -139,10 +139,14 @@ pub fn sanitize_for_responses(schema: &mut Value) -> Option { /// verbatim because no record rejects them. Root composition branch /// properties are merged into the object and the dropped exactly-one / /// at-least-one constraint is surfaced as a description note so the model -/// still knows which parameter groups apply; where that merge leaves a -/// populated object schema whose root declared no `type`, the object type -/// is restored — the previous release line always forced it, and the -/// model-facing contract otherwise loses its object shape. +/// still knows which parameter groups apply. Where the stripped composition +/// leaves a root with no declared `type`, the object type is restored — the +/// previous release line always forced it, and the model-facing contract +/// otherwise loses its object shape even when the branches carried no +/// properties to merge (an `allOf` of `$ref`s, required-only or boolean +/// branches). When the merge contributes no properties at all, the dropped +/// composition is surfaced as a generic note so the model is not left +/// without any compensating guidance. /// /// The shared [`sanitize`] normalization still applies first, so a root /// nullable union can collapse (rewriting the root `type`), a bare object @@ -169,9 +173,10 @@ pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { return constraint_note; }; - let strips_composition = ["oneOf", "anyOf", "allOf"] + let stripped_composition = ["oneOf", "anyOf", "allOf"] .iter() - .any(|key| obj.contains_key(*key)); + .find(|key| obj.contains_key(**key)) + .copied(); merge_root_composition_properties(obj); obj.remove("oneOf"); obj.remove("anyOf"); @@ -180,11 +185,33 @@ pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { .get("properties") .and_then(Value::as_object) .is_some_and(|properties| !properties.is_empty()); - if strips_composition && !obj.contains_key("type") && merged_properties { + if stripped_composition.is_some() && !obj.contains_key("type") { obj.insert("type".into(), Value::String("object".to_string())); } prune_dangling_required(schema); - constraint_note + constraint_note.or_else(|| { + // A composition whose branches carry no properties (an allOf of + // $refs, required-only or boolean branches) merges nothing, so the + // argument contract is erased without any required-group note to + // compensate; surface a generic note instead of leaving the model + // without guidance about the dropped constraint. + stripped_composition + .filter(|_| !merged_properties) + .map(|key| match key { + "oneOf" => { + "Exactly one of the original argument alternatives must be provided (top-level `oneOf` is not supported by this API and was removed)." + .to_string() + } + "anyOf" => { + "At least one of the original argument alternatives must be provided (top-level `anyOf` is not supported by this API and was removed)." + .to_string() + } + _ => { + "All of the original argument alternatives must be satisfied (top-level `allOf` is not supported by this API and was removed)." + .to_string() + } + }) + }) } fn strict_schema_supported(schema: &Value) -> bool { @@ -1356,6 +1383,216 @@ mod tests { assert_eq!(note, None); } + #[test] + fn anthropic_messages_sanitize_restores_the_object_floor_for_property_less_compositions() { + // A composition whose branches carry no properties — an allOf of + // $refs (the classic OpenAPI-to-MCP shape), required-only or boolean + // branches — merges nothing, and without the type floor the wire + // would carry a constraint-free typeless schema with no note. The + // previous release line always forced type:"object"; keep that floor + // for every stripped composition and surface a generic note when the + // merge contributed nothing. + let mut refs = json!({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$defs": { + "patch": {"type": "object", "properties": {"patch": {"type": "string"}}} + }, + "allOf": [ + {"$ref": "#/$defs/patch"}, + {"$ref": "#/$defs/missing"} + ] + }); + + let note = sanitize_for_anthropic_messages(&mut refs); + + assert_eq!( + refs["type"], "object", + "a stripped composition must not ship a typeless root even when no properties merged: {refs}" + ); + assert!( + refs.get("allOf").is_none(), + "the recorded keyword is still stripped: {refs}" + ); + assert!( + refs.get("properties").is_none(), + "no properties are invented when the branches reference $defs: {refs}" + ); + assert_eq!( + note.as_deref(), + Some( + "All of the original argument alternatives must be satisfied (top-level `allOf` is not supported by this API and was removed)." + ), + "the erased argument contract is surfaced as a note: {note:?}" + ); + + let mut booleans = json!({"oneOf": [true, false]}); + + let note = sanitize_for_anthropic_messages(&mut booleans); + + assert_eq!( + booleans, + json!({"type": "object"}), + "boolean branches merge nothing but keep the object floor: {booleans}" + ); + assert_eq!( + note.as_deref(), + Some( + "Exactly one of the original argument alternatives must be provided (top-level `oneOf` is not supported by this API and was removed)." + ), + "the note names the dropped exactly-one constraint: {note:?}" + ); + } + + #[test] + fn anthropic_messages_sanitize_preserves_nested_compositions() { + // The recorded 400 names only top-level composition keywords, so the + // strip is root-only exactly like the Responses pass; a nested oneOf + // inside a property must reach the wire untouched. Without this pin + // a mutation widening the strip tree-wide stays green. + let mut nested = json!({ + "oneOf": [ + {"type": "object", "properties": {"a": {"type": "string"}}}, + {"type": "object", "properties": {"b": {"type": "integer"}}} + ], + "properties": { + "payload": { + "oneOf": [ + {"type": "object", "properties": {"x": {"type": "string"}}, "required": ["x"]}, + {"type": "object", "properties": {"y": {"type": "integer"}}, "required": ["y"]} + ] + } + } + }); + + let note = sanitize_for_anthropic_messages(&mut nested); + + assert!( + nested.get("oneOf").is_none(), + "the root oneOf is stripped: {nested}" + ); + assert_eq!( + nested["properties"]["payload"]["oneOf"][0]["required"], + json!(["x"]), + "a nested composition is outside the recorded root-only scope: {nested}" + ); + assert_eq!(note, None); + } + + #[test] + fn anthropic_messages_sanitize_keeps_root_properties_over_same_named_branch_properties() { + // merge_root_composition_properties resolves a same-named property + // first-wins: a property declared at the root keeps its own + // definition over a branch's redeclaration. Pin the precedence so a + // silent flip to last-wins cannot narrow the declared contract. + let mut precedence = json!({ + "type": "object", + "properties": {"mode": {"type": "string"}}, + "oneOf": [ + {"type": "object", "properties": {"mode": {"type": "integer"}}}, + {"type": "object", "properties": {"extra": {"type": "string"}}} + ] + }); + + let note = sanitize_for_anthropic_messages(&mut precedence); + + assert_eq!( + precedence["properties"]["mode"], + json!({"type": "string"}), + "the root-declared property wins over a same-named branch property: {precedence}" + ); + assert_eq!( + precedence["properties"]["extra"], + json!({"type": "string"}), + "branch properties without a root collision still merge: {precedence}" + ); + assert_eq!(note, None); + } + + #[test] + fn anthropic_messages_sanitize_notes_only_the_first_composition_keyword() { + // A root carrying two compositions strips both, but the note + // machinery surfaces only the first keyword's constraint (oneOf + // before anyOf before allOf, the recorded-keyword order); pin the + // precedence so a reordering cannot silently change model-facing + // copy. + let mut both = json!({ + "oneOf": [ + {"type": "object", "properties": {"a": {"type": "string"}}, "required": ["a"]}, + {"type": "object", "properties": {"b": {"type": "string"}}, "required": ["b"]} + ], + "anyOf": [ + {"type": "object", "properties": {"c": {"type": "string"}}, "required": ["c"]}, + {"type": "object", "properties": {"d": {"type": "string"}}, "required": ["d"]} + ] + }); + + let note = sanitize_for_anthropic_messages(&mut both); + + assert_eq!( + note.as_deref(), + Some("Exactly one of these parameter groups must be provided: `a` | `b`."), + "the oneOf note wins the fixed keyword precedence: {note:?}" + ); + assert!( + both.get("anyOf").is_none(), + "both compositions are stripped: {both}" + ); + } + + #[test] + fn anthropic_messages_pass_matches_the_previous_release_line_on_the_recorded_tools() { + // The PR's headline behavior claim: apply_patch and finance — the + // two model-visible built-ins whose root compositions produced the + // recorded Anthropic 400 — ship byte-identically to the previous + // release line, which ran sanitize_for_responses on every lane. The + // registry pre-pass (registry.rs) both lines share runs first. + use crate::tools::apply_patch::ApplyPatchTool; + use crate::tools::finance::FinanceTool; + use crate::tools::schema_canonicalize::canonicalize_schema; + use crate::tools::spec::ToolSpec as _; + + fn previous_line_pipeline(mut schema: Value) -> (Value, Option) { + sanitize(&mut schema); + canonicalize_schema(&mut schema); + let note = sanitize_for_responses(&mut schema); + (schema, note) + } + + fn current_line_pipeline(mut schema: Value) -> (Value, Option) { + sanitize(&mut schema); + canonicalize_schema(&mut schema); + let note = sanitize_for_anthropic_messages(&mut schema); + (schema, note) + } + + for (name, previous_input, current_input) in [ + ( + "apply_patch", + ApplyPatchTool.input_schema(), + ApplyPatchTool.input_schema(), + ), + ( + "finance", + FinanceTool::new().input_schema(), + FinanceTool::new().input_schema(), + ), + ] { + let (previous, previous_note) = previous_line_pipeline(previous_input); + let (current, current_note) = current_line_pipeline(current_input); + + assert_eq!( + previous, current, + "{name}: the narrowed pass must match the previous release line" + ); + assert_eq!( + previous.to_string(), + current.to_string(), + "{name}: key order must match too (prompt-cache stability)" + ); + assert_eq!(previous_note, current_note, "{name}: notes must match"); + } + } + #[test] fn xai_sanitize_flattens_apply_patch_root_one_of() { // The exact shape that produced the live 400: From 63003dca57aefad61bf3573a8030a9a821f5398a Mon Sep 17 00:00:00 2001 From: asto18089 Date: Thu, 12 Nov 2026 13:30:56 +0800 Subject: [PATCH 06/11] fix(tui): note the stripped root composition even when properties merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a root composition's branches merged properties into the object, the generic note was suppressed in favor of a required-group note — but branches can merge properties without any required groups: an anyOf of two additionalProperties:false object branches (file vs url) originally admitted an object with exactly one field, while the merged root accepts both. The union contract was relaxed silently and the model never learned the branch/at-least-one constraint was dropped. Surface the generic note whenever a root composition is stripped without the more specific required-group note, merged properties or not. Schema bytes for the recorded built-ins stay identical to the previous release line; the note is the deliberate, reviewed divergence. Requested by review on #146 (P2: merged properties suppressed the generic note). Signed-off-by: asto18089 --- crates/tui/src/tools/schema_sanitize.rs | 107 +++++++++++++++++++----- 1 file changed, 88 insertions(+), 19 deletions(-) diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index 10cd5c739a..73e71321c5 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -144,9 +144,12 @@ pub fn sanitize_for_responses(schema: &mut Value) -> Option { /// previous release line always forced it, and the model-facing contract /// otherwise loses its object shape even when the branches carried no /// properties to merge (an `allOf` of `$ref`s, required-only or boolean -/// branches). When the merge contributes no properties at all, the dropped -/// composition is surfaced as a generic note so the model is not left -/// without any compensating guidance. +/// branches). Whenever no specific required-group note exists, the dropped +/// composition is surfaced as a generic note — merged properties or not +/// (review 4539837534): a union of `additionalProperties:false` branches +/// whose properties merge into the root accepts a mixture no original +/// branch admits, so the model must still be told the branch contract was +/// relaxed. /// /// The shared [`sanitize`] normalization still applies first, so a root /// nullable union can collapse (rewriting the root `type`), a bare object @@ -181,23 +184,20 @@ pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { obj.remove("oneOf"); obj.remove("anyOf"); obj.remove("allOf"); - let merged_properties = obj - .get("properties") - .and_then(Value::as_object) - .is_some_and(|properties| !properties.is_empty()); if stripped_composition.is_some() && !obj.contains_key("type") { obj.insert("type".into(), Value::String("object".to_string())); } prune_dangling_required(schema); constraint_note.or_else(|| { - // A composition whose branches carry no properties (an allOf of - // $refs, required-only or boolean branches) merges nothing, so the - // argument contract is erased without any required-group note to - // compensate; surface a generic note instead of leaving the model - // without guidance about the dropped constraint. - stripped_composition - .filter(|_| !merged_properties) - .map(|key| match key { + // Merged properties do not compensate for the dropped composition: + // a union of `additionalProperties:false` branches merges its branch + // properties into the root and then accepts a mixture no original + // branch admits (review 4539837534), and a property-less + // composition (an allOf of $refs, required-only or boolean branches) + // erases the argument contract outright. Whenever the specific + // required-group note is missing, surface the generic note so the + // model always learns the branch contract was relaxed. + stripped_composition.map(|key| match key { "oneOf" => { "Exactly one of the original argument alternatives must be provided (top-level `oneOf` is not supported by this API and was removed)." .to_string() @@ -1228,6 +1228,48 @@ mod tests { ); } + #[test] + fn anthropic_messages_sanitize_notes_the_relaxed_union_when_branches_merge_without_required() { + // Review 4539837534: merged properties do not compensate for the + // dropped composition. An `anyOf` of two closed object branches whose + // properties merge into the root — each with + // `additionalProperties:false` and no `required`, so no required-group + // note exists — originally admitted an object with exactly one of the + // two fields; the merged root accepts both. The model must still be + // told the branch contract was relaxed via the generic note. + let mut schema = json!({ + "anyOf": [ + { + "type": "object", + "properties": {"file": {"type": "string"}}, + "additionalProperties": false + }, + { + "type": "object", + "properties": {"url": {"type": "string"}}, + "additionalProperties": false + } + ] + }); + + let note = sanitize_for_anthropic_messages(&mut schema); + + assert!(schema.get("anyOf").is_none(), "{schema}"); + assert!(schema["properties"].get("file").is_some(), "{schema}"); + assert!(schema["properties"].get("url").is_some(), "{schema}"); + assert!( + schema.get("additionalProperties").is_none(), + "branch closures must not leak onto the merged root: {schema}" + ); + assert_eq!( + note.as_deref(), + Some( + "At least one of the original argument alternatives must be provided (top-level `anyOf` is not supported by this API and was removed)." + ), + "the relaxed at-least-one contract must be surfaced even though properties merged: {note:?}" + ); + } + #[test] fn anthropic_messages_sanitize_does_not_inject_type_without_a_root_composition() { // Type restoration is scoped to stripped compositions: a declared @@ -1475,7 +1517,13 @@ mod tests { json!(["x"]), "a nested composition is outside the recorded root-only scope: {nested}" ); - assert_eq!(note, None); + assert_eq!( + note.as_deref(), + Some( + "Exactly one of the original argument alternatives must be provided (top-level `oneOf` is not supported by this API and was removed)." + ), + "the root branches carry properties but no required groups, so the dropped exactly-one contract is surfaced as the generic note (review 4539837534): {note:?}" + ); } #[test] @@ -1505,7 +1553,13 @@ mod tests { json!({"type": "string"}), "branch properties without a root collision still merge: {precedence}" ); - assert_eq!(note, None); + assert_eq!( + note.as_deref(), + Some( + "Exactly one of the original argument alternatives must be provided (top-level `oneOf` is not supported by this API and was removed)." + ), + "the branches merge properties without required groups, so no group note exists and the generic note still names the dropped union (review 4539837534): {note:?}" + ); } #[test] @@ -1545,7 +1599,11 @@ mod tests { // two model-visible built-ins whose root compositions produced the // recorded Anthropic 400 — ship byte-identically to the previous // release line, which ran sanitize_for_responses on every lane. The - // registry pre-pass (registry.rs) both lines share runs first. + // registry pre-pass (registry.rs) both lines share runs first. The + // notes intentionally diverge after review 4539837534: the registry + // pre-pass leaves no required groups, so the previous line surfaced + // nothing while this pass names the dropped union contract — the + // model-facing compensation the narrowed pass exists to keep. use crate::tools::apply_patch::ApplyPatchTool; use crate::tools::finance::FinanceTool; use crate::tools::schema_canonicalize::canonicalize_schema; @@ -1589,7 +1647,18 @@ mod tests { current.to_string(), "{name}: key order must match too (prompt-cache stability)" ); - assert_eq!(previous_note, current_note, "{name}: notes must match"); + // Bytes stay identical; the note is the deliberate divergence: + // the previous line dropped the union contract silently, this + // pass names it whenever no required-group note exists. + match (previous_note, current_note) { + (previous_note, Some(current_note)) + if previous_note.is_none() + || previous_note == Some(current_note.clone()) => {} + (previous_note, current_note) => panic!( + "{name}: notes diverged beyond the reviewed generic-note gap: \ + previous={previous_note:?} current={current_note:?}" + ), + } } } From ba32127506255d356acf4ea69560fbef48457cf6 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Sat, 14 Nov 2026 01:47:49 +0800 Subject: [PATCH 07/11] fix(tools): resolve required-only composition branches against the enclosing properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registry sanitize pruned the builtin apply_patch schema's required-only oneOf branches into `[{}, {}, {}]`: `prune_dangling_required` resolved each branch's `required` against the branch's own (absent) `properties`, while JSON Schema binds `required` to the instance the enclosing object already validates. Every branch then matched every instance, so the exactly-one contract admitted no instance at all, and this PR's verbatim Messages lanes forwarded the damaged union verbatim (review 5885719142) where the previous unconditional Responses pass had stripped it before serialization. `sanitize` now threads one instance scope's declared property names into the walk: composition keywords (oneOf/anyOf/allOf/not/if/then/else) inherit the enclosing names, every other keyword resets to a fresh instance, and a branch with no local `properties` at all resolves its `required` against the inherited names before a name is declared dangling. A branch that declares its own `properties` — including an injected empty map — keeps strictly local resolution, so the DeepSeek strict-validator workaround stays intact for genuinely dangling names. Pinned at three layers: schema_sanitize unit tests (preserved branches, still-pruned ghosts, nested-instance reset, local-properties isolation), the registry-produced apply_patch catalog shape, and an end-to-end pin running the actual registry tool through the DeepSeek Anthropic-compatible lane with a jsonschema usability check for replacement-only and patch-only calls. Signed-off-by: asto18089 --- crates/tui/src/client/anthropic.rs | 58 ++++++++ crates/tui/src/tools/registry/tests.rs | 37 +++++ crates/tui/src/tools/schema_sanitize.rs | 178 ++++++++++++++++++++++-- 3 files changed, 265 insertions(+), 8 deletions(-) diff --git a/crates/tui/src/client/anthropic.rs b/crates/tui/src/client/anthropic.rs index e8af845ad7..92ebd505ac 100644 --- a/crates/tui/src/client/anthropic.rs +++ b/crates/tui/src/client/anthropic.rs @@ -1376,6 +1376,64 @@ mod tests { ); } + #[test] + fn registry_produced_patch_tool_ships_a_valid_union_on_the_verbatim_lane() { + // The damage this PR's regression guards against starts upstream of + // the adapter: the real registry pipeline (sanitize + + // canonicalize_schema) pruned the builtin apply_patch schema's + // required-only oneOf branches into `[{}, {}, {}]`, and the + // verbatim Messages lanes forwarded it — a contract no argument can + // satisfy, since every object matches all three empty branches + // (review 5885719142). The end-to-end pin runs the ACTUAL + // registry-produced tool through a non-native Messages lane: the + // union reaches the wire with every branch group intact, so a + // replacement-only call conforms to the advertised builtin + // contract. + let tmp = tempfile::tempdir().expect("tempdir"); + let context = crate::tools::spec::ToolContext::new(tmp.path().to_path_buf()); + let registry = crate::tools::ToolRegistryBuilder::new() + .with_patch_tools() + .build(context); + let patch_tool = registry + .to_api_tools() + .into_iter() + .find(|tool| tool.name == "apply_patch") + .expect("apply_patch registered"); + + let client = deepseek_test_client(crate::config::DEFAULT_DEEPSEEK_ANTHROPIC_BASE_URL); + let mut request = request_with("deepseek-chat", None, None, None); + request.tools = Some(vec![patch_tool.clone()]); + + let body = client.build_anthropic_body(&request, true); + + let schema = &body["tools"][0]["input_schema"]; + assert_eq!( + schema["oneOf"], + json!([ + {"required": ["patch"]}, + {"required": ["replace"]}, + {"required": ["changes"]} + ]), + "the registry-produced union must reach the wire intact: {schema}" + ); + + // The shipped contract is satisfiable for every supported call + // shape: a compiled validator accepts a replacement-only argument + // — the exact case the pruned `oneOf: [{}, {}, {}]` shape rejected. + let validator = jsonschema::options() + .with_draft(jsonschema::Draft::Draft202012) + .build(schema) + .expect("the shipped builtin contract must compile"); + assert!( + validator.is_valid(&json!({"replace": [{"path": "a.txt", "content": "hi"}]})), + "a replacement-only call must conform to the shipped union: {schema}" + ); + assert!( + validator.is_valid(&json!({"patch": "--- a/x\n+++ b/x\n"})), + "a patch-only call must conform to the shipped union: {schema}" + ); + } + #[test] fn anthropic_identity_narrows_even_on_a_proxy_base_url() { // Provider identity stays in the gate because ANTHROPIC_BASE_URL-style diff --git a/crates/tui/src/tools/registry/tests.rs b/crates/tui/src/tools/registry/tests.rs index a56963ee52..2ac16867e5 100644 --- a/crates/tui/src/tools/registry/tests.rs +++ b/crates/tui/src/tools/registry/tests.rs @@ -1714,3 +1714,40 @@ fn classic_truncation_footer_teaches_a_followable_retry() { "the classic truncation footer must keep naming the followable retrieval route" ); } + +#[test] +fn registered_apply_patch_schema_keeps_its_required_only_branch_groups() { + // The builtin apply_patch schema declares root properties beside + // `oneOf: [{required: [patch]}, {required: [replace]}, {required: + // [changes]}]` — required-only branches resolving against the + // ENCLOSING properties. Pruning those groups (the pre-context walk + // did) left `oneOf: [{}, {}, {}]`, whose every branch matches every + // instance: under exactly-one semantics the advertised builtin + // contract admits no instance at all, and the verbatim Messages lanes + // forwarded the damage verbatim (review 5885719142). The registry + // catalog must carry the intact branch groups instead. + let tmp = tempdir().expect("tempdir"); + let ctx = ToolContext::new(tmp.path().to_path_buf()); + let registry = ToolRegistryBuilder::new().with_patch_tools().build(ctx); + let tools = registry.to_api_tools(); + let patch = tools + .iter() + .find(|tool| tool.name == "apply_patch") + .expect("apply_patch registered"); + assert_eq!( + patch.input_schema["oneOf"], + json!([ + {"required": ["patch"]}, + {"required": ["replace"]}, + {"required": ["changes"]} + ]), + "the registered apply_patch union must keep every required-only branch: {}", + patch.input_schema + ); + for field in ["patch", "replace", "changes"] { + assert!( + patch.input_schema["properties"].get(field).is_some(), + "every branch name stays declared at the root: {field}" + ); + } +} diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index 73e71321c5..38b8f15a29 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -28,18 +28,52 @@ use crate::models::Tool; /// - Collapse single-element `oneOf` / `allOf` /// - Walk recursively through all subschemas pub fn sanitize(schema: &mut Value) { + sanitize_in_instance(schema, &[]); +} + +/// Sanitize a schema for one JSON instance scope. +/// +/// `enclosing_properties` carries the property names declared by the +/// enclosing object schema for the *same* instance: a `required` array +/// inside a composition branch refers to that instance, so a required-only +/// branch such as `oneOf: [{required: ["patch"]}, ...]` (the builtin +/// `apply_patch` shape) keeps its entries — they resolve against the root +/// `properties` — instead of being pruned into `{}`, which under `oneOf` +/// semantics would reject every instance (all-empty branches match +/// everything, so exactly-one-match is unsatisfiable). Every other keyword +/// (`properties.*`, `items`, `additionalProperties`, ...) validates a +/// different instance, so the context resets there and only genuinely +/// dangling names are pruned. +fn sanitize_in_instance(schema: &mut Value, enclosing_properties: &[String]) { collapse_nullable_unions(schema); inject_properties_on_bare_objects(schema); - prune_dangling_required(schema); + prune_dangling_required(schema, enclosing_properties); collapse_single_element_unions(schema); // Recurse into all sub-schemas if let Some(obj) = schema.as_object_mut() { - for (_, v) in obj.iter_mut() { - sanitize(v); + let local_properties: Vec = obj + .get("properties") + .and_then(|v| v.as_object()) + .map(|props| props.keys().cloned().collect()) + .unwrap_or_default(); + for (key, v) in obj.iter_mut() { + if matches!( + key.as_str(), + "oneOf" | "anyOf" | "allOf" | "not" | "if" | "then" | "else" + ) { + sanitize_in_instance(v, &local_properties); + } else { + sanitize_in_instance(v, &[]); + } } } else if let Some(arr) = schema.as_array_mut() { + // Composition branch lists arrive as the array value of a + // composition keyword, whose context the parent already chose — + // thread it so each branch prunes against the enclosing + // properties. Non-composition arrays enter with an empty context + // and keep it. for v in arr.iter_mut() { - sanitize(v); + sanitize_in_instance(v, enclosing_properties); } } } @@ -121,7 +155,7 @@ pub fn sanitize_for_responses(schema: &mut Value) -> Option { obj.remove("enum"); obj.remove("not"); ensure_properties_object(obj); - prune_dangling_required(schema); + prune_dangling_required(schema, &[]); combine_constraint_notes(constraint_note, dependent_note) } @@ -187,7 +221,7 @@ pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { if stripped_composition.is_some() && !obj.contains_key("type") { obj.insert("type".into(), Value::String("object".to_string())); } - prune_dangling_required(schema); + prune_dangling_required(schema, &[]); constraint_note.or_else(|| { // Merged properties do not compensate for the dropped composition: // a union of `additionalProperties:false` branches merges its branch @@ -304,16 +338,29 @@ fn inject_properties_on_bare_objects(schema: &mut Value) { } /// Remove entries from `required` that aren't keys in `properties`. -fn prune_dangling_required(schema: &mut Value) { +/// +/// An object with no local `properties` declaration at all (a required-only +/// composition branch) resolves names against `enclosing_properties` — the +/// same-instance property names declared by the enclosing object schema — +/// before a name is declared dangling. A local `properties` key, even empty, +/// keeps the strictly local resolution (the DeepSeek strict validator reads +/// required against the schema object it validates, so an injected empty +/// properties map must not silently adopt the enclosing names). +fn prune_dangling_required(schema: &mut Value, enclosing_properties: &[String]) { let Some(obj) = schema.as_object_mut() else { return; }; // Collect known property names first (immutable borrow), then prune. - let known_keys: Vec = obj + let local_keys: Vec = obj .get("properties") .and_then(|v| v.as_object()) .map(|props| props.keys().cloned().collect()) .unwrap_or_default(); + let known_keys: &[String] = if obj.contains_key("properties") { + &local_keys + } else { + enclosing_properties + }; let Some(required) = obj.get_mut("required").and_then(|v| v.as_array_mut()) else { return; }; @@ -648,6 +695,121 @@ mod tests { assert!(schema.get("required").is_none()); } + #[test] + fn required_only_composition_branches_resolve_against_the_enclosing_properties() { + // `required` inside a composition branch refers to the SAME instance + // the enclosing schema validates, so a required-only branch + // (`{required: ["patch"]}` with the field declared in the enclosing + // `properties`) is not dangling: pruning it would leave `oneOf: + // [{}, {}, {}]`, whose every branch matches every instance — under + // exactly-one semantics that contract admits no instance at all + // (review 5885719142: the registry handed exactly that shape for + // the builtin apply_patch tool, and the verbatim Messages lanes + // forwarded it). + let mut schema = json!({ + "type": "object", + "properties": { + "patch": {"type": "string"}, + "replace": {"type": "array"} + }, + "oneOf": [ + {"required": ["patch"]}, + {"required": ["replace"]} + ] + }); + sanitize(&mut schema); + assert_eq!( + schema["oneOf"], + json!([{"required": ["patch"]}, {"required": ["replace"]}]), + "required-only branches must keep their enclosing-resolvable names" + ); + } + + #[test] + fn genuinely_dangling_branch_required_still_prunes_against_the_enclosing_properties() { + // The fallback resolves names only against what the enclosing + // schema actually declares: a branch requiring a name that exists + // nowhere stays dangling and is pruned, keeping the DeepSeek + // strict-validator workaround intact. + let mut schema = json!({ + "type": "object", + "properties": {"patch": {"type": "string"}}, + "oneOf": [ + {"required": ["patch"]}, + {"required": ["ghost"]} + ] + }); + sanitize(&mut schema); + assert_eq!( + schema["oneOf"], + json!([{"required": ["patch"]}, {}]), + "an enclosing-unresolvable branch name must still prune" + ); + } + + #[test] + fn nested_instance_scopes_do_not_inherit_the_enclosing_required_context() { + // `properties.*` and `items` validate DIFFERENT instances: a + // required-only branch inside them must not resolve against the + // outer object's property names, or a genuinely dangling nested + // name would survive on the strength of an unrelated sibling. + let mut schema = json!({ + "type": "object", + "properties": { + "patch": {"type": "string"}, + "handler": { + "type": "object", + "properties": {"kind": {"type": "string"}}, + "oneOf": [ + {"required": ["ghost"]}, + {"type": "string"} + ] + } + } + }); + sanitize(&mut schema); + assert_eq!( + schema["properties"]["handler"]["oneOf"], + json!([{}, {"type": "string"}]), + "a nested instance scope resets the property context" + ); + } + + #[test] + fn branch_with_local_properties_keeps_strictly_local_required_resolution() { + // A branch declaring its own `properties` (the self-contained + // alternative shape) resolves `required` locally exactly as before: + // the DeepSeek strict validator reads required against the branch's + // own advertised properties, so an injected or declared empty + // properties map must not silently adopt the enclosing names. + let mut schema = json!({ + "type": "object", + "properties": { + "patch": {"type": "string"}, + "handler": { + "type": "object", + "properties": {"kind": {"type": "string"}}, + "required": ["kind", "patch"], + "oneOf": [ + {"type": "object", "properties": {"kind": {"type": "string"}}, "required": ["kind", "patch"]}, + {"type": "string"} + ] + } + } + }); + sanitize(&mut schema); + assert_eq!( + schema["properties"]["handler"]["required"], + json!(["kind"]), + "a local-properties branch prunes against its own map only" + ); + assert_eq!( + schema["properties"]["handler"]["oneOf"][0]["required"], + json!(["kind"]), + "the local-properties branch alternative prunes the same way" + ); + } + #[test] fn collapses_single_element_oneof() { let mut schema = json!({ From 7fa882c2869327f177cf255af52416db2a3cd46b Mon Sep 17 00:00:00 2001 From: asto18089 Date: Sat, 14 Nov 2026 03:45:28 +0800 Subject: [PATCH 08/11] fix(tools): inherit property names through nested same-instance composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A composition branch with no local `properties` still introduces no property scope of its own: `sanitize_in_instance` computed each scope's handed-down context from the scope's own `properties` map alone, so a bare `{oneOf: [...]}` intermediate branch passed an empty context to the composition nested inside it and the inner required-only branches pruned to `{}` — the inner union became unsatisfiable and the sibling non-object branch is incompatible with the object root, so no argument stayed valid (review 5896057559), although `{"a":"x"}` was valid before sanitization. A scope with no local `properties` declaration now forwards the names it inherited; a scope that declares its own map (including an injected empty one) keeps strictly local resolution, and instance-changing keywords still reset the context. Pinned by Jensen's exact nested counterexample with a jsonschema usability check, verified red when the inheritance is reverted. Signed-off-by: asto18089 --- crates/tui/src/tools/schema_sanitize.rs | 64 +++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index 38b8f15a29..5373f20b14 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -51,11 +51,22 @@ fn sanitize_in_instance(schema: &mut Value, enclosing_properties: &[String]) { collapse_single_element_unions(schema); // Recurse into all sub-schemas if let Some(obj) = schema.as_object_mut() { - let local_properties: Vec = obj - .get("properties") - .and_then(|v| v.as_object()) - .map(|props| props.keys().cloned().collect()) - .unwrap_or_default(); + // The context this scope hands its same-instance applicators: a + // local `properties` map replaces the inherited names, while an + // object with NO local declaration introduces no property scope of + // its own and passes the inherited names through — a nested + // composition inside a bare `{oneOf: [...]}` branch still resolves + // against the ancestor that declared the properties (review + // thread on 5896057559: dropping the inheritance there pruned the + // inner required-only branches into `{}` and left no valid + // argument). + let local_properties: Vec = match obj.get("properties") { + Some(props) => props + .as_object() + .map(|props| props.keys().cloned().collect()) + .unwrap_or_default(), + None => enclosing_properties.to_vec(), + }; for (key, v) in obj.iter_mut() { if matches!( key.as_str(), @@ -775,6 +786,49 @@ mod tests { ); } + #[test] + fn inherited_property_names_flow_through_nested_same_instance_composition() { + // A composition branch with no local `properties` introduces no + // property scope of its own: the ancestor's declared names must + // reach compositions nested inside it, or a bare + // `{oneOf: [...]}` intermediate branch prunes its inner + // required-only branches into `{}` — the inner union becomes + // unsatisfiable and the sibling non-object branch is incompatible + // with the object root, so no argument stays valid (review + // 5896057559). `{"a":"x"}` is valid before sanitization and the + // schema must keep admitting it. + let mut schema = json!({ + "type": "object", + "properties": { + "a": {"type": "string"}, + "b": {"type": "string"} + }, + "oneOf": [ + { + "oneOf": [ + {"required": ["a"]}, + {"required": ["b"]} + ] + }, + {"type": "string"} + ] + }); + sanitize(&mut schema); + assert_eq!( + schema["oneOf"][0]["oneOf"], + json!([{"required": ["a"]}, {"required": ["b"]}]), + "the nested required-only branches must resolve against the root properties: {schema}" + ); + let validator = jsonschema::options() + .with_draft(jsonschema::Draft::Draft202012) + .build(&schema) + .expect("the sanitized schema must compile"); + assert!( + validator.is_valid(&json!({"a": "x"})), + "the object form must stay valid after sanitization: {schema}" + ); + } + #[test] fn branch_with_local_properties_keeps_strictly_local_required_resolution() { // A branch declaring its own `properties` (the self-contained From cdc72bf1148265a1fd9c7ea8a8ec83d1603bd88f Mon Sep 17 00:00:00 2001 From: asto18089 Date: Sat, 14 Nov 2026 04:36:49 +0800 Subject: [PATCH 09/11] fix(tools): do not read the injected compatibility map as a declared scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `inject_properties_on_bare_objects` runs before the context match and inserts a synthetic empty `properties` map on a bare typed object, so an intermediate `{type: "object", oneOf: [...]}` branch carried Some(empty) and the context derivation treated it as a local declaration — the same valid-schema loss as the untyped case, one layer down (review 5897491074): the nested required-only branches pruned into `{}`, the inner union became unsatisfiable, and no object argument stayed valid. The context now distinguishes a DECLARED properties map (present before normalization, any content — strictly local, DeepSeek strict behavior unchanged) from a synthetic one: a bare typed object's injected empty map forwards the inherited names, while a non-empty map merged in by a nullable-union collapse still reads as the schema's own declaration. Jensen's typed-object counterexample is the regression pin with a jsonschema usability check; mutation-verified red when the derivation reverts to the post-state-only map. Signed-off-by: asto18089 --- crates/tui/src/tools/schema_sanitize.rs | 84 ++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 10 deletions(-) diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index 5373f20b14..c3ae24afc4 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -45,6 +45,16 @@ pub fn sanitize(schema: &mut Value) { /// different instance, so the context resets there and only genuinely /// dangling names are pruned. fn sanitize_in_instance(schema: &mut Value, enclosing_properties: &[String]) { + // Whether THIS schema declared a properties map before normalization: + // `inject_properties_on_bare_objects` below inserts a synthetic empty + // map on a bare typed object, and a nullable-union collapse can merge an + // arm's map in — only a pre-existing declaration is a real property + // scope (review thread on 5897491074: treating the injected map as + // declared discarded the inherited context for typed-object + // intermediates). + let declared_properties = schema + .as_object() + .is_some_and(|obj| obj.contains_key("properties")); collapse_nullable_unions(schema); inject_properties_on_bare_objects(schema); prune_dangling_required(schema, enclosing_properties); @@ -52,20 +62,29 @@ fn sanitize_in_instance(schema: &mut Value, enclosing_properties: &[String]) { // Recurse into all sub-schemas if let Some(obj) = schema.as_object_mut() { // The context this scope hands its same-instance applicators: a - // local `properties` map replaces the inherited names, while an - // object with NO local declaration introduces no property scope of - // its own and passes the inherited names through — a nested - // composition inside a bare `{oneOf: [...]}` branch still resolves - // against the ancestor that declared the properties (review - // thread on 5896057559: dropping the inheritance there pruned the - // inner required-only branches into `{}` and left no valid - // argument). + // DECLARED `properties` map replaces the inherited names, while an + // object with no declaration of its own introduces no property + // scope and passes the inherited names through — a nested + // composition inside an untyped or bare-typed `{oneOf: [...]}` + // branch still resolves against the ancestor that declared the + // properties (review threads on 5896057559 and 5897491074: the + // injected compatibility map must not read as a declaration, or + // the inner required-only branches prune into `{}` and no valid + // argument survives). let local_properties: Vec = match obj.get("properties") { - Some(props) => props + Some(props) if declared_properties => props .as_object() .map(|props| props.keys().cloned().collect()) .unwrap_or_default(), - None => enclosing_properties.to_vec(), + Some(props) if props.as_object().is_some_and(|m| !m.is_empty()) => { + // A collapse merged a union arm's real property map into + // this scope: that map is the schema's own declaration. + props + .as_object() + .map(|props| props.keys().cloned().collect()) + .unwrap_or_default() + } + _ => enclosing_properties.to_vec(), }; for (key, v) in obj.iter_mut() { if matches!( @@ -829,6 +848,51 @@ mod tests { ); } + #[test] + fn injected_compatibility_map_is_not_a_declared_property_scope() { + // `inject_properties_on_bare_objects` inserts a synthetic empty + // `properties` map on a bare typed object BEFORE the context is + // derived, so an intermediate `{type: "object", oneOf: [...]}` + // branch carries Some(empty) even though the schema declared + // nothing locally: reading that map as a declaration discarded the + // inherited names and the inner required-only branches pruned into + // `{}` — the inner union became unsatisfiable and the sibling + // non-object branch is incompatible with the object root (review + // 5897491074). `{"a":"x"}` is valid before sanitization and must + // stay valid after. + let mut schema = json!({ + "type": "object", + "properties": { + "a": {"type": "string"}, + "b": {"type": "string"} + }, + "oneOf": [ + { + "type": "object", + "oneOf": [ + {"required": ["a"]}, + {"required": ["b"]} + ] + }, + {"type": "string"} + ] + }); + sanitize(&mut schema); + assert_eq!( + schema["oneOf"][0]["oneOf"], + json!([{"required": ["a"]}, {"required": ["b"]}]), + "the typed-object intermediate must forward the inherited names: {schema}" + ); + let validator = jsonschema::options() + .with_draft(jsonschema::Draft::Draft202012) + .build(&schema) + .expect("the sanitized schema must compile"); + assert!( + validator.is_valid(&json!({"a": "x"})), + "the object form must stay valid after sanitization: {schema}" + ); + } + #[test] fn branch_with_local_properties_keeps_strictly_local_required_resolution() { // A branch declaring its own `properties` (the self-contained From 0a7b5c5d65407ecfd2b4d8c2d547430f46a6b631 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Sat, 14 Nov 2026 05:40:19 +0800 Subject: [PATCH 10/11] fix(tools): keep the synthetic-map provenance stable across sanitize passes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two provenance edges on the inherited-context fix (review 5897819279): - Idempotency: the registry sanitizes first and `sanitize_for_responses` sanitizes the same schema again. The synthetic `properties: {}` injected on a bare typed object persisted, so the second pass read it as a declaration and re-pruned the nested required-only branches. Composition -bearing bare typed objects now receive no synthetic map at all — the strict validator the injection exists for rejects compositions outright, so the map bought nothing there — and the pass is idempotent, pinned by a double-sanitize regression whose second pass must be a no-op. - Provenance capture order: `declared_properties` is now captured after the nullable-union collapse but before the injection pass, so an arm's explicitly EMPTY properties map merged by the collapse reads as a declared scope (strictly local, like any directly declared empty map) instead of falling through the nonempty-map guard into inheritance. Pinned by a merged-arm/undeclared-sibling contrast at one instance. Both regressions verified red against targeted mutations (re-enabling the injection on composition intermediates; capturing before the collapse). Signed-off-by: asto18089 --- crates/tui/src/tools/schema_sanitize.rs | 125 ++++++++++++++++++++++-- 1 file changed, 118 insertions(+), 7 deletions(-) diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index c3ae24afc4..573314c210 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -45,17 +45,18 @@ pub fn sanitize(schema: &mut Value) { /// different instance, so the context resets there and only genuinely /// dangling names are pruned. fn sanitize_in_instance(schema: &mut Value, enclosing_properties: &[String]) { - // Whether THIS schema declared a properties map before normalization: - // `inject_properties_on_bare_objects` below inserts a synthetic empty - // map on a bare typed object, and a nullable-union collapse can merge an - // arm's map in — only a pre-existing declaration is a real property - // scope (review thread on 5897491074: treating the injected map as - // declared discarded the inherited context for typed-object + // Whether THIS schema declared a properties map before the injection + // pass: capture AFTER the nullable-union collapse (a merged arm's map — + // even an explicitly empty one — is the schema's own declaration, + // review thread on 5897819279) but BEFORE + // `inject_properties_on_bare_objects`, whose synthetic map is not a + // declaration (review threads on 5896057559 and 5897491074: treating + // it as one discarded the inherited context for composition // intermediates). + collapse_nullable_unions(schema); let declared_properties = schema .as_object() .is_some_and(|obj| obj.contains_key("properties")); - collapse_nullable_unions(schema); inject_properties_on_bare_objects(schema); prune_dangling_required(schema, enclosing_properties); collapse_single_element_unions(schema); @@ -364,6 +365,19 @@ fn inject_properties_on_bare_objects(schema: &mut Value) { if obj.contains_key("properties") || obj.contains_key("additionalProperties") { return; } + // A composition-bearing bare object never reaches the strict-mode + // validator this injection exists for (strict mode rejects root and + // nested compositions outright), so the synthetic map buys nothing + // there — while persisting it manufactures provenance the next + // sanitize pass mistakes for a declared empty scope and re-prunes the + // nested required-only branches (review thread on 5897819279: the + // registry sanitize injects, then sanitize_for_responses sanitizes the + // same schema again). Skipped here, the branch stays property-less, + // the inherited-context rule keeps forwarding ancestor names, and the + // pass is idempotent. + if obj.contains_key("oneOf") || obj.contains_key("anyOf") || obj.contains_key("allOf") { + return; + } obj.insert("properties".into(), Value::Object(Map::new())); } @@ -893,6 +907,103 @@ mod tests { ); } + #[test] + fn sanitize_is_idempotent_for_typed_composition_intermediates() { + // The registry sanitizes first, and `sanitize_for_responses` + // sanitizes the same schema again: a synthetic compatibility map + // injected on pass one must not read as a declaration on pass two, + // or the typed-composition intermediate re-prunes its nested + // required-only branches on the second pass (review 5897819279). + // Composition-bearing bare typed objects now receive no synthetic + // map at all — the strict validator the injection exists for + // rejects compositions outright — so the pass is idempotent and + // the inherited context survives. + let mut schema = json!({ + "type": "object", + "properties": { + "a": {"type": "string"}, + "b": {"type": "string"} + }, + "oneOf": [ + { + "type": "object", + "oneOf": [ + {"required": ["a"]}, + {"required": ["b"]} + ] + }, + {"type": "string"} + ] + }); + sanitize(&mut schema); + let once = schema.clone(); + sanitize(&mut schema); + assert_eq!( + schema, once, + "a second sanitize pass must be a no-op here: {schema}" + ); + assert!( + !schema["oneOf"][0] + .as_object() + .expect("object branch") + .contains_key("properties"), + "the composition-bearing bare typed branch must not gain a synthetic map: {schema}" + ); + assert_eq!( + schema["oneOf"][0]["oneOf"], + json!([{"required": ["a"]}, {"required": ["b"]}]), + "the nested required-only branches must survive both passes: {schema}" + ); + } + + #[test] + fn empty_property_map_merged_from_a_nullable_arm_is_a_declared_scope() { + // Provenance is captured after the nullable-union collapse: an arm + // carrying an explicitly EMPTY properties map merges that map into + // the scope, and it reads as a declaration — strictly local, like + // any directly declared empty map — not as a synthetic map to + // inherit through (review 5897819279). The untyped sibling branch + // at the same instance shows the inherited reading still applies + // when nothing was declared. + let mut schema = json!({ + "type": "object", + "properties": { + "a": {"type": "string"}, + "b": {"type": "string"} + }, + "oneOf": [ + { + "anyOf": [ + {"type": "object", "properties": {}}, + {"type": "null"} + ], + "oneOf": [ + {"required": ["a"]}, + {"type": "string"} + ] + }, + { + "oneOf": [ + {"required": ["a"]}, + {"type": "string"} + ] + }, + {"type": "string"} + ] + }); + sanitize(&mut schema); + assert_eq!( + schema["oneOf"][0]["oneOf"], + json!([{}, {"type": "string"}]), + "the merged explicit empty map is a declared scope: required prunes locally: {schema}" + ); + assert_eq!( + schema["oneOf"][1]["oneOf"], + json!([{"required": ["a"]}, {"type": "string"}]), + "the undeclared sibling branch inherits the ancestor names: {schema}" + ); + } + #[test] fn branch_with_local_properties_keeps_strictly_local_required_resolution() { // A branch declaring its own `properties` (the self-contained From b97a4e6cfecacc01504769bacf5b75db97ce17a4 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Sat, 14 Nov 2026 06:40:36 +0800 Subject: [PATCH 11/11] fix(tui): surface every stripped root composition in the Anthropic note The native Anthropic lane strips all three root composition keywords but the note machinery surfaced only the first matching constraint: `root_composition_constraint_note` returned on the first keyword with two labeled groups and the generic fallback named only the first stripped keyword (review 5898539144). A valid root stacking `oneOf` (a|b) and `anyOf` (c|d) shipped with neither keyword and a description advertising only the exactly-one contract, so the model was never told that `c` or `d` is also required and could emit arguments outside the original contract. The Anthropic pass now builds one note per STRIPPED keyword, in the recorded keyword order: the specific required-group sentence when that keyword's branches carry two or more labeled groups (captured at entry, as the first-match note read them), otherwise the generic sentence. A keyword erased by the shared normalization before stripping (a nullable-union collapse) takes no note, exactly as before. The shared helper aggregates for the Responses/xAI lanes the same way. The `notes_only_the_first_composition_keyword` pin becomes an aggregation pin with a mixed specific+generic control; mutation-verified red under first-note truncation. Signed-off-by: asto18089 --- crates/tui/src/tools/schema_sanitize.rs | 186 +++++++++++++++++------- 1 file changed, 132 insertions(+), 54 deletions(-) diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index 573314c210..bd4c3c28e5 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -204,17 +204,16 @@ pub fn sanitize_for_responses(schema: &mut Value) -> Option { /// verbatim because no record rejects them. Root composition branch /// properties are merged into the object and the dropped exactly-one / /// at-least-one constraint is surfaced as a description note so the model -/// still knows which parameter groups apply. Where the stripped composition -/// leaves a root with no declared `type`, the object type is restored — the -/// previous release line always forced it, and the model-facing contract -/// otherwise loses its object shape even when the branches carried no -/// properties to merge (an `allOf` of `$ref`s, required-only or boolean -/// branches). Whenever no specific required-group note exists, the dropped -/// composition is surfaced as a generic note — merged properties or not -/// (review 4539837534): a union of `additionalProperties:false` branches -/// whose properties merge into the root accepts a mixture no original -/// branch admits, so the model must still be told the branch contract was -/// relaxed. +/// still knows which parameter groups apply. Every stripped composition is +/// surfaced: its specific required-group note when the branches carry two +/// or more labeled groups, otherwise the generic sentence — merged +/// properties or not (review 4539837534): a union of +/// `additionalProperties:false` branches whose properties merge into the +/// root accepts a mixture no original branch admits, so the model must +/// still be told the branch contract was relaxed. The notes aggregate +/// across all stripped keywords (review 5898539144): a first-match note +/// left a root stacking `oneOf` and `anyOf` advertising only the +/// exactly-one contract while the wire schema dropped both. /// /// The shared [`sanitize`] normalization still applies first, so a root /// nullable union can collapse (rewriting the root `type`), a bare object @@ -227,9 +226,38 @@ pub fn sanitize_for_responses(schema: &mut Value) -> Option { /// /// Returns the same style of description note as [`sanitize_for_responses`]. pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { - let constraint_note = schema - .as_object() - .and_then(root_composition_constraint_note); + // The specific required-group sentences for the keywords that carry + // two or more labeled groups, captured at entry exactly as the + // first-match note read them; which keywords actually SURVIVE to be + // stripped is decided after the shared normalization (a nullable-union + // collapse can erase a composition entirely, and a composition that + // never reaches the wire takes no note with it). + let mut specific_notes: std::collections::HashMap<&'static str, String> = + std::collections::HashMap::new(); + if let Some(obj) = schema.as_object() { + for (key, prefix) in [ + ("oneOf", "Exactly one"), + ("anyOf", "At least one"), + ("allOf", "All"), + ] { + let Some(items) = obj.get(key).and_then(Value::as_array) else { + continue; + }; + let mut groups: Vec = + items.iter().filter_map(required_group_label).collect(); + groups.sort(); + groups.dedup(); + if groups.len() >= 2 { + specific_notes.insert( + key, + format!( + "{prefix} of these parameter groups must be provided: {}.", + groups.join(" | ") + ), + ); + } + } + } sanitize(schema); @@ -238,45 +266,44 @@ pub fn sanitize_for_anthropic_messages(schema: &mut Value) -> Option { } let Some(obj) = schema.as_object_mut() else { - return constraint_note; + return specific_notes.values().next().cloned(); }; - let stripped_composition = ["oneOf", "anyOf", "allOf"] + let stripped_compositions: Vec<&'static str> = ["oneOf", "anyOf", "allOf"] .iter() - .find(|key| obj.contains_key(**key)) - .copied(); + .filter(|key| obj.contains_key(**key)) + .copied() + .collect(); merge_root_composition_properties(obj); obj.remove("oneOf"); obj.remove("anyOf"); obj.remove("allOf"); - if stripped_composition.is_some() && !obj.contains_key("type") { + if !stripped_compositions.is_empty() && !obj.contains_key("type") { obj.insert("type".into(), Value::String("object".to_string())); } prune_dangling_required(schema, &[]); - constraint_note.or_else(|| { - // Merged properties do not compensate for the dropped composition: - // a union of `additionalProperties:false` branches merges its branch - // properties into the root and then accepts a mixture no original - // branch admits (review 4539837534), and a property-less - // composition (an allOf of $refs, required-only or boolean branches) - // erases the argument contract outright. Whenever the specific - // required-group note is missing, surface the generic note so the - // model always learns the branch contract was relaxed. - stripped_composition.map(|key| match key { - "oneOf" => { - "Exactly one of the original argument alternatives must be provided (top-level `oneOf` is not supported by this API and was removed)." - .to_string() - } - "anyOf" => { - "At least one of the original argument alternatives must be provided (top-level `anyOf` is not supported by this API and was removed)." - .to_string() - } - _ => { - "All of the original argument alternatives must be satisfied (top-level `allOf` is not supported by this API and was removed)." - .to_string() - } - }) - }) + // One note per STRIPPED root composition keyword, in the recorded + // keyword order: the specific sentence when that keyword carried two or + // more labeled groups, otherwise the generic sentence (review 4539837534 + // — merged properties do not compensate for the dropped composition, and + // a property-less composition erases the argument contract outright). + // Aggregating across ALL stripped keywords keeps the model informed of + // every dropped constraint (review 5898539144: the pass strips all + // three, but a first-match note advertised only the first). + let notes: Vec = stripped_compositions + .iter() + .map(|key| { + specific_notes + .get(key) + .cloned() + .unwrap_or_else(|| root_composition_generic_note(key).to_string()) + }) + .collect(); + if notes.is_empty() { + None + } else { + Some(notes.join(" ")) + } } fn strict_schema_supported(schema: &Value) -> bool { @@ -551,6 +578,13 @@ fn merge_root_composition_properties(obj: &mut Map) { } fn root_composition_constraint_note(obj: &Map) -> Option { + // Aggregate the required-group note for EVERY root composition keyword, + // in the recorded keyword order: this pass strips all three keywords + // but a first-match note left the model ignorant of every dropped + // constraint after the first (review 5898539144 — a root stacking + // `oneOf` and `anyOf` advertised only the exactly-one contract on the + // wire). + let mut notes: Vec = Vec::new(); for (key, prefix) in [ ("oneOf", "Exactly one"), ("anyOf", "At least one"), @@ -563,13 +597,34 @@ fn root_composition_constraint_note(obj: &Map) -> Option groups.sort(); groups.dedup(); if groups.len() >= 2 { - return Some(format!( + notes.push(format!( "{prefix} of these parameter groups must be provided: {}.", groups.join(" | ") )); } } - None + if notes.is_empty() { + None + } else { + Some(notes.join(" ")) + } +} + +/// The generic sentence for one stripped root composition keyword: the +/// model-facing compensation for a composition whose branches carry no +/// labeled required groups (review 4539837534). +fn root_composition_generic_note(key: &str) -> &'static str { + match key { + "oneOf" => { + "Exactly one of the original argument alternatives must be provided (top-level `oneOf` is not supported by this API and was removed)." + } + "anyOf" => { + "At least one of the original argument alternatives must be provided (top-level `anyOf` is not supported by this API and was removed)." + } + _ => { + "All of the original argument alternatives must be satisfied (top-level `allOf` is not supported by this API and was removed)." + } + } } /// Strip `dependentSchemas` / `dependentRequired` from every node. @@ -1954,12 +2009,13 @@ mod tests { } #[test] - fn anthropic_messages_sanitize_notes_only_the_first_composition_keyword() { - // A root carrying two compositions strips both, but the note - // machinery surfaces only the first keyword's constraint (oneOf - // before anyOf before allOf, the recorded-keyword order); pin the - // precedence so a reordering cannot silently change model-facing - // copy. + fn anthropic_messages_sanitize_notes_every_stripped_composition_keyword() { + // A root carrying two compositions strips both, and the notes + // aggregate in the recorded keyword order (review 5898539144: the + // first-match note told the model only about `a` | `b` while the + // wire schema also dropped the at-least-one `c` | `d` constraint, + // so the model could emit arguments outside the original + // contract). let mut both = json!({ "oneOf": [ {"type": "object", "properties": {"a": {"type": "string"}}, "required": ["a"]}, @@ -1975,13 +2031,35 @@ mod tests { assert_eq!( note.as_deref(), - Some("Exactly one of these parameter groups must be provided: `a` | `b`."), - "the oneOf note wins the fixed keyword precedence: {note:?}" + Some("Exactly one of these parameter groups must be provided: `a` | `b`. At least one of these parameter groups must be provided: `c` | `d`."), + "both dropped constraints must be surfaced: {note:?}" ); assert!( - both.get("anyOf").is_none(), + both.get("oneOf").is_none() && both.get("anyOf").is_none(), "both compositions are stripped: {both}" ); + + // A stripped keyword whose branches carry no labeled groups + // contributes its generic sentence beside the sibling's specific + // one instead of being silently dropped by the aggregation. + let mut mixed = json!({ + "oneOf": [ + {"type": "object", "properties": {"a": {"type": "string"}}, "required": ["a"]}, + {"type": "object", "properties": {"b": {"type": "string"}}, "required": ["b"]} + ], + "anyOf": [ + {"type": "object", "properties": {"e": {"type": "string"}}}, + {"type": "object", "properties": {"f": {"type": "string"}}} + ] + }); + + let note = sanitize_for_anthropic_messages(&mut mixed); + + assert_eq!( + note.as_deref(), + Some("Exactly one of these parameter groups must be provided: `a` | `b`. At least one of the original argument alternatives must be provided (top-level `anyOf` is not supported by this API and was removed)."), + "the generic sentence fills in for the group-less keyword: {note:?}" + ); } #[test]